Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix download error bug #1513

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion azure-pipelines/end-to-end-tests-dir/asset-caching.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ $actual = Run-VcpkgAndCaptureOutput -TestArgs ($commonArgs + @("install", "vcpkg
$actual = $actual -replace "`r`n", "`n"

$expected = @(
"A suitable version of .* was not found \(required v[0-9\.]+\)."
"Asset cache miss; downloading from .*"
"Successfully stored .* to .*."
) -join "`n"
Expand Down
8 changes: 4 additions & 4 deletions include/vcpkg/base/message-data.inc.h
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,10 @@ DECLARE_MESSAGE(ArtifactsSwitchX86, (), "", "Forces host detection to x86 when a
DECLARE_MESSAGE(ArtifactsSwitchWindows, (), "", "Forces host detection to Windows when acquiring artifacts")
DECLARE_MESSAGE(AssetCacheHit, (msg::path, msg::url), "", "Asset cache hit for {path}; downloaded from: {url}")
DECLARE_MESSAGE(AssetCacheMiss, (msg::url), "", "Asset cache miss; downloading from {url}")
DECLARE_MESSAGE(AssetCacheMissBlockOrigin,
(msg::path),
"x-block-origin is a vcpkg term. Do not translate",
"Asset cache miss for {path} and downloads are blocked by x-block-origin.")
DECLARE_MESSAGE(DownloadingUrl, (msg::url), "", "Downloading {url}")
DECLARE_MESSAGE(AssetCacheProviderAcceptsNoArguments,
(msg::value),
Expand Down Expand Up @@ -1228,10 +1232,6 @@ DECLARE_MESSAGE(MissingShaVariable,
(),
"{{sha}} should not be translated",
"The {{sha}} variable must be used in the template if other variables are used.")
DECLARE_MESSAGE(AssetCacheMissBlockOrigin,
(msg::path),
"x-block-origin is a vcpkg term. Do not translate",
"Asset cache miss for {path} and downloads are blocked by x-block-origin.")
DECLARE_MESSAGE(FailedToExtract, (msg::path), "", "Failed to extract \"{path}\":")
DECLARE_MESSAGE(FailedToFetchRepo, (msg::url), "", "Failed to fetch {url}.")
DECLARE_MESSAGE(FailedToFindPortFeature,
Expand Down
9 changes: 4 additions & 5 deletions src/vcpkg/base/downloads.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -936,6 +936,10 @@ namespace vcpkg
msg::url = replace_secrets(read_url, m_config.m_secrets));
return read_url;
}
else
{
msg::println(msgAssetCacheMissBlockOrigin, msg::path = download_path.filename());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not seem correct to me, can you show how we know x-block-origin is set here?

}
}
else if (auto script = m_config.m_script.get())
{
Expand Down Expand Up @@ -1012,11 +1016,6 @@ namespace vcpkg
}
}
}
// Asset cache is not configured and x-block-origin enabled
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous comment is incorrect: the problem is not every code path on line 988 if (!m_config.m_block_origin) returns or exits. We probably need to add a if (m_config.m_block_origin) conditional here or refactor both sections.

if (m_config.m_read_url_template.has_value())
{
msg::println(msgAssetCacheMissBlockOrigin, msg::path = download_path.filename());
}
else
{
msg::println_error(msgMissingAssetBlockOrigin, msg::path = download_path.filename());
Expand Down
Loading