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

Remove stale extension factors #22378

Closed
wants to merge 3 commits into from
Closed

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented May 15, 2024

JSON-based merge conflict resolution for MODULE.bazel.lock can end up producing entries with, e.g., both a general and an os:Linux factor result when the OS/arch dependence of the extension changes. BazelLockFileModule now invalidates each factor result individually.

Also sort the extension factors.

@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 May 15, 2024
@fmeum
Copy link
Collaborator Author

fmeum commented May 15, 2024

@Wyverald I'm currently planning to implement the automated merge conflict resolution for MODULE.bazel.lock as a very simple, standalone jq script that can be used as a git merge driver. This is what I need to make this strategy work, but I hope that the new implementation is also cleaner than the previous one (which just grew and shrank organically).

@fmeum
Copy link
Collaborator Author

fmeum commented May 15, 2024

@bazel-io fork 7.2.0

@fmeum fmeum force-pushed the factors-merge branch 2 times, most recently from 686da36 to a44e25c Compare May 16, 2024 19:26
@fmeum
Copy link
Collaborator Author

fmeum commented May 16, 2024

I also added a commit to sort the eval factors.

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.

LGTM -- just nits

fmeum added 3 commits May 21, 2024 09:03
JSON-based merge conflict resolution for `MODULE.bazel.lock` can end up producing entries with, e.g., both a `general` and an `os:Linux` factor result when the OS/arch dependence of the extension changes. `BazelLockFileModule` now invalidates each factor result individually.
@fmeum fmeum requested a review from Wyverald May 21, 2024 07:09
@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 May 21, 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 May 21, 2024
bazel-io pushed a commit to bazel-io/bazel that referenced this pull request May 21, 2024
JSON-based merge conflict resolution for `MODULE.bazel.lock` can end up producing entries with, e.g., both a `general` and an `os:Linux` factor result when the OS/arch dependence of the extension changes. `BazelLockFileModule` now invalidates each factor result individually.

Also sort the extension factors.

Closes bazelbuild#22378.

PiperOrigin-RevId: 635913837
Change-Id: I0064098806c856f16e8f4c0270f609f06cebc945
@fmeum fmeum deleted the factors-merge branch May 21, 2024 20:44
github-merge-queue bot pushed a commit that referenced this pull request May 21, 2024
JSON-based merge conflict resolution for `MODULE.bazel.lock` can end up
producing entries with, e.g., both a `general` and an `os:Linux` factor
result when the OS/arch dependence of the extension changes.
`BazelLockFileModule` now invalidates each factor result individually.

Also sort the extension factors.

Closes #22378.

PiperOrigin-RevId: 635913837
Change-Id: I0064098806c856f16e8f4c0270f609f06cebc945

Commit
c4092e9

Co-authored-by: Fabian Meumertzheim <[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.

None yet

2 participants