-
Notifications
You must be signed in to change notification settings - Fork 363
Add new baseline-diff command #1980
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
4a51845
93cc331
650a931
1945ffd
5492f16
0bfc39b
a211bba
12d0661
96ebbe9
e7bcd2d
cdc0bef
f6943f7
0b38e43
3e54317
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| #pragma once | ||
|
|
||
| #include <vcpkg/fwd/triplet.h> | ||
| #include <vcpkg/fwd/vcpkgcmdarguments.h> | ||
| #include <vcpkg/fwd/vcpkgpaths.h> | ||
|
|
||
| namespace vcpkg | ||
|
autoantwort marked this conversation as resolved.
|
||
| { | ||
|
autoantwort marked this conversation as resolved.
|
||
| extern const CommandMetadata CommandBaselineDiffMetadata; | ||
| void command_baseline_diff_and_exit(const VcpkgCmdArguments& args, | ||
| const VcpkgPaths& paths, | ||
| vcpkg::Triplet default_triplet, | ||
| vcpkg::Triplet host_triplet); | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,183 @@ | ||||||||||
| #include <vcpkg/base/fwd/message_sinks.h> | ||||||||||
|
|
||||||||||
| #include <vcpkg/base/contractual-constants.h> | ||||||||||
| #include <vcpkg/base/files.h> | ||||||||||
| #include <vcpkg/base/messages.h> | ||||||||||
| #include <vcpkg/base/parse.h> | ||||||||||
| #include <vcpkg/base/path.h> | ||||||||||
| #include <vcpkg/base/strings.h> | ||||||||||
| #include <vcpkg/base/util.h> | ||||||||||
|
|
||||||||||
| #include <vcpkg/binarycaching.h> | ||||||||||
| #include <vcpkg/cmakevars.h> | ||||||||||
| #include <vcpkg/commands.baseline-diff.h> | ||||||||||
| #include <vcpkg/commands.install.h> | ||||||||||
| #include <vcpkg/configuration.h> | ||||||||||
| #include <vcpkg/dependencies.h> | ||||||||||
| #include <vcpkg/installeddatabase.h> | ||||||||||
| #include <vcpkg/installedpaths.h> | ||||||||||
| #include <vcpkg/paragraphs.h> | ||||||||||
| #include <vcpkg/portfileprovider.h> | ||||||||||
| #include <vcpkg/vcpkgcmdarguments.h> | ||||||||||
| #include <vcpkg/vcpkgpaths.h> | ||||||||||
|
|
||||||||||
| #include <map> | ||||||||||
| #include <memory> | ||||||||||
| #include <string> | ||||||||||
| #include <vector> | ||||||||||
|
|
||||||||||
| using namespace vcpkg; | ||||||||||
|
|
||||||||||
| namespace | ||||||||||
| { | ||||||||||
| constexpr CommandSwitch switches[] = { | ||||||||||
| {SwitchXNoDefaultFeatures, msgHelpTxtOptManifestNoDefault}, | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| static constexpr CommandMultiSetting multisettings[] = { | ||||||||||
| {SwitchXFeature, msgHelpTxtOptManifestFeature}, | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| bool print_lines(LocalizedString&& header, std::vector<std::string>&& lines) | ||||||||||
| { | ||||||||||
| if (!lines.empty()) | ||||||||||
| { | ||||||||||
| Util::sort_unique_erase(lines); | ||||||||||
| msg::println(header.append_raw(':')); | ||||||||||
|
autoantwort marked this conversation as resolved.
Outdated
|
||||||||||
| for (const auto& line : lines) | ||||||||||
| { | ||||||||||
| msg::write_unlocalized_text(Color::none, line); | ||||||||||
| msg::write_unlocalized_text(Color::none, "\n"); | ||||||||||
| } | ||||||||||
| msg::write_unlocalized_text(Color::none, "\n"); | ||||||||||
| return true; | ||||||||||
| } | ||||||||||
| return false; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| void check_for_valid_sha(StringView sha) | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this use check_commit_exists (from git.h) instead?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see we can't do that in the general case because this kinda just blindly overwrites the baseline with a SHA and we don't necessarily have a git repo present for that. However that makes this kinda broken because filesystem registries' baselines are not SHAs. We should only be checking that the input is a SHA when we know that registry has that constraint.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code is registry.h/cpp could really benefit from some comments. What is a |
||||||||||
| { | ||||||||||
| if (sha.size() < 7 || !Util::all_of(sha, ParserBase::is_hex_digit_lower)) | ||||||||||
| { | ||||||||||
| Checks::msg_exit_with_error(VCPKG_LINE_INFO, msgInvalidCommitId, msg::commit_sha = sha); | ||||||||||
| } | ||||||||||
|
autoantwort marked this conversation as resolved.
Outdated
|
||||||||||
| } | ||||||||||
|
autoantwort marked this conversation as resolved.
Outdated
autoantwort marked this conversation as resolved.
Outdated
|
||||||||||
|
|
||||||||||
| } // unnamed namespace | ||||||||||
|
|
||||||||||
| namespace vcpkg | ||||||||||
| { | ||||||||||
| constexpr CommandMetadata CommandBaselineDiffMetadata{ | ||||||||||
| "x-baseline-diff", | ||||||||||
| msgCmdBaselineDiffSynopsis, | ||||||||||
| {"vcpkg x-baseline-diff <old_commit_sha> <newer_commit_sha>", | ||||||||||
| "vcpkg x-baseline-diff $(git rev-parse 2026.02.27) $(git rev-parse 2026.03.18)"}, | ||||||||||
|
||||||||||
| {"vcpkg x-baseline-diff <old_commit_sha> <newer_commit_sha>", | |
| "vcpkg x-baseline-diff $(git rev-parse 2026.02.27) $(git rev-parse 2026.03.18)"}, | |
| {msgCmdBaselineDiffExample1, msgCmdBaselineDiffExample2}, |
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 must say that i am always very confused if such examples gets translated. Also only old and new would be translated here (The people that uses git also understand git without translations...).
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.
In general if there was "English" in the example we made it be translated, and if there was not we did not. So for instance, remove has one that is translated (vcpkg remove <package name>... because "package name" is translatable) and two that are not:
vcpkg-tool/src/vcpkg/commands.remove.cpp
Line 174 in 4a0ae3e
| {msgCmdRemoveExample1, "vcpkg remove zlib zlib:x64-windows curl boost", "vcpkg remove --outdated"}, |
So I believe in this case the one that says <old_commit_sha> should get translated but the one with rev-parse should not. (However I'm not sure we should show the rev-parse example at all as that syntax makes shell assumptions)
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 think we should just accept the tags here rather than needing the user to say rev-parse? (I fixed a similar bug in #1935 )
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 think we need a docs page for this.
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.
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 not sure I agree with this design: Because the user is explicitly passing in multiple baseline SHAs, it seems to really be more of a classic mode command as other registries aren't accounted for.
It seems like what you actually wanted is effectively this same diff but before/after what x-update-baseline does?
If this stays manifest only, it seems like one of the SHAs should be the one already in the manifest?
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.
It seems like what you actually wanted is effectively this same diff but before/after what x-update-baseline does?
Yes. I wanted to know what x-update-baseline does mean to me in terms of port versions, or what it would mean.
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.
If this stays manifest only, it seems like one of the SHAs should be the one already in the manifest?
At least if there is no second one. You are right, this would be more developer friendly.
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 not sure if
installableconveys the meaning but I'd prefer a terse description.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.
Maybe installed? Because the packages listed in the vcpkg.json are normally installed.