[vcpkg] Use a tag file rather than conditional compilation to permanently disable metrics. (#15470)

9 files changed, 124 insertions, 54 deletions

diff --git a/scripts/azure-pipelines/end-to-end-tests-dir/disable-metrics.ps1 b/scripts/azure-pipelines/end-to-end-tests-dir/disable-metrics.ps1
new file mode 100644
index 000000000..efec472b4
--- /dev/null
+++ b/scripts/azure-pipelines/end-to-end-tests-dir/disable-metrics.ps1
@@ -0,0 +1,61 @@
+. $PSScriptRoot/../end-to-end-tests-prelude.ps1

+

+# Test that metrics are on by default

+$metricsTagName = 'vcpkg.disable-metrics'

+$metricsAreDisabledMessage = 'Warning: passed --sendmetrics, but metrics are disabled.'

+

+function Test-Metrics-Enabled() {

+    Param(

+        [Parameter(ValueFromRemainingArguments)]

+        [string[]]$TestArgs

+    )

+

+    $actualArgs = @('version', '--sendmetrics')

+    if ($TestArgs.Length -ne 0) {

+        $actualArgs += $TestArgs

+    }

+

+    $vcpkgOutput = Run-Vcpkg $actualArgs

+    if ($vcpkgOutput -contains $metricsAreDisabledMessage) {

+        Write-Host 'Metrics are disabled'

+        return $false

+    }

+

+    Write-Host 'Metrics are enabled'

+    return $true

+}

+

+# By default, metrics are enabled.

+Require-FileNotExists $metricsTagName

+if (-Not (Test-Metrics-Enabled)) {

+    throw "Metrics were not on by default."

+}

+

+if (Test-Metrics-Enabled '--disable-metrics') {

+    throw "Metrics were not disabled by switch."

+}

+

+$env:VCPKG_DISABLE_METRICS = 'ON'

+try {

+    if (Test-Metrics-Enabled) {

+        throw "Environment variable did not disable metrics."

+    }

+

+    if (-Not (Test-Metrics-Enabled '--no-disable-metrics')) {

+        throw "Environment variable to disable metrics could not be overridden by switch."

+    }

+} finally {

+    Remove-Item env:VCPKG_DISABLE_METRICS

+}

+

+# If the disable-metrics tag file exists, metrics are disabled even if attempted to be enabled on

+# the command line.

+Set-Content -Path $metricsTagName -Value ""

+try {

+    if (Test-Metrics-Enabled '--disable-metrics') {

+        throw "Metrics were not force-disabled by the disable-metrics tag file."

+    }

+}

+finally {

+    Remove-Item $metricsTagName

+}

diff --git a/scripts/azure-pipelines/end-to-end-tests-prelude.ps1 b/scripts/azure-pipelines/end-to-end-tests-prelude.ps1
index 0c675e9bf..1e340eff7 100644
--- a/scripts/azure-pipelines/end-to-end-tests-prelude.ps1
+++ b/scripts/azure-pipelines/end-to-end-tests-prelude.ps1
@@ -59,9 +59,17 @@ function Run-Vcpkg {
         [Parameter(ValueFromRemainingArguments)]

         [string[]]$TestArgs

     )

-    $Script:CurrentTest = "./vcpkg $($testArgs -join ' ')"

+

+    if ($IsWindows) {

+        $vcpkgName = 'vcpkg.exe'

+    } else {

+        $vcpkgName = 'vcpkg'

+    }

+

+    $Script:CurrentTest = "./$vcpkgName $($testArgs -join ' ')"

     Write-Host $Script:CurrentTest

-    ./vcpkg @testArgs

+    & "./$vcpkgName" @testArgs | Tee-Object -Variable 'ConsoleOutput'

+    return $ConsoleOutput

 }

 

 Refresh-TestRoot

