-
Notifications
You must be signed in to change notification settings - Fork 351
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 all commits
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 | ||
| { | ||
| 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,184 @@ | ||||||||||
| #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(const msg::MessageT<>& header, std::vector<std::string>&& lines) | ||||||||||
| { | ||||||||||
| if (lines.empty()) | ||||||||||
| { | ||||||||||
| return false; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| Util::sort_unique_erase(lines); | ||||||||||
| msg::print(msg::format(header).append_raw(":\n")); | ||||||||||
| 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; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| 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. |
||||||||||
| { | ||||||||||
| 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.
|
||||||||||
| } | ||||||||||
|
autoantwort marked this conversation as resolved.
|
||||||||||
|
|
||||||||||
| } // 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)"}, | ||||||||||
|
Comment on lines
+74
to
+75
|
||||||||||
| {"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.
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.
| check_for_valid_sha(options.command_arguments.at(0)); | |
| check_for_valid_sha(options.command_arguments.at(1)); | |
| check_for_valid_sha(options.command_arguments[0]); | |
| check_for_valid_sha(options.command_arguments[1]); |
Check failure on line 100 in src/vcpkg/commands.baseline-diff.cpp
GitHub Actions / builds / build (ubuntu-24.04-arm, linux-arm64-ci)
invalid initialization of reference of type ‘const vcpkg::InstallAndBuildDatabaseLock&’ from expression of type ‘vcpkg::InstalledDatabaseLock’
Check failure on line 100 in src/vcpkg/commands.baseline-diff.cpp
GitHub Actions / builds / build (macos-26, macos-ci)
no viable conversion from 'InstalledDatabaseLock' to 'const InstallAndBuildDatabaseLock'
Check failure on line 100 in src/vcpkg/commands.baseline-diff.cpp
GitHub Actions / builds / build (windows-2025, windows-ci)
'std::unique_ptr<vcpkg::CMakeVars::CMakeVarProvider,std::default_delete<vcpkg::CMakeVars::CMakeVarProvider>> vcpkg::CMakeVars::make_triplet_cmake_var_provider(const vcpkg::VcpkgPaths &,const vcpkg::InstallAndBuildDatabaseLock &)': cannot convert argument 2 from 'vcpkg::InstalledDatabaseLock' to 'const vcpkg::InstallAndBuildDatabaseLock &'
Check failure on line 100 in src/vcpkg/commands.baseline-diff.cpp
GitHub Actions / builds / build (ubuntu-24.04, linux-ci)
invalid initialization of reference of type ‘const vcpkg::InstallAndBuildDatabaseLock&’ from expression of type ‘vcpkg::InstalledDatabaseLock’
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.
| for (int i = 0; i < 2; ++i) | |
| for (size_t i = 0; i < 2; ++i) |
to avoid any sugned/unsigned mismatch warnings from vector.
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.
| default_reg->baseline = options.command_arguments.at(i); | |
| default_reg->baseline = options.command_arguments[i]; |
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.
| synthesized_registry.baseline = options.command_arguments.at(i); | |
| synthesized_registry.baseline = options.command_arguments[i]; |
Uh oh!
There was an error while loading. Please reload this page.