Re-bootstrap vcpkg in manifest mode if updated #27692

Neumann-A · 2022-11-07T12:36:30Z

Solves: People being annoyed manually deleting vcpkg${CMAKE_EXECUTABLE_SUFFIX}
Solves: Issues due to vcpkg_minimum_required since you will always use latest vcpkg now!

closes #27635

autoantwort · 2022-11-07T13:15:24Z

We need a way to disable this. Where I worked we had to use a self compiled version of vcpkg because of #25521. If this is merged the version is replaced with the broken version distributed by Microsoft. So an option to disable this automatically update is welcomed.

Neumann-A · 2022-11-07T13:34:00Z

If this is merged the version is replaced with the broken version distributed by Microsoft.

Unless your version is higher than the one from the bootstrap script which you probably always want. If you update vcpkg repo you probably want to recompile your vcpkg-tool and bump the version.

So I also thought about providing an option to disable it but from the cases I thought of it was always like -> requires an update of vcpkg-tool any way.

Neumann-A · 2022-11-07T13:37:43Z

same baseline issue as #27689

BillyONeal · 2022-11-08T19:57:07Z

scripts/buildsystems/vcpkg.cmake

+    ERROR_VARIABLE Z_VCPKG_MANIFEST_VERSION_LOGTEXT
+    RESULT_VARIABLE Z_VCPKG_MANIFEST_VERSION_RESULT
+)
+string(REGEX MATCH "program version ([0-9][0-9][0-9][0-9]-[0-9]?[0-9]-[0-9]?[0-9])" z_vcpkg_ver_bin "${Z_VCPKG_MANIFEST_VERSION_LOGTEXT}")


This won't do the right thing when localized :(

Give me --version-only cli. I never get why ms programs are always so verbose.

Also: will currently do the correct thing. Localization is in the future and a problem for someone else.

Localization is not "in the future", this is broken with vcpkg-tool@main.

We could use the semi-standard --x-json:

C:\vcpkg> ./vcpkg version --x-json { "version": "2022-10-17-3247920fbdd47d08f36cbd480addd9890d3c2435" }

I think that @ras0219-msft has laid out the appropriate course of action. @Neumann-A, please let us know if you would like to continue with @ras0219-ms suggestions.

One problem with --z-require-version to keep in mind is that old versions will simply error out because they don't know the --z-require-version switch. And you can't differentiate this error from a normal installation fails error.

@JavierMatosD I rather take the --x-json approach than the new cmd line switch stuff. If the process invoke is so a big issue I could use a file(TIMESTAMP <filename> <variable> [<format>] [UTC]) (is available in CMake 3.7.2) of the bootstrap and vcpkg binary and observe changes to these files if necessary.

We could use the semi-standard --x-json:

C:\vcpkg> ./vcpkg version --x-json { "version": "2022-10-17-3247920fbdd47d08f36cbd480addd9890d3c2435" }

That doesn't exist?

One problem with --z-require-version to keep in mind is that old versions will simply error out because they don't know the --z-require-version switch. And you can't differentiate this error from a normal installation fails error.

We could interpret failing to run that as more-or-less unconditionally too old. (For example with a specific command added for this purpose)

scripts/buildsystems/vcpkg.cmake

Neumann-A · 2022-12-11T15:09:06Z

@JavierMatosD I disagree #27692 (comment) is not implementable for current users. You can add the mentioned functionallity to vcpkg-tool first and after a year switch the behavior. But currently there is no other way to implement and auto update mechanism.

Adela0814 · 2022-12-22T08:20:45Z

@JavierMatosD Please review this PR again. Thanks.

BillyONeal · 2023-01-31T23:48:29Z

We'll take a look at this one more time but I don't think the read on the situation has changed.

BillyONeal · 2023-02-02T21:46:51Z

@ras0219-msft @dan-shaw @JavierMatosD and I discussed this today.

Let's break the problem trying to be solved here apart.

Updating the copy of vcpkg.exe for the user for them by rerunning bootstrap if they have somehow switched to an incompatible version.
Not giving the user an inconsistent installed state because we are running with an old copy of vcpkg that does not know how to provide something.

We observe that solving (2) can already be achieved without building any new tech by bumping the vcpkg_minimum_required in ports.cmake here:

vcpkg/scripts/ports.cmake

Line 87 in a548ef5

vcpkg_minimum_required(VERSION 2022-10-12)

This does mean that if we actually build something the UX is slightly worse than the solution proposed here because the result will be an error message "please go update vcpkg" rather than updating it for them, which is case (1) above.

Regarding case (1), we have a host of bad tradeoffs:

Attempting to inspect anything about stdout in a normal command invocation is going to be vulnerable to localization.
Attempting to change the exit code to something different is going to require substantial plumbing inside the tool to change the success case exit code.
Running the tool again (as is done here with version) or with some form of version check command adds a minimum ~40ms cost to every cmake configure that uses vcpkg.cmake on Windows.

Speaking personally, I think that leaves us with at least 4 choices:

We don't care about solving problem (1) above; we will just always bump ports.cmake's constraint every time we ship a new tool.
We pay the additional cost for another command (for example vcpkg z-check-that-my-version-is-blah EXPECTEDVERSION) that can do the version check. (It probably shouldn't use the existing version command as proposed here unless we can make absolutely positively sure that that would not be vulnerable to being broken by localization).
We build some other form of back channel and a new switch that activates that back channel, such as "hey vcpkg if your version is OK but there's a failure please touch this file to say the version was OK"
(This came late :P) We could pass the --z-require-version to install as requested above and only try to interrogate the version (as in (2)) on failure to disambiguate between 'failed because version mismatch' and 'failed because installing the dependencies failed'

Stack ranking our preferences:

@ras0219-msft (4) (2) (3) (1) (Also, we should do (1) regardless of whether we do (2) or (3))
@dan-shaw Same as @ras0219-msft, "I think we need to fix the issue somehow" (opposed to (1))
@JavierMatosD (4), no preference between (1) and (2), opposed to (3)
@BillyONeal (4), (1), then (2), opposed to (3)

Neumann-A · 2023-02-02T22:31:05Z

Stack ranking our preferences:

so you want 4 which requires a tool PR. Got it.

Adela0814 · 2023-05-05T06:42:22Z

@Neumann-A If you are ready to review, please active this PR.

Neumann-A · 2023-05-05T09:57:50Z

@Neumann-A If you are ready to review, please active this PR.

As stated in #27692 (comment) the teams wants 4 so unless that has been implemented in the tool there is currently no way forward for this PR which is why this PR stays in Draft mode.

Adela0814 · 2024-01-08T08:40:00Z

Closing this PR since it seems that no progress is being made. Please ping us to reopen if work is still being done.

Neumann-A added 2 commits November 7, 2022 13:32

Rebootstrap vcpkg in manifest mode if updated

bf21d3e

remove CACHE variables

33f0795

github-actions bot previously approved these changes Nov 7, 2022

View reviewed changes

Cheney-W assigned Adela0814 Nov 8, 2022

Adela0814 added the category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed label Nov 8, 2022

BillyONeal reviewed Nov 8, 2022

View reviewed changes

Neumann-A commented Nov 8, 2022

View reviewed changes

scripts/buildsystems/vcpkg.cmake Outdated Show resolved Hide resolved

only use numbers to detect version of binary

459c8f9

Neumann-A dismissed github-actions[bot]’s stale review via 459c8f9 November 8, 2022 21:40

github-actions bot approved these changes Nov 8, 2022

View reviewed changes

Adela0814 added the info:reviewed Pull Request changes follow basic guidelines label Nov 28, 2022

Adela0814 approved these changes Nov 28, 2022

View reviewed changes

JavierMatosD added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines labels Nov 28, 2022

quyykk mentioned this pull request Jan 7, 2023

catch2 build failure #28762

Closed

Adela0814 added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Jan 31, 2023

BillyONeal added requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. and removed info:reviewed Pull Request changes follow basic guidelines labels Jan 31, 2023

BillyONeal added requires:author-response and removed requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. labels Feb 2, 2023

Neumann-A marked this pull request as draft February 2, 2023 22:31

Adela0814 removed the requires:author-response label May 5, 2023

Adela0814 closed this Jan 8, 2024

Neumann-A deleted the rebootstrap_outdated_vcpkg branch November 13, 2024 09:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Re-bootstrap vcpkg in manifest mode if updated #27692

Re-bootstrap vcpkg in manifest mode if updated #27692

Neumann-A commented Nov 7, 2022 •

edited

Loading

autoantwort commented Nov 7, 2022

Neumann-A commented Nov 7, 2022

Neumann-A commented Nov 7, 2022

BillyONeal Nov 8, 2022

Neumann-A Nov 8, 2022 •

edited

Loading

Neumann-A Nov 8, 2022

BillyONeal Nov 8, 2022

vicroms Nov 8, 2022 •

edited

Loading

JavierMatosD Nov 28, 2022

autoantwort Nov 28, 2022

Neumann-A Nov 28, 2022

Neumann-A Dec 11, 2022

BillyONeal Feb 2, 2023 •

edited

Loading

Neumann-A commented Dec 11, 2022

Adela0814 commented Dec 22, 2022

BillyONeal commented Jan 31, 2023

BillyONeal commented Feb 2, 2023

Neumann-A commented Feb 2, 2023

Adela0814 commented May 5, 2023 •

edited

Loading

Neumann-A commented May 5, 2023

Adela0814 commented Jan 8, 2024

Re-bootstrap vcpkg in manifest mode if updated #27692

Re-bootstrap vcpkg in manifest mode if updated #27692

Conversation

Neumann-A commented Nov 7, 2022 • edited Loading

autoantwort commented Nov 7, 2022

Neumann-A commented Nov 7, 2022

Neumann-A commented Nov 7, 2022

BillyONeal Nov 8, 2022

Choose a reason for hiding this comment

Neumann-A Nov 8, 2022 • edited Loading

Choose a reason for hiding this comment

Neumann-A Nov 8, 2022

Choose a reason for hiding this comment

BillyONeal Nov 8, 2022

Choose a reason for hiding this comment

vicroms Nov 8, 2022 • edited Loading

Choose a reason for hiding this comment

JavierMatosD Nov 28, 2022

Choose a reason for hiding this comment

autoantwort Nov 28, 2022

Choose a reason for hiding this comment

Neumann-A Nov 28, 2022

Choose a reason for hiding this comment

Neumann-A Dec 11, 2022

Choose a reason for hiding this comment

BillyONeal Feb 2, 2023 • edited Loading

Choose a reason for hiding this comment

Neumann-A commented Dec 11, 2022

Adela0814 commented Dec 22, 2022

BillyONeal commented Jan 31, 2023

BillyONeal commented Feb 2, 2023

Neumann-A commented Feb 2, 2023

Adela0814 commented May 5, 2023 • edited Loading

Neumann-A commented May 5, 2023

Adela0814 commented Jan 8, 2024

Neumann-A commented Nov 7, 2022 •

edited

Loading

Neumann-A Nov 8, 2022 •

edited

Loading

vicroms Nov 8, 2022 •

edited

Loading

BillyONeal Feb 2, 2023 •

edited

Loading

Adela0814 commented May 5, 2023 •

edited

Loading