Skip to content
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

Omit unique module versions from canonical repo names #21035

Closed
wants to merge 9 commits into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Jan 25, 2024

The canonical repository name of a module rules_foo is now rules_foo~ instead of e.g. rules_foo~1.2.3 if there is only a single version of the module in the entire dep graph. This also applies to overrides, which previously used the canonical repository name rules_foo~override.

This improves cacheability of actions across module version changes and also prevents the output base from filling up with outdated module and extension repositories. See the long comment in ModuleKey#getCanonicalRepoName for a detailed explanation of why this particular scheme was chosen.

The change includes a bump of the lockfile version as the canonical repo names of label attributes in extension usages are affected.

RELNOTES: The scheme for generating canonical repository names has changed to improve cacheability of actions across dependency version updates. Note that canonical names are not considered to be public API and can change at any time. See https://bazel.build/external/module#repository_names_and_strict_deps for advice on how to avoid hardcoding canonical repository names.

Fixes #20997

@fmeum fmeum force-pushed the 20997-elide-version branch 5 times, most recently from 35962c8 to 6c55580 Compare February 1, 2024 21:54
@fmeum fmeum force-pushed the 20997-elide-version branch from 6c55580 to 3264860 Compare February 5, 2024 20:11
@fmeum fmeum changed the title TMP: Omit unique module versions Omit unique module versions from canonical repo names Feb 5, 2024
@fmeum fmeum marked this pull request as ready for review February 5, 2024 20:14
@github-actions github-actions bot added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. awaiting-review PR is awaiting review from an assigned reviewer labels Feb 5, 2024
@fmeum
Copy link
Collaborator Author

fmeum commented Feb 5, 2024

@Wyverald @meteorcloudy Could you already take a look at the implementation? If you agree with the new naming scheme and the overall implementation, I would go ahead and fix all the tests.

Copy link
Member

@Wyverald Wyverald left a comment

Choose a reason for hiding this comment

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

overall LGTM

@fmeum fmeum force-pushed the 20997-elide-version branch 2 times, most recently from 4398798 to c077db3 Compare February 6, 2024 19:52
@fmeum fmeum requested review from a team, gregestren and fweikert as code owners February 6, 2024 19:52
@fmeum fmeum removed request for a team, gregestren and fweikert February 6, 2024 19:52
@fmeum fmeum force-pushed the 20997-elide-version branch 6 times, most recently from 5cda597 to f7379b1 Compare February 6, 2024 20:59
@fmeum
Copy link
Collaborator Author

fmeum commented Feb 8, 2024

@Wyverald @meteorcloudy Tests are passing now, I had to update the canonical name of r_j_e.

Copy link
Member

@meteorcloudy meteorcloudy left a comment

Choose a reason for hiding this comment

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

Thank you! ❤️

site/en/external/module.md Outdated Show resolved Hide resolved
@fmeum fmeum force-pushed the 20997-elide-version branch from 67bdda8 to 2bddce6 Compare February 9, 2024 13:59
@fmeum fmeum force-pushed the 20997-elide-version branch from 2bddce6 to 8cbb5f2 Compare February 9, 2024 14:02
@fmeum fmeum requested a review from meteorcloudy February 9, 2024 14:04
@fmeum
Copy link
Collaborator Author

fmeum commented Feb 9, 2024

@meteorcloudy I addressed your comment and also added RELNOTES to the PR description.

@Wyverald Wyverald added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Feb 9, 2024
@copybara-service copybara-service bot closed this in a54a393 Feb 9, 2024
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Feb 9, 2024
@fmeum fmeum deleted the 20997-elide-version branch February 10, 2024 00:09
@fmeum
Copy link
Collaborator Author

fmeum commented Feb 10, 2024

@bazel-io fork 7.1.0

Wyverald pushed a commit that referenced this pull request Feb 12, 2024
The canonical repository name of a module `rules_foo` is now `rules_foo~` instead of e.g. `rules_foo~1.2.3` if there is only a single version of the module in the entire dep graph. This also applies to overrides, which previously used the canonical repository name `rules_foo~override`.

This improves cacheability of actions across module version changes and also prevents the output base from filling up with outdated module and extension repositories. See the long comment in `ModuleKey#getCanonicalRepoName` for a detailed explanation of why this particular scheme was chosen.

The change includes a bump of the lockfile version as the canonical repo names of label attributes in extension usages are affected.

RELNOTES: The scheme for generating canonical repository names has changed to improve cacheability of actions across dependency version updates. Note that canonical names are not considered to be public API and can change at any time. See https://bazel.build/external/module#repository_names_and_strict_deps for advice on how to avoid hardcoding canonical repository names.

Fixes #20997

Closes #21035.

PiperOrigin-RevId: 605730371
Change-Id: Ica1be1ba5493d3636248a79a6549a0927021bef9
Wyverald pushed a commit that referenced this pull request Feb 12, 2024
The canonical repository name of a module `rules_foo` is now `rules_foo~` instead of e.g. `rules_foo~1.2.3` if there is only a single version of the module in the entire dep graph. This also applies to overrides, which previously used the canonical repository name `rules_foo~override`.

This improves cacheability of actions across module version changes and also prevents the output base from filling up with outdated module and extension repositories. See the long comment in `ModuleKey#getCanonicalRepoName` for a detailed explanation of why this particular scheme was chosen.

The change includes a bump of the lockfile version as the canonical repo names of label attributes in extension usages are affected.

RELNOTES: The scheme for generating canonical repository names has changed to improve cacheability of actions across dependency version updates. Note that canonical names are not considered to be public API and can change at any time. See https://bazel.build/external/module#repository_names_and_strict_deps for advice on how to avoid hardcoding canonical repository names.

Fixes #20997

Closes #21035.

PiperOrigin-RevId: 605730371
Change-Id: Ica1be1ba5493d3636248a79a6549a0927021bef9
github-merge-queue bot pushed a commit that referenced this pull request Feb 13, 2024
The canonical repository name of a module `rules_foo` is now
`rules_foo~` instead of e.g. `rules_foo~1.2.3` if there is only a single
version of the module in the entire dep graph. This also applies to
overrides, which previously used the canonical repository name
`rules_foo~override`.

This improves cacheability of actions across module version changes and
also prevents the output base from filling up with outdated module and
extension repositories. See the long comment in
`ModuleKey#getCanonicalRepoName` for a detailed explanation of why this
particular scheme was chosen.

The change includes a bump of the lockfile version as the canonical repo
names of label attributes in extension usages are affected.

RELNOTES: The scheme for generating canonical repository names has
changed to improve cacheability of actions across dependency version
updates. Note that canonical names are not considered to be public API
and can change at any time. See
https://bazel.build/external/module#repository_names_and_strict_deps for
advice on how to avoid hardcoding canonical repository names.

Fixes #20997

Closes #21035.

PiperOrigin-RevId: 605730371
Change-Id: Ica1be1ba5493d3636248a79a6549a0927021bef9

Co-authored-by: Fabian Meumertzheim <[email protected]>
@iancha1992
Copy link
Member

The changes in this PR have been included in Bazel 7.1.0 RC1. Please test out the release candidate and report any issues as soon as possible. If you're using Bazelisk, you can point to the latest RC by setting USE_BAZEL_VERSION=last_rc.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Usage of bzlmod causes cache invalidations due to module version being in file path
4 participants