Skip to content

Add new baseline-diff command - #1980

Open
autoantwort wants to merge 14 commits into
microsoft:mainfrom
autoantwort:feature/baseline-diff
Open

Add new baseline-diff command#1980
autoantwort wants to merge 14 commits into
microsoft:mainfrom
autoantwort:feature/baseline-diff

Conversation

@autoantwort

Copy link
Copy Markdown
Contributor

If we use x-update-baseline we wanted to know which packages gets updated for our manifest file.

Copilot AI review requested due to automatic review settings April 8, 2026 09:40
@autoantwort
autoantwort force-pushed the feature/baseline-diff branch 2 times, most recently from 04a5e7a to 9d4ae6d Compare April 8, 2026 09:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a new experimental x-baseline-diff command to help users understand which manifest dependencies would change when moving between two baselines (commit refs), integrating it into the vcpkg command dispatcher and localization system.

Changes:

  • Adds x-baseline-diff command implementation and registers it in command dispatch.
  • Extends configuration with a helper intended to detect whether the default registry is the built-in registry.
  • Adds new localized strings for command synopsis/output headings and “no change” output.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
src/vcpkg/configuration.cpp Adds a helper intended to detect built-in default registry kind (currently incorrect).
include/vcpkg/configuration.h Exposes the new configuration helper in the public header.
src/vcpkg/commands.cpp Wires the new command into the command table (currently has a syntax error in the include).
src/vcpkg/commands.baseline-diff.cpp Implements the baseline comparison by generating two install plans and diffing versions.
include/vcpkg/commands.baseline-diff.h Declares the new command entrypoint/metadata.
locales/messages.json Adds user-facing strings for the command and output labels (some grammar issues).
include/vcpkg/base/message-data.inc.h Declares new messages for localization enforcement (some grammar issues).

Comment thread src/vcpkg/configuration.cpp Outdated
Comment thread include/vcpkg/configuration.h Outdated
Comment thread src/vcpkg/commands.baseline-diff.cpp Outdated
Comment thread src/vcpkg/commands.baseline-diff.cpp Outdated
Comment thread src/vcpkg/commands.baseline-diff.cpp
Comment thread include/vcpkg/commands.baseline-diff.h
Comment thread locales/messages.json Outdated
Comment thread locales/messages.json Outdated
Comment thread include/vcpkg/base/message-data.inc.h Outdated
Comment thread include/vcpkg/base/message-data.inc.h Outdated
@autoantwort
autoantwort force-pushed the feature/baseline-diff branch from 9d4ae6d to a9849db Compare April 8, 2026 09:54
If we use x-update-baseline we wanted to know which packages gets updated for our manifest file.
Copilot AI review requested due to automatic review settings April 8, 2026 10:14
@autoantwort
autoantwort force-pushed the feature/baseline-diff branch from a9849db to 4a51845 Compare April 8, 2026 10:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Comment thread src/vcpkg/commands.baseline-diff.cpp Outdated
Comment thread src/vcpkg/commands.baseline-diff.cpp
Comment thread src/vcpkg/commands.baseline-diff.cpp
Comment thread src/vcpkg/commands.baseline-diff.cpp
Comment thread src/vcpkg/commands.baseline-diff.cpp Outdated
return false;
}

void check_for_valid_sha(StringView sha)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can this use check_commit_exists (from git.h) instead?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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 LockFile and LockFile::lockdata maps what to what?

@BillyONeal Billy O'Neal (BillyONeal) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the general idea of such a command makes sense.

Most of what is here is nitpicks but the one about 'why manifest only' and 'what about non-git registries' are kinda foundational.

Should we have output like this printed by x-update-baseline instead of/in addition to what you've done here?

This needs at least one end to end test.

Thanks for the new command submission!

Comment thread src/vcpkg/commands.baseline-diff.cpp Outdated
Comment thread src/vcpkg/commands.baseline-diff.cpp Outdated
Comment thread src/vcpkg/commands.baseline-diff.cpp Outdated
Comment thread src/vcpkg/commands.baseline-diff.cpp Outdated
Comment thread src/vcpkg/commands.baseline-diff.cpp
Comment thread src/vcpkg/commands.baseline-diff.cpp Outdated
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)"},
Undocumented,

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Comment thread src/vcpkg/commands.baseline-diff.cpp Outdated
Comment thread src/vcpkg/commands.baseline-diff.cpp Outdated
Triplet host_triplet)
{
const auto* manifest = paths.get_manifest();
if (manifest == nullptr)

Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Contributor Author

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.

Copy link
Copy Markdown
Contributor Author

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.

Comment thread src/vcpkg/commands.baseline-diff.cpp Outdated
return false;
}

void check_for_valid_sha(StringView sha)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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.

@BillyONeal Billy O'Neal (BillyONeal) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(paraphrased by Billy O'Neal (@BillyONeal) )
Victor Romero (@vicroms): Anything that makes baselines more helpful and less obscure is a good thing
Augustin Popa (@AugP): This concept sounds like a good idea and I have no objections to it

