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

Make bazel mod tidy work with include() #22063

Closed
Wyverald opened this issue Apr 19, 2024 · 3 comments
Closed

Make bazel mod tidy work with include() #22063

Wyverald opened this issue Apr 19, 2024 · 3 comments
Assignees
@Wyverald
Labels
area-Bzlmod Bzlmod-specific PRs, issues, and feature requests P2 We'll consider working on this in future. (Assignee optional) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: bug
Milestone
Mainline issues t...

Comments

@Wyverald
Copy link
Member

After #21855 is in, we'll need to address the case where include()d files need tidying. This would require Bazel itself to generate proper fixup events for included files (potentially only for extension proxies that are bound to a variable, IOW 'exported'), and for Buildozer to properly handle files that are MODULE.bazel-like but not exactly named "MODULE.bazel" (per discussion in the PR, they would be called foo.MODULE.bazel).
@Wyverald Wyverald added type: bug P2 We'll consider working on this in future. (Assignee optional) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. area-Bzlmod Bzlmod-specific PRs, issues, and feature requests labels Apr 19, 2024
@Wyverald Wyverald self-assigned this Apr 19, 2024
@Wyverald Wyverald mentioned this issue Apr 19, 2024
Closed
@fmeum fmeum mentioned this issue Apr 22, 2024
Merged
@fmeum
Copy link
Collaborator

fmeum commented Apr 22, 2024 •
edited
Loading

Since every repo can only be use_repoed in a single segment and the extension proxy needs to have been exported in the same segment, could we do the following?

  1. For every import to remove, obtain the file that contained the use_repo and emit a use_repo_remove command for that file and the particular extension proxy.
  2. For every import to add, find the first file with an extension usage (of the correct prod/dev) type for it and add the use_repo_add command for it.
  3. For all segments, emit a format command.

That is, we just need to know which extension proxy comes from which segment and results in which imports.

@Wyverald
Copy link
Member Author

Sounds good to me!

Wyverald added a commit that referenced this issue May 9, 2024
@Wyverald
Store information about each proxy in ModuleExtensionUsage
4486392 
The current ModuleExtensionUsage class was designed in the days before we allowed multiple `use_extension` calls for the same extension in the same module. This PR brings the class up to speed to better reflect the actual API.

This will also lay the groundwork for `bazel mod tidy` working with `include()`d segments.

Work towards #22063
Wyverald added a commit that referenced this issue May 9, 2024
@Wyverald
Store information about each proxy in ModuleExtensionUsage
125b6b8 
The current ModuleExtensionUsage class was designed in the days before we allowed multiple `use_extension` calls for the same extension in the same module. This PR brings the class up to speed to better reflect the actual API.

This will also lay the groundwork for `bazel mod tidy` working with `include()`d segments.

Work towards #22063
Wyverald added a commit that referenced this issue May 9, 2024
@Wyverald
Store information about each proxy in ModuleExtensionUsage
d9ee6df 
The current ModuleExtensionUsage class was designed in the days before we allowed multiple `use_extension` calls for the same extension in the same module. This PR brings the class up to speed to better reflect the actual API.

This will also lay the groundwork for `bazel mod tidy` working with `include()`d segments.

Work towards #22063
@Wyverald Wyverald mentioned this issue May 9, 2024
Closed
copybara-service bot pushed a commit that referenced this issue May 10, 2024
@Wyverald @copybara-github
Store information about each proxy in ModuleExtensionUsage
40ec2cd 
The current ModuleExtensionUsage class was designed in the days before we allowed multiple `use_extension` calls for the same extension in the same module. This PR brings the class up to speed to better reflect the actual API.

This will also lay the groundwork for `bazel mod tidy` working with `include()`d segments.

Work towards #22063

Closes #22307.

PiperOrigin-RevId: 632377764
Change-Id: I282a68bc7962088ae4583418f73b2e60a0ec88f0
Kila2 pushed a commit to Kila2/bazel that referenced this issue May 13, 2024
@Wyverald @Kila2
Store information about each proxy in ModuleExtensionUsage
ca12099 
The current ModuleExtensionUsage class was designed in the days before we allowed multiple `use_extension` calls for the same extension in the same module. This PR brings the class up to speed to better reflect the actual API.

This will also lay the groundwork for `bazel mod tidy` working with `include()`d segments.

Work towards bazelbuild#22063

Closes bazelbuild#22307.

PiperOrigin-RevId: 632377764
Change-Id: I282a68bc7962088ae4583418f73b2e60a0ec88f0
Wyverald added a commit that referenced this issue May 13, 2024
@Wyverald
Store information about each proxy in ModuleExtensionUsage
f7274ce 
The current ModuleExtensionUsage class was designed in the days before we allowed multiple `use_extension` calls for the same extension in the same module. This PR brings the class up to speed to better reflect the actual API.

This will also lay the groundwork for `bazel mod tidy` working with `include()`d segments.

Work towards #22063

Closes #22307.

PiperOrigin-RevId: 632377764
Change-Id: I282a68bc7962088ae4583418f73b2e60a0ec88f0
Wyverald added a commit that referenced this issue May 20, 2024
@Wyverald
Make bazel mod tidy work with include()
fa61bf7 
We now generate fixup commands targeting `include()`d segments in addition to the root MODULE.bazel file itself. Fixup commands are grouped by file, and then passed to Buildozer using `-f -` (i.e. multiple commands for multiple files, passed in using stdin).

- For repos to add, we find the first usage with the correct prod/dev type and add them there.
- For repos to remove, we remove them from whichever usage had them.
- At the end, all module files and included segments are formatted.

Fixes #22063
@Wyverald Wyverald mentioned this issue May 20, 2024
Closed
Wyverald added a commit that referenced this issue May 23, 2024
@Wyverald
Make bazel mod tidy work with include()
106f83a 
We now generate fixup commands targeting `include()`d segments in addition to the root MODULE.bazel file itself. Fixup commands are grouped by file, and then passed to Buildozer using `-f -` (i.e. multiple commands for multiple files, passed in using stdin).

- For repos to add, we find the first usage with the correct prod/dev type and add them there.
- For repos to remove, we remove them from whichever usage had them.
- At the end, all module files and included segments are formatted.

Fixes #22063
@Wyverald
Copy link
Member Author

@bazel-io fork 7.2.0

@bazel-io bazel-io mentioned this issue May 23, 2024
Closed
bazel-io pushed a commit to bazel-io/bazel that referenced this issue May 23, 2024
@Wyverald @bazel-io
Make bazel mod tidy work with include()
1b25e76 
We now generate fixup commands targeting `include()`d segments in addition to the root MODULE.bazel file itself. Fixup commands are grouped by file, and then passed to Buildozer using `-f -` (i.e. multiple commands for multiple files, passed in using stdin).

- For repos to add, we find the first usage with the correct prod/dev type and add them there.
- For repos to remove, we remove them from whichever usage had them.
- At the end, all module files and included segments are formatted.

Fixes bazelbuild#22063

Closes bazelbuild#22455.

PiperOrigin-RevId: 636680730
Change-Id: Id894a449d8d1cdf39271ea3a724a91aebc51befb
@bazel-io bazel-io mentioned this issue May 23, 2024
Closed
Wyverald added a commit that referenced this issue May 24, 2024
@Wyverald
Make bazel mod tidy work with include()
b70e8f6 
We now generate fixup commands targeting `include()`d segments in addition to the root MODULE.bazel file itself. Fixup commands are grouped by file, and then passed to Buildozer using `-f -` (i.e. multiple commands for multiple files, passed in using stdin).

- For repos to add, we find the first usage with the correct prod/dev type and add them there.
- For repos to remove, we remove them from whichever usage had them.
- At the end, all module files and included segments are formatted.

Fixes #22063

Closes #22455.

PiperOrigin-RevId: 636680730
Change-Id: Id894a449d8d1cdf39271ea3a724a91aebc51befb
@Wyverald Wyverald mentioned this issue May 24, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue May 24, 2024
@Wyverald
[7.2.0] Make bazel mod tidy work with include() (#22540)
3b6069d 
We now generate fixup commands targeting `include()`d segments in
addition to the root MODULE.bazel file itself. Fixup commands are
grouped by file, and then passed to Buildozer using `-f -` (i.e.
multiple commands for multiple files, passed in using stdin).

- For repos to add, we find the first usage with the correct prod/dev
type and add them there.
- For repos to remove, we remove them from whichever usage had them.
- At the end, all module files and included segments are formatted.

Fixes #22063

Closes #22455.

PiperOrigin-RevId: 636680730
Change-Id: Id894a449d8d1cdf39271ea3a724a91aebc51befb
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 P2 We'll consider working on this in future. (Assignee optional) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: bug
Projects
Bzlmod
Archived in project
Milestone
Mainline issues targeted for 7.2.0
Development

Successfully merging a pull request may close this issue.

Make bazel mod tidy work with include() bazelbuild/bazel
2 participants
@Wyverald @fmeum