Skip to content

Add x-azcopy and x-azcopy-sas binary caching sources#1738

Merged
vicroms merged 26 commits intomicrosoft:mainfrom
vicroms:azcopy-provider
Aug 19, 2025
Merged

Add x-azcopy and x-azcopy-sas binary caching sources#1738
vicroms merged 26 commits intomicrosoft:mainfrom
vicroms:azcopy-provider

Conversation

@vicroms
Copy link
Copy Markdown
Member

@vicroms vicroms commented Jul 29, 2025 •
edited
Loading

Adds two new binary sources that interact with Azure Storage blob containers using the azcopy executable.

  • x-azcopy-sas,<url>,<sas>[,<rw>]: Intended as a drop-in replacement for azblob, it uses SAS tokens for authentication.
  • x-azcopy,<url>[,<rw>]: Intended for non-SAS authentication methods like Microsoft Entra ID; requires preloading credentials or/and the AZCOPY_AUTO_LOGIN_TYPE.

Internally, we use azcopy copy --from-to BlobLocal {destination} --include-path {files} argument to download multiple files in a single azcopy job, this improves performance and brings it more in line with azblob, that uses a single curl invocation to download a batch of files. Upload happens one file at a time in the background.

Small test

x-azblob:
    Restored 5 package(s) from HTTP servers in 3.8 s. Use --debug to see more details.

x-azcopy:
    Restored 5 package(s) from Azure Storage in 1.5 min. Use --debug to see more details.

x-azcopy-sas:
    Restored 5 package(s) from Azure Storage in 4.7 s. Use --debug to see more details.

Large test

azcopy-sas (with --include-path):
    Restored 141 package(s) from Azure Storage in 1.5 min. Use --debug to see more details.

azblob:
    Restored 141 package(s) from HTTP servers in 3 min. Use --debug to see more details.

Related:

vicroms
vicroms commented Jul 31, 2025
View reviewed changes
Comment thread src/vcpkg/binarycaching.cpp Outdated
@vicroms vicroms changed the title Add x-azcopy provider Add x-azcopy and x-azcopy-sas binary caching sources Aug 1, 2025
@vicroms vicroms marked this pull request as ready for review August 1, 2025 18:43
BillyONeal
BillyONeal reviewed Aug 1, 2025
View reviewed changes
Comment thread include/vcpkg/metrics.h
Comment thread src/vcpkg/binarycaching.cpp Outdated
Comment thread src/vcpkg/binarycaching.cpp Outdated
Comment thread src/vcpkg/binarycaching.cpp Outdated
Comment thread src/vcpkg/binarycaching.cpp Outdated
Comment thread src/vcpkg/binarycaching.cpp Outdated
vicroms
vicroms commented Aug 2, 2025
View reviewed changes
Comment thread src/vcpkg/binarycaching.cpp Outdated
@vicroms vicroms mentioned this pull request Aug 2, 2025
Merged
@vicroms vicroms mentioned this pull request Aug 4, 2025
Merged
@vicroms vicroms marked this pull request as draft August 5, 2025 19:24
@vicroms
Copy link
Copy Markdown
Member Author

vicroms commented Aug 5, 2025

Back to draft while the x64-osx upload failures are being investigated: https://dev.azure.com/vcpkg/public/_build/results?buildId=118621&view=logs&j=7b75bd19-17d3-53d4-00fd-23f1a49a8ba4&t=c0c57043-5abe-5a13-5ef1-aa66ea261ab3&l=95

@vicroms vicroms marked this pull request as ready for review August 7, 2025 19:19
BillyONeal
BillyONeal reviewed Aug 11, 2025
View reviewed changes
Comment thread src/vcpkg/binarycaching.cpp Outdated
BillyONeal
BillyONeal requested changes Aug 11, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@BillyONeal BillyONeal left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The only comment I'm 'request changes' over is the one on line 1078 where the for loop seems clearly broken. The other things are nitpicks.

Comment thread src/vcpkg/binarycaching.cpp Outdated
Comment thread src/vcpkg/binarycaching.cpp Outdated
Comment thread src/vcpkg/binarycaching.cpp Outdated
Comment thread src/vcpkg/binarycaching.cpp Outdated
Comment thread src/vcpkg/binarycaching.cpp Outdated
Comment thread src/vcpkg/binarycaching.cpp
Comment thread src/vcpkg/binarycaching.cpp Outdated
BillyONeal
BillyONeal previously approved these changes Aug 12, 2025
View reviewed changes
Thomas1664
Thomas1664 reviewed Aug 12, 2025
View reviewed changes
Comment thread src/vcpkg/binarycaching.cpp Outdated
@ras0219-msft ras0219-msft requested a review from Copilot August 12, 2025 22:10
Copilot AI reviewed Aug 12, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds two new binary caching sources for vcpkg that use Azure Storage blob containers via the azcopy executable. The implementation provides both SAS token-based authentication and Microsoft Entra ID authentication options.

Key changes:

  • Introduces x-azcopy and x-azcopy-sas binary cache providers as alternatives to existing Azure blob storage options
  • Implements batch downloading using azcopy copy --include-path for improved performance compared to individual file downloads
  • Adds comprehensive test coverage for both new provider types with various configuration scenarios

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/vcpkg/metrics.cpp Adds metrics tracking for the two new azcopy binary cache providers
src/vcpkg/binarycaching.cpp Core implementation of AzCopy providers with batch download/upload logic and configuration parsing
src/vcpkg-test/configparser.cpp Comprehensive unit tests for both azcopy provider variants with various configuration scenarios
locales/messages.json Adds localized message for Azure Storage restoration feedback
include/vcpkg/metrics.h Defines new metric types for tracking azcopy provider usage
include/vcpkg/binarycaching.h Declares AzCopyUrl structure and adds provider state fields
include/vcpkg/base/message-data.inc.h Declares the new Azure Storage restoration message

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread src/vcpkg/binarycaching.cpp Outdated
Comment thread src/vcpkg/binarycaching.cpp Outdated
Comment thread src/vcpkg/binarycaching.cpp Outdated
Comment thread src/vcpkg/binarycaching.cpp
BillyONeal
BillyONeal previously approved these changes Aug 19, 2025
View reviewed changes
Comment thread src/vcpkg/binarycaching.cpp
struct AzCopyStorageProvider : ZipReadBinaryProvider
{
AzCopyStorageProvider(
ZipTool zip, const Filesystem& fs, const Path& buildtrees, AzCopyUrl&& az_url, const Path& tool)
Copy link
Copy Markdown
Member

@BillyONeal BillyONeal Aug 19, 2025

Choose a reason for hiding this comment

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

Copy/move antipattern on ZipTool seems bad to me but the other providers are already doing it so no change requested.

Copy link
Copy Markdown
Member Author

@vicroms vicroms Aug 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think this would be greatly improved by something like @autoantwort's #911 but out of the scope of this PR.

Comment thread src/vcpkg/binarycaching.cpp Outdated
@vicroms vicroms merged commit 1109746 into microsoft:main Aug 19, 2025
7 checks passed
@vicroms vicroms deleted the azcopy-provider branch August 26, 2025 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@BillyONeal BillyONeal BillyONeal approved these changes

+1 more reviewer

@Thomas1664 Thomas1664 Thomas1664 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vicroms @BillyONeal @Thomas1664