diff --git a/scripts/bootstrap.ps1 b/scripts/bootstrap.ps1
index 67a4bbbef..018ba3f99 100644
--- a/scripts/bootstrap.ps1
+++ b/scripts/bootstrap.ps1
@@ -370,7 +370,6 @@ else
 
 $arguments = (
 "`"/p:VCPKG_VERSION=-unknownhash`"",
-"`"/p:DISABLE_METRICS=$disableMetricsValue`"",
 "/p:Configuration=Release",
 "/p:Platform=$platform",
 "/p:PlatformToolset=$platformToolset",
@@ -413,8 +412,14 @@ if ($ec -ne 0)
 
 Write-Host "`nBuilding vcpkg.exe... done.`n"
 
-if (-not $disableMetrics)
+if ($disableMetrics)
+{
+    Set-Content -Value "" -Path "$vcpkgRootDir\vcpkg.disable-metrics" -Force
+}
+elseif (-Not (Test-Path "$vcpkgRootDir\vcpkg.disable-metrics"))
 {
+    # Note that we intentionally leave any existing vcpkg.disable-metrics; once a user has
+    # opted out they should stay opted out.
     Write-Host @"
 Telemetry
 ---------
diff --git a/scripts/bootstrap.sh b/scripts/bootstrap.sh
index fa1a64622..ba76f4f63 100644
--- a/scripts/bootstrap.sh
+++ b/scripts/bootstrap.sh
@@ -280,13 +280,17 @@ buildDir="$vcpkgRootDir/toolsrc/build.rel"
 rm -rf "$buildDir"
 mkdir -p "$buildDir"
 
-(cd "$buildDir" && CXX="$CXX" "$cmakeExe" .. -DCMAKE_BUILD_TYPE=Release -G "Ninja" "-DCMAKE_MAKE_PROGRAM=$ninjaExe" "-DBUILD_TESTING=$vcpkgBuildTests" "-DVCPKG_DEVELOPMENT_WARNINGS=OFF" "-DVCPKG_DISABLE_METRICS=$vcpkgDisableMetrics" "-DVCPKG_ALLOW_APPLE_CLANG=$vcpkgAllowAppleClang") || exit 1
+(cd "$buildDir" && CXX="$CXX" "$cmakeExe" .. -DCMAKE_BUILD_TYPE=Release -G "Ninja" "-DCMAKE_MAKE_PROGRAM=$ninjaExe" "-DBUILD_TESTING=$vcpkgBuildTests" "-DVCPKG_DEVELOPMENT_WARNINGS=OFF" "-DVCPKG_ALLOW_APPLE_CLANG=$vcpkgAllowAppleClang") || exit 1
 (cd "$buildDir" && "$cmakeExe" --build .) || exit 1
 
 rm -rf "$vcpkgRootDir/vcpkg"
 cp "$buildDir/vcpkg" "$vcpkgRootDir/"
 
-if ! [ "$vcpkgDisableMetrics" = "ON" ]; then
+if [ "$vcpkgDisableMetrics" = "ON" ]; then
+    touch "$vcpkgRootDir/vcpkg.disable-metrics"
+elif ! [ -f "$vcpkgRootDir/vcpkg.disable-metrics" ]; then
+    # Note that we intentionally leave any existing vcpkg.disable-metrics; once a user has
+    # opted out they should stay opted out.
     cat <<EOF
 Telemetry
 ---------
diff --git a/toolsrc/CMakeLists.txt b/toolsrc/CMakeLists.txt
index 3f0c55cff..317d000cc 100644
--- a/toolsrc/CMakeLists.txt
+++ b/toolsrc/CMakeLists.txt
@@ -15,7 +15,6 @@ include(cmake/utilities.cmake)
 include(CMakeDependentOption)
 
 option(BUILD_TESTING "Option for enabling testing" ON)
-option(VCPKG_DISABLE_METRICS "Option for disabling metrics" OFF)
 option(VCPKG_ALLOW_APPLE_CLANG "Option for allowing apple clang, even versions that we don't know will work" OFF)
 option(VCPKG_DEVELOPMENT_WARNINGS "Option for turning on all warnings" ON)
 option(VCPKG_WARNINGS_AS_ERRORS "Set warnings to be errors" ${VCPKG_DEVELOPMENT_WARNINGS})
@@ -28,9 +27,9 @@ CMAKE_DEPENDENT_OPTION(VCPKG_BUILD_BENCHMARKING "Option for enabling benchmarkin
 if(WERROR)
     message(DEPRECATION "-DWERROR is no longer a supported flag. It doesn't do anything.")
 endif()
-if(DEFINE_DISABLE_METRICS)
-    message(DEPRECATION "DEFINE_DISABLE_METRICS is now called VCPKG_DISABLE_METRICS.")
-    set(VCPKG_DISABLE_METRICS ${DEFINE_DISABLE_METRICS} CACHE BOOL "Option for disabling metrics" FORCE)
+if(DEFINE_DISABLE_METRICS OR VCPKG_DISABLE_METRICS)
+    message(DEPRECATION "DEFINE_DISABLE_METRICS / VCPKG_DISABLE_METRICS are now handled by creating a "
+        "file vcpkg.disable_metrics next to the binary.")
 endif()
 
 # =============
diff --git a/toolsrc/include/vcpkg/commands.upload-metrics.h b/toolsrc/include/vcpkg/commands.upload-metrics.h
index dcd855ffb..4cfb66092 100644
--- a/toolsrc/include/vcpkg/commands.upload-metrics.h
+++ b/toolsrc/include/vcpkg/commands.upload-metrics.h
@@ -1,13 +1,5 @@
 #pragma once
 
-#if !VCPKG_DISABLE_METRICS && defined(_WIN32)
-#define VCPKG_ENABLE_X_UPLOAD_METRICS_COMMAND 1
-#else
-#define VCPKG_ENABLE_X_UPLOAD_METRICS_COMMAND 0
-#endif // !VCPKG_DISABLE_METRICS && defined(_WIN32)
-
-#if VCPKG_ENABLE_X_UPLOAD_METRICS_COMMAND
-
 #include <vcpkg/commands.interface.h>
 
 namespace vcpkg::Commands::UploadMetrics
@@ -18,5 +10,3 @@ namespace vcpkg::Commands::UploadMetrics
         virtual void perform_and_exit(const VcpkgCmdArguments& args, Files::Filesystem& fs) const override;
     };
 }
-
-#endif // VCPKG_ENABLE_X_UPLOAD_METRICS_COMMAND
diff --git a/toolsrc/src/vcpkg.cpp b/toolsrc/src/vcpkg.cpp
index f128210fd..f3a6ac6a0 100644
--- a/toolsrc/src/vcpkg.cpp
+++ b/toolsrc/src/vcpkg.cpp
@@ -227,24 +227,40 @@ int main(const int argc, const char* const* const argv)
 #endif
 
     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();
     args.check_feature_flag_consistency();
 
-    if (const auto p = args.disable_metrics.get()) Metrics::g_metrics.lock()->set_disabled(*p);
-    if (const auto p = args.print_metrics.get()) Metrics::g_metrics.lock()->set_print_metrics(*p);
-    if (const auto p = args.send_metrics.get()) Metrics::g_metrics.lock()->set_send_metrics(*p);
-    if (const auto p = args.debug.get()) Debug::g_debugging = *p;
-
-    if (args.send_metrics.has_value() && !Metrics::g_metrics.lock()->metrics_enabled())
     {
-        System::print2(System::Color::warning,
-                       "Warning: passed either --sendmetrics or --no-sendmetrics, but metrics are disabled.\n");
-    }
-    if (args.print_metrics.has_value() && !Metrics::g_metrics.lock()->metrics_enabled())
-    {
-        System::print2(System::Color::warning,
-                       "Warning: passed either --printmetrics or --no-printmetrics, but metrics are disabled.\n");
-    }
+        auto metrics = Metrics::g_metrics.lock();
+        if (const auto p = args.disable_metrics.get())
+        {
+            metrics->set_disabled(*p);
+        }
+
+        auto disable_metrics_tag_file_path =
+            System::get_exe_path_of_current_process().replace_filename(fs::u8path("vcpkg.disable-metrics"));
+        std::error_code ec;
+        if (fs.exists(disable_metrics_tag_file_path, ec) || ec)
+        {
+            metrics->set_disabled(true);
+        }
+
+        if (const auto p = args.print_metrics.get())
+        {
+            metrics->set_print_metrics(*p);
+        }
+
+        if (const auto p = args.send_metrics.get())
+        {
+            metrics->set_send_metrics(*p);
+        }
+
+        if (args.send_metrics.value_or(true) && !metrics->metrics_enabled())
+        {
+            System::print2(System::Color::warning, "Warning: passed --sendmetrics, but metrics are disabled.\n");
+        }
+    } // unlock Metrics::g_metrics
 
     args.debug_print_feature_flags();
     args.track_feature_flag_metrics();
