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

Add a repository_ctx field to accommodate default repo target names #24467

Closed
mbland opened this issue Nov 22, 2024 · 4 comments
Closed

Add a repository_ctx field to accommodate default repo target names #24467

mbland opened this issue Nov 22, 2024 · 4 comments
Assignees
Labels
help wanted Someone outside the Bazel team could own this P3 We're not considering working on this, but happy to review a PR. (No assignee) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request

Comments

@mbland
Copy link

mbland commented Nov 22, 2024

Description of the feature request:

A repository_ctx parameter holding the original, user-provided name value, facilitating the creation of default repo target names without workarounds

Which category does this issue belong to?

Rules API

What underlying problem are you trying to solve with this feature?

Label supports default target names based on repo names. For example, @repo expands to @repo//:repo.

Under Bzlmod, repository_ctx.name is now the canonical repo name, invalidating previous repository_rule implementations that used this value to construct a default target name under WORKSPACE. One can work around this issue by adding a separate attr to duplicate the name parameter of a given repository rule, but this degrades the interface somewhat and is potentially error prone.

I've implemented a pattern in bazelbuild/rules_scala#1650 to wrap a repository_rule in a macro to handle this duplication on behalf of the user, keeping the ergonomics of the original rule, but with trade-offs. This came after my first workaround, parsing the original name from repository_ctx.name, from bazelbuild/rules_scala#1621 and the withdrawn bazelbuild/bazel-skylib#548. Both flavors of workaround, however, reflect extra effort to grab information already passed as the name parameter of a repository_rule.

As originally discussed with @Wyverald and @fmeum in a #bzlmod channel thread in the Bazel Slack Workspace, a dedicated repository_ctx field would eliminate the need for such workarounds in the future. I'd imagine it could be backported to Bazel 7, and even to Bazel 6, to remove friction from Bzlmod migrations.

Which operating system are you running Bazel on?

N/A

What is the output of bazel info release?

N/A

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

N/A

What's the output of git remote get-url origin; git rev-parse HEAD ?

N/A

Have you found anything relevant by searching the web?

N/A

Any other information, logs, or outputs that you want to share?

N/A

@github-actions github-actions bot added the team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts label Nov 22, 2024
@mbland
Copy link
Author

mbland commented Nov 26, 2024

BTW, just found out this other clever method of parsing out the name parameter of a repository_rule:

SEPARATOR = Label("@local").workspace_name.removesuffix("local")[-1]
target = rctx.attr.target or rctx.attr.name.rsplit(SEPARATOR, 1)[1]

@comius comius added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website and removed team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts labels Jan 7, 2025
@comius
Copy link
Contributor

comius commented Jan 7, 2025

Reassigning to team-OSS.

@meteorcloudy meteorcloudy removed the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Jan 7, 2025
@Wyverald Wyverald added P3 We're not considering working on this, but happy to review a PR. (No assignee) help wanted Someone outside the Bazel team could own this and removed untriaged labels Jan 8, 2025
@fmeum fmeum self-assigned this Jan 29, 2025
fmeum added a commit to fmeum/bazel that referenced this issue Feb 4, 2025
This new attribute contains the original value of the `name` attribute at the instantiation site of the repo rule (e.g., `rctx.original_name` would be `foo` if `rctx.name` is `+ext+foo`).

Fixes bazelbuild#24467

Closes bazelbuild#25121.

RELNOTES: Added `repository_ctx.original_name`, which contains the original value of the `name` attribute as specified at the repo rule call site.
PiperOrigin-RevId: 722731393
Change-Id: I2f7dada0c44b6bd4c0d2622fa1e97223382a8547
(cherry picked from commit 8bcfb06)
fmeum added a commit to fmeum/bazel that referenced this issue Feb 4, 2025
This new attribute contains the original value of the `name` attribute at the instantiation site of the repo rule (e.g., `rctx.original_name` would be `foo` if `rctx.name` is `+ext+foo`).

Fixes bazelbuild#24467

Closes bazelbuild#25121.

RELNOTES: Added `repository_ctx.original_name`, which contains the original value of the `name` attribute as specified at the repo rule call site.
PiperOrigin-RevId: 722731393
Change-Id: I2f7dada0c44b6bd4c0d2622fa1e97223382a8547
(cherry picked from commit 8bcfb06)
fmeum added a commit to fmeum/bazel that referenced this issue Feb 4, 2025
This new attribute contains the original value of the `name` attribute at the instantiation site of the repo rule (e.g., `rctx.original_name` would be `foo` if `rctx.name` is `+ext+foo`).

