Skip to content

x-add-version: Regenerate baseline on --all.#2003

Merged
BillyONeal merged 1 commit intomicrosoft:mainfrom
BillyONeal:x-add-version-regenerate-all
Apr 30, 2026
Merged

x-add-version: Regenerate baseline on --all.#2003
BillyONeal merged 1 commit intomicrosoft:mainfrom
BillyONeal:x-add-version-regenerate-all

Conversation

@BillyONeal
Copy link
Copy Markdown
Member

In microsoft/vcpkg#51431 I observed that x-add-version --all does not remove removed entries which seems incorrect. This changes --all to completely regenerate the baseline file. It also changes x-add-version to create the file if it doesn't exist.

Drive by refactor: deduplicate all the "file not found" std::errc comparisons.

In microsoft/vcpkg#51431 I observed that `x-add-version --all` does not remove removed entries which seems incorrect. This changes `--all` to completely regenerate the baseline file. It also changes `x-add-version` to create the file if it doesn't exist.

Drive by refactor: deduplicate all the "file not found" std::errc comparisons.
Copilot AI review requested due to automatic review settings April 29, 2026 04:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 updates x-add-version --all to fully regenerate the builtin versions/baseline.json (removing stale entries), allows x-add-version to create the baseline file when missing, and refactors repeated filesystem “not found” error checks into a shared helper.

Changes:

  • Change x-add-version --all to rebuild the baseline from scratch; allow baseline creation when absent.
  • Introduce vcpkg::is_not_found_errc() and replace several direct std::errc comparisons across the codebase.
  • Add end-to-end coverage for x-add-version --all baseline rewrite/create/recreate scenarios.

Reviewed changes

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

Show a summary per file
File Description
src/vcpkg/commands.add-version.cpp Adjusts baseline handling so --all can regenerate baseline and missing baseline can be created.
src/vcpkg/registries.cpp Updates baseline loading behavior and refactors versions file “not found” checks to use is_not_found_errc().
src/vcpkg/base/files.cpp Adds the is_not_found_errc() helper and refactors existing internal checks to use it.
include/vcpkg/base/files.h Declares is_not_found_errc() for shared use.
src/vcpkg/paragraphs.cpp Replaces direct ENOENT checks with is_not_found_errc() when probing for manifest/CONTROL.
src/vcpkg/commands.z-applocal.cpp Uses is_not_found_errc() for copy/open error handling paths.
src/vcpkg/base/hash.cpp Uses is_not_found_errc() to classify open errors as “file not found”.
include/vcpkg/base/message-data.inc.h Removes the no-longer-used AddVersionFileNotFound message declaration.
locales/messages.json Removes the corresponding message text entry.
azure-pipelines/end-to-end-tests-dir/versions.ps1 Adds e2e assertions for baseline rewrite/create/recreate behavior under x-add-version --all.
Comments suppressed due to low confidence (5)

src/vcpkg/paragraphs.cpp:456

  • Same issue for the CONTROL read: with !is_not_found_errc(ec), errors like not_a_directory / too_many_symbolic_link_levels are treated as “CONTROL missing” and the function returns an empty load result instead of an I/O error. Consider restricting the “missing file” case to no_such_file_or_directory so non-ENOENT failures remain actionable.
        if (!is_not_found_errc(ec))
        {
            return PortLoadResult{LocalizedString::from_raw(port_location.port_directory)
                                      .append_raw(": ")
                                      .append(format_filesystem_call_error(ec, "read_contents", {control_path})),
                                  std::string{}};
        }

src/vcpkg/commands.z-applocal.cpp:523

  • deploy_binary() now treats any is_not_found_errc(ec) from copy_file() as “source didn’t exist” and returns false. That expands the ignored error set beyond “file missing” (e.g. not_a_directory / symlink-loop), which can hide real filesystem/layout problems during deployment. Consider narrowing this branch to only no_such_file_or_directory and treating other errc values as fatal.
            else if (is_not_found_errc(ec))
            {
                Debug::println("Attempted to deploy ", source, ", but it didn't exist");
                return false;
            }

src/vcpkg/commands.z-applocal.cpp:642

  • For the initial open_for_read() failure, using is_not_found_errc(ec) will classify more conditions than “binary path doesn’t exist” as a warning (e.g. not_a_directory / symlink-loop). That changes the diagnostic severity compared to the previous no_such_file_or_directory check and may make genuine I/O errors harder to notice. Consider restricting the warning case to no_such_file_or_directory and leaving other errors as Color::error.
            if (is_not_found_errc(ec))
            {
                msg::print(Color::warning,
                           LocalizedString::from_raw(target_binary_path)
                               .append_raw(": ")
                               .append_raw(WarningPrefix)
                               .append_raw(io_error)
                               .append_raw('\n'));

src/vcpkg/paragraphs.cpp:435

  • Switching this check to is_not_found_errc(ec) changes behavior for errors like not_a_directory / too_many_symbolic_link_levels: those will now be treated as “manifest missing” and fall through to trying CONTROL instead of returning an I/O error for the manifest path. That can mask real filesystem problems and produce misleading diagnostics. Consider keeping the “try CONTROL” fallback limited to no_such_file_or_directory and surfacing other error codes.
        if (!is_not_found_errc(ec))
        {
            return PortLoadResult{LocalizedString::from_raw(port_location.port_directory)
                                      .append_raw(": ")
                                      .append(format_filesystem_call_error(ec, "read_contents", {manifest_path})),
                                  std::string{}};
        }

src/vcpkg/base/hash.cpp:570

  • get_file_hash() now treats too_many_symbolic_link_levels as HashPrognosis::FileNotFound via is_not_found_errc(ec). A symlink loop typically indicates a filesystem problem rather than a missing file, so this can downgrade a real error into a “not found” result. Consider handling too_many_symbolic_link_levels as OtherError (or remove it from is_not_found_errc() and keep the explicit no_such_file_or_directory / not_a_directory handling here).
            if (is_not_found_errc(ec))
            {
                result.prognosis = HashPrognosis::FileNotFound;
                return result;
            }

Comment thread src/vcpkg/registries.cpp
Comment thread src/vcpkg/commands.add-version.cpp
Comment thread src/vcpkg/registries.cpp
Comment thread src/vcpkg/registries.cpp
@BillyONeal BillyONeal merged commit 196a76c into microsoft:main Apr 30, 2026
11 checks passed
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.

3 participants