Skip to content

[VCPKG] Adding support for COS binary caching#23538

Merged
strega-nil-ms merged 1 commit intomicrosoft:masterfrom
day253:add-cos-tools
Apr 13, 2022
Merged

[VCPKG] Adding support for COS binary caching#23538
strega-nil-ms merged 1 commit intomicrosoft:masterfrom
day253:add-cos-tools

Conversation

@day253
Copy link
Copy Markdown
Contributor

@day253 day253 commented Mar 14, 2022

Describe the pull request

  • What does your PR fix?

    Adding support for COS binary caching

depends on microsoft/vcpkg-tool#435

  • Which triplets are supported/not supported? Have you updated the CI baseline?

    all, No

  • Does your PR follow the maintainer guide?

    Yes

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

    Yes

If you are still working on the PR, open it as a Draft: https://github.blog/2019-02-14-introducing-draft-pull-requests/
No

Copy link
Copy Markdown
Contributor

@JackBoosY JackBoosY left a comment

Choose a reason for hiding this comment

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

The tools contained in this file are required by vcpkg instead of ports, please do not put them here.

Comment thread scripts/vcpkgTools.xml Outdated
Comment thread scripts/vcpkgTools.xml Outdated
Comment thread scripts/vcpkgTools.xml Outdated
@JackBoosY
Copy link
Copy Markdown
Contributor

JackBoosY commented Mar 14, 2022

Any reason to add this tool?

@JackBoosY
Copy link
Copy Markdown
Contributor

Depends on microsoft/vcpkg-tool#435.

@JackBoosY JackBoosY added category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed depends:different-pr This PR or Issue depends on a PR which has been filed and removed invalid requires:author-response labels Mar 14, 2022
@day253
Copy link
Copy Markdown
Contributor Author

day253 commented Mar 31, 2022

Depends on microsoft/vcpkg-tool#435.

Because of the code conflict. Depends on microsoft/vcpkg-tool#476.

@day253
Copy link
Copy Markdown
Contributor Author

day253 commented Mar 31, 2022

@JackBoosY I found vcpkg will not make the directly downloaded binary executable. The tar package will preserve the right permission. I am not really familiar with codebase. Where is the right place to add this function? I think the nuget.exe have the same problem.

@PhoebeHui PhoebeHui changed the title Adding support for COS binary caching [VCPKG] Adding support for COS binary caching Apr 1, 2022
@JackBoosY
Copy link
Copy Markdown
Contributor

@day253
Copy link
Copy Markdown
Contributor Author

day253 commented Apr 13, 2022

Depends on microsoft/vcpkg-tool#435.

The upstream PR microsoft/vcpkg-tool#476 has been merged. Can you please approve this PR?

@JackBoosY

@JackBoosY JackBoosY removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Apr 13, 2022
@JackBoosY
Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@day253 day253 requested a review from JackBoosY April 13, 2022 02:52
@day253
Copy link
Copy Markdown
Contributor Author

day253 commented Apr 13, 2022

@JackBoosY All check have passed.

Copy link
Copy Markdown
Contributor

@JackBoosY JackBoosY left a comment

Choose a reason for hiding this comment

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

Can you please have a test with this tools?

@day253
Copy link
Copy Markdown
Contributor Author

day253 commented Apr 13, 2022

Can you please have a test with this tools?

Another PR or this time?I have to find a way to test this tool.

@day253
Copy link
Copy Markdown
Contributor Author

day253 commented Apr 13, 2022

@day253
Copy link
Copy Markdown
Contributor Author

day253 commented Apr 13, 2022

Can you please have a test with this tools?

I open an issue track this test problem.

#24113

@JackBoosY
Copy link
Copy Markdown
Contributor

Can you please have a test with this tools?

Another PR or this time?I have to find a way to test this tool.

Just test this locally would be good.

@day253
Copy link
Copy Markdown
Contributor Author

day253 commented Apr 13, 2022

Can you please have a test with this tools?

Another PR or this time?I have to find a way to test this tool.

Just test this locally would be good.

We have deployed this tool in our production enviroment. So this PR will not break current codebase.

@JackBoosY JackBoosY added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Apr 13, 2022
@strega-nil-ms
Copy link
Copy Markdown
Contributor

LGTM! Thanks :)

@strega-nil-ms strega-nil-ms merged commit 859cdf1 into microsoft:master Apr 13, 2022
@day253 day253 deleted the add-cos-tools branch April 14, 2022 00:51
@PavelCurtis
Copy link
Copy Markdown
Contributor

The SHA-512 hash used for the Windows executable appears to now be incorrect. I don't know if the file changed recently on the server or if perhaps the hash was always incorrect.

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

Labels

category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants