-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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] Restore embedded git hash behind an option. #15472
Conversation
@@ -24,17 +24,15 @@ namespace vcpkg::Commands::Version | |||
; | |||
} | |||
|
|||
const std::string& version() | |||
const char* version() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -55,7 +53,7 @@ namespace vcpkg::Commands::Version | |||
const auto num1 = scan3(version_contents->c_str(), "\"%d.%d.%d\"", &maj1, &min1, &rev1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noting here that VERSION.txt
will need to be copied into the parent repo so that we don't need the submodule at runtime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to touch anything about this. When the tool is deployed in binary form there is no version.txt which means this check is disabled. (The if on line 50 protects it)
But in registries world we can't ever make the kinds of changes this was trying to protect against anyway so I'm not sure it matters....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to merge this as-is for now and introduce vcpkg_minimum_required
as another PR.
No description provided.