diff --git a/toolsrc/src/vcpkg/metrics.cpp b/toolsrc/src/vcpkg/metrics.cpp
index fd5abea4b..9e9f43c15 100644
--- a/toolsrc/src/vcpkg/metrics.cpp
+++ b/toolsrc/src/vcpkg/metrics.cpp
@@ -240,20 +240,14 @@ namespace vcpkg::Metrics
 
     static MetricMessage g_metricmessage;
     static bool g_should_send_metrics =
-#if defined(NDEBUG) && (VCPKG_DISABLE_METRICS == 0)
+#if defined(NDEBUG)
         true
 #else
         false
 #endif
         ;
     static bool g_should_print_metrics = false;
-    static bool g_metrics_disabled =
-#if VCPKG_DISABLE_METRICS
-        true
-#else
-        false
-#endif
-        ;
+    static bool g_metrics_disabled = false;
 
     std::string get_MAC_user()
     {
@@ -305,14 +299,7 @@ namespace vcpkg::Metrics
 
     void Metrics::set_disabled(bool disabled) { g_metrics_disabled = disabled; }
 
-    bool Metrics::metrics_enabled()
-    {
-#if VCPKG_DISABLE_METRICS
-        return false;
-#else
-        return !g_metrics_disabled;
-#endif
-    }
+    bool Metrics::metrics_enabled() { return !g_metrics_disabled; }
 
     void Metrics::track_metric(const std::string& name, double value)
     {
diff --git a/toolsrc/windows-bootstrap/vcpkg.vcxproj b/toolsrc/windows-bootstrap/vcpkg.vcxproj
index 9f035bb37..7aa153598 100644
--- a/toolsrc/windows-bootstrap/vcpkg.vcxproj
+++ b/toolsrc/windows-bootstrap/vcpkg.vcxproj
@@ -96,7 +96,7 @@
       <Optimization>Disabled</Optimization>

       <SDLCheck>true</SDLCheck>

       <AdditionalIncludeDirectories>..\include</AdditionalIncludeDirectories>

-      <PreprocessorDefinitions>VCPKG_DISABLE_METRICS=$(DISABLE_METRICS);VCPKG_VERSION=$(VCPKG_VERSION);_MBCS;%(PreprocessorDefinitions)</PreprocessorDefinitions>

+      <PreprocessorDefinitions>VCPKG_VERSION=$(VCPKG_VERSION);_MBCS;%(PreprocessorDefinitions)</PreprocessorDefinitions>

       <AdditionalOptions>/std:c++latest %(AdditionalOptions)</AdditionalOptions>

       <MultiProcessorCompilation>true</MultiProcessorCompilation>

       <MinimalRebuild>false</MinimalRebuild>

@@ -114,7 +114,7 @@
       <Optimization>Disabled</Optimization>

       <SDLCheck>true</SDLCheck>

       <AdditionalIncludeDirectories>..\include</AdditionalIncludeDirectories>

-      <PreprocessorDefinitions>VCPKG_DISABLE_METRICS=$(DISABLE_METRICS);VCPKG_VERSION=$(VCPKG_VERSION);_MBCS;%(PreprocessorDefinitions)</PreprocessorDefinitions>

+      <PreprocessorDefinitions>VCPKG_VERSION=$(VCPKG_VERSION);_MBCS;%(PreprocessorDefinitions)</PreprocessorDefinitions>

       <AdditionalOptions>/std:c++latest %(AdditionalOptions)</AdditionalOptions>

       <MultiProcessorCompilation>true</MultiProcessorCompilation>

       <MinimalRebuild>false</MinimalRebuild>

@@ -134,7 +134,7 @@
       <IntrinsicFunctions>true</IntrinsicFunctions>

       <SDLCheck>true</SDLCheck>

       <AdditionalIncludeDirectories>..\include</AdditionalIncludeDirectories>

-      <PreprocessorDefinitions>VCPKG_DISABLE_METRICS=$(DISABLE_METRICS);VCPKG_VERSION=$(VCPKG_VERSION);_MBCS;NDEBUG;%(PreprocessorDefinitions)</PreprocessorDefinitions>

+      <PreprocessorDefinitions>VCPKG_VERSION=$(VCPKG_VERSION);_MBCS;NDEBUG;%(PreprocessorDefinitions)</PreprocessorDefinitions>

       <PreprocessorDefinitions>_MBCS;NDEBUG;%(PreprocessorDefinitions)</PreprocessorDefinitions>

       <AdditionalOptions>/std:c++latest %(AdditionalOptions)</AdditionalOptions>

       <MultiProcessorCompilation>true</MultiProcessorCompilation>

@@ -156,7 +156,7 @@
       <IntrinsicFunctions>true</IntrinsicFunctions>

       <SDLCheck>true</SDLCheck>

       <AdditionalIncludeDirectories>..\include</AdditionalIncludeDirectories>

-      <PreprocessorDefinitions>VCPKG_DISABLE_METRICS=$(DISABLE_METRICS);VCPKG_VERSION=$(VCPKG_VERSION);_MBCS;NDEBUG;%(PreprocessorDefinitions)</PreprocessorDefinitions>

+      <PreprocessorDefinitions>VCPKG_VERSION=$(VCPKG_VERSION);_MBCS;NDEBUG;%(PreprocessorDefinitions)</PreprocessorDefinitions>

       <PreprocessorDefinitions>_MBCS;NDEBUG;%(PreprocessorDefinitions)</PreprocessorDefinitions>

       <AdditionalOptions>/std:c++latest %(AdditionalOptions)</AdditionalOptions>

       <MultiProcessorCompilation>true</MultiProcessorCompilation>