wheeeee more fixes

6 files changed, 90 insertions, 116 deletions

diff --git a/toolsrc/include/vcpkg/base/strings.h b/toolsrc/include/vcpkg/base/strings.h
index a1906790f..aa4c4d690 100644
--- a/toolsrc/include/vcpkg/base/strings.h
+++ b/toolsrc/include/vcpkg/base/strings.h
@@ -185,8 +185,8 @@ namespace vcpkg::Strings
 
     bool contains(StringView haystack, StringView needle);
 
-    // base 64 encoding with URL and filesafe alphabet (base64url)
-    // based on IETF RFC 4648
-    // ignores padding, since one implicitly knows the length from the size of x
-    std::string b64url_encode(std::uint64_t x) noexcept;
+    // base 32 encoding, since base64 encoding requires lowercase letters,
+	// which are not distinct from uppercase letters on macOS or Windows filesystems.
+	// follows RFC 4648
+    std::string b32_encode(std::uint64_t x) noexcept;
 }
diff --git a/toolsrc/include/vcpkg/base/work_queue.h b/toolsrc/include/vcpkg/base/work_queue.h
index d6666770b..69ca387f3 100644
--- a/toolsrc/include/vcpkg/base/work_queue.h
+++ b/toolsrc/include/vcpkg/base/work_queue.h
@@ -36,7 +36,7 @@ namespace vcpkg
         {
             m_line_info = li;
 
-            m_unjoined_workers = num_threads;
+            set_unjoined_workers(num_threads);
             m_threads.reserve(num_threads);
             for (std::size_t i = 0; i < num_threads; ++i)
             {
@@ -93,20 +93,16 @@ namespace vcpkg
                 }
             }
 
-            for (;;)
+            while (unjoined_workers())
             {
-                auto lck = std::unique_lock<std::mutex>(m_mutex);
-                if (!m_unjoined_workers)
-                    break;
-
-                else if (!m_running_workers)
+                if (!running_workers())
                 {
-                    lck.unlock();
-                    m_cv.notify_all();
-                }
+                    m_cv.notify_one();
+          
+				}
             }
 
-            // all threads have returned -- now, it's time to join them
+            // wait for all threads to join
             for (auto& thrd : m_threads)
             {
                 thrd.join();
@@ -143,50 +139,6 @@ namespace vcpkg
             m_cv.notify_one();
         }
 
-        template<class Rng>
-        void enqueue_all_actions_by_move(Rng&& rng) const
-        {
-            {
-                using std::begin;
-                using std::end;
-
-                auto lck = std::unique_lock<std::mutex>(m_mutex);
-
-                const auto first = begin(rng);
-                const auto last = end(rng);
-
-                m_actions.reserve(m_actions.size() + (last - first));
-
-                std::move(first, last, std::back_inserter(rng));
-
-                if (m_state == State::BeforeRun) return;
-            }
-
-            m_cv.notify_all();
-        }
-
-        template<class Rng>
-        void enqueue_all_actions(Rng&& rng) const
-        {
-            {
-                using std::begin;
-                using std::end;
-
-                auto lck = std::unique_lock<std::mutex>(m_mutex);
-
-                const auto first = begin(rng);
-                const auto last = end(rng);
-
-                m_actions.reserve(m_actions.size() + (last - first));
-
-                std::copy(first, last, std::back_inserter(rng));
-
-                if (m_state == State::BeforeRun) return;
-            }
-
-            m_cv.notify_all();
-        }
-
     private:
         struct Worker
         {
@@ -201,6 +153,7 @@ namespace vcpkg
 
                 work_queue->m_cv.wait(lck, [&] { return work_queue->m_state != State::BeforeRun; });
 
+                work_queue->increment_running_workers();
                 for (;;)
                 {
                     const auto state = work_queue->m_state;
@@ -212,15 +165,15 @@ namespace vcpkg
 
                     if (work_queue->m_actions.empty())
                     {
-                        if (state == State::Running || work_queue->m_running_workers > 1)
+                        if (state == State::Running || work_queue->running_workers() > 1)
                         {
-                            --work_queue->m_running_workers;
+                            work_queue->decrement_running_workers();
                             work_queue->m_cv.wait(lck);
-                            ++work_queue->m_running_workers;
+                            work_queue->increment_running_workers();
                             continue;
                         }
 
-                        // the queue isn't running, and we are the only worker
+                        // the queue is joining, and we are the only worker running
                         // no more work!
                         break;
                     }
@@ -229,11 +182,13 @@ namespace vcpkg
                     work_queue->m_actions.pop_back();
 
                     lck.unlock();
+                    work_queue->m_cv.notify_one();
                     detail::call_moved_action(action, *work_queue, tld);
                     lck.lock();
                 }
 
-                --work_queue->m_unjoined_workers;
+                work_queue->decrement_running_workers();
+                work_queue->decrement_unjoined_workers();
             }
         };
 
