From fcccba2bbc681e3f143c8c139418b330b84026fc Mon Sep 17 00:00:00 2001 From: Javier Matos Date: Wed, 13 Nov 2024 13:37:44 -0500 Subject: [PATCH 1/5] improve messaging for x-script --- .../asset-caching/failing-script.ps1 | 1 + .../asset-caching/success-script.ps1 | 22 +++++++++++++++ .../end-to-end-tests-dir/asset-caching.ps1 | 28 +++++++++++++++++++ include/vcpkg/base/message-data.inc.h | 4 +++ locales/messages.json | 2 ++ src/vcpkg/base/downloads.cpp | 7 +++-- 6 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 azure-pipelines/e2e-assets/asset-caching/failing-script.ps1 create mode 100644 azure-pipelines/e2e-assets/asset-caching/success-script.ps1 diff --git a/azure-pipelines/e2e-assets/asset-caching/failing-script.ps1 b/azure-pipelines/e2e-assets/asset-caching/failing-script.ps1 new file mode 100644 index 0000000000..c232cc056d --- /dev/null +++ b/azure-pipelines/e2e-assets/asset-caching/failing-script.ps1 @@ -0,0 +1 @@ +throw "Script download error" diff --git a/azure-pipelines/e2e-assets/asset-caching/success-script.ps1 b/azure-pipelines/e2e-assets/asset-caching/success-script.ps1 new file mode 100644 index 0000000000..7906eb1084 --- /dev/null +++ b/azure-pipelines/e2e-assets/asset-caching/success-script.ps1 @@ -0,0 +1,22 @@ +param ( + [string]$Url, + [string]$ExpectedSHA512, + [string]$Destination +) + +# Ensure the destination folder exists +$downloadFolder = Split-Path -Path $Destination -Parent +if (-not (Test-Path $downloadFolder)) { + New-Item -Path $downloadFolder -ItemType Directory | Out-Null +} + +# Download the file directly to the specified destination path +Invoke-WebRequest -Uri $Url -OutFile $Destination + +# Confirm that the file exists and is accessible +if (-not (Test-Path $Destination)) { + Throw "Download failed: File not found at $Destination" + exit 1 +} + +exit 0 \ No newline at end of file diff --git a/azure-pipelines/end-to-end-tests-dir/asset-caching.ps1 b/azure-pipelines/end-to-end-tests-dir/asset-caching.ps1 index b3725fcb9b..1d907f8deb 100644 --- a/azure-pipelines/end-to-end-tests-dir/asset-caching.ps1 +++ b/azure-pipelines/end-to-end-tests-dir/asset-caching.ps1 @@ -197,3 +197,31 @@ $actual = Run-VcpkgAndCaptureOutput -TestArgs ($commonArgs + @("x-download", "$d if (-not ($actual.Contains("Asset cache hit for example3.html; downloaded from: file://$AssetCache"))) { throw "Failure: azurl (yes), x-block-origin (yes), asset-cache (hit), download (n/a)" } + +# Testing x-download failure with asset cache (x-script) and x-block-origin settings +$env:X_VCPKG_ASSET_SOURCES = "clear;x-script,pwsh $PSScriptRoot/../e2e-assets/asset-caching/failing-script.ps1 {url} {sha512} {dst};x-block-origin" +$actual = Run-VcpkgAndCaptureOutput -TestArgs ($commonArgs + @("x-download", "$downloadsRoot/example3.html", "--url", "https://example.com", "--sha512", "d06b93c883f8126a04589937a884032df031b05518eed9d433efb6447834df2596aebd500d69b8283e5702d988ed49655ae654c1683c7a4ae58bfa6b92f2b73a")) +if (-not ($actual.Contains("The script download failed.") -and + $actual.Contains("error: failed with exit code: (1).") -and + $actual.Contains("error: Missing example3.html and downloads are blocked by x-block-origin."))) { + throw "Failure: x-script downloads failure messaging" +} + +# Testing x-download success with asset cache (x-script) and x-block-origin settings +Refresh-TestRoot +$env:X_VCPKG_ASSET_SOURCES = "clear;x-script,pwsh $PSScriptRoot/../e2e-assets/asset-caching/success-script.ps1 {url} {sha512} {dst};x-block-origin" +$actual = Run-VcpkgAndCaptureOutput -TestArgs ($commonArgs + @("x-download", "$downloadsRoot/example3.html", "--url", "https://example.com", "--sha512", "d06b93c883f8126a04589937a884032df031b05518eed9d433efb6447834df2596aebd500d69b8283e5702d988ed49655ae654c1683c7a4ae58bfa6b92f2b73a")) +if (-not ($actual.Contains("Successfully downloaded example3.html."))) { + throw "Failure: x-script download success message" +} + +# Testing x-download failure with mismatched sha +Refresh-TestRoot +$env:X_VCPKG_ASSET_SOURCES = "clear;x-script,pwsh $PSScriptRoot/../e2e-assets/asset-caching/success-script.ps1 {url} {sha512} {dst};x-block-origin" +$actual = Run-VcpkgAndCaptureOutput -TestArgs ($commonArgs + @("x-download", "$downloadsRoot/example3.html", "--url", "https://example.com", "--sha512", "d06b93c883f8126a04589937a884032df031b05518eed9d433efb6447834df2596aebd500d69b8283e5702d988ed49655ae654c1683c7a4ae58bfa6b92f2b73b")) +if (-not ($actual.Contains("error: File does not have the expected hash:") -and + $actual.Contains("url: ") -and + $actual.Contains("Expected hash: d06b93c883f8126a04589937a884032df031b05518eed9d433efb6447834df2596aebd500d69b8283e5702d988ed49655ae654c1683c7a4ae58bfa6b92f2b73b") -and + $actual.Contains("Actual hash: d06b93c883f8126a04589937a884032df031b05518eed9d433efb6447834df2596aebd500d69b8283e5702d988ed49655ae654c1683c7a4ae58bfa6b92f2b73a"))) { + throw "Failure: x-script download failure message - sha mismatch" +} \ No newline at end of file diff --git a/include/vcpkg/base/message-data.inc.h b/include/vcpkg/base/message-data.inc.h index 3131761a91..c22fbc73ec 100644 --- a/include/vcpkg/base/message-data.inc.h +++ b/include/vcpkg/base/message-data.inc.h @@ -1144,6 +1144,10 @@ DECLARE_MESSAGE(ErrorRequirePackagesList, (), "", "`vcpkg install` requires a list of packages to install in classic mode.") +DECLARE_MESSAGE(ErrorScriptDownloadFailed, + (msg::error_msg), + "", + "The script download failed.\n{error_msg}.") DECLARE_MESSAGE(ErrorInvalidExtractOption, (msg::option, msg::value), "The keyword 'AUTO' should not be localized", diff --git a/locales/messages.json b/locales/messages.json index d33ef52815..203e796ff1 100644 --- a/locales/messages.json +++ b/locales/messages.json @@ -663,6 +663,8 @@ "_ErrorParsingBinaryParagraph.comment": "An example of {spec} is zlib:x64-windows.", "ErrorRequireBaseline": "this vcpkg instance requires a manifest with a specified baseline in order to interact with ports. Please add 'builtin-baseline' to the manifest or add a 'vcpkg-configuration.json' that redefines the default registry.", "ErrorRequirePackagesList": "`vcpkg install` requires a list of packages to install in classic mode.", + "ErrorScriptDownloadFailed": "The script download failed.\n{error_msg}.", + "_ErrorScriptDownloadFailed.comment": "An example of {error_msg} is File Not Found.", "ErrorUnableToDetectCompilerInfo": "vcpkg was unable to detect the active compiler's information. See above for the CMake failure output.", "_ErrorUnableToDetectCompilerInfo.comment": "failure output will be displayed at the top of this", "ErrorVcvarsUnsupported": "in triplet {triplet}: Use of Visual Studio's Developer Prompt is unsupported on non-Windows hosts.\nDefine 'VCPKG_CMAKE_SYSTEM_NAME' or 'VCPKG_CHAINLOAD_TOOLCHAIN_FILE' in the triplet file.", diff --git a/src/vcpkg/base/downloads.cpp b/src/vcpkg/base/downloads.cpp index 56e61316c5..340cba94cb 100644 --- a/src/vcpkg/base/downloads.cpp +++ b/src/vcpkg/base/downloads.cpp @@ -974,6 +974,7 @@ namespace vcpkg RedirectedProcessLaunchSettings settings; settings.environment = get_clean_environment(); settings.echo_in_debug = EchoInDebug::Show; + auto maybe_res = flatten(cmd_execute_and_capture_output(cmd, settings), ""); if (maybe_res) { @@ -982,14 +983,14 @@ namespace vcpkg if (maybe_success) { fs.rename(download_path_part_path, download_path, VCPKG_LINE_INFO); + msg::println(msgDownloadSuccesful, msg::path = download_path.filename()); return urls[0]; } - - errors.push_back(std::move(maybe_success).error()); + msg::println_error(maybe_success.error()); } else { - errors.push_back(std::move(maybe_res).error()); + msg::println(msgErrorScriptDownloadFailed, msg::error_msg = maybe_res.error()); } } } From 3eae222fbf021bdc0311521ccd233d637fbaaa04 Mon Sep 17 00:00:00 2001 From: Javier Matos Date: Wed, 13 Nov 2024 13:40:48 -0500 Subject: [PATCH 2/5] add newlines @ EOF --- azure-pipelines/e2e-assets/asset-caching/success-script.ps1 | 2 +- azure-pipelines/end-to-end-tests-dir/asset-caching.ps1 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/azure-pipelines/e2e-assets/asset-caching/success-script.ps1 b/azure-pipelines/e2e-assets/asset-caching/success-script.ps1 index 7906eb1084..603f8ef6d5 100644 --- a/azure-pipelines/e2e-assets/asset-caching/success-script.ps1 +++ b/azure-pipelines/e2e-assets/asset-caching/success-script.ps1 @@ -19,4 +19,4 @@ if (-not (Test-Path $Destination)) { exit 1 } -exit 0 \ No newline at end of file +exit 0 diff --git a/azure-pipelines/end-to-end-tests-dir/asset-caching.ps1 b/azure-pipelines/end-to-end-tests-dir/asset-caching.ps1 index 1d907f8deb..772c9697f0 100644 --- a/azure-pipelines/end-to-end-tests-dir/asset-caching.ps1 +++ b/azure-pipelines/end-to-end-tests-dir/asset-caching.ps1 @@ -224,4 +224,4 @@ if (-not ($actual.Contains("error: File does not have the expected hash:") -and $actual.Contains("Expected hash: d06b93c883f8126a04589937a884032df031b05518eed9d433efb6447834df2596aebd500d69b8283e5702d988ed49655ae654c1683c7a4ae58bfa6b92f2b73b") -and $actual.Contains("Actual hash: d06b93c883f8126a04589937a884032df031b05518eed9d433efb6447834df2596aebd500d69b8283e5702d988ed49655ae654c1683c7a4ae58bfa6b92f2b73a"))) { throw "Failure: x-script download failure message - sha mismatch" -} \ No newline at end of file +} From f0800c8f3798557b463bb5bd6e63ff2fa3c839a4 Mon Sep 17 00:00:00 2001 From: Javier Matos Date: Wed, 13 Nov 2024 14:04:24 -0500 Subject: [PATCH 3/5] format --- include/vcpkg/base/message-data.inc.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/include/vcpkg/base/message-data.inc.h b/include/vcpkg/base/message-data.inc.h index c22fbc73ec..1e6259b065 100644 --- a/include/vcpkg/base/message-data.inc.h +++ b/include/vcpkg/base/message-data.inc.h @@ -1144,10 +1144,7 @@ DECLARE_MESSAGE(ErrorRequirePackagesList, (), "", "`vcpkg install` requires a list of packages to install in classic mode.") -DECLARE_MESSAGE(ErrorScriptDownloadFailed, - (msg::error_msg), - "", - "The script download failed.\n{error_msg}.") +DECLARE_MESSAGE(ErrorScriptDownloadFailed, (msg::error_msg), "", "The script download failed.\n{error_msg}.") DECLARE_MESSAGE(ErrorInvalidExtractOption, (msg::option, msg::value), "The keyword 'AUTO' should not be localized", From 42e09a7162125156865efdcae1ffba6fccefa62b Mon Sep 17 00:00:00 2001 From: Javier Matos Date: Wed, 13 Nov 2024 16:07:08 -0500 Subject: [PATCH 4/5] respond to Billy feedback --- .../asset-caching/success-script.ps1 | 22 ------------------- .../end-to-end-tests-dir/asset-caching.ps1 | 17 +++----------- include/vcpkg/base/message-data.inc.h | 1 - locales/messages.json | 2 -- src/vcpkg/base/downloads.cpp | 2 +- 5 files changed, 4 insertions(+), 40 deletions(-) delete mode 100644 azure-pipelines/e2e-assets/asset-caching/success-script.ps1 diff --git a/azure-pipelines/e2e-assets/asset-caching/success-script.ps1 b/azure-pipelines/e2e-assets/asset-caching/success-script.ps1 deleted file mode 100644 index 603f8ef6d5..0000000000 --- a/azure-pipelines/e2e-assets/asset-caching/success-script.ps1 +++ /dev/null @@ -1,22 +0,0 @@ -param ( - [string]$Url, - [string]$ExpectedSHA512, - [string]$Destination -) - -# Ensure the destination folder exists -$downloadFolder = Split-Path -Path $Destination -Parent -if (-not (Test-Path $downloadFolder)) { - New-Item -Path $downloadFolder -ItemType Directory | Out-Null -} - -# Download the file directly to the specified destination path -Invoke-WebRequest -Uri $Url -OutFile $Destination - -# Confirm that the file exists and is accessible -if (-not (Test-Path $Destination)) { - Throw "Download failed: File not found at $Destination" - exit 1 -} - -exit 0 diff --git a/azure-pipelines/end-to-end-tests-dir/asset-caching.ps1 b/azure-pipelines/end-to-end-tests-dir/asset-caching.ps1 index 772c9697f0..d626bfd801 100644 --- a/azure-pipelines/end-to-end-tests-dir/asset-caching.ps1 +++ b/azure-pipelines/end-to-end-tests-dir/asset-caching.ps1 @@ -201,27 +201,16 @@ if (-not ($actual.Contains("Asset cache hit for example3.html; downloaded from: # Testing x-download failure with asset cache (x-script) and x-block-origin settings $env:X_VCPKG_ASSET_SOURCES = "clear;x-script,pwsh $PSScriptRoot/../e2e-assets/asset-caching/failing-script.ps1 {url} {sha512} {dst};x-block-origin" $actual = Run-VcpkgAndCaptureOutput -TestArgs ($commonArgs + @("x-download", "$downloadsRoot/example3.html", "--url", "https://example.com", "--sha512", "d06b93c883f8126a04589937a884032df031b05518eed9d433efb6447834df2596aebd500d69b8283e5702d988ed49655ae654c1683c7a4ae58bfa6b92f2b73a")) -if (-not ($actual.Contains("The script download failed.") -and - $actual.Contains("error: failed with exit code: (1).") -and +if (-not ($actual.Contains("error: failed with exit code: (1).") -and $actual.Contains("error: Missing example3.html and downloads are blocked by x-block-origin."))) { throw "Failure: x-script downloads failure messaging" } # Testing x-download success with asset cache (x-script) and x-block-origin settings Refresh-TestRoot -$env:X_VCPKG_ASSET_SOURCES = "clear;x-script,pwsh $PSScriptRoot/../e2e-assets/asset-caching/success-script.ps1 {url} {sha512} {dst};x-block-origin" -$actual = Run-VcpkgAndCaptureOutput -TestArgs ($commonArgs + @("x-download", "$downloadsRoot/example3.html", "--url", "https://example.com", "--sha512", "d06b93c883f8126a04589937a884032df031b05518eed9d433efb6447834df2596aebd500d69b8283e5702d988ed49655ae654c1683c7a4ae58bfa6b92f2b73a")) +$env:X_VCPKG_ASSET_SOURCES = "clear;x-script,$TestScriptAssetCacheExe {url} {sha512} {dst};x-block-origin" +$actual = Run-VcpkgAndCaptureOutput -TestArgs ($commonArgs + @("x-download", "$downloadsRoot/example3.html", "--url", "https://example.com/hello-world.txt", "--sha512", "09e1e2a84c92b56c8280f4a1203c7cffd61b162cfe987278d4d6be9afbf38c0e8934cdadf83751f4e99d111352bffefc958e5a4852c8a7a29c95742ce59288a8")) if (-not ($actual.Contains("Successfully downloaded example3.html."))) { throw "Failure: x-script download success message" } -# Testing x-download failure with mismatched sha -Refresh-TestRoot -$env:X_VCPKG_ASSET_SOURCES = "clear;x-script,pwsh $PSScriptRoot/../e2e-assets/asset-caching/success-script.ps1 {url} {sha512} {dst};x-block-origin" -$actual = Run-VcpkgAndCaptureOutput -TestArgs ($commonArgs + @("x-download", "$downloadsRoot/example3.html", "--url", "https://example.com", "--sha512", "d06b93c883f8126a04589937a884032df031b05518eed9d433efb6447834df2596aebd500d69b8283e5702d988ed49655ae654c1683c7a4ae58bfa6b92f2b73b")) -if (-not ($actual.Contains("error: File does not have the expected hash:") -and - $actual.Contains("url: ") -and - $actual.Contains("Expected hash: d06b93c883f8126a04589937a884032df031b05518eed9d433efb6447834df2596aebd500d69b8283e5702d988ed49655ae654c1683c7a4ae58bfa6b92f2b73b") -and - $actual.Contains("Actual hash: d06b93c883f8126a04589937a884032df031b05518eed9d433efb6447834df2596aebd500d69b8283e5702d988ed49655ae654c1683c7a4ae58bfa6b92f2b73a"))) { - throw "Failure: x-script download failure message - sha mismatch" -} diff --git a/include/vcpkg/base/message-data.inc.h b/include/vcpkg/base/message-data.inc.h index 1e6259b065..3131761a91 100644 --- a/include/vcpkg/base/message-data.inc.h +++ b/include/vcpkg/base/message-data.inc.h @@ -1144,7 +1144,6 @@ DECLARE_MESSAGE(ErrorRequirePackagesList, (), "", "`vcpkg install` requires a list of packages to install in classic mode.") -DECLARE_MESSAGE(ErrorScriptDownloadFailed, (msg::error_msg), "", "The script download failed.\n{error_msg}.") DECLARE_MESSAGE(ErrorInvalidExtractOption, (msg::option, msg::value), "The keyword 'AUTO' should not be localized", diff --git a/locales/messages.json b/locales/messages.json index 203e796ff1..d33ef52815 100644 --- a/locales/messages.json +++ b/locales/messages.json @@ -663,8 +663,6 @@ "_ErrorParsingBinaryParagraph.comment": "An example of {spec} is zlib:x64-windows.", "ErrorRequireBaseline": "this vcpkg instance requires a manifest with a specified baseline in order to interact with ports. Please add 'builtin-baseline' to the manifest or add a 'vcpkg-configuration.json' that redefines the default registry.", "ErrorRequirePackagesList": "`vcpkg install` requires a list of packages to install in classic mode.", - "ErrorScriptDownloadFailed": "The script download failed.\n{error_msg}.", - "_ErrorScriptDownloadFailed.comment": "An example of {error_msg} is File Not Found.", "ErrorUnableToDetectCompilerInfo": "vcpkg was unable to detect the active compiler's information. See above for the CMake failure output.", "_ErrorUnableToDetectCompilerInfo.comment": "failure output will be displayed at the top of this", "ErrorVcvarsUnsupported": "in triplet {triplet}: Use of Visual Studio's Developer Prompt is unsupported on non-Windows hosts.\nDefine 'VCPKG_CMAKE_SYSTEM_NAME' or 'VCPKG_CHAINLOAD_TOOLCHAIN_FILE' in the triplet file.", diff --git a/src/vcpkg/base/downloads.cpp b/src/vcpkg/base/downloads.cpp index 340cba94cb..c3b2014954 100644 --- a/src/vcpkg/base/downloads.cpp +++ b/src/vcpkg/base/downloads.cpp @@ -990,7 +990,7 @@ namespace vcpkg } else { - msg::println(msgErrorScriptDownloadFailed, msg::error_msg = maybe_res.error()); + msg::println_error(maybe_res.error()); } } } From f0b0bdd813c968aae9a197080eaf5bd950933629 Mon Sep 17 00:00:00 2001 From: Javier Matos Date: Mon, 18 Nov 2024 15:17:36 -0500 Subject: [PATCH 5/5] test order of messaging --- .../end-to-end-tests-dir/asset-caching.ps1 | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/azure-pipelines/end-to-end-tests-dir/asset-caching.ps1 b/azure-pipelines/end-to-end-tests-dir/asset-caching.ps1 index d626bfd801..e00df0f361 100644 --- a/azure-pipelines/end-to-end-tests-dir/asset-caching.ps1 +++ b/azure-pipelines/end-to-end-tests-dir/asset-caching.ps1 @@ -201,9 +201,20 @@ if (-not ($actual.Contains("Asset cache hit for example3.html; downloaded from: # Testing x-download failure with asset cache (x-script) and x-block-origin settings $env:X_VCPKG_ASSET_SOURCES = "clear;x-script,pwsh $PSScriptRoot/../e2e-assets/asset-caching/failing-script.ps1 {url} {sha512} {dst};x-block-origin" $actual = Run-VcpkgAndCaptureOutput -TestArgs ($commonArgs + @("x-download", "$downloadsRoot/example3.html", "--url", "https://example.com", "--sha512", "d06b93c883f8126a04589937a884032df031b05518eed9d433efb6447834df2596aebd500d69b8283e5702d988ed49655ae654c1683c7a4ae58bfa6b92f2b73a")) -if (-not ($actual.Contains("error: failed with exit code: (1).") -and - $actual.Contains("error: Missing example3.html and downloads are blocked by x-block-origin."))) { - throw "Failure: x-script downloads failure messaging" +# Check for the expected messages in order +$expectedOrder = @( + "error: failed with exit code: (1).", + "error: Missing example3.html and downloads are blocked by x-block-origin." +) + +# Verify order +$index = 0 +foreach ($message in $expectedOrder) { + $index = $actual.IndexOf($message, $index) + if ($index -lt 0) { + throw "Failure: Expected message '$message' not found in the correct order." + } + $index += $message.Length } # Testing x-download success with asset cache (x-script) and x-block-origin settings