fix more comments

9 files changed, 164 insertions, 58 deletions

diff --git a/toolsrc/src/vcpkg/archives.cpp b/toolsrc/src/vcpkg/archives.cpp
index 69a916828..d22e841de 100644
--- a/toolsrc/src/vcpkg/archives.cpp
+++ b/toolsrc/src/vcpkg/archives.cpp
@@ -15,9 +15,10 @@ namespace vcpkg::Archives
 #endif
             ;
 
+        fs.remove_all(to_path, VCPKG_LINE_INFO);
+        fs.remove_all(to_path_partial, VCPKG_LINE_INFO);
+        // TODO: check this error code
         std::error_code ec;
-        fs.remove_all(to_path, ec);
-        fs.remove_all(to_path_partial, ec);
         fs.create_directories(to_path_partial, ec);
         const auto ext = archive.extension();
 #if defined(_WIN32)
diff --git a/toolsrc/src/vcpkg/base/files.cpp b/toolsrc/src/vcpkg/base/files.cpp
index 8bc37819a..a4e67a142 100644
--- a/toolsrc/src/vcpkg/base/files.cpp
+++ b/toolsrc/src/vcpkg/base/files.cpp
@@ -5,8 +5,8 @@
 #include <vcpkg/base/system.h>
 #include <vcpkg/base/system.print.h>
 #include <vcpkg/base/system.process.h>
-#include <vcpkg/base/work_queue.h>
 #include <vcpkg/base/util.h>
+#include <vcpkg/base/work_queue.h>
 
 #if defined(__linux__) || defined(__APPLE__)
 #include <fcntl.h>
@@ -21,8 +21,10 @@
 #include <copyfile.h>
 #endif
 