@@ -255,11 +210,21 @@ namespace vcpkg
         mutable std::mutex m_mutex{};
         // these are all under m_mutex
         mutable State m_state = State::BeforeRun;
-        mutable std::uint16_t m_running_workers = 0;
-        mutable std::uint16_t m_unjoined_workers = 0; // num_threads
         mutable std::vector<Action> m_actions{};
         mutable std::condition_variable m_cv{};
 
+        mutable std::atomic<std::uint32_t> m_workers;
+        // = unjoined_workers << 16 | running_workers
+
+        void set_unjoined_workers(std::uint16_t threads) { m_workers = std::uint32_t(threads) << 16; }
+        void decrement_unjoined_workers() const { m_workers -= 1 << 16; }
+
+        std::uint16_t unjoined_workers() const { return std::uint16_t(m_workers >> 16); }
+
+        void increment_running_workers() const { ++m_workers; }
+        void decrement_running_workers() const { --m_workers; }
+        std::uint16_t running_workers() const { return std::uint16_t(m_workers); }
+
         std::vector<std::thread> m_threads{};
         LineInfo m_line_info;
     };
diff --git a/toolsrc/src/tests.files.cpp b/toolsrc/src/tests.files.cpp
index c99dbb286..56b0ceac6 100644
--- a/toolsrc/src/tests.files.cpp
+++ b/toolsrc/src/tests.files.cpp
@@ -4,6 +4,7 @@
 #include <vcpkg/base/strings.h>
 
 #include <iostream>
+#include <filesystem> // required for filesystem::create_{directory_}symlink
 #include <random>
 
 #include <windows.h>
@@ -21,18 +22,20 @@ namespace UnitTest1
         {
             HKEY key;
             const auto status = RegOpenKeyExW(
-                HKEY_LOCAL_MACHINE, LR"(SOFTWARE\Microsoft\Windows\CurrentVersion\AppModelUnlock)", 0, KEY_READ, &key);
+                HKEY_LOCAL_MACHINE, LR"(SOFTWARE\Microsoft\Windows\CurrentVersion\AppModelUnlock)", 0, 0, &key);
 
-            if (!status)
+            if (status == ERROR_FILE_NOT_FOUND)
             {
                 ALLOW_SYMLINKS = false;
                 std::clog << "Symlinks are not allowed on this system\n";
             }
             else
             {
+				// if we get a permissions error, we still know that we're in developer mode
                 ALLOW_SYMLINKS = true;
-                RegCloseKey(key);
             }
+
+			if (status == ERROR_SUCCESS) RegCloseKey(key);
         }
 
     private:
@@ -62,9 +65,9 @@ namespace UnitTest1
 
             std::clog << "temp dir is: " << temp_dir << '\n';
 
-            std::error_code ec;
             create_directory_tree(urbg, fs, 0, temp_dir);
 
+            std::error_code ec;
             fs::path fp;
             fs.remove_all(temp_dir, ec, fp);
             Assert::IsFalse(bool(ec));
@@ -80,7 +83,7 @@ namespace UnitTest1
             return std::mt19937_64{index + 9223372036854775837};
         }
 