autoantwort and others added 2 commits April 23, 2026 17:25
Co-authored-by: Billy O'Neal <bion@microsoft.com>
Copilot AI review requested due to automatic review settings May 24, 2026 21:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Comment thread include/vcpkg/commands.baseline-diff.h
Comment thread src/vcpkg/commands.baseline-diff.cpp Outdated
Comment thread src/vcpkg/commands.baseline-diff.cpp
Comment thread src/vcpkg/commands.baseline-diff.cpp
Comment thread src/vcpkg/commands.baseline-diff.cpp
Comment thread locales/messages.json Outdated
Comment thread include/vcpkg/base/message-data.inc.h Outdated
DECLARE_MESSAGE(BaselineDiffNoChange,
(),
"commit is a git commit here",
"No package that would have been installed was updated between the two commits")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
"No package that would have been installed was updated between the two commits")
"No changes to installable packages between baselines")

I'm not sure if installable conveys the meaning but I'd prefer a terse description.

Copy link
Copy Markdown
Contributor Author

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.

Comment thread include/vcpkg/base/message-data.inc.h Outdated
Comment thread include/vcpkg/base/message-data.inc.h Outdated
Copilot AI review requested due to automatic review settings June 8, 2026 12:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Comment thread src/vcpkg/commands.baseline-diff.cpp Outdated
Comment thread src/vcpkg/commands.baseline-diff.cpp
Comment thread src/vcpkg/commands.baseline-diff.cpp Outdated
Comment thread src/vcpkg/commands.baseline-diff.cpp
Comment thread locales/messages.json
Copilot AI review requested due to automatic review settings June 14, 2026 12:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Comment thread src/vcpkg/commands.baseline-diff.cpp Outdated
Comment thread locales/messages.json Outdated
Comment thread src/vcpkg/commands.baseline-diff.cpp Outdated
Comment thread azure-pipelines/end-to-end-tests-dir/baseline-diff.ps1
Copilot AI review requested due to automatic review settings June 14, 2026 13:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Comment thread src/vcpkg/commands.baseline-diff.cpp Outdated
Comment thread locales/messages.json Outdated
Comment thread locales/messages.json Outdated
Comment thread locales/messages.json
Comment thread locales/messages.json Outdated
…ry because there is also a builtin git registry which is not meant by is_default_builtin_registry
Comment on lines +189 to +207
if (!local_git_root.empty())
{
const auto* git_exe = paths.get_tool_path(console_diagnostic_context, Tools::GIT);
if (!git_exe) Checks::exit_fail(VCPKG_LINE_INFO);

GitRepoLocator locator{GitRepoLocatorKind::CurrentDirectory, local_git_root};
const auto resolve_baseline_ref = [&](StringView baseline) {
if (is_git_sha(baseline))
{
return baseline.to_string();
}
auto maybe_resolved = git_resolve_to_full_sha(console_diagnostic_context, *git_exe, locator, baseline);
Checks::msg_check_exit(
VCPKG_LINE_INFO, maybe_resolved.has_value(), msgInvalidGitRef, msg::value = baseline);
return *maybe_resolved.get();
};
old_baseline = resolve_baseline_ref(old_baseline_ref);
new_baseline = resolve_baseline_ref(new_baseline_ref);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand why we are trying to look at a local git root when this is a manifest mode command. What the local git situation is is totally irrelevant to manifest mode.

@BillyONeal

Copy link
Copy Markdown
Member

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.

The problem with this is that x-update-baseline will update all registries in the manifest and this only cares about the default one. Only talking about the default registry means we could be showing an inaccurate view of what changes.

Inside x-update-baseline, we have this data for every registry we are updating and can show you the diff between the old world order and the new world order. Since the thing you seem to be actually trying to get out of this is what that diff looks like, should we fix x-update-baseline to print that rather than trying to add a new difficult-to-specify command like this?

I think this kind of "compare these two SHAs" should be a classic mode universe thing (which is exactly what portsdiff already intends to do) and the kind of comparison you're trying to do here would be better to compare 2 different manifests instead.

That said, I have some sympathy for "99% of people only use one registry so maybe this does what they actually want most of the time" but it sounds like even you really want this from x-update-baseline instead?

Billy O'Neal (BillyONeal) added a commit that referenced this pull request Aug 5, 2026
This is a psuedo-alternative to #1980 or at least to get similar results
as intended there.

I used GPT 5.5 in authoring this change.

```console
PS C:\Dev\vcpkg-tool\src> C:\Dev\vcpkg-tool\out\build\Win-x64-Debug-WithArtifacts\vcpkg.exe x-update-baseline --dry-run --vcpkg-root C:\Dev\vcpkg 
Fetching registry information from https://github.com/microsoft/vcpkg (HEAD)...
updated registry 'https://github.com/microsoft/vcpkg': e03dc9b29710050cd1018bc5674688108658d327 -> 42e4e33e1505c9f47b58c21e0f557c1571b751ee
Updating baselines has resulted in the following version updates:

Direct dependencies:
curl: 8.20.0#1 -> 8.21.0#1

Transitive dependencies:
zlib: 1.3.2 -> 1.3.2#1
```
@BillyONeal

Copy link
Copy Markdown
Member

I believe I implemented the outcome you were trying to get to here in #2057

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants