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

Streamline ModuleExtensionId#toString() #24450

Closed
wants to merge 1 commit into from
Closed

Conversation

Wyverald
Copy link
Member

Currently ModuleExtensionId#toString() uses the default Java record repr, which isn't very user-friendly. There's really no reason not to use the asTargetString() method instead. This PR just renames asTargetString() to toString() and updates all call sites.

@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 Nov 22, 2024
@meteorcloudy meteorcloudy 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 Nov 22, 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 Nov 25, 2024
@Wyverald Wyverald deleted the wyv-fix-tostring branch November 27, 2024 20:31
fmeum pushed a commit to fmeum/bazel that referenced this pull request Jan 7, 2025
Currently `ModuleExtensionId#toString()` uses the default Java record repr, which isn't very user-friendly. There's really no reason not to use the `asTargetString()` method instead. This PR just renames `asTargetString()` to `toString()` and updates all call sites.

Closes bazelbuild#24450.

PiperOrigin-RevId: 700050362
Change-Id: I82238d2134e1642694f0b20235fcfe9307ceaa7d
(cherry picked from commit d23421a)
fmeum pushed a commit to fmeum/bazel that referenced this pull request Jan 7, 2025
Currently `ModuleExtensionId#toString()` uses the default Java record repr, which isn't very user-friendly. There's really no reason not to use the `asTargetString()` method instead. This PR just renames `asTargetString()` to `toString()` and updates all call sites.

Closes bazelbuild#24450.

PiperOrigin-RevId: 700050362
Change-Id: I82238d2134e1642694f0b20235fcfe9307ceaa7d
(cherry picked from commit d23421a)

Fix edge cases in lockfile handling

* Don't run the core logic when `--lockfile_mode` is `off` or `error` but the command doesn't forward options to Skyframe.
* Honor `reproducible` per extension eval factor, not per extension.
* Fix encoding conflict between isolation key and `use_repo_rule`'s fake extension names

This doesn't require a lockfile version bump as `use_repo_rule`'s fake extension (so far) isn't included in the lockfile.

Work towards bazelbuild#24723

Closes bazelbuild#24754.

PiperOrigin-RevId: 712623562
Change-Id: I61fd439539031a01ddec4488276ff2d0484849f2
(cherry picked from commit cfda178)
meteorcloudy pushed a commit that referenced this pull request Jan 8, 2025
… lockfile handling (#24845)

### Streamline `ModuleExtensionId#toString()`

Currently `ModuleExtensionId#toString()` uses the default Java record
repr, which isn't very user-friendly. There's really no reason not to
use the `asTargetString()` method instead. This PR just renames
`asTargetString()` to `toString()` and updates all call sites.

Closes #24450.

PiperOrigin-RevId: 700050362
Change-Id: I82238d2134e1642694f0b20235fcfe9307ceaa7d 
(cherry picked from commit d23421a)

### Fix edge cases in lockfile handling

* Don't run the core logic when `--lockfile_mode` is `off` or `error`
but the command doesn't forward options to Skyframe.
* Honor `reproducible` per extension eval factor, not per extension.
* Fix encoding conflict between isolation key and `use_repo_rule`'s fake
extension names

This doesn't require a lockfile version bump as `use_repo_rule`'s fake
extension (so far) isn't included in the lockfile.

Work towards #24723

Closes #24754.

PiperOrigin-RevId: 712623562
Change-Id: I61fd439539031a01ddec4488276ff2d0484849f2 
(cherry picked from commit cfda178)

Fixes #24754

Co-authored-by: Xdng Yng <[email protected]>
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.

2 participants