Fixes bazelbuild#24467

Closes bazelbuild#25121.

RELNOTES: Added `repository_ctx.original_name`, which contains the original value of the `name` attribute as specified at the repo rule call site.
PiperOrigin-RevId: 722731393
Change-Id: I2f7dada0c44b6bd4c0d2622fa1e97223382a8547
(cherry picked from commit 8bcfb06)
github-merge-queue bot pushed a commit that referenced this issue Feb 5, 2025
This new attribute contains the original value of the `name` attribute
at the instantiation site of the repo rule (e.g., `rctx.original_name`
would be `foo` if `rctx.name` is `+ext+foo`).

Fixes #24467

Closes #25121.

RELNOTES: Added `repository_ctx.original_name`, which contains the
original value of the `name` attribute as specified at the repo rule
call site.

PiperOrigin-RevId: 722731393
Change-Id: I2f7dada0c44b6bd4c0d2622fa1e97223382a8547 
(cherry picked from commit 8bcfb06)

Fixes #25147
@iancha1992
Copy link
Member

A fix for this issue has been included in Bazel 8.1.0 RC2. Please test out the release candidate and report any issues as soon as possible.
If you're using Bazelisk, you can point to the latest RC by setting USE_BAZEL_VERSION=8.1.0rc2. Thanks!

@mbland
Copy link
Author

mbland commented Feb 6, 2025

Took repository_ctx.original_name for a test drive, and can confirm it works perfectly. Thanks!

mbland added a commit to mbland/rules_scala that referenced this issue Feb 6, 2025
Prepares for `repository_ctx.original_name` support in Bazel 8.1.0 per
bazelbuild/bazel#24467 and bumps Go to 1.23.6. Part of bazelbuild#1482 and bazelbuild#1652.

The underlying repo rules will now use `repository_ctx.original_name` if
it's available.

I've confirmed that these changes use `repository_ctx.original_name`
under Bazel 8.1.0rc2 after commenting out the wrappers and restoring the
original repo rule symbols.

---

This change ensures compatibility with future Bazel releases. New
comments indicate where to simplify code after dropping support for
Bazel versions that don't have `repository_ctx.original_name`.

The Go 1.23.6 bump is a convenience update that isn't essential to the
rest of the change.

Under Bzlmod, `repository_ctx.name` contains the canonical repository
name. This broke the `_alias_repository` and `jvm_import_external`
repository rules, which used `repository_ctx.name` to generate default
top level target names for their repos. bazelbuild#1650 added wrapper macros
passing the original name as an extra attribute to the underlying repo
rules as a portable workaround.

The Bazel maintainers eventually recognized this as a common use case
and added `repository_ctx.original_name` to eliminate the need for such
wrappers (eventually).

- https://bazel.build/rules/lib/builtins/repository_ctx#name
- https://bazel.build/rules/lib/builtins/repository_ctx#original_name
mbland added a commit to mbland/rules_scala that referenced this issue Feb 6, 2025
Prepares for `repository_ctx.original_name` support in Bazel 8.1.0 per
bazelbuild/bazel#24467 and bumps Go to 1.23.6. Part of bazelbuild#1482 and bazelbuild#1652.

The underlying repo rules will now use `repository_ctx.original_name` if
it's available.

I've confirmed that these changes use `repository_ctx.original_name`
under Bazel 8.1.0rc2 after commenting out the wrappers and restoring the
original repo rule symbols.

---

This change ensures compatibility with future Bazel releases. New
comments indicate where to simplify code after dropping support for
Bazel versions that don't have `repository_ctx.original_name`.

The Go 1.23.6 bump is a convenience update that isn't essential to the
rest of the change.

Under Bzlmod, `repository_ctx.name` contains the canonical repository
name. This broke the `_alias_repository` and `jvm_import_external`
repository rules, which used `repository_ctx.name` to generate default
top level target names for their repos. bazelbuild#1650 added wrapper macros
passing the original name as an extra attribute to the underlying repo
rules as a portable workaround.

The Bazel maintainers eventually recognized this as a common use case
and added `repository_ctx.original_name` to eliminate the need for such
wrappers (eventually).

- https://bazel.build/rules/lib/builtins/repository_ctx#name
- https://bazel.build/rules/lib/builtins/repository_ctx#original_name
simuons pushed a commit to bazelbuild/rules_scala that referenced this issue Feb 13, 2025
Prepares for `repository_ctx.original_name` support in Bazel 8.1.0 per
bazelbuild/bazel#24467 and bumps Go to 1.23.6. Part of #1482 and #1652.

The underlying repo rules will now use `repository_ctx.original_name` if
it's available.

I've confirmed that these changes use `repository_ctx.original_name`
under Bazel 8.1.0rc2 after commenting out the wrappers and restoring the
original repo rule symbols.

---

This change ensures compatibility with future Bazel releases. New
comments indicate where to simplify code after dropping support for
Bazel versions that don't have `repository_ctx.original_name`.

The Go 1.23.6 bump is a convenience update that isn't essential to the
rest of the change.

Under Bzlmod, `repository_ctx.name` contains the canonical repository
name. This broke the `_alias_repository` and `jvm_import_external`
repository rules, which used `repository_ctx.name` to generate default
top level target names for their repos. #1650 added wrapper macros
passing the original name as an extra attribute to the underlying repo
rules as a portable workaround.

The Bazel maintainers eventually recognized this as a common use case
and added `repository_ctx.original_name` to eliminate the need for such
wrappers (eventually).

- https://bazel.build/rules/lib/builtins/repository_ctx#name
- https://bazel.build/rules/lib/builtins/repository_ctx#original_name
mbland added a commit to mbland/rules_scala that referenced this issue Feb 14, 2025
Works around breakages when building under `WORKSPACE` with Bazel 8.1.0,
as described in bazelbuild/bazel#25286. Part of bazelbuild#1652.

PR bazelbuild#1694 added support for `rctx.original_name` after the implementation
of bazelbuild/bazel#24467 landed in 8.1.0rc2. However, I hadn't tested
`WORKSPACE` builds with 8.1.0rc2 before 8.1.0 came out.

This resolves the breakage, but should only be necessary if a Bazel 8
release containing a fix for bazelbuild/bazel#25286 isn't yet available.
mbland added a commit to mbland/rules_scala that referenced this issue Feb 18, 2025
Works around breakages when building under `WORKSPACE` with Bazel 8.1.0,
as described in bazelbuild/bazel#25286. Part of bazelbuild#1652.

PR bazelbuild#1694 added support for `rctx.original_name` after the implementation
of bazelbuild/bazel#24467 landed in 8.1.0rc2. However, I hadn't tested
`WORKSPACE` builds with 8.1.0rc2 before 8.1.0 came out.

This resolves the breakage until a Bazel 8 release containing a fix for
bazelbuild/bazel#25286 becomes available (possibly Bazel 8.2.0).
mbland added a commit to mbland/rules_scala that referenced this issue Feb 18, 2025
Works around breakages when building under `WORKSPACE` with Bazel 8.1.0,
as described in bazelbuild/bazel#25286. Part of bazelbuild#1652.

PR bazelbuild#1694 added support for `rctx.original_name` after the implementation
of bazelbuild/bazel#24467 landed in 8.1.0rc2. However, I hadn't tested
`WORKSPACE` builds with 8.1.0rc2 before 8.1.0 came out.

This resolves the breakage until a Bazel 8 release containing a fix for
bazelbuild/bazel#25286 becomes available (possibly Bazel 8.2.0).
mbland added a commit to mbland/rules_scala that referenced this issue Feb 19, 2025
Works around breakages when building under `WORKSPACE` with Bazel 8.1.0,
as described in bazelbuild/bazel#25286. Part of bazelbuild#1652.

PR bazelbuild#1694 added support for `rctx.original_name` after the implementation
of bazelbuild/bazel#24467 landed in 8.1.0rc2. However, I hadn't tested
`WORKSPACE` builds with 8.1.0rc2 before 8.1.0 came out.

This resolves the breakage until a Bazel 8 release containing a fix for
bazelbuild/bazel#25286 becomes available (possibly Bazel 8.2.0).
mbland added a commit to mbland/rules_scala that referenced this issue Feb 20, 2025
Works around breakages when building under `WORKSPACE` with Bazel 8.1.0,
as described in bazelbuild/bazel#25286. Part of bazelbuild#1652.

PR bazelbuild#1694 added support for `rctx.original_name` after the implementation
of bazelbuild/bazel#24467 landed in 8.1.0rc2. However, I hadn't tested
`WORKSPACE` builds with 8.1.0rc2 before 8.1.0 came out.

This resolves the breakage until a Bazel 8 release containing a fix for
bazelbuild/bazel#25286 becomes available (possibly Bazel 8.2.0).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Someone outside the Bazel team could own this P3 We're not considering working on this, but happy to review a PR. (No assignee) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants