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

[8.0.0] Allow implicit creation of input files within a macro's namespace #24077

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

bazel-io
Copy link
Member

Recent experience with symbolic macro migration showed that we need to relax the restrictions on the implicit creation of input files.

Previously it was an error to refer to a non-existent target "foo_input" if there happened to be a symbolic macro named "foo" in the package. The rationale was that we don't want to have to evaluate foo just to decide whether or not foo_input is in fact an InputFile.

But this restriction means that the common idiom:

bzl_library(
    name = "foo",
    srcs = ["foo.bzl"],
)

blocks bzl_library from being a symbolic macro (e.g. wrapping the underlying rule). We believe this will be a common blocker well beyond this one example.

Therefore, we will now only block the implicit creation of input files when they collide exactly with a target or macro, and not when they merely fall within a macro's namespace.

A consequence of this design change is that it complicates how input files are handled when we have lazy macro evaluation. But we'll deal with that when the time comes.

Package.java

  • Update and clarify javadoc for createAssumedInputFiles() and maybeCreateAssumedInput(). Update check in the latter to only care about exact matches of macro names.

TargetRecorder.java

  • Factor out an accessor for checking whether a macro with the given name exists (insulating the caller from having to worry about the detail of how macro ids are constructed).

PackageFactoryTest.java

  • Make implicit-input-file-creation tests more orthogonal and uniformly named, and update for the semantics change in this CL. (It may be easier to review this file by ignoring the diff and just reviewing the new test content.)

Fixes #24064.

PiperOrigin-RevId: 689374256
Change-Id: I218338af9f9638e1ec03c34e57e7c9f6e10beaaa

Commit 2d4c8eb

Recent experience with symbolic macro migration showed that we need to relax the restrictions on the implicit creation of input files.

Previously it was an error to refer to a non-existent target `"foo_input"` if there happened to be a symbolic macro named `"foo"` in the package. The rationale was that we don't want to have to evaluate `foo` just to decide whether or not `foo_input` is in fact an `InputFile`.

But this restriction means that the common idiom:

```starlark
bzl_library(
    name = "foo",
    srcs = ["foo.bzl"],
)
```

blocks `bzl_library` from being a symbolic macro (e.g. wrapping the underlying rule). We believe this will be a common blocker well beyond this one example.

Therefore, we will now only block the implicit creation of input files when they collide exactly with a target or macro, and not when they merely fall within a macro's namespace.

A consequence of this design change is that it complicates how input files are handled when we have lazy macro evaluation. But we'll deal with that when the time comes.

Package.java
- Update and clarify javadoc for `createAssumedInputFiles()` and `maybeCreateAssumedInput()`. Update check in the latter to only care about exact matches of macro names.

TargetRecorder.java
- Factor out an accessor for checking whether a macro with the given name exists (insulating the caller from having to worry about the detail of how macro ids are constructed).

PackageFactoryTest.java
- Make implicit-input-file-creation tests more orthogonal and uniformly named, and update for the semantics change in this CL. (It may be easier to review this file by ignoring the diff and just reviewing the new test content.)

Fixes bazelbuild#24064.

PiperOrigin-RevId: 689374256
Change-Id: I218338af9f9638e1ec03c34e57e7c9f6e10beaaa
@bazel-io bazel-io requested a review from a team as a code owner October 24, 2024 14:35
@bazel-io bazel-io added awaiting-review PR is awaiting review from an assigned reviewer team-Loading-API BUILD file and macro processing: labels, package(), visibility, glob labels Oct 24, 2024
@iancha1992 iancha1992 added this pull request to the merge queue Oct 25, 2024
Merged via the queue into bazelbuild:release-8.0.0 with commit ed9bbd0 Oct 25, 2024
46 checks passed
@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Loading-API BUILD file and macro processing: labels, package(), visibility, glob
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants