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] Correct git version #25358

Closed
wants to merge 2 commits into from
Closed

Conversation

JackBoosY
Copy link
Contributor

@JackBoosY JackBoosY commented Jun 21, 2022

Git was updated in #22985, but git version was wrongly restored by PR #23424.

Fixes #25339.

@JackBoosY JackBoosY added category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) info:internal This PR or Issue was filed by the vcpkg team. labels Jun 21, 2022
@Cheney-W Cheney-W added the info:reviewed Pull Request changes follow basic guidelines label Jun 21, 2022
@JackBoosY JackBoosY changed the title Correct git version [vcpkg tool] Correct git version Jun 21, 2022
@JackBoosY JackBoosY changed the title [vcpkg tool] Correct git version [vcpkg] Correct git version Jun 21, 2022
@Neumann-A
Copy link
Contributor

@JackBoosY: Your are probably not fixing stuff here. The message should tell: Downloading git .... without mentioning the version

@JackBoosY
Copy link
Contributor Author

@Neumann-A Nonono, in fact, even if no explicit download tool information is required, the version corresponding to the download url is incorrect.

Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

The version in <version> is the minimum version we will accept from the system, not necessarily the version we are downloading here.

@JackBoosY
Copy link
Contributor Author

The version in <version> is the minimum version we will accept from the system, not necessarily the version we are downloading here.

Oh, so we need to correct the warning info.

@JackBoosY JackBoosY closed this Jun 23, 2022
@BillyONeal
Copy link
Member

We need vcpkgTools.xml to die

@JackBoosY JackBoosY deleted the dev/jack/25339 branch June 24, 2022 02:21
@cenit
Copy link
Contributor

cenit commented Jun 27, 2022

in any case, if you force the VCPKG_FORCE_DOWNLOADED_BINARIES, this is what happens and it's not clear enough to the end user. Sad the PR has been closed

@BillyONeal
Copy link
Member

in any case, if you force the VCPKG_FORCE_DOWNLOADED_BINARIES, this is what happens and it's not clear enough to the end user. Sad the PR has been closed

If the problem is that the message we display isn't clear, that can't be fixed from here. The effect of rejecting perfectly good copies of git the user already has is not an improvement.

@cenit
Copy link
Contributor

cenit commented Jun 27, 2022

the pr should then modify the downloaded version, putting an older version of git there. But at least be consistent with what it says it will download and what it downloads afterwards

@BillyONeal
Copy link
Member

the pr should then modify the downloaded version, putting an older version of git there

That doesn't make any sense to me. Just because we'll accept an existing copy has no bearing on the version of the copy we download. If we're going to download it anyway, we may as well download a current one.

But at least be consistent with what it says it will download and what it downloads afterwards

That seems reasonable to me, but again, nothing in this PR could achieve that effect. It needs to be fixed over in vcpkg-tool

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) info:internal This PR or Issue was filed by the vcpkg team. info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

inconsistency between messages and real requests
5 participants