From 3b0080e3b0d395de7551cc2ce1ec89189e539eab Mon Sep 17 00:00:00 2001 From: Billy O'Neal Date: Tue, 19 May 2020 13:43:30 -0700 Subject: [PATCH] [vcpkg] Harden file removals and clean directory contents in "CI" inside vcpkg itself. (#11432) `files.h/files.cpp`: * Add end and else comments to all macros. * Add "remove_all_inside" function which empties a directory without actually deleting the directory. This is necessary to handle the case where the directory is actually a directory symlink. * Change remove_all to use std::remove when VCPKG_USE_STD_FILESYSTEM is set; this will engage POSIX delete support available on current Win10. `commands.ci.cpp`: empty "installed". `*/initialize_environment.*`: No longer clean the directories outside the tool. `ci-step.ps1`: Remove unused console output tee-ing. --- .../linux/initialize-environment.sh | 4 - .../osx/initialize-environment.sh | 9 -- .../windows/azure-pipelines.yml | 6 - scripts/azure-pipelines/windows/ci-step.ps1 | 6 +- .../windows/initialize-environment.ps1 | 29 ++-- toolsrc/include/vcpkg/base/files.h | 3 + toolsrc/src/vcpkg/base/files.cpp | 139 ++++++++++++++---- toolsrc/src/vcpkg/commands.ci.cpp | 6 + 8 files changed, 138 insertions(+), 64 deletions(-) diff --git a/scripts/azure-pipelines/linux/initialize-environment.sh b/scripts/azure-pipelines/linux/initialize-environment.sh index 5346345a5..1cbdd3326 100755 --- a/scripts/azure-pipelines/linux/initialize-environment.sh +++ b/scripts/azure-pipelines/linux/initialize-environment.sh @@ -6,7 +6,3 @@ if [ ! -d "archives" ]; then ln -s /archives archives fi - -rm -rf installed -rm -rf buildtrees -rm -rf packages/* diff --git a/scripts/azure-pipelines/osx/initialize-environment.sh b/scripts/azure-pipelines/osx/initialize-environment.sh index 5eeba4255..6f42a809a 100755 --- a/scripts/azure-pipelines/osx/initialize-environment.sh +++ b/scripts/azure-pipelines/osx/initialize-environment.sh @@ -4,29 +4,20 @@ # Sets up the environment for MacOS runs of vcpkg CI -rm -rf installed || true mkdir -p ~/Data/installed || true ln -s ~/Data/installed -rm -rf ~/Data/installed/* || true -rm -rf buildtrees || true mkdir -p ~/Data/buildtrees || true ln -s ~/Data/buildtrees -rm -rf ~/Data/buildtrees/* || true -rm -rf packages || true mkdir -p ~/Data/packages || true ln -s ~/Data/packages -rm -rf ~/Data/packages/* || true rm archives || rm -rf archives || true ln -s ~/Data/archives -rm -rf downloads || true mkdir -p ~/Data/downloads || true ln -s ~/Data/downloads -if [ -d downloads/ ]; then #delete downloaded files that have not been used in 7 days find downloads/ -maxdepth 1 -type f ! -atime 7 -exec rm -f {} \; -fi diff --git a/scripts/azure-pipelines/windows/azure-pipelines.yml b/scripts/azure-pipelines/windows/azure-pipelines.yml index 71ffc7828..9d4de6794 100644 --- a/scripts/azure-pipelines/windows/azure-pipelines.yml +++ b/scripts/azure-pipelines/windows/azure-pipelines.yml @@ -41,12 +41,6 @@ jobs: cp $xmlPath $(Build.ArtifactStagingDirectory) Move-Item $xmlPath -Destination $outputXmlPath - # already in DevOps, no need for extra copies - rm $(System.DefaultWorkingDirectory)\console-out.txt -ErrorAction Ignore - - Remove-Item "$(System.DefaultWorkingDirectory)\buildtrees\*" -Recurse -errorAction silentlycontinue - Remove-Item "$(System.DefaultWorkingDirectory)\packages\*" -Recurse -errorAction silentlycontinue - Remove-Item "$(System.DefaultWorkingDirectory)\installed\*" -Recurse -errorAction silentlycontinue displayName: 'Collect logs and cleanup build' - task: PowerShell@2 diff --git a/scripts/azure-pipelines/windows/ci-step.ps1 b/scripts/azure-pipelines/windows/ci-step.ps1 index 0e07895e0..f0aee4dfd 100644 --- a/scripts/azure-pipelines/windows/ci-step.ps1 +++ b/scripts/azure-pipelines/windows/ci-step.ps1 @@ -125,14 +125,13 @@ if (!$?) { throw "bootstrap failed" } Write-Host "Bootstrapping vcpkg ... done." $ciXmlPath = "$vcpkgRootDir\test-full-ci.xml" -$consoleOuputPath = "$vcpkgRootDir\console-out.txt" Remove-VcpkgItem $ciXmlPath $env:VCPKG_FEATURE_FLAGS = "binarycaching" if (![string]::IsNullOrEmpty($OnlyIncludePorts)) { ./vcpkg install --triplet $Triplet $OnlyIncludePorts $AdditionalVcpkgFlags ` - "--x-xunit=$ciXmlPath" | Tee-Object -FilePath "$consoleOuputPath" + "--x-xunit=$ciXmlPath" } else { $exclusions = "" @@ -155,9 +154,6 @@ else { | ForEach-Object { if ($_ -is [System.Management.Automation.ErrorRecord]) { $_.ToString() } else { $_ } } - - # Phasing out the console output (it is already saved in DevOps) Create a dummy file for now. - Set-Content -LiteralPath "$consoleOuputPath" -Value '' } Write-Host "CI test is complete" diff --git a/scripts/azure-pipelines/windows/initialize-environment.ps1 b/scripts/azure-pipelines/windows/initialize-environment.ps1 index 4bbc15665..8b1d8e8e4 100644 --- a/scripts/azure-pipelines/windows/initialize-environment.ps1 +++ b/scripts/azure-pipelines/windows/initialize-environment.ps1 @@ -68,21 +68,24 @@ else } Write-Host "Linking archives => $archivesPath" -Remove-DirectorySymlink archives -cmd /c "mklink /D archives $archivesPath" +if (-Not (Test-Path archives)) { + cmd /c "mklink /D archives $archivesPath" +} Write-Host 'Linking installed => E:\installed' -Remove-DirectorySymlink installed -Remove-Item E:\installed -Recurse -Force -ErrorAction SilentlyContinue -mkdir E:\installed -cmd /c "mklink /D installed E:\installed" +if (-Not (Test-Path E:\installed)) { + mkdir E:\installed +} + +if (-Not (Test-Path installed)) { + cmd /c "mklink /D installed E:\installed" +} Write-Host 'Linking downloads => D:\downloads' -Remove-DirectorySymlink downloads -cmd /c "mklink /D downloads D:\downloads" +if (-Not (Test-Path D:\downloads)) { + mkdir D:\downloads +} -Write-Host 'Cleaning buildtrees' -Remove-Item buildtrees\* -Recurse -Force -errorAction silentlycontinue - -Write-Host 'Cleaning packages' -Remove-Item packages\* -Recurse -Force -errorAction silentlycontinue +if (-Not (Test-Path downloads)) { + cmd /c "mklink /D downloads D:\downloads" +} diff --git a/toolsrc/include/vcpkg/base/files.h b/toolsrc/include/vcpkg/base/files.h index 8f278cd20..0fba3a55f 100644 --- a/toolsrc/include/vcpkg/base/files.h +++ b/toolsrc/include/vcpkg/base/files.h @@ -134,6 +134,9 @@ namespace vcpkg::Files virtual void remove_all(const fs::path& path, std::error_code& ec, fs::path& failure_point) = 0; void remove_all(const fs::path& path, LineInfo li); void remove_all(const fs::path& path, ignore_errors_t); + virtual void remove_all_inside(const fs::path& path, std::error_code& ec, fs::path& failure_point) = 0; + void remove_all_inside(const fs::path& path, LineInfo li); + void remove_all_inside(const fs::path& path, ignore_errors_t); bool exists(const fs::path& path, std::error_code& ec) const; bool exists(LineInfo li, const fs::path& path) const; bool exists(const fs::path& path, ignore_errors_t = ignore_errors) const; diff --git a/toolsrc/src/vcpkg/base/files.cpp b/toolsrc/src/vcpkg/base/files.cpp index 2d12b1757..24e2038d9 100644 --- a/toolsrc/src/vcpkg/base/files.cpp +++ b/toolsrc/src/vcpkg/base/files.cpp @@ -13,12 +13,12 @@ #include #include #include -#endif +#endif // ^^^ !defined(_WIN32) #if defined(__linux__) #include #elif defined(__APPLE__) #include -#endif +#endif // ^^^ defined(__APPLE__) namespace vcpkg::Files { @@ -80,7 +80,7 @@ namespace vcpkg::Files return fs::file_status(ft, permissions); -#else +#else // ^^^ defined(_WIN32) // !defined(_WIN32) vvv auto result = follow_symlinks ? fs::stdfs::status(p, ec) : fs::stdfs::symlink_status(p, ec); // libstdc++ doesn't correctly not-set ec on nonexistent paths if (ec.value() == ENOENT || ec.value() == ENOTDIR) @@ -89,7 +89,7 @@ namespace vcpkg::Files return fs::file_status(file_type::not_found, perms::unknown); } return fs::file_status(result.type(), result.permissions()); -#endif +#endif// ^^^ !defined(_WIN32) } fs::file_status status(const fs::path& p, std::error_code& ec) noexcept @@ -119,7 +119,7 @@ namespace vcpkg::Files { ec.assign(GetLastError(), std::system_category()); } -#else +#else // ^^^ defined(_WIN32) // !defined(_WIN32) vvv struct stat s; if (lstat(path.c_str(), &s)) { @@ -137,7 +137,7 @@ namespace vcpkg::Files ec.assign(errno, std::system_category()); } } -#endif +#endif // ^^^ !defined(_WIN32) } } @@ -272,6 +272,31 @@ namespace vcpkg::Files this->remove_all(path, ec, failure_point); } + void Filesystem::remove_all_inside(const fs::path& path, LineInfo li) + { + std::error_code ec; + fs::path failure_point; + + this->remove_all_inside(path, ec, failure_point); + + if (ec) + { + Checks::exit_with_message(li, + "Failure to remove_all_inside(%s) due to file %s: %s", + path.string(), + failure_point.string(), + ec.message()); + } + } + + void Filesystem::remove_all_inside(const fs::path& path, ignore_errors_t) + { + std::error_code ec; + fs::path failure_point; + + this->remove_all_inside(path, ec, failure_point); + } + fs::path Filesystem::absolute(LineInfo li, const fs::path& path) const { std::error_code ec; @@ -477,7 +502,7 @@ namespace vcpkg::Files auto written_bytes = sendfile(o_fd, i_fd, &bytes, info.st_size); #elif defined(__APPLE__) auto written_bytes = fcopyfile(i_fd, o_fd, 0, COPYFILE_ALL); -#else +#else // ^^^ defined(__APPLE__) // !(defined(__APPLE__) || defined(__linux__)) vvv ssize_t written_bytes = 0; { constexpr std::size_t buffer_length = 4096; @@ -505,7 +530,7 @@ namespace vcpkg::Files copy_failure:; } -#endif +#endif // ^^^ !(defined(__APPLE__) || defined(__linux__)) if (written_bytes == -1) { ec.assign(errno, std::generic_category()); @@ -522,7 +547,7 @@ namespace vcpkg::Files if (ec) return; this->remove(oldpath, ec); } -#endif +#endif // ^^^ !defined(_WIN32) } virtual bool remove(const fs::path& path, std::error_code& ec) override { return fs::stdfs::remove(path, ec); } virtual void remove_all(const fs::path& path, std::error_code& ec, fs::path& failure_point) override @@ -574,13 +599,20 @@ namespace vcpkg::Files { ec.assign(GetLastError(), std::system_category()); } -#else +#else // ^^^ defined(_WIN32) // !defined(_WIN32) vvv if (rmdir(current_path.c_str())) { ec.assign(errno, std::system_category()); } -#endif +#endif // ^^^ !defined(_WIN32) } +#if VCPKG_USE_STD_FILESYSTEM + else + { + fs::stdfs::remove(current_path, ec); + if (check_ec(ec, current_path, err)) return; + } +#else // ^^^ VCPKG_USE_STD_FILESYSTEM // !VCPKG_USE_STD_FILESYSTEM vvv #if defined(_WIN32) else if (path_type == fs::file_type::directory_symlink) { @@ -596,7 +628,7 @@ namespace vcpkg::Files ec.assign(GetLastError(), std::system_category()); } } -#else +#else // ^^^ defined(_WIN32) // !defined(_WIN32) vvv else { if (unlink(current_path.c_str())) @@ -604,7 +636,8 @@ namespace vcpkg::Files ec.assign(errno, std::system_category()); } } -#endif +#endif // ^^^ !defined(_WIN32) +#endif // ^^^ !VCPKG_USE_STD_FILESYSTEM check_ec(ec, current_path, err); } @@ -651,6 +684,58 @@ namespace vcpkg::Files ec = std::move(err.ec); failure_point = std::move(err.failure_point); } + + virtual void remove_all_inside(const fs::path& path, std::error_code& ec, fs::path& failure_point) override + { + fs::directory_iterator last{}; + fs::directory_iterator first(path, ec); + if (ec) + { + failure_point = path; + return; + } + + for (;;) + { + if (first == last) + { + return; + } + + auto stats = first->status(ec); + if (ec) + { + break; + } + + auto& thisPath = first->path(); + if (stats.type() == fs::stdfs::file_type::directory) + { + this->remove_all(thisPath, ec, failure_point); + if (ec) + { + return; // keep inner failure_point + } + } + else + { + this->remove(thisPath, ec); + if (ec) + { + break; + } + } + + first.increment(ec); + if (ec) + { + break; + } + } + + failure_point = first->path(); + } + virtual bool is_directory(const fs::path& path) const override { return fs::stdfs::is_directory(path); } virtual bool is_regular_file(const fs::path& path) const override { return fs::stdfs::is_regular_file(path); } virtual bool is_empty(const fs::path& path) const override { return fs::stdfs::is_empty(path); } @@ -693,10 +778,10 @@ namespace vcpkg::Files FILE* f = nullptr; #if defined(_WIN32) auto err = _wfopen_s(&f, file_path.native().c_str(), L"wb"); -#else +#else // ^^^ defined(_WIN32) // !defined(_WIN32) vvv f = fopen(file_path.native().c_str(), "wb"); int err = f != nullptr ? 0 : 1; -#endif +#endif // ^^^ !defined(_WIN32) if (err != 0) { ec.assign(err, std::system_category()); @@ -720,19 +805,22 @@ namespace vcpkg::Files #if VCPKG_USE_STD_FILESYSTEM return fs::stdfs::absolute(path, ec); #else // ^^^ VCPKG_USE_STD_FILESYSTEM / !VCPKG_USE_STD_FILESYSTEM vvv -#if _WIN32 +#if defined(_WIN32) // absolute was called system_complete in experimental filesystem return fs::stdfs::system_complete(path, ec); -#else // ^^^ _WIN32 / !_WIN32 vvv - if (path.is_absolute()) { +#else // ^^^ defined(_WIN32) / !defined(_WIN32) vvv + if (path.is_absolute()) + { return path; - } else { + } + else + { auto current_path = this->current_path(ec); if (ec) return fs::path(); return std::move(current_path) / path; } -#endif -#endif +#endif // ^^^ !defined(_WIN32) +#endif // ^^^ !VCPKG_USE_STD_FILESYSTEM } virtual fs::path canonical(const fs::path& path, std::error_code& ec) const override @@ -740,20 +828,17 @@ namespace vcpkg::Files return fs::stdfs::canonical(path, ec); } - virtual fs::path current_path(std::error_code& ec) const override - { - return fs::stdfs::current_path(ec); - } + virtual fs::path current_path(std::error_code& ec) const override { return fs::stdfs::current_path(ec); } virtual std::vector find_from_PATH(const std::string& name) const override { #if defined(_WIN32) static constexpr StringLiteral EXTS[] = {".cmd", ".exe", ".bat"}; auto paths = Strings::split(System::get_environment_variable("PATH").value_or_exit(VCPKG_LINE_INFO), ";"); -#else +#else // ^^^ defined(_WIN32) // !defined(_WIN32) vvv static constexpr StringLiteral EXTS[] = {""}; auto paths = Strings::split(System::get_environment_variable("PATH").value_or_exit(VCPKG_LINE_INFO), ":"); -#endif +#endif // ^^^ !defined(_WIN32) std::vector ret; std::error_code ec; diff --git a/toolsrc/src/vcpkg/commands.ci.cpp b/toolsrc/src/vcpkg/commands.ci.cpp index 0ea23cd82..eadfd8bdd 100644 --- a/toolsrc/src/vcpkg/commands.ci.cpp +++ b/toolsrc/src/vcpkg/commands.ci.cpp @@ -397,6 +397,12 @@ namespace vcpkg::Commands::CI const ParsedArguments options = args.parse_arguments(COMMAND_STRUCTURE); + auto& filesystem = paths.get_filesystem(); + if (filesystem.is_directory(paths.installed)) + { + filesystem.remove_all_inside(paths.installed, VCPKG_LINE_INFO); + } + std::set exclusions_set; auto it_exclusions = options.settings.find(OPTION_EXCLUDE); if (it_exclusions != options.settings.end())