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

Polish Bazel external dependencies #20042

Closed
wants to merge 2 commits into from

Conversation

meteorcloudy
Copy link
Member

@meteorcloudy meteorcloudy commented Nov 3, 2023

Clean up and polish Bazel's external dependencies declarations:

  • Gathered all repository declarations in repositories.bzl file.
  • Removed most of repo definitions in distdir_deps.bzl and renamed it to workspace_deps.bzl, the rest repo definitions are only used in WORKSPACE suffix and test setup.
  • Added a repo cache for the workspace repos so that we can decouple repo cache used by
    • dependencies used to build and test Bazel: cached by //src:test_repos
    • dependencies embedded in MODULE.tools: cached by @bazel_tools_repo_cache//:files
    • dependencies embedded in WORKSPACE suffix: cached by @workspace_repo_cache//:files
  • Removed unused macros from distdir.bzl

This PR largely simplifies distdir_deps.bzl, which is also loaded in internal codebase because gen_workspace_stanza is a build rule.

@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Nov 3, 2023
@meteorcloudy meteorcloudy force-pushed the workspace_suffix branch 2 times, most recently from a3bade0 to 18d39cb Compare November 3, 2023 12:05
@meteorcloudy meteorcloudy marked this pull request as draft November 3, 2023 12:08
@meteorcloudy meteorcloudy changed the title WIP: Polish Bazel deps Polish Bazel external dependencies Nov 6, 2023
@meteorcloudy meteorcloudy marked this pull request as ready for review November 6, 2023 15:51
distdir_deps.bzl Outdated
"~grpc_repo_deps_ext~envoy_api",
"~grpc_repo_deps_ext~rules_cc", # TODO: Should be removed
]]
"""WORKSPACE default repository definitions."""
Copy link
Member

Choose a reason for hiding this comment

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

IIUC there's no reason this file should be called 'distdir_deps' anymore, right? It only contains information about WORKSPACE suffix repos, which are (for example) not used by Bzlmod builds at all.

Copy link
Member Author

Choose a reason for hiding this comment

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

Make sense, I renamed it to workspace_deps.bzl

@Wyverald
Copy link
Member

Wyverald commented Nov 6, 2023

This is a really nice cleanup! I'd like to wait for Tony's review too.

repositories.bzl Show resolved Hide resolved
@sgowroji sgowroji added the team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. label Nov 7, 2023
@copybara-service copybara-service bot closed this in c1f2aff Nov 7, 2023
@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Nov 7, 2023
meteorcloudy added a commit to meteorcloudy/bazel that referenced this pull request Nov 8, 2023
Clean up and polish Bazel's external dependencies declarations:

 - Gathered all repository declarations in `repositories.bzl` file.
 - Removed most of repo definitions in `distdir_deps.bzl` and renamed it to `workspace_deps.bzl`, the rest repo definitions are only used in WORKSPACE suffix and test setup.
 - Added a repo cache for the workspace repos so that we can decouple repo cache used by
     - dependencies used to build and test Bazel: cached by `//src:test_repos`
     - dependencies embedded in MODULE.tools: cached by `@bazel_tools_repo_cache//:files`
     - dependencies embedded in WORKSPACE suffix: cached by `@workspace_repo_cache//:files`
 - Removed unused macros from `distdir.bzl`

This PR largely simplifies `distdir_deps.bzl`, which is also loaded in internal codebase because `gen_workspace_stanza` is a build rule.

Closes bazelbuild#20042.

PiperOrigin-RevId: 580212706
Change-Id: I91a9f7bbf89b9af15fdb98f387d95a40a89e7700
meteorcloudy added a commit to meteorcloudy/bazel that referenced this pull request Nov 8, 2023
Clean up and polish Bazel's external dependencies declarations:

 - Gathered all repository declarations in `repositories.bzl` file.
 - Removed most of repo definitions in `distdir_deps.bzl` and renamed it to `workspace_deps.bzl`, the rest repo definitions are only used in WORKSPACE suffix and test setup.
 - Added a repo cache for the workspace repos so that we can decouple repo cache used by
     - dependencies used to build and test Bazel: cached by `//src:test_repos`
     - dependencies embedded in MODULE.tools: cached by `@bazel_tools_repo_cache//:files`
     - dependencies embedded in WORKSPACE suffix: cached by `@workspace_repo_cache//:files`
 - Removed unused macros from `distdir.bzl`

This PR largely simplifies `distdir_deps.bzl`, which is also loaded in internal codebase because `gen_workspace_stanza` is a build rule.

Closes bazelbuild#20042.

PiperOrigin-RevId: 580212706
Change-Id: I91a9f7bbf89b9af15fdb98f387d95a40a89e7700
meteorcloudy added a commit that referenced this pull request Nov 8, 2023
Clean up and polish Bazel's external dependencies declarations:

 - Gathered all repository declarations in `repositories.bzl` file.
- Removed most of repo definitions in `distdir_deps.bzl` and renamed it
to `workspace_deps.bzl`, the rest repo definitions are only used in
WORKSPACE suffix and test setup.
- Added a repo cache for the workspace repos so that we can decouple
repo cache used by
- dependencies used to build and test Bazel: cached by
`//src:test_repos`
- dependencies embedded in MODULE.tools: cached by
`@bazel_tools_repo_cache//:files`
- dependencies embedded in WORKSPACE suffix: cached by
`@workspace_repo_cache//:files`
 - Removed unused macros from `distdir.bzl`

This PR largely simplifies `distdir_deps.bzl`, which is also loaded in
internal codebase because `gen_workspace_stanza` is a build rule.

Closes #20042.

PiperOrigin-RevId: 580212706
Change-Id: I91a9f7bbf89b9af15fdb98f387d95a40a89e7700
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.

4 participants