[vcpkg] Apply fixes needed to extract vcpkg-tool

utilities.cmake: Disable warnings that are firing on Azure Pipelines CI machines due to different Clang version. expected.h: Fix a bug I introduced in microsoft#15638 because I was under the impression expected worked like optional (in that value_or_exit for it should be treated as a program bug) build.cpp: Add (void)s to silence warnings. binarycaching.cpp: Repair assumption that the current directory is C: which isn't true on the Hosted Azure Pipelines agents. others: Make unit tests respect %VCPKG_ROOT%, as necessary in the vcpkg_tool repo. Note that this required splitting vcpkgcmdarguments::ImbueFromEnvironment into the once-only process modifying part and the just imbue from environment part.
BillyONeal · Feb 3, 2021 · 5205184 · 5205184
1 parent 401b26c
commit 5205184
Show file tree

Hide file tree

Showing 9 changed files with 77 additions and 64 deletions.
diff --git a/toolsrc/cmake/utilities.cmake b/toolsrc/cmake/utilities.cmake
@@ -204,7 +204,13 @@ function(vcpkg_target_add_warning_options TARGET)
         if(VCPKG_DEVELOPMENT_WARNINGS)
             target_compile_options(${TARGET} PRIVATE -W4)
             if(VCPKG_COMPILER STREQUAL "clang")
-                target_compile_options(${TARGET} PRIVATE -Wmissing-prototypes -Wno-missing-field-initializers)
+                # -Wno-range-loop-analysis is due to an LLVM bug which will be fixed in a
+                # future version of clang https://reviews.llvm.org/D73007
+                target_compile_options(${TARGET} PRIVATE
+                    -Wmissing-prototypes
+                    -Wno-missing-field-initializers
+                    -Wno-range-loop-analysis
+                    )
             else()
                 target_compile_options(${TARGET} PRIVATE -analyze)
             endif()
@@ -219,13 +225,21 @@ function(vcpkg_target_add_warning_options TARGET)
         if(VCPKG_DEVELOPMENT_WARNINGS)
             target_compile_options(${TARGET} PRIVATE
                 -Wall -Wextra -Wpedantic
-                -Wno-unknown-pragmas -Wno-missing-field-initializers -Wno-redundant-move)
+                -Wno-unknown-pragmas
+                -Wno-missing-field-initializers
+                -Wno-redundant-move
+                )
 
             # GCC and clang have different names for the same warning
             if(VCPKG_COMPILER STREQUAL "gcc")
-                target_compile_options(${TARGET} PRIVATE -Wmissing-declarations)
+                target_compile_options(${TARGET} PRIVATE
+                    -Wmissing-declarations
+                    )
             elseif(VCPKG_COMPILER STREQUAL "clang")
-                target_compile_options(${TARGET} PRIVATE -Wmissing-prototypes)
+                target_compile_options(${TARGET} PRIVATE
+                    -Wmissing-prototypes
+                    -Wno-range-loop-analysis
+                    )
             endif()
         endif()
 

diff --git a/toolsrc/include/vcpkg/base/expected.h b/toolsrc/include/vcpkg/base/expected.h
@@ -229,7 +229,7 @@ namespace vcpkg
             if (m_s.has_error())
             {
                 System::print2(System::Color::error, m_s.to_string(), "\n");
-                Checks::unreachable(line_info);
+                Checks::exit_fail(line_info);
             }
         }
 

diff --git a/toolsrc/include/vcpkg/vcpkgcmdarguments.h b/toolsrc/include/vcpkg/vcpkgcmdarguments.h
@@ -212,7 +212,6 @@ namespace vcpkg
         }
 
         bool output_json() const { return json.value_or(false); }
-        bool is_recursive_invocation() const { return m_is_recursive_invocation; }
 
         std::string command;
         std::vector<std::string> command_arguments;
@@ -221,13 +220,16 @@ namespace vcpkg
 
         void imbue_from_environment();
 
+        // Applies recursive settings from the environment or sets a global environment variable
+        // to be consumed by subprocesses; may only be called once per process.
+        static void imbue_or_apply_process_recursion(VcpkgCmdArguments& args);
+
         void check_feature_flag_consistency() const;
 
         void debug_print_feature_flags() const;
         void track_feature_flag_metrics() const;
 
     private:
-        bool m_is_recursive_invocation = false;
         std::unordered_set<std::string> command_switches;
         std::unordered_map<std::string, std::vector<std::string>> command_options;
     };

