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.

Sign up for GitHub

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

Jump to bottom

--incompatible_use_plus_in_repo_names #23127

Closed
5 tasks done
Wyverald opened this issue Jul 26, 2024 · 1 comment
Closed
5 tasks done

--incompatible_use_plus_in_repo_names #23127

Wyverald opened this issue Jul 26, 2024 · 1 comment
Assignees
@Wyverald
Labels
area-Bzlmod Bzlmod-specific PRs, issues, and feature requests breaking-change-8.0 incompatible-change Incompatible/breaking change migration-ready Incompatible flag is ready for migration with Bazel rolling releases or Bazel@last_green P1 I'll work on this now. (Assignee required) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: process

Comments

@Wyverald
Copy link
Member

Wyverald commented Jul 26, 2024 •
edited
Loading

Motivation

In #22865, it was discovered that the ~ character currently used as a separator in canonical repo names can have significant performance impact on Windows.

We reviewed all potential separators and found that + could serve as a good replacement.

Description

When set to true, the flag causes the canonical repo name format to use + wherever ~ is currently used. That is:

  • For module repos: $NAME+
  • For extension repos: $MODULE+$EXTENSION_NAME+$APPARENT_REPO_NAME

Incompatible Flag

--incompatible_use_plus_in_repo_names

Migration Guide

Ideally, people should have already not been depending on the canonical repo name format, as it was declared to be unstable.

But for those who have been parsing repo names, especially those relying on the existence of a ~ character, should try to switch to other alternatives:

In which Bazel LTS version will this incompatible change be enabled?

Bazel 8

Additional Context

Note that in Bazel 8, this flag will be flipped to true by default, and the flag will immediately become a no-op (that is, setting it to false will have no effect). This is to avoid having to maintain the different repo name formats for another LTS cycle, especially as it was never a stable API to begin with.

TODO List
@Wyverald Wyverald added untriaged incompatible-change Incompatible/breaking change labels Jul 26, 2024
@Wyverald Wyverald added P1 I'll work on this now. (Assignee required) type: process team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. area-Bzlmod Bzlmod-specific PRs, issues, and feature requests and removed untriaged labels Jul 26, 2024
@Wyverald Wyverald self-assigned this Jul 26, 2024
@Wyverald Wyverald added the migration-ready Incompatible flag is ready for migration with Bazel rolling releases or Bazel@last_green label Jul 29, 2024
@peakschris
Copy link

7.3.0rc1 is working well for us, fixes #22865, thank you for your work on this!

plobsing added a commit to plobsing/rules_jvm_external that referenced this issue Aug 4, 2024
@plobsing
Update query-result normalization logic to support plus repo name del…
306b589 
…imiter

The delimiter used in canonical repo names is changing from `~` to
`+`.[^1] This logic, which tries to normalize query results for the
purposes of diff-testing against golden results, label parsing seems
unavoidable, so we fall back to the non-preferred approach of supporting
both delimiters in our patterns.

Also, refine the patterns to be more exact and better reflect the intent
to only act agaisnt the repo-name part of each label:
  * anchor the patterns to match only at start of each label
  * only operate before the first `/`

[^1]: bazelbuild/bazel#23127
plobsing added a commit to plobsing/rules_jvm_external that referenced this issue Aug 4, 2024
@plobsing
Derive jvm_import debug label from explicit attribute
9042f39 
The delimiter used in Bzlmod for repo names has never been stable and
will be changing soon.[^1] String-parsing to derive shorter,
user-friendly names for labels will be broken by this change and is
brittle in general. Instead, passing the right values down from the
contexts where the true values are known is preferred.

[^1]: bazelbuild/bazel#23127
Wyverald pushed a commit to bazel-contrib/rules_jvm_external that referenced this issue Aug 5, 2024
@plobsing
Update query-result normalization logic to support plus repo name del…
e8bdad7 
…imiter (#1211)

The delimiter used in canonical repo names is changing from `~` to
`+`.[^1] This logic, which tries to normalize query results for the
purposes of diff-testing against golden results, label parsing seems
unavoidable, so we fall back to the non-preferred approach of supporting
both delimiters in our patterns.

Also, refine the patterns to be more exact and better reflect the intent
to only act agaisnt the repo-name part of each label:
  * anchor the patterns to match only at start of each label
  * only operate before the first `/`

[^1]: bazelbuild/bazel#23127
Wyverald added a commit that referenced this issue Aug 19, 2024
@Wyverald
Ignore invalid entries in the marker file
3bab496 
Especially due to #23127, older marker files may contain entries that don't even parse correctly. Instead of crashing Bazel, we should just ignore such entries.

Fixes #23322.
@Wyverald Wyverald mentioned this issue Aug 19, 2024
Closed
Wyverald added a commit that referenced this issue Aug 19, 2024
@Wyverald
Ignore invalid entries in the marker file
0a6880d 
Especially due to #23127, older marker files may contain entries that don't even parse correctly. Instead of crashing Bazel, we should just ignore such entries.

Fixes #23322.
@meteorcloudy meteorcloudy mentioned this issue Aug 20, 2024
Merged
@peakschris peakschris mentioned this issue Aug 21, 2024
Closed
@albertocavalcante albertocavalcante mentioned this issue Aug 22, 2024
Merged
copybara-service bot pushed a commit that referenced this issue Aug 22, 2024
@Wyverald @copybara-github
Consider invalid entries in the marker file reason for refetch
d62e0a0 
Especially due to #23127, older marker files may contain entries that don't even parse correctly. Instead of crashing Bazel, we should signal a refetch.

Fixes #23322.

Closes #23336.

PiperOrigin-RevId: 666416942
Change-Id: Ie5507654f69825d93f9523d9c209d417fb3cdaf6
@peakschris peakschris mentioned this issue Aug 25, 2024
Open
@dizzy57 dizzy57 mentioned this issue Sep 4, 2024
Merged
bazel-io pushed a commit to bazel-io/bazel that referenced this issue Sep 10, 2024
@Wyverald @bazel-io
Consider invalid entries in the marker file reason for refetch
697fcc5 
Especially due to bazelbuild#23127, older marker files may contain entries that don't even parse correctly. Instead of crashing Bazel, we should signal a refetch.

Fixes bazelbuild#23322.

Closes bazelbuild#23336.

PiperOrigin-RevId: 666416942
Change-Id: Ie5507654f69825d93f9523d9c209d417fb3cdaf6
@bazel-io bazel-io mentioned this issue Sep 10, 2024
Open
iancha1992 pushed a commit to bazel-io/bazel that referenced this issue Sep 12, 2024
@Wyverald @iancha1992
Consider invalid entries in the marker file reason for refetch
cd40eef 
Especially due to bazelbuild#23127, older marker files may contain entries that don't even parse correctly. Instead of crashing Bazel, we should signal a refetch.

Fixes bazelbuild#23322.

Closes bazelbuild#23336.

PiperOrigin-RevId: 666416942
Change-Id: Ie5507654f69825d93f9523d9c209d417fb3cdaf6
@meteorcloudy meteorcloudy mentioned this issue Sep 20, 2024
Closed
@kczulko kczulko mentioned this issue Sep 26, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Wyverald Wyverald

Labels
area-Bzlmod Bzlmod-specific PRs, issues, and feature requests breaking-change-8.0 incompatible-change Incompatible/breaking change migration-ready Incompatible flag is ready for migration with Bazel rolling releases or Bazel@last_green P1 I'll work on this now. (Assignee required) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: process
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@Wyverald @meteorcloudy @sgowroji @peakschris @iancha1992 @satyanandak