Skip to content

[vcpkg] support Google Cloud Storage as binary cache#11

Merged
ras0219-msft merged 4 commits intomicrosoft:mainfrom
coryan:feat-binary-cache-with-gcs
Feb 18, 2021
Merged

[vcpkg] support Google Cloud Storage as binary cache#11
ras0219-msft merged 4 commits intomicrosoft:mainfrom
coryan:feat-binary-cache-with-gcs

Conversation

@coryan
Copy link
Contributor

@coryan coryan commented Feb 14, 2021

Adds a new binary cache provider (configured via x-gcs) to use
Google Cloud Storage as the backend. All uploads and downloads
are done via the gsutil tool.

I expect this PR to need a lot of additional work, mostly I am seeking
feedback.

There is plenty of room for optimization, this uploads and downloads
one object at a time, and does not take advantage of gsutil parallelization
either (aka the -m option).

Another future improvement would be to structure the cache to support
gsutil rsync.

@coryan
Copy link
Contributor Author

coryan commented Feb 14, 2021

/cc: @ras0219-msft as we discussed earlier, here is an initial PR.

@coryan coryan marked this pull request as ready for review February 15, 2021 00:04
vicroms
vicroms reviewed Feb 15, 2021
View reviewed changes
Copy link
Member

@vicroms vicroms left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I know it's still a draft but I took a peek and left some comments.

ras0219
ras0219 reviewed Feb 16, 2021
View reviewed changes
Copy link
Contributor

@ras0219 ras0219 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.

Looks great so far 👍. I think this is close to mergeable as an initial implementation, though supporting parallelism during download would be really great.

src/vcpkg/binarycaching.cpp
bool gsutil_stat(const std::string& url)
{
System::Command cmd;
cmd.string_arg("gsutil").string_arg("-q").string_arg("stat").string_arg(url);
Copy link
Contributor

@ras0219 ras0219 Feb 16, 2021

Choose a reason for hiding this comment

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

We may want to add gsutil to https://github.com/microsoft/vcpkg-tool/blob/main/src/vcpkg/tools.cpp to eventually support automatic download via vcpkgTools.xml (https://github.com/microsoft/vcpkg/blob/master/scripts/vcpkgTools.xml).

Copy link
Contributor Author

@coryan coryan Feb 16, 2021

Choose a reason for hiding this comment

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

That does seem like a good idea. I doubt I will have time to do this until the weekend. We can wait or merge now, your call, but I prefer to merge sooner rather than later.

coryan and others added 2 commits February 16, 2021 14:11
@coryan @ras0219
Apply suggestions from code review
dce9238 
Co-authored-by: ras0219 <533828+ras0219@users.noreply.github.com>
@coryan
Fix formatting
d2f5a3b
@coryan
Copy link
Contributor Author

coryan commented Feb 16, 2021

Looks great so far 👍. I think this is close to mergeable as an initial implementation,

Cool.

though supporting parallelism during download would be really great.

gsutil likes to puts all the downloaded files in a single destination directory. It seems to me that vcpkg (when using curl) takes advantage of the -o option to put each file in separate directories? Is there an acceptable directory to use as staging for the downloads and then do an fs::???? operation to move them? Sorry if this should be obvious, the code is fairly clear, but I am still figuring out the lay of the land.

@ras0219-msft
Copy link
Collaborator

ras0219-msft commented Feb 16, 2021

You can use a subdirectory of buildtrees that contains an illegal identifier character, such as _. So, paths.buildtrees / "_gsutil", for example.

@ras0219-msft
Copy link
Collaborator

ras0219-msft commented Feb 18, 2021

@coryan Let me know if you'd like this to be merged or if you're still working on it.

@coryan
Copy link
Contributor Author

coryan commented Feb 18, 2021

Let's merge as-is, I am working on the parallel download but will take me a while. Thanks for checking on me.

@ras0219-msft ras0219-msft merged commit d276ea1 into microsoft:main Feb 18, 2021
@ras0219-msft
Copy link
Collaborator

ras0219-msft commented Feb 18, 2021

Awesome, thanks!

When we push a binary tool update, I'll ping you to potentially add a docs blurb to binarycaching.md

@coryan coryan deleted the feat-binary-cache-with-gcs branch February 18, 2021 23:32
This was referenced Feb 18, 2021
Closed
Closed
@coryan
Copy link
Contributor Author

coryan commented Feb 18, 2021

Happy to send patches to the documentation too. I also created a couple of bugs to implement the suggestions (requests?) that came up during the review. Feel free to assign them to me.

vicroms added a commit to vicroms/vcpkg-tool that referenced this pull request Aug 28, 2025
@vicroms
# This is a combination of 30 commits.
2c4a760 
# This is the 1st commit message:

WIP

# The commit message #2 will be skipped:

# Fix build error

# The commit message microsoft#3 will be skipped:

# Implement bulk operation

# The commit message microsoft#4 will be skipped:

# Remove unused struct

# The commit message microsoft#5 will be skipped:

# Follow redirects

# The commit message microsoft#6 will be skipped:

# Get HTTP response code

# The commit message microsoft#7 will be skipped:

# Map URL response code to the correct index

# The commit message microsoft#8 will be skipped:

# Map URL response code to the correct index

# The commit message microsoft#9 will be skipped:

# Undo changes to workflows

# The commit message microsoft#10 will be skipped:

# Implement store to asset cache

# The commit message microsoft#11 will be skipped:

# handle file:// protocol

# The commit message microsoft#12 will be skipped:

# Implement try_download_file

# The commit message microsoft#13 will be skipped:

# Add missing calls to curl_easy_cleanup

# The commit message microsoft#14 will be skipped:

# Fix global init

# The commit message microsoft#15 will be skipped:

# Replace all calls in downloads.cpp and fix unit tests

# The commit message microsoft#16 will be skipped:

# use libcurl to submit metrics payload

# The commit message microsoft#17 will be skipped:

# Use Schannel on Windows

# The commit message microsoft#18 will be skipped:

# Apply suggestions from code review
#
# Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

# The commit message microsoft#19 will be skipped:

# Fix unit test

# The commit message microsoft#20 will be skipped:

# Change curl error messages

# The commit message microsoft#21 will be skipped:

# generate message map

# The commit message microsoft#22 will be skipped:

# Always call multi_remove_handle

# The commit message microsoft#23 will be skipped:

# Fix end-to-end tests

# The commit message microsoft#24 will be skipped:

# Cleanup header lists

# The commit message microsoft#25 will be skipped:

# curl global init order

# The commit message microsoft#26 will be skipped:

# Use external libcurl on non-Windows

# The commit message microsoft#27 will be skipped:

# Install libcurl4 dev package on Linux

# The commit message microsoft#28 will be skipped:

# Disable metrics by default in debug builds

# The commit message microsoft#29 will be skipped:

# Set user agent using CURLOPT_USERAGENT

# The commit message microsoft#30 will be skipped:

# curl initialization again
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vicroms vicroms vicroms approved these changes

+1 more reviewer

@ras0219 ras0219 ras0219 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

@coryan @ras0219-msft @ras0219 @vicroms