diff --git a/toolsrc/src/vcpkg-test/binarycaching.cpp b/toolsrc/src/vcpkg-test/binarycaching.cpp
@@ -58,7 +58,11 @@ TEST_CASE ("generate_nuspec", "[generate_nuspec]")
 {
     auto& fsWrapper = Files::get_real_filesystem();
     VcpkgCmdArguments args = VcpkgCmdArguments::create_from_arg_sequence(nullptr, nullptr);
+    args.imbue_from_environment();
     args.packages_root_dir = std::make_unique<std::string>("/");
+    auto pkgPath = fsWrapper.absolute(VCPKG_LINE_INFO, fs::u8path("/zlib2_x64-windows")) / fs::u8path("**");
+    pkgPath.make_preferred();
+    const auto pkgPathStr = fs::u8string(pkgPath);
     VcpkgPaths paths(fsWrapper, args);
 
     auto pghs = Paragraphs::parse_paragraphs(R"(
@@ -101,11 +105,6 @@ Build-Depends: bzip
 
     {
         auto nuspec = generate_nuspec(paths, ipa, ref, {});
-#ifdef _WIN32
-#define PKGPATH "C:\\zlib2_x64-windows\\**"
-#else
-#define PKGPATH "/zlib2_x64-windows/**"
-#endif
         std::string expected = R"(<package>
   <metadata>
     <id>zlib2_x64-windows</id>
@@ -125,19 +124,15 @@ Features: a, b
 </description>
     <packageTypes><packageType name="vcpkg"/></packageTypes>
   </metadata>
-  <files><file src=")" PKGPATH R"(" target=""/></files>
+  <files><file src=")" + pkgPathStr +
+                               R"(" target=""/></files>
 </package>
 )";
         REQUIRE_EQUAL_TEXT(nuspec, expected);
     }
 
     {
         auto nuspec = generate_nuspec(paths, ipa, ref, {"urlvalue"});
-#ifdef _WIN32
-#define PKGPATH "C:\\zlib2_x64-windows\\**"
-#else
-#define PKGPATH "/zlib2_x64-windows/**"
-#endif
         std::string expected = R"(<package>
   <metadata>
     <id>zlib2_x64-windows</id>
@@ -158,18 +153,14 @@ Features: a, b
     <packageTypes><packageType name="vcpkg"/></packageTypes>
     <repository type="git" url="urlvalue"/>
   </metadata>
-  <files><file src=")" PKGPATH R"(" target=""/></files>
+  <files><file src=")" + pkgPathStr +
+                               R"(" target=""/></files>
 </package>
 )";
         REQUIRE_EQUAL_TEXT(nuspec, expected);
     }
     {
         auto nuspec = generate_nuspec(paths, ipa, ref, {"urlvalue", "branchvalue", "commitvalue"});
-#ifdef _WIN32
-#define PKGPATH "C:\\zlib2_x64-windows\\**"
-#else
-#define PKGPATH "/zlib2_x64-windows/**"
-#endif
         std::string expected = R"(<package>
   <metadata>
     <id>zlib2_x64-windows</id>
@@ -190,7 +181,8 @@ Features: a, b
     <packageTypes><packageType name="vcpkg"/></packageTypes>
     <repository type="git" url="urlvalue" branch="branchvalue" commit="commitvalue"/>
   </metadata>
-  <files><file src=")" PKGPATH R"(" target=""/></files>
+  <files><file src=")" + pkgPathStr +
+                               R"(" target=""/></files>
 </package>
 )";
         REQUIRE_EQUAL_TEXT(nuspec, expected);

diff --git a/toolsrc/src/vcpkg-test/commands.build.cpp b/toolsrc/src/vcpkg-test/commands.build.cpp
@@ -20,6 +20,7 @@ TEST_CASE ("build smoke test", "[commands-build]")
 
     auto& fs_wrapper = Files::get_real_filesystem();
     VcpkgCmdArguments args = VcpkgCmdArguments::create_from_arg_sequence(std::begin(args_raw), std::end(args_raw));
+    args.imbue_from_environment();
     args.binary_caching = false;
     args.buildtrees_root_dir =
         std::make_unique<std::string>(fs::u8string(Test::base_temporary_directory() / fs::u8path("buildtrees")));

diff --git a/toolsrc/src/vcpkg-test/manifests.cpp b/toolsrc/src/vcpkg-test/manifests.cpp
@@ -681,6 +681,7 @@ TEST_CASE ("Serialize all the ports", "[manifests]")
     std::vector<std::string> args_list = {"format-manifest"};
     auto& fs = Files::get_real_filesystem();
     auto args = VcpkgCmdArguments::create_from_arg_sequence(args_list.data(), args_list.data() + args_list.size());
+    args.imbue_from_environment();
     VcpkgPaths paths{fs, args};
 
     std::vector<SourceControlFile> scfs;

diff --git a/toolsrc/src/vcpkg.cpp b/toolsrc/src/vcpkg.cpp
@@ -226,6 +226,7 @@ int main(const int argc, const char* const* const argv)
     VcpkgCmdArguments args = VcpkgCmdArguments::create_from_command_line(fs, argc, argv);
     if (const auto p = args.debug.get()) Debug::g_debugging = *p;
     args.imbue_from_environment();
+    VcpkgCmdArguments::imbue_or_apply_process_recursion(args);
     args.check_feature_flag_consistency();
 
     {

diff --git a/toolsrc/src/vcpkg/build.cpp b/toolsrc/src/vcpkg/build.cpp
@@ -344,6 +344,9 @@ namespace vcpkg::Build
                        "for more information.\n");
         Checks::exit_maybe_upgrade(VCPKG_LINE_INFO);
 #else
+        (void)target_architecture;
+        (void)toolset;
+        (void)all_toolsets;
         Checks::exit_with_message(VCPKG_LINE_INFO,
                                   "Error: vcvars-based toolchains are only usable on Windows platforms.");
 #endif

diff --git a/toolsrc/src/vcpkg/vcpkgcmdarguments.cpp b/toolsrc/src/vcpkg/vcpkgcmdarguments.cpp
@@ -648,13 +648,6 @@ namespace vcpkg
 
     void VcpkgCmdArguments::imbue_from_environment()
     {
-        static bool s_reentrancy_guard = false;
-        Checks::check_exit(VCPKG_LINE_INFO,
-                           !s_reentrancy_guard,
-                           "VcpkgCmdArguments::imbue_from_environment() modifies global state and thus may only be "
-                           "called once per process.");
-        s_reentrancy_guard = true;
-
         if (!disable_metrics)
         {
             const auto vcpkg_disable_metrics_env = System::get_environment_variable(DISABLE_METRICS_ENV);
@@ -701,46 +694,52 @@ namespace vcpkg
                 parse_feature_flags(flags, *this);
             }
         }
+    }
 
-        {
-            auto maybe_vcpkg_recursive_data = System::get_environment_variable(RECURSIVE_DATA_ENV);
-            if (auto vcpkg_recursive_data = maybe_vcpkg_recursive_data.get())
-            {
-                m_is_recursive_invocation = true;
-
-                auto rec_doc = Json::parse(*vcpkg_recursive_data).value_or_exit(VCPKG_LINE_INFO).first;
-                const auto& obj = rec_doc.object();
-
-                if (auto entry = obj.get(DOWNLOADS_ROOT_DIR_ENV))
-                {
-                    downloads_root_dir = std::make_unique<std::string>(entry->string().to_string());
-                }
+    void VcpkgCmdArguments::imbue_or_apply_process_recursion(VcpkgCmdArguments& args)
+    {
+        static bool s_reentrancy_guard = false;
+        Checks::check_exit(
+            VCPKG_LINE_INFO,
+            !s_reentrancy_guard,
+            "VcpkgCmdArguments::imbue_or_apply_process_recursion() modifies global state and thus may only be "
+            "called once per process.");
+        s_reentrancy_guard = true;
 
-                if (obj.get(DISABLE_METRICS_ENV))
-                {
-                    disable_metrics = true;
-                }
+        auto maybe_vcpkg_recursive_data = System::get_environment_variable(RECURSIVE_DATA_ENV);
+        if (auto vcpkg_recursive_data = maybe_vcpkg_recursive_data.get())
+        {
+            auto rec_doc = Json::parse(*vcpkg_recursive_data).value_or_exit(VCPKG_LINE_INFO).first;
+            const auto& obj = rec_doc.object();
 
-                // Setting the recursive data to 'poison' prevents more than one level of recursion because
-                // Json::parse() will fail.
-                System::set_environment_variable(RECURSIVE_DATA_ENV, "poison");
+            if (auto entry = obj.get(DOWNLOADS_ROOT_DIR_ENV))
+            {
+                args.downloads_root_dir = std::make_unique<std::string>(entry->string().to_string());
             }
-            else
+
+            if (obj.get(DISABLE_METRICS_ENV))
             {
-                Json::Object obj;
-                if (downloads_root_dir)
-                {
-                    obj.insert(DOWNLOADS_ROOT_DIR_ENV, Json::Value::string(*downloads_root_dir.get()));
-                }
+                args.disable_metrics = true;
+            }
 
-                if (disable_metrics)
-                {
-                    obj.insert(DISABLE_METRICS_ENV, Json::Value::boolean(true));
-                }
+            // Setting the recursive data to 'poison' prevents more than one level of recursion because
+            // Json::parse() will fail.
+            System::set_environment_variable(RECURSIVE_DATA_ENV, "poison");
+        }
+        else
+        {
+            Json::Object obj;
+            if (args.downloads_root_dir)
+            {
+                obj.insert(DOWNLOADS_ROOT_DIR_ENV, Json::Value::string(*args.downloads_root_dir.get()));
+            }
 
-                System::set_environment_variable(RECURSIVE_DATA_ENV,
-                                                 Json::stringify(obj, Json::JsonStyle::with_spaces(0)));
+            if (args.disable_metrics)
+            {
+                obj.insert(DISABLE_METRICS_ENV, Json::Value::boolean(true));
             }
+
+            System::set_environment_variable(RECURSIVE_DATA_ENV, Json::stringify(obj, Json::JsonStyle::with_spaces(0)));
         }
     }