-namespace fs::detail {
-    file_status symlink_status_t::operator()(const path& p, std::error_code& ec) const noexcept {
+namespace fs::detail
+{
+    file_status symlink_status_t::operator()(const path& p, std::error_code& ec) const noexcept
+    {
 #if defined(_WIN32)
         /*
             do not find the permissions of the file -- it's unnecessary for the
@@ -34,14 +36,21 @@ namespace fs::detail {
 
         WIN32_FILE_ATTRIBUTE_DATA file_attributes;
         file_type ft = file_type::unknown;
-        if (!GetFileAttributesExW(p.c_str(), GetFileExInfoStandard, &file_attributes)) {
+        if (!GetFileAttributesExW(p.c_str(), GetFileExInfoStandard, &file_attributes))
+        {
             ft = file_type::not_found;
-        } else if (file_attributes.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) {
+        }
+        else if (file_attributes.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT)
+        {
             // check for reparse point -- if yes, then symlink
             ft = file_type::symlink;
-        } else if (file_attributes.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) {
+        }
+        else if (file_attributes.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
+        {
             ft = file_type::directory;
-        } else {
+        }
+        else
+        {
             // otherwise, the file is a regular file
             ft = file_type::regular;
         }
@@ -53,12 +62,13 @@ namespace fs::detail {
 #endif
     }
 
-    file_status symlink_status_t::operator()(const path& p) const noexcept {
+    file_status symlink_status_t::operator()(const path& p, vcpkg::LineInfo li) const noexcept
+    {
         std::error_code ec;
         auto result = symlink_status(p, ec);
-        if (ec) vcpkg::Checks::exit_with_message(VCPKG_LINE_INFO, "error getting status of path %s: %s", p.string(), ec.message());
+        if (ec) vcpkg::Checks::exit_with_message(li, "error getting status of path %s: %s", p.string(), ec.message());
 
-				return result;
+        return result;
     }
 }
 
@@ -105,6 +115,25 @@ namespace vcpkg::Files
         if (ec) Checks::exit_with_message(linfo, "error writing lines: %s: %s", path.u8string(), ec.message());
     }
 
+    std::uintmax_t Filesystem::remove_all(const fs::path& path, LineInfo li)
+    {
+        std::error_code ec;
+        fs::path failure_point;
+
+        const auto result = this->remove_all(path, ec, failure_point);
+
+        if (ec)
+        {
+            Checks::exit_with_message(li,
+                                      "Failure to remove_all(%s) due to file %s: %s",
+                                      path.string(),
+                                      failure_point.string(),
+                                      ec.message());
+        }
+
+        return result;
+    }
+
     struct RealFilesystem final : Filesystem
     {
         virtual Expected<std::string> read_contents(const fs::path& file_path) const override
@@ -296,7 +325,7 @@ namespace vcpkg::Files
 #endif
         }
         virtual bool remove(const fs::path& path, std::error_code& ec) override { return fs::stdfs::remove(path, ec); }
-        virtual std::uintmax_t remove_all(const fs::path& path, std::error_code& ec) override
+        virtual std::uintmax_t remove_all(const fs::path& path, std::error_code& ec, fs::path& failure_point) override
         {
             /*
                 does not use the std::filesystem call since it is buggy, and can
@@ -311,8 +340,10 @@ namespace vcpkg::Files
                 and then inserts `actually_remove{current_path}` into the work
                 queue.
             */
-            struct remove {
-                struct tld {
+            struct remove
+            {
+                struct tld
+                {
                     const fs::path& tmp_directory;
                     std::uint64_t index;
 
@@ -320,6 +351,7 @@ namespace vcpkg::Files
 
                     std::mutex& ec_mutex;
                     std::error_code& ec;
+                    fs::path& failure_point;
                 };
 
                 struct actually_remove;
@@ -331,52 +363,68 @@ namespace vcpkg::Files
 
                     else, just calls remove.
                 */
-                struct actually_remove {
+                struct actually_remove
+                {
                     fs::path current_path;
 
-                    void operator()(tld& info, const queue& queue) const {
+                    void operator()(tld& info, const queue& queue) const
+                    {
                         std::error_code ec;
                         const auto path_type = fs::symlink_status(current_path, ec).type();
 
-                        if (check_ec(ec, info, queue)) return;
+                        if (check_ec(ec, info, queue, current_path)) return;
 
-                        if (path_type == fs::file_type::directory) {
-                            for (const auto& entry : fs::stdfs::directory_iterator(current_path)) {
+                        if (path_type == fs::file_type::directory)
+                        {
+                            for (const auto& entry : fs::stdfs::directory_iterator(current_path))
+                            {
                                 remove{}(entry, info, queue);
                             }
                         }
 
-                        if (fs::stdfs::remove(current_path, ec)) {
+                        if (fs::stdfs::remove(current_path, ec))
+                        {
                             info.files_deleted.fetch_add(1, std::memory_order_relaxed);
-                        } else {
-                            check_ec(ec, info, queue);
+                        }
+                        else
+                        {
+                            check_ec(ec, info, queue, current_path);
                         }
                     }
                 };
 
-                static bool check_ec(const std::error_code& ec, tld& info, const queue& queue) {
-                    if (ec) {
+                static bool check_ec(const std::error_code& ec,
+                                     tld& info,
+                                     const queue& queue,
+                                     const fs::path& failure_point)
+                {
+                    if (ec)
+                    {
                         queue.terminate();
 
                         auto lck = std::unique_lock<std::mutex>(info.ec_mutex);
-                        if (!info.ec) {
+                        if (!info.ec)
+                        {
                             info.ec = ec;
                         }
 
                         return true;
-                    } else {
+                    }
+                    else
+                    {
                         return false;
                     }
                 }
 
-                void operator()(const fs::path& current_path, tld& info, const queue& queue) const {
+                void operator()(const fs::path& current_path, tld& info, const queue& queue) const
+                {
                     std::error_code ec;
 
                     const auto tmp_name = Strings::b64url_encode(info.index++);
                     const auto tmp_path = info.tmp_directory / tmp_name;
 
                     fs::stdfs::rename(current_path, tmp_path, ec);
-                    if (check_ec(ec, info, queue)) return;
+                    if (check_ec(ec, info, queue, current_path)) return;
 
                     queue.enqueue_action(actually_remove{std::move(tmp_path)});
                 }
@@ -386,22 +434,28 @@ namespace vcpkg::Files
 
             std::atomic<std::uintmax_t> files_deleted{0};
 
-            if (path_type == fs::file_type::directory) {
+            if (path_type == fs::file_type::directory)
+            {
                 std::uint64_t index = 0;
                 std::mutex ec_mutex;
 
-                remove::queue queue{[&] {
+                auto const tld_gen = [&] {
                     index += static_cast<std::uint64_t>(1) << 32;
-                    return remove::tld{path, index, files_deleted, ec_mutex, ec};
-                }};
+                    return remove::tld{path, index, files_deleted, ec_mutex, ec, failure_point};
+                };
 
-                index += static_cast<std::uint64_t>(1) << 32;
-                auto main_tld = remove::tld{path, index, files_deleted, ec_mutex, ec};
-                for (const auto& entry : fs::stdfs::directory_iterator(path)) {
+                remove::queue queue{4, VCPKG_LINE_INFO, tld_gen};
+
+                // note: we don't actually start the queue running until the
+                // `join()`. This allows us to rename all the top-level files in
+                // peace, so that we don't get collisions.
+                auto main_tld = tld_gen();
+                for (const auto& entry : fs::stdfs::directory_iterator(path))
+                {
                     remove{}(entry, main_tld, queue);
                 }
 
-                queue.join();
+                queue.join(VCPKG_LINE_INFO);
             }
 
             /*
@@ -410,14 +464,17 @@ namespace vcpkg::Files
                 directory, and so we can only delete the directory after all the
                 lower levels have been deleted.
             */
-            for (int backoff = 0; backoff < 5; ++backoff) {
-                if (backoff) {
+            for (int backoff = 0; backoff < 5; ++backoff)
+            {
+                if (backoff)
+                {
                     using namespace std::chrono_literals;
                     auto backoff_time = 100ms * backoff;
                     std::this_thread::sleep_for(backoff_time);
                 }
 
-                if (fs::stdfs::remove(path, ec)) {
+                if (fs::stdfs::remove(path, ec))
+                {
                     files_deleted.fetch_add(1, std::memory_order_relaxed);
                     break;
                 }
diff --git a/toolsrc/src/vcpkg/base/strings.cpp b/toolsrc/src/vcpkg/base/strings.cpp
index 54a74a7a1..16543046e 100644
--- a/toolsrc/src/vcpkg/base/strings.cpp
+++ b/toolsrc/src/vcpkg/base/strings.cpp
@@ -288,3 +288,45 @@ bool Strings::contains(StringView haystack, StringView needle)
 {
     return Strings::search(haystack, needle) != haystack.end();
 }
+
+namespace vcpkg::Strings::detail {
+
+    template <class Integral>
+    std::string b64url_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[0x41] =
+            /*     0123456789ABCDEF */
+            /*0*/ "ABCDEFGHIJKLMNOP"
+            /*1*/ "QRSTUVWXYZabcdef"
+            /*2*/ "ghijklmnopqrstuv"
+            /*3*/ "wxyz0123456789-_"
+        ;
+
+        constexpr static int shift = 5;
+        constexpr static auto mask = (static_cast<Unsigned>(1) << shift) - 1;
+
+        std::string result;
+        // reserve ceiling(number of bits / 3)
+        result.resize((sizeof(value) * 8 + 2) / 3, map[0]);
+
+        for (char& c: result) {
+            if (value == 0) {
+                break;
+            }
+            c = map[value & mask];
+            value >>= shift;
+        }
+
+        return result;
+    }
+
+    std::string b64url_encode_t::operator()(std::uint64_t x) const noexcept{
+        return b64url_encode_implementation(x);
+    }
+
+}
+
diff --git a/toolsrc/src/vcpkg/build.cpp b/toolsrc/src/vcpkg/build.cpp
index 68df1f965..c50acce7e 100644
--- a/toolsrc/src/vcpkg/build.cpp
+++ b/toolsrc/src/vcpkg/build.cpp
@@ -363,7 +363,8 @@ namespace vcpkg::Build
         const Triplet& triplet = spec.triplet();

         const auto& triplet_file_path = paths.get_triplet_file_path(spec.triplet()).u8string();

 

-        if (!Strings::case_insensitive_ascii_starts_with(triplet_file_path, paths.triplets.u8string()))

+        if (!Strings::case_insensitive_ascii_starts_with(triplet_file_path,

+                                                         paths.triplets.u8string()))

         {

             System::printf("-- Loading triplet configuration from: %s\n", triplet_file_path);

         }

@@ -495,7 +496,8 @@ namespace vcpkg::Build
                 if (fs.is_directory(file)) // Will only keep the logs

                 {

                     std::error_code ec;

-                    fs.remove_all(file, ec);

+                    fs::path failure_point;

+                    fs.remove_all(file, ec, failure_point);

                 }

             }

         }

@@ -610,8 +612,8 @@ namespace vcpkg::Build
         auto& fs = paths.get_filesystem();

 

         auto pkg_path = paths.package_dir(spec);

+        fs.remove_all(pkg_path, VCPKG_LINE_INFO);

         std::error_code ec;

-        fs.remove_all(pkg_path, ec);

         fs.create_directories(pkg_path, ec);

         auto files = fs.get_files_non_recursive(pkg_path);

         Checks::check_exit(VCPKG_LINE_INFO, files.empty(), "unable to clear path: %s", pkg_path.u8string());

@@ -794,7 +796,7 @@ namespace vcpkg::Build
                     fs.rename_or_copy(tmp_failure_zip, archive_tombstone_path, ".tmp", ec);

 

                     // clean up temporary directory

-                    fs.remove_all(tmp_log_path, ec);

+                    fs.remove_all(tmp_log_path, VCPKG_LINE_INFO);

                 }

             }

 

@@ -1018,7 +1020,7 @@ namespace vcpkg::Build
                 hash += "-";

                 hash += Hash::get_file_hash(fs, *p, "SHA1");

             }

-            else if (pre_build_info.cmake_system_name.empty() || 

+            else if (pre_build_info.cmake_system_name.empty() ||

                      pre_build_info.cmake_system_name == "WindowsStore")

             {

                 hash += "-";

diff --git a/toolsrc/src/vcpkg/commands.exportifw.cpp b/toolsrc/src/vcpkg/commands.exportifw.cpp
index f0946110c..3d963a297 100644
--- a/toolsrc/src/vcpkg/commands.exportifw.cpp
+++ b/toolsrc/src/vcpkg/commands.exportifw.cpp
@@ -352,13 +352,15 @@ namespace vcpkg::Export::IFW
         System::print2("Generating repository ", repository_dir.generic_u8string(), "...\n");
 
         std::error_code ec;
+        fs::path failure_point;
         Files::Filesystem& fs = paths.get_filesystem();
 
-        fs.remove_all(repository_dir, ec);
+        fs.remove_all(repository_dir, ec, failure_point);
         Checks::check_exit(VCPKG_LINE_INFO,
                            !ec,
-                           "Could not remove outdated repository directory %s",
-                           repository_dir.generic_u8string());
+                           "Could not remove outdated repository directory %s due to file %s",
+                           repository_dir.generic_u8string(),
+                           failure_point.string());
 
         const auto cmd_line = Strings::format(R"("%s" --packages "%s" "%s" > nul)",
                                               repogen_exe.u8string(),
@@ -414,16 +416,18 @@ namespace vcpkg::Export::IFW
                    const VcpkgPaths& paths)
     {
         std::error_code ec;
+        fs::path failure_point;
         Files::Filesystem& fs = paths.get_filesystem();
 
         // Prepare packages directory
         const fs::path ifw_packages_dir_path = get_packages_dir_path(export_id, ifw_options, paths);
 
-        fs.remove_all(ifw_packages_dir_path, ec);
+        fs.remove_all(ifw_packages_dir_path, ec, failure_point);
         Checks::check_exit(VCPKG_LINE_INFO,
                            !ec,
-                           "Could not remove outdated packages directory %s",
-                           ifw_packages_dir_path.generic_u8string());
+                           "Could not remove outdated packages directory %s due to file %s",
+                           ifw_packages_dir_path.generic_u8string(),
+                           failure_point.string());
 
         fs.create_directory(ifw_packages_dir_path, ec);
         Checks::check_exit(
diff --git a/toolsrc/src/vcpkg/commands.portsdiff.cpp b/toolsrc/src/vcpkg/commands.portsdiff.cpp
index b30c38f43..cddc274b8 100644
--- a/toolsrc/src/vcpkg/commands.portsdiff.cpp
+++ b/toolsrc/src/vcpkg/commands.portsdiff.cpp
@@ -105,7 +105,7 @@ namespace vcpkg::Commands::PortsDiff
         std::map<std::string, VersionT> names_and_versions;
         for (auto&& port : all_ports)
             names_and_versions.emplace(port->core_paragraph->name, port->core_paragraph->version);
-        fs.remove_all(temp_checkout_path, ec);
+        fs.remove_all(temp_checkout_path, VCPKG_LINE_INFO);
         return names_and_versions;
     }
 
diff --git a/toolsrc/src/vcpkg/export.cpp b/toolsrc/src/vcpkg/export.cpp
index 88c1526c5..f306bf4e6 100644
--- a/toolsrc/src/vcpkg/export.cpp
+++ b/toolsrc/src/vcpkg/export.cpp
@@ -400,8 +400,10 @@ namespace vcpkg::Export
         Files::Filesystem& fs = paths.get_filesystem();
         const fs::path export_to_path = paths.root;
         const fs::path raw_exported_dir_path = export_to_path / export_id;
+        fs.remove_all(raw_exported_dir_path, VCPKG_LINE_INFO);
+
+        // TODO: error handling
         std::error_code ec;
-        fs.remove_all(raw_exported_dir_path, ec);
         fs.create_directory(raw_exported_dir_path, ec);
 
         // execute the plan
@@ -476,7 +478,7 @@ With a project open, go to Tools->NuGet Package Manager->Package Manager Console
 
         if (!opts.raw)
         {
-            fs.remove_all(raw_exported_dir_path, ec);
+            fs.remove_all(raw_exported_dir_path, VCPKG_LINE_INFO);
         }
     }
 
diff --git a/toolsrc/src/vcpkg/install.cpp b/toolsrc/src/vcpkg/install.cpp
index de19c360a..981a9233f 100644
--- a/toolsrc/src/vcpkg/install.cpp
+++ b/toolsrc/src/vcpkg/install.cpp
@@ -355,8 +355,7 @@ namespace vcpkg::Install
             {
                 auto& fs = paths.get_filesystem();
                 const fs::path package_dir = paths.package_dir(action.spec);
-                std::error_code ec;
-                fs.remove_all(package_dir, ec);
+                fs.remove_all(package_dir, VCPKG_LINE_INFO);
             }
 
             if (action.build_options.clean_downloads == Build::CleanDownloads::YES)
diff --git a/toolsrc/src/vcpkg/remove.cpp b/toolsrc/src/vcpkg/remove.cpp
index a40b27bd7..84ec6c981 100644
--- a/toolsrc/src/vcpkg/remove.cpp
+++ b/toolsrc/src/vcpkg/remove.cpp
@@ -179,8 +179,7 @@ namespace vcpkg::Remove
         {
             System::printf("Purging package %s...\n", display_name);
             Files::Filesystem& fs = paths.get_filesystem();
-            std::error_code ec;
-            fs.remove_all(paths.packages / action.spec.dir(), ec);
+            fs.remove_all(paths.packages / action.spec.dir(), VCPKG_LINE_INFO);
             System::printf(System::Color::success, "Purging package %s... done\n", display_name);
         }
     }