From 8414e15973097e70fe40149e922c402799804b3d Mon Sep 17 00:00:00 2001 From: Billy O'Neal Date: Wed, 13 Jan 2021 14:06:06 -0800 Subject: [vcpkg] Use a tag file rather than conditional compilation to permanently disable metrics. (#15470) --- .../end-to-end-tests-dir/disable-metrics.ps1 | 61 ++++++++++++++++++++++ .../azure-pipelines/end-to-end-tests-prelude.ps1 | 12 ++++- scripts/bootstrap.ps1 | 9 +++- scripts/bootstrap.sh | 8 ++- toolsrc/CMakeLists.txt | 7 ++- toolsrc/include/vcpkg/commands.upload-metrics.h | 10 ---- toolsrc/src/vcpkg.cpp | 44 +++++++++++----- toolsrc/src/vcpkg/metrics.cpp | 19 ++----- toolsrc/windows-bootstrap/vcpkg.vcxproj | 8 +-- 9 files changed, 124 insertions(+), 54 deletions(-) create mode 100644 scripts/azure-pipelines/end-to-end-tests-dir/disable-metrics.ps1 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 < 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 @@ Disabled true ..\include - VCPKG_DISABLE_METRICS=$(DISABLE_METRICS);VCPKG_VERSION=$(VCPKG_VERSION);_MBCS;%(PreprocessorDefinitions) + VCPKG_VERSION=$(VCPKG_VERSION);_MBCS;%(PreprocessorDefinitions) /std:c++latest %(AdditionalOptions) true false @@ -114,7 +114,7 @@ Disabled true ..\include - VCPKG_DISABLE_METRICS=$(DISABLE_METRICS);VCPKG_VERSION=$(VCPKG_VERSION);_MBCS;%(PreprocessorDefinitions) + VCPKG_VERSION=$(VCPKG_VERSION);_MBCS;%(PreprocessorDefinitions) /std:c++latest %(AdditionalOptions) true false @@ -134,7 +134,7 @@ true true ..\include - VCPKG_DISABLE_METRICS=$(DISABLE_METRICS);VCPKG_VERSION=$(VCPKG_VERSION);_MBCS;NDEBUG;%(PreprocessorDefinitions) + VCPKG_VERSION=$(VCPKG_VERSION);_MBCS;NDEBUG;%(PreprocessorDefinitions) _MBCS;NDEBUG;%(PreprocessorDefinitions) /std:c++latest %(AdditionalOptions) true @@ -156,7 +156,7 @@ true true ..\include - VCPKG_DISABLE_METRICS=$(DISABLE_METRICS);VCPKG_VERSION=$(VCPKG_VERSION);_MBCS;NDEBUG;%(PreprocessorDefinitions) + VCPKG_VERSION=$(VCPKG_VERSION);_MBCS;NDEBUG;%(PreprocessorDefinitions) _MBCS;NDEBUG;%(PreprocessorDefinitions) /std:c++latest %(AdditionalOptions) true -- cgit v1.2.3