-        std::string get_random_filename(std::mt19937_64& urbg) { return vcpkg::Strings::b64url_encode(uid{}(urbg)); }
+        std::string get_random_filename(std::mt19937_64& urbg) { return vcpkg::Strings::b32_encode(uid{}(urbg)); }
 
         void create_directory_tree(std::mt19937_64& urbg,
                                    vcpkg::Files::Filesystem& fs,
@@ -88,46 +91,57 @@ namespace UnitTest1
                                    const fs::path& base)
         {
             std::random_device rd;
-            constexpr auto max_depth = std::uint64_t(3);
-            const auto width = depth ? uid{0, (max_depth - depth) * 3 / 2}(urbg) : 5;
+            constexpr std::uint64_t max_depth = 5;
+            constexpr std::uint64_t width = 5;
+            const auto type = depth < max_depth ? uid{0, 9}(urbg) : uid{7, 9}(urbg);
+
+			// 0 <= type < 7 : directory
+			// 7 = type : regular
+			// 8 = type : regular symlink (regular file if !ALLOW_SYMLINKS)
+			// 9 = type : directory symlink (^^)
 
             std::error_code ec;
-            if (width == 0)
+            if (type >= 7)
             {
                 // I don't want to move urbg forward conditionally
-                const auto type = uid{0, 3}(urbg);
-                if (type == 0 || !ALLOW_SYMLINKS)
+                if (type == 7 || !ALLOW_SYMLINKS)
                 {
-                    // 0 is a regular file
+					// regular file
                     fs.write_contents(base, "", ec);
                 }
-                else if (type == 1)
+                else if (type == 8)
                 {
-                    // 1 is a regular symlink
+                    // regular symlink
                     fs.write_contents(base, "", ec);
                     Assert::IsFalse(bool(ec));
-                    fs::path base_link = base;
-                    base_link.append("-link");
-                    fs::stdfs::create_symlink(base, base_link, ec);
+                    const std::filesystem::path basep = base.native();
+                    auto basep_link = basep;
+                    basep_link.replace_filename(basep.filename().native() + L"-link");
+                    std::filesystem::create_symlink(basep, basep_link, ec);
                 }
                 else
                 {
-                    // 2 is a directory symlink
-                    fs::stdfs::create_directory_symlink(".", base, ec);
+                    // directory symlink
+                    std::filesystem::path basep = base.native();
+                    std::filesystem::create_directory_symlink(basep / "..", basep, ec);
                 }
 
                 Assert::IsFalse(bool(ec));
 
-                return;
             }
-
-            fs.create_directory(base, ec);
-            Assert::IsFalse(bool(ec));
-
-            for (int i = 0; i < width; ++i)
+            else
             {
-                create_directory_tree(urbg, fs, depth + 1, base / get_random_filename(urbg));
+				// directory
+                fs.create_directory(base, ec);
+                Assert::IsFalse(bool(ec));
+
+                for (int i = 0; i < width; ++i)
+                {
+                    create_directory_tree(urbg, fs, depth + 1, base / get_random_filename(urbg));
+                }
+        
             }
+
         }
     };
 }
