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 way for a macro or rule in an external repository to convert caller relative apparent labels to canonical form strings #17260

Closed
brentleyjones opened this issue Jan 19, 2023 · 10 comments
Assignees
Labels
area-Bzlmod Bzlmod-specific PRs, issues, and feature requests P1 I'll work on this now. (Assignee required) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request

Comments

@brentleyjones
Copy link
Contributor

brentleyjones commented Jan 19, 2023

Description of the feature request:

Add a way for a Label to be constructed relative to the caller of a macros or the package a rule is defined in.

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

With bzlmod enabled, rules_xcodeproj can't convert caller produced label strings (e.g. "@SwiftLint//:swiftlint") to canonical form strings (e.g. @@swiftlint~override//:swiftlint). This is because Label() called in a macro or rule uses the repo mapping from the point of view of the repo the macro or rule is defined in, instead of where the macro is called from or the target is defined in, and Label() can't be called in BUILD files to allow the caller to instantiate the label with the correct mapping. Also, rules_xcodeproj can't use an attr.label() or attr.label_list() for this use case, because these are "nodep" labels (I do not want/cannot allow these labels to cause targets to get analyzed, as they refer to targets further down the dependency tree, where they are already transitioned to another configuration).

Which operating system are you running Bazel on?

macOS

What is the output of bazel info release?

release 6.0.0

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

No response

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

No response

Have you found anything relevant by searching the web?

#17258 is related, as it shows the unknown mapping happening.

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

No response

@brentleyjones
Copy link
Contributor Author

I've been talking with @fmeum about this, and he said @Wyverald might know a work around.

@fmeum
Copy link
Collaborator

fmeum commented Jan 19, 2023

Delving into the history of the Label constructor, it once had a relative_to_caller_repository argument. However, according to its docs, that seemed to work only in rule implementation functions, not in BUILD threads.

@brentleyjones
Copy link
Contributor Author

Ideally whatever solution is come up with would work in both locations (we currently create the labels in macros), but I could probably get by if it was only in implementation functions.

@Wyverald
Copy link
Member

I happen to be working on this right now too (what are the chances??). We essentially need the relative_to_caller_repository thing back, but maybe under a better name. I'm thinking something like native.label_in_current_package.

We considered just using Starlark for this, but it turns out it doesn't work. We can synthesize a label in the current package using native.repository_name and native.package_name, but label.relative(s) doesn't work as expected if s has a @repo part -- it actually uses the repo mapping of the current .bzl file.

To confirm, you should only ever need this in macros, right? Or do you also need it in a rule impl function?

@Wyverald Wyverald added P1 I'll work on this now. (Assignee required) and removed untriaged labels Jan 19, 2023
@Wyverald Wyverald self-assigned this Jan 19, 2023
@brentleyjones
Copy link
Contributor Author

brentleyjones commented Jan 19, 2023

To confirm, you should only ever need this in macros, right? Or do you also need it in a rule impl function?

I happen to be doing this in macros right now, but I could easily see myself needing it in a rule impl function as well in the future.

@Wyverald
Copy link
Member

Hmm. Using it in a rule impl function sounds a bit magical to me, kind of just the right level of magical that macros afford. It also makes the naming a bit hard as "current package" doesn't really make sense in a rule impl function.

I'll focus on the macro use case for now, since you could probably just wrap the rule in a macro if you need it in the rule impl function.

@Wyverald
Copy link
Member

I wrote a short design doc about the proposed API: https://docs.google.com/document/d/1_kMVWRHSBVkSsw1SLWPf3e-3RNSt55ZZYlzFzJkkwMo/edit

@brentleyjones
Copy link
Contributor Author

Looks like there is some minor feedback, but otherwise looks good? Just hoping it will make it into 6.1.

@Wyverald
Copy link
Member

Yeah I'm just waiting on the reviewers now. The name change sgtm

@Wyverald
Copy link
Member

Wyverald commented Feb 7, 2023

@bazel-io fork 6.1.0

Wyverald added a commit that referenced this issue Feb 7, 2023
This essentially brings back the old `relative_to_caller_repository` param to the Label constructor, but with an arguably better API. It converts a label string into a Label object using the context of the calling package; this is useful for macro authors that want to convert string inputs into labels for reasons such as deduping.

Also made `Label()` accept relative labels (like `:foo`) because there really isn't a good reason not to do so.

Short design doc about the proposed API: https://docs.google.com/document/d/1_kMVWRHSBVkSsw1SLWPf3e-3RNSt55ZZYlzFzJkkwMo/edit

Fixes #17260

RELNOTES: Added a `native.package_relative_label()` function, which converts a label string to a Label object in the context of the calling package, in contrast to `Label()`, which does so in the context of the current .bzl file. Both functions now also accept relative labels such as `:foo`, and are idempotent.
PiperOrigin-RevId: 507836895
Change-Id: Ic870fe564d96a77f05dd7258d32c031fca8cacb1
Wyverald added a commit that referenced this issue Feb 7, 2023
This essentially brings back the old `relative_to_caller_repository` param to the Label constructor, but with an arguably better API. It converts a label string into a Label object using the context of the calling package; this is useful for macro authors that want to convert string inputs into labels for reasons such as deduping.

Also made `Label()` accept relative labels (like `:foo`) because there really isn't a good reason not to do so.

Short design doc about the proposed API: https://docs.google.com/document/d/1_kMVWRHSBVkSsw1SLWPf3e-3RNSt55ZZYlzFzJkkwMo/edit

Fixes #17260

RELNOTES: Added a `native.package_relative_label()` function, which converts a label string to a Label object in the context of the calling package, in contrast to `Label()`, which does so in the context of the current .bzl file. Both functions now also accept relative labels such as `:foo`, and are idempotent.
PiperOrigin-RevId: 507836895
Change-Id: Ic870fe564d96a77f05dd7258d32c031fca8cacb1
hvadehra pushed a commit that referenced this issue Feb 14, 2023
This essentially brings back the old `relative_to_caller_repository` param to the Label constructor, but with an arguably better API. It converts a label string into a Label object using the context of the calling package; this is useful for macro authors that want to convert string inputs into labels for reasons such as deduping.

Also made `Label()` accept relative labels (like `:foo`) because there really isn't a good reason not to do so.

Short design doc about the proposed API: https://docs.google.com/document/d/1_kMVWRHSBVkSsw1SLWPf3e-3RNSt55ZZYlzFzJkkwMo/edit

Fixes #17260

RELNOTES: Added a `native.package_relative_label()` function, which converts a label string to a Label object in the context of the calling package, in contrast to `Label()`, which does so in the context of the current .bzl file. Both functions now also accept relative labels such as `:foo`, and are idempotent.
PiperOrigin-RevId: 507836895
Change-Id: Ic870fe564d96a77f05dd7258d32c031fca8cacb1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Bzlmod Bzlmod-specific PRs, issues, and feature requests P1 I'll work on this now. (Assignee required) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request
Projects
None yet
Development

No branches or pull requests

4 participants
@brentleyjones @Wyverald @fmeum and others