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

[vcpkg] Download vcpkg.exe rather than building it in bootstrap on Windows. #15474

Merged
merged 7 commits into from
Feb 4, 2021

Conversation

BillyONeal
Copy link
Member

@BillyONeal BillyONeal commented Jan 6, 2021

This reduces bootstrap cost for Windows customers, resolving the issue initially submitted as #12502 .

The toolsrc tree was extracted to https://github.com/microsoft/vcpkg-tool. bootstrap.sh was changed to download the right source tarball, extract, and build it. This was chosen over the previous attempt, a submodule, over concerns of accidentally destroying people's local modifications.

We already have macos signed binaries ready in our infrastructure but the work to edit bootstrap.sh to consume them is not yet here.

Next steps:

  • Replay any changes to toolsrc in this repo in the tool repo landing after microsoft/vcpkg-tool@913beb7
  • Move / copy infrastructure from this main repo like C++ formatting rules that we want to enforce in the "tool" repo.
  • MacOS (in a future PR)
  • Investigate doing the same for Linux, e.g. by statically linking everything but glibc and ripping out std::filesystem use. (In future PR(s))

Depends on:

Clean build of vcpkg-tool repo example: https://dev.azure.com/vcpkg/public/_build/results?buildId=48871&view=results

@BillyONeal BillyONeal added info:internal This PR or Issue was filed by the vcpkg team. category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed labels Jan 6, 2021
@BillyONeal BillyONeal force-pushed the downloads_on_windows branch 2 times, most recently from 3975c2a to f265319 Compare January 6, 2021 07:54
@BillyONeal BillyONeal added the depends:different-pr This PR or Issue depends on a PR which has been filed label Jan 6, 2021
@JackBoosY JackBoosY self-assigned this Jan 7, 2021
@JackBoosY JackBoosY linked an issue Jan 7, 2021 that may be closed by this pull request
@JackBoosY
Copy link
Contributor

Closes #14995.

@StarGate-One
Copy link
Contributor

@BillyONeal - Thanks a lot for the hard work so far.👍👍👍

So to confirm/verify:

  1. The vcpkg-tool repository replaces the [vcpkg-root]/toolsrc directory?
  2. Any changes to the vcpkg binary will need a PR will be submitted to the vcpkg-tool repository or the issue reported on vcpkg repository and PR on vcpkg-tool repository linked to the vcpkg repository issue or ...?
  3. Will there be scripts to build the vcpkg-tool, a bootstrap-vcpkg-tool or something to that effect?

Again thanks a lot for all the work you do!👏👏👏
Have a wonderful day and super week!😎😎😎

@BillyONeal
Copy link
Member Author

The vcpkg-tool repository replaces the [vcpkg-root]/toolsrc directory?

Correct. I prepared this using git filter-branch.

Any changes to the vcpkg binary will need a PR will be submitted to the vcpkg-tool repository

Correct. Issue tracking remains here.

Will there be scripts to build the vcpkg-tool, a bootstrap-vcpkg-tool or something to that effect?

In that repo, it's just a cmake project like any other, so there are no specific scripts. Here, bootstrap on the POSIX platforms still needs to be fixed to download and build the sources.

@BillyONeal BillyONeal force-pushed the downloads_on_windows branch from cd8d74d to 7ab1489 Compare February 3, 2021 05:26
…ndows.

This reduces bootstrap cost for Windows customers, resolving the issue initially submitted as microsoft#12502 .

The toolsrc tree was extracted to https://github.com/microsoft/vcpkg-tool, and created as a submodule "vcpkg-tool" here. The name is explicitly chosen to be different from toolsrc because git doesn't like an ordinary directory becoming a submodule when changing branches or vice versa. Given that the name had to change, I set the name to the same name as the actual github repo.

Clean build of vcpkg-tool repo example: https://dev.azure.com/vcpkg/public/_build/results?buildId=48871&view=results
@BillyONeal BillyONeal force-pushed the downloads_on_windows branch from 7ab1489 to f48c39d Compare February 3, 2021 05:29
@BillyONeal BillyONeal added the depends:different-pr This PR or Issue depends on a PR which has been filed label Feb 3, 2021
# Conflicts:
#	scripts/azure-pipelines/end-to-end-tests-dir/registries.ps1
#	toolsrc/cmake/utilities.cmake
#	toolsrc/include/vcpkg/base/expected.h
#	toolsrc/include/vcpkg/vcpkgcmdarguments.h
#	toolsrc/src/vcpkg-test/binarycaching.cpp
#	toolsrc/src/vcpkg-test/json.cpp
#	toolsrc/src/vcpkg-test/manifests.cpp
#	toolsrc/src/vcpkg.cpp
#	toolsrc/src/vcpkg/base/json.cpp
#	toolsrc/src/vcpkg/base/parse.cpp
#	toolsrc/src/vcpkg/build.cpp
#	toolsrc/src/vcpkg/dependencies.cpp
#	toolsrc/src/vcpkg/registries.cpp
#	toolsrc/src/vcpkg/vcpkgcmdarguments.cpp
@BillyONeal BillyONeal removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Feb 4, 2021
srcDir="$srcBaseDir/vcpkg-tool-$vcpkgToolReleaseTag"

if [ -e "$tarballPath" ]; then
vcpkgCheckEqualFileHash "$vcpkgToolUrl" "$tarballPath" "$vcpkgToolReleaseSha"
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, we should probably check the file hash, and if they're not equal, delete it and download again, as opposed to just failing.

@BillyONeal
Copy link
Member Author

Considering the status quo for all the other downloads in this script (or elsewhere in the tool -- that's why we had to add FILE_DISAMBIGUATOR to places) doesn't do that would you be OK just turning that into an issue?

@strega-nil
Copy link
Contributor

@BillyONeal yep, I'm good with that.

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:internal This PR or Issue was filed by the vcpkg team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide pre-built binaries through GitHub action
6 participants