Ignore the VCPKG_ROOT environment variable when we detect a vcpkg_root.

This resolves https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1634907

In actions/runner-images#6196 we have yet another CI system that is trying to helpfully set VCPKG_ROOT for customers to find their vcpkg instance which yet again breaks classic-mode-as-submodule customers. This change mitigates the issue by ignoring the environment variable when we detect a valid VCPKG_ROOT, which the user can override by using the command line option.

azure-pipelines/end-to-end-tests-dir/autocomplete.ps1

@@ -29,7 +29,7 @@ $privateCommands = @(
 )
 
 function getOptionsForPrefix($prefix, $commands) {
-    $commands | Sort-Object | ? { $_.StartsWith($prefix) }
+    ($commands | Sort-Object | ? { $_.StartsWith($prefix) }) -join '`n'
 }
 function arraysEqual($arr1, $arr2) {
     if ($arr1.Length -ne $arr2.Length) {
@@ -68,7 +68,7 @@ function testPrefixes($toTest, $commands) {
         $expected = getOptionsForPrefix $_ $commands
         $found = Run-Vcpkg autocomplete $_
         Throw-IfFailed
-        if (-not (arraysEqual @($expected) @($found))) {
+        if (-not $expected -eq $found) {
             Write-Host "unexpected result from vcpkg autocomplete:
         expected: `"$($expected -join '" "')`"
         found   : `"$($found -join '" "')`""

azure-pipelines/end-to-end-tests-dir/build-command.ps1

@@ -3,7 +3,7 @@
 $CurrentTest = "Build Command"
 
 # Test that the build command fails if dependencies are missing
-$out = (Run-Vcpkg -TestArgs ($commonArgs + @("build","vcpkg-hello-world-1","--host-triplet",$Triplet)) | Out-String)
+$out = Run-Vcpkg -TestArgs ($commonArgs + @("build","vcpkg-hello-world-1","--host-triplet",$Triplet))
 Throw-IfNotFailed
 if ($out -notmatch "The build command requires all dependencies to be already installed\.")
 {
@@ -22,7 +22,7 @@ Throw-IfFailed
 # Regression test https://github.com/microsoft/vcpkg/issues/13933
 Run-Vcpkg -TestArgs ($commonArgs + @("install","vcpkg-hello-world-1","--host-triplet",$Triplet))
 Throw-IfFailed
-$out = (Run-Vcpkg -TestArgs ($commonArgs + @("build","vcpkg-hello-world-1","--host-triplet",$Triplet)) | Out-String)
+$out = Run-Vcpkg -TestArgs ($commonArgs + @("build","vcpkg-hello-world-1","--host-triplet",$Triplet))
 Throw-IfNotFailed
 if ($out -notmatch "is already installed; please remove")
 {

azure-pipelines/end-to-end-tests-dir/classic-versions.ps1

@@ -1,7 +1,7 @@
 . $PSScriptRoot/../end-to-end-tests-prelude.ps1
 
 # Not a number
-$out = Run-Vcpkg @commonArgs install classic-versions-b | Out-String
+$out = Run-Vcpkg @commonArgs install classic-versions-b
 Throw-IfNotFailed
 if ($out -notmatch ".*warning:.*dependency classic-versions-a.*at least version 2.*is currently 1.*")
 {

azure-pipelines/end-to-end-tests-dir/disable-metrics.ps1

@@ -16,7 +16,7 @@ function Test-MetricsEnabled() {
     }
 
     $vcpkgOutput = Run-Vcpkg $actualArgs
-    if ($vcpkgOutput -contains $metricsAreDisabledMessage) {
+    if ($vcpkgOutput.Contains($metricsAreDisabledMessage)) {
         Write-Host 'Metrics are disabled'
         return $false
     }
@@ -43,7 +43,7 @@ try {
 
     # Also test that you get no message without --sendmetrics
     $vcpkgOutput = Run-Vcpkg list
-    if ($vcpkgOutput -contains $metricsAreDisabledMessage) {
+    if ($vcpkgOutput.Contains($metricsAreDisabledMessage)) {
         throw "Disabled metrics emit message even without --sendmetrics"
     }
 

azure-pipelines/end-to-end-tests-dir/new.ps1

@@ -9,7 +9,7 @@ Push-Location $manifestDir
 $result = Run-Vcpkg new
 Pop-Location
 Throw-IfNotFailed
-if (-not $result -contains '--application') {
+if (-not $result.Contains('--application')) {
     throw "New without --name or --version didn't require setting --application"
 }
 
@@ -34,7 +34,7 @@ Push-Location $manifestDir
 $result = Run-Vcpkg new --application
 Pop-Location
 Throw-IfNotFailed
-if (-not $result -contains 'A manifest is already present at') {
+if (-not $result.Contains('A manifest is already present at')) {
     throw "New didn't detect existing manifest correctly"
 }
 
@@ -43,7 +43,7 @@ Push-Location $manifestDir
 $result = Run-Vcpkg new --application
 Pop-Location
 Throw-IfNotFailed
-if (-not $result -contains 'Creating a manifest would overwrite a vcpkg-configuration.json') {
+if (-not $result.Contains('Creating a manifest would overwrite a vcpkg-configuration.json')) {
     throw "New didn't detect existing configuration correctly"
 }
 

azure-pipelines/end-to-end-tests-dir/vcpkg-root.ps1

@@ -0,0 +1,39 @@
+. $PSScriptRoot/../end-to-end-tests-prelude.ps1
+
+$CurrentTest = "VCPKG_ROOT"
+
+$targetMessage = 'Ignoring VCPKG_ROOT environment variable'
+
+$defaultOutput = Run-Vcpkg -TestArgs ($commonArgs + @('install', 'vcpkg-empty-port'))
+Throw-IfFailed
+if ($defaultOutput.Contains($targetMessage)) {
+	throw 'Expected no warning about VCPKG_ROOT when using the environment variable.'
+}
+
+$actualVcpkgRoot = $env:VCPKG_ROOT
+
+pushd $actualVcpkgRoot
+try {
+	$samePathOutput = Run-Vcpkg -TestArgs ($commonArgs + @('install', 'vcpkg-empty-port'))
+	Throw-IfFailed
+	if ($samePathOutput.Contains($targetMessage)) {
+		throw 'Expected no warning about VCPKG_ROOT when the detected path is the same as the configured path.'
+	}
+
+	$env:VCPKG_ROOT = Join-Path $actualVcpkgRoot 'ports' # any existing directory that isn't the detected root
+	$differentPathOutput = Run-Vcpkg -TestArgs ($commonArgs + @('install', 'vcpkg-empty-port', '--debug'))
+	Throw-IfFailed
+	if (-not ($differentPathOutput.Contains($targetMessage))) {
+		throw 'Expected a warning about VCPKG_ROOT differing when the detected path differs from the configured path.'
+	}
+
+	$setWithArgOutput = Run-Vcpkg -TestArgs ($commonArgs + @('install', 'vcpkg-empty-port', '--vcpkg-root', $actualVcpkgRoot, '--debug'))
+	echo $setWithArgOutput
+	Throw-IfFailed
+	if ($setWithArgOutput.Contains($targetMessage)) {
+		throw 'Expected no warning about VCPKG_ROOT when the path is configured with a command line argument.'
+	}
+} finally {
+	$env:VCPKG_ROOT = $actualVcpkgRoot
+	popd
+}

azure-pipelines/end-to-end-tests-dir/versions.ps1

@@ -69,7 +69,7 @@ Run-Vcpkg @portsRedirectArgsIncomplete x-ci-verify-versions --verbose
 Throw-IfFailed
 
 $CurrentTest = "default baseline"
-$out = Run-Vcpkg @commonArgs "--feature-flags=versions" install --x-manifest-root=$versionFilesPath/default-baseline-1 2>&1 | Out-String
+$out = Run-Vcpkg @commonArgs "--feature-flags=versions" install --x-manifest-root=$versionFilesPath/default-baseline-1
 Throw-IfNotFailed
 if ($out -notmatch ".*error: while checking out baseline\.*")
 {
@@ -78,7 +78,7 @@ if ($out -notmatch ".*error: while checking out baseline\.*")
 }
 
 $CurrentTest = "mismatched version database"
-$out = Run-Vcpkg @commonArgs "--feature-flags=versions" install --x-manifest-root="$PSScriptRoot/../e2e_ports/mismatched-version-database" 2>&1 | Out-String
+$out = Run-Vcpkg @commonArgs "--feature-flags=versions" install --x-manifest-root="$PSScriptRoot/../e2e_ports/mismatched-version-database"
 Throw-IfNotFailed
 if (($out -notmatch ".*error: Failed to load port because versions are inconsistent*") -or
   ($out -notmatch ".*version database indicates that it should be arrow@6.0.0.20210925#4.*") -or

azure-pipelines/end-to-end-tests-prelude.ps1

@@ -101,7 +101,7 @@ function Run-Vcpkg {
     )
     $Script:CurrentTest = "$VcpkgExe $($testArgs -join ' ')"
     if (!$EndToEndTestSilent) { Write-Host $Script:CurrentTest }
-    & $VcpkgExe @testArgs
+    (& $VcpkgExe @testArgs) | Out-String
 }
 
 Refresh-TestRoot

azure-pipelines/end-to-end-tests.ps1

@@ -66,6 +66,8 @@ if ([string]::IsNullOrEmpty($VcpkgExe))
     }
 }
 
+$VcpkgExe = (Get-Item $VcpkgExe).FullName
+
 [Array]$AllTests = Get-ChildItem $PSScriptRoot/end-to-end-tests-dir/*.ps1
 if ($Filter -ne $Null) {
     $AllTests = $AllTests | ? { $_.Name -match $Filter }

include/vcpkg/base/messages.h

@@ -1014,6 +1014,12 @@ namespace vcpkg
     DECLARE_MESSAGE(HelpUpdateCommand, (), "", "List packages that can be updated.");
     DECLARE_MESSAGE(HelpUpgradeCommand, (), "", "Rebuild all outdated packages.");
     DECLARE_MESSAGE(HelpVersionCommand, (), "", "Display version information.");
+    DECLARE_MESSAGE(
+        IgnoringVcpkgRootEnvironment,
+        (msg::path, msg::actual),
+        "{actual} is the path we actually used",
+        "Ignoring VCPKG_ROOT environment variable; use --vcpkg-root \"{path}\" to use the environment value or unset "
+        "the VCPKG_ROOT environment variable to suppress this message. Using detected vcpkg root: \"{actual}\".");
     DECLARE_MESSAGE(IllegalFeatures, (), "", "List of features is not allowed in this context");
     DECLARE_MESSAGE(IllegalPlatformSpec, (), "", "Platform qualifier is not allowed in this context");
     DECLARE_MESSAGE(ImproperShaLength,

include/vcpkg/vcpkgcmdarguments.h

@@ -120,7 +120,11 @@ namespace vcpkg
 
         constexpr static StringLiteral VCPKG_ROOT_DIR_ENV = "VCPKG_ROOT";
         constexpr static StringLiteral VCPKG_ROOT_DIR_ARG = "vcpkg-root";
-        Optional<std::string> vcpkg_root_dir;
+
+        constexpr static StringLiteral VCPKG_ROOT_ARG_NAME = "VCPKG_ROOT_ARG";
+        Optional<std::string> vcpkg_root_dir_arg;
+        constexpr static StringLiteral VCPKG_ROOT_ENV_NAME = "VCPKG_ROOT_ENV";
+        Optional<std::string> vcpkg_root_dir_env;
         constexpr static StringLiteral MANIFEST_ROOT_DIR_ARG = "x-manifest-root";
         Optional<std::string> manifest_root_dir;
 

locales/messages.en.json

@@ -221,6 +221,7 @@
   "HelpUpdateCommand": "List packages that can be updated.",
   "HelpUpgradeCommand": "Rebuild all outdated packages.",
   "HelpVersionCommand": "Display version information.",
+  "IgnoringVcpkgRootEnvironment": "Ignoring VCPKG_ROOT environment variable; use --vcpkg-root \"{path}\" to use the environment value or unset the VCPKG_ROOT environment variable to suppress this message. Using detected vcpkg root: \"{actual}\".",
   "IllegalFeatures": "List of features is not allowed in this context",
   "IllegalPlatformSpec": "Platform qualifier is not allowed in this context",
   "ImproperShaLength": "SHA512's must be 128 hex characters: {value}",

locales/messages.json

@@ -361,6 +361,8 @@
   "HelpUpdateCommand": "List packages that can be updated.",
   "HelpUpgradeCommand": "Rebuild all outdated packages.",
   "HelpVersionCommand": "Display version information.",
+  "IgnoringVcpkgRootEnvironment": "Ignoring VCPKG_ROOT environment variable; use --vcpkg-root \"{path}\" to use the environment value or unset the VCPKG_ROOT environment variable to suppress this message. Using detected vcpkg root: \"{actual}\".",
+  "_IgnoringVcpkgRootEnvironment.comment": "{actual} is the path we actually used An example of {path} is /foo/bar.",
   "IllegalFeatures": "List of features is not allowed in this context",
   "IllegalPlatformSpec": "Platform qualifier is not allowed in this context",
   "ImproperShaLength": "SHA512's must be 128 hex characters: {value}",

src/vcpkg-test/arguments.cpp

@@ -26,7 +26,8 @@ TEST_CASE ("VcpkgCmdArguments from lowercase argument sequence", "[arguments]")
                                   "--overlay-triplets=C:\\tripletsB"};
     auto v = VcpkgCmdArguments::create_from_arg_sequence(t.data(), t.data() + t.size());
 
-    REQUIRE(v.vcpkg_root_dir.value_or_exit(VCPKG_LINE_INFO) == "C:\\vcpkg");
+    REQUIRE(v.vcpkg_root_dir_arg.value_or_exit(VCPKG_LINE_INFO) == "C:\\vcpkg");
+    REQUIRE(!v.vcpkg_root_dir_env.has_value());
     REQUIRE(v.scripts_root_dir.value_or_exit(VCPKG_LINE_INFO) == "C:\\scripts");
     REQUIRE(v.builtin_ports_root_dir.value_or_exit(VCPKG_LINE_INFO) == "C:\\ports");
     REQUIRE(v.builtin_registry_versions_dir.value_or_exit(VCPKG_LINE_INFO) == "C:\\versions");
@@ -62,7 +63,8 @@ TEST_CASE ("VcpkgCmdArguments from uppercase argument sequence", "[arguments]")
                                   "--OVERLAY-TRIPLETS=C:\\tripletsB"};
     auto v = VcpkgCmdArguments::create_from_arg_sequence(t.data(), t.data() + t.size());
 
-    REQUIRE(v.vcpkg_root_dir.value_or_exit(VCPKG_LINE_INFO) == "C:\\vcpkg");
+    REQUIRE(v.vcpkg_root_dir_arg.value_or_exit(VCPKG_LINE_INFO) == "C:\\vcpkg");
+    REQUIRE(!v.vcpkg_root_dir_env.has_value());
     REQUIRE(v.scripts_root_dir.value_or_exit(VCPKG_LINE_INFO) == "C:\\scripts");
     REQUIRE(v.builtin_ports_root_dir.value_or_exit(VCPKG_LINE_INFO) == "C:\\ports");
     REQUIRE(v.builtin_registry_versions_dir.value_or_exit(VCPKG_LINE_INFO) == "C:\\versions");
@@ -121,14 +123,14 @@ TEST_CASE ("vcpkg_root parse with arg separator", "[arguments]")
 {
     std::vector<std::string> t = {"--vcpkg-root", "C:\\vcpkg"};
     auto v = VcpkgCmdArguments::create_from_arg_sequence(t.data(), t.data() + t.size());
-    REQUIRE(v.vcpkg_root_dir.value_or_exit(VCPKG_LINE_INFO) == "C:\\vcpkg");
+    REQUIRE(v.vcpkg_root_dir_arg.value_or_exit(VCPKG_LINE_INFO) == "C:\\vcpkg");
 }
 
 TEST_CASE ("vcpkg_root parse with equal separator", "[arguments]")
 {
     std::vector<std::string> t = {"--vcpkg-root=C:\\vcpkg"};
     auto v = VcpkgCmdArguments::create_from_arg_sequence(t.data(), t.data() + t.size());
-    REQUIRE(v.vcpkg_root_dir.value_or_exit(VCPKG_LINE_INFO) == "C:\\vcpkg");
+    REQUIRE(v.vcpkg_root_dir_arg.value_or_exit(VCPKG_LINE_INFO) == "C:\\vcpkg");
 }
 
 TEST_CASE ("Combine asset cache params", "[arguments]")

src/vcpkg/base/messages.cpp

@@ -599,6 +599,7 @@ namespace vcpkg
     REGISTER_MESSAGE(HelpUpdateCommand);
     REGISTER_MESSAGE(HelpUpgradeCommand);
     REGISTER_MESSAGE(HelpVersionCommand);
+    REGISTER_MESSAGE(IgnoringVcpkgRootEnvironment);
     REGISTER_MESSAGE(IllegalFeatures);
     REGISTER_MESSAGE(IllegalPlatformSpec);
     REGISTER_MESSAGE(ImproperShaLength);

src/vcpkg/commands.zbootstrap-standalone.cpp

@@ -23,13 +23,13 @@ namespace vcpkg::Commands
     void ZBootstrapStandaloneCommand::perform_and_exit(const VcpkgCmdArguments& args, Filesystem& fs) const
     {
         DownloadManager download_manager{{}};
-        const auto maybe_vcpkg_root_arg = args.vcpkg_root_dir.get();
-        if (!maybe_vcpkg_root_arg)
+        const auto maybe_vcpkg_root_env = args.vcpkg_root_dir_env.get();
+        if (!maybe_vcpkg_root_env)
         {
             Checks::msg_exit_with_message(VCPKG_LINE_INFO, msgVcpkgRootRequired);
         }
 
-        const auto& vcpkg_root = fs.almost_canonical(*maybe_vcpkg_root_arg, VCPKG_LINE_INFO);
+        const auto& vcpkg_root = fs.almost_canonical(*maybe_vcpkg_root_env, VCPKG_LINE_INFO);
         fs.create_directories(vcpkg_root, VCPKG_LINE_INFO);
         const auto bundle_tarball = vcpkg_root / "vcpkg-standalone-bundle.tar.gz";
 #if defined(VCPKG_STANDALONE_BUNDLE_SHA)

src/vcpkg/vcpkgcmdarguments.cpp

@@ -124,7 +124,7 @@ namespace vcpkg
 
     static void parse_cojoined_value(StringView new_value, StringView option_name, Optional<std::string>& option_field)
     {
-        if (nullptr != option_field)
+        if (option_field.has_value())
         {
             msg::println_error(msgDuplicateOptions, msg::value = option_name);
             LockGuardPtr<Metrics>(g_metrics)->track_string_property(StringMetric::Error,
@@ -334,7 +334,7 @@ namespace vcpkg
             // basic_arg[0] == '-' && basic_arg[1] == '-'
             StringView arg = StringView(basic_arg).substr(2);
             constexpr static std::pair<StringView, Optional<std::string> VcpkgCmdArguments::*> cojoined_values[] = {
-                {VCPKG_ROOT_DIR_ARG, &VcpkgCmdArguments::vcpkg_root_dir},
+                {VCPKG_ROOT_DIR_ARG, &VcpkgCmdArguments::vcpkg_root_dir_arg},
                 {TRIPLET_ARG, &VcpkgCmdArguments::triplet},
                 {HOST_TRIPLET_ARG, &VcpkgCmdArguments::host_triplet},
                 {MANIFEST_ROOT_DIR_ARG, &VcpkgCmdArguments::manifest_root_dir},
@@ -746,7 +746,7 @@ namespace vcpkg
 
         from_env(get_env, TRIPLET_ENV, triplet);
         from_env(get_env, HOST_TRIPLET_ENV, host_triplet);
-        from_env(get_env, VCPKG_ROOT_DIR_ENV, vcpkg_root_dir);
+        vcpkg_root_dir_env = get_environment_variable(VCPKG_ROOT_DIR_ENV);
         from_env(get_env, DOWNLOADS_ROOT_DIR_ENV, downloads_root_dir);
         from_env(get_env, DEFAULT_VISUAL_STUDIO_PATH_ENV, default_visual_studio_path);
         from_env(get_env, ASSET_SOURCES_ENV, asset_sources_template_env);
@@ -812,10 +812,16 @@ namespace vcpkg
             auto rec_doc = Json::parse(*vcpkg_recursive_data).value_or_exit(VCPKG_LINE_INFO).first;
             const auto& obj = rec_doc.object(VCPKG_LINE_INFO);
 
-            if (auto entry = obj.get(VCPKG_ROOT_DIR_ENV))
+            if (auto entry = obj.get(VCPKG_ROOT_ARG_NAME))
             {
                 auto as_sv = entry->string(VCPKG_LINE_INFO);
-                args.vcpkg_root_dir.emplace(as_sv.data(), as_sv.size());
+                args.vcpkg_root_dir_arg.emplace(as_sv.data(), as_sv.size());
+            }
+
+            if (auto entry = obj.get(VCPKG_ROOT_ENV_NAME))
+            {
+                auto as_sv = entry->string(VCPKG_LINE_INFO);
+                args.vcpkg_root_dir_env.emplace(as_sv.data(), as_sv.size());
             }
 
             if (auto entry = obj.get(DOWNLOADS_ROOT_DIR_ENV))
@@ -842,9 +848,14 @@ namespace vcpkg
         else
         {
             Json::Object obj;
-            if (auto vcpkg_root_dir = args.vcpkg_root_dir.get())
+            if (auto vcpkg_root_dir_arg = args.vcpkg_root_dir_arg.get())
+            {
+                obj.insert(VCPKG_ROOT_ARG_NAME, Json::Value::string(*vcpkg_root_dir_arg));
+            }
+
+            if (auto vcpkg_root_dir_env = args.vcpkg_root_dir_env.get())
             {
-                obj.insert(VCPKG_ROOT_DIR_ENV, Json::Value::string(*vcpkg_root_dir));
+                obj.insert(VCPKG_ROOT_ENV_NAME, Json::Value::string(*vcpkg_root_dir_env));
             }
 
             if (auto downloads_root_dir = args.downloads_root_dir.get())

src/vcpkg/vcpkgpaths.cpp

@@ -589,9 +589,9 @@ namespace vcpkg
     static Path determine_root(const Filesystem& fs, const Path& original_cwd, const VcpkgCmdArguments& args)
     {
         Path ret;
-        if (auto vcpkg_root_dir = args.vcpkg_root_dir.get())
+        if (auto vcpkg_root_dir_arg = args.vcpkg_root_dir_arg.get())
         {
-            ret = fs.almost_canonical(*vcpkg_root_dir, VCPKG_LINE_INFO);
+            ret = fs.almost_canonical(*vcpkg_root_dir_arg, VCPKG_LINE_INFO);
         }
         else
         {
@@ -603,6 +603,20 @@ namespace vcpkg
                                                 ".vcpkg-root",
                                                 VCPKG_LINE_INFO);
             }
+
+            if (auto vcpkg_root_dir_env = args.vcpkg_root_dir_env.get())
+            {
+                auto canonical_root_dir_env = fs.almost_canonical(*vcpkg_root_dir_env, VCPKG_LINE_INFO);
+                if (ret.empty())
+                {
+                    ret = std::move(canonical_root_dir_env);
+                }
+                else if (ret != canonical_root_dir_env)
+                {
+                    msg::println_warning(
+                        msgIgnoringVcpkgRootEnvironment, msg::path = *vcpkg_root_dir_env, msg::actual = ret);
+                }
+            }
         }
 
         if (ret.empty())