diff --git a/toolsrc/src/tests.strings.cpp b/toolsrc/src/tests.strings.cpp
index 14b449e86..6301ea05d 100644
--- a/toolsrc/src/tests.strings.cpp
+++ b/toolsrc/src/tests.strings.cpp
@@ -16,21 +16,21 @@ namespace UnitTest1 {
 
             std::vector<std::pair<std::uint64_t, std::string>> map;
 
-            map.emplace_back(0, "AAAAAAAAAAA");
-            map.emplace_back(1, "BAAAAAAAAAA");
+            map.emplace_back(0, "AAAAAAAAAAAAA");
+            map.emplace_back(1, "BAAAAAAAAAAAA");
 
-            map.emplace_back(u64(1) << 32, "AAAAAEAAAAA");
-            map.emplace_back((u64(1) << 32) + 1, "BAAAAEAAAAA");
+            map.emplace_back(u64(1) << 32, "AAAAAAEAAAAAA");
+            map.emplace_back((u64(1) << 32) + 1, "BAAAAAEAAAAAA");
 
-            map.emplace_back(0xE4D0'1065'D11E'0229, "pIgHRXGEQTO");
-            map.emplace_back(0xA626'FE45'B135'07FF, "_fQNxWk_mYK");
-            map.emplace_back(0xEE36'D228'0C31'D405, "FQdMMgi024O");
-            map.emplace_back(0x1405'64E7'FE7E'A88C, "Miqf-fOZFQB");
-            map.emplace_back(0xFFFF'FFFF'FFFF'FFFF, "__________P");
+            map.emplace_back(0xE4D0'1065'D11E'0229, "JRA4RIXMQAUJO");
+            map.emplace_back(0xA626'FE45'B135'07FF, "77BKTYWI6XJMK");
+            map.emplace_back(0xEE36'D228'0C31'D405, "FAVDDGAFSWN4O");
+            map.emplace_back(0x1405'64E7'FE7E'A88C, "MEK5H774ELBIB");
+            map.emplace_back(0xFFFF'FFFF'FFFF'FFFF, "777777777777P");
 
             std::string result;
             for (const auto& pr : map) {
-                result = vcpkg::Strings::b64url_encode(pr.first);
+                result = vcpkg::Strings::b32_encode(pr.first);
                 Assert::AreEqual(result, pr.second);
             }
         }
diff --git a/toolsrc/src/vcpkg/base/files.cpp b/toolsrc/src/vcpkg/base/files.cpp
index e40822705..6c6945e44 100644
--- a/toolsrc/src/vcpkg/base/files.cpp
+++ b/toolsrc/src/vcpkg/base/files.cpp
@@ -423,7 +423,7 @@ namespace vcpkg::Files
                 {
                     std::error_code ec;
 
-                    const auto tmp_name = Strings::b64url_encode(info.index++);
+                    const auto tmp_name = Strings::b32_encode(info.index++);
                     const auto tmp_path = info.tmp_directory / tmp_name;
 
                     fs::stdfs::rename(current_path, tmp_path, ec);
diff --git a/toolsrc/src/vcpkg/base/strings.cpp b/toolsrc/src/vcpkg/base/strings.cpp
index ade4384a9..7970e1b46 100644
--- a/toolsrc/src/vcpkg/base/strings.cpp
+++ b/toolsrc/src/vcpkg/base/strings.cpp
@@ -294,26 +294,21 @@ namespace vcpkg::Strings
     namespace
     {
         template<class Integral>
-        std::string b64url_encode_implementation(Integral x)
+        std::string b32_encode_implementation(Integral x)
         {
             static_assert(std::is_integral<Integral>::value, "b64url_encode must take an integer type");
             using Unsigned = std::make_unsigned_t<Integral>;
             auto value = static_cast<Unsigned>(x);
 
-            // 64 values, plus the implicit \0
-            constexpr static char map[65] =
-                /*     0123456789ABCDEF */
-                /*0*/ "ABCDEFGHIJKLMNOP"
-                      /*1*/ "QRSTUVWXYZabcdef"
-                      /*2*/ "ghijklmnopqrstuv"
-                      /*3*/ "wxyz0123456789-_";
+            // 32 values, plus the implicit \0
+            constexpr static char map[33] = "ABCDEFGHIJKLMNOP" "QRSTUVWXYZ234567";
 
-            // log2(64)
-            constexpr static int shift = 6;
-            // 64 - 1
-            constexpr static auto mask = 63;
+            // log2(32)
+            constexpr static int shift = 5;
+            // 32 - 1
+            constexpr static auto mask = 31;
 
-            // ceiling(bitsize(Integral) / log2(64))
+            // ceiling(bitsize(Integral) / log2(32))
             constexpr static auto result_size = (sizeof(value) * 8 + shift - 1) / shift;
 
             std::string result;
@@ -329,6 +324,6 @@ namespace vcpkg::Strings
         }
     }
 
-    std::string b64url_encode(std::uint64_t x) noexcept { return b64url_encode_implementation(x); }
+    std::string b32_encode(std::uint64_t x) noexcept { return b32_encode_implementation(x); }
 
 }