Expose current repository name to Java with @AutoBazelRepository by fmeum · Pull Request #16534 · bazelbuild/bazel

fmeum · 2022-10-23T20:15:23Z

Java targets depending on @bazel_tools//tools/java/runfiles can add the new @AutoBazelRepository to a class to have an annotation processor generate a companion class with a BAZEL_REPOSITORY constant containing the repository name of the target that compiled the class.

This requires a small addition to JavaBuilder to parse the repository name out of the target label and pass it to javac as a processor option.

Work towards #16124

fmeum · 2022-10-24T10:19:24Z

@cushon I followed your recommendation in #16281 (comment) and experimented with annotation processing rather than JavaBuilder magic. The ergonomics seem decent enough and the only change to JavaBuilder is passing the repository name via -A.

The integration test is most likely not in the correctly place, I only put it there so that the test only runs against java_tools built from HEAD. How would I do this properly - split up the PR into two parts with a java_tools update in between?

@Wyverald Do you think this is usable?

fmeum · 2022-10-24T10:33:20Z

I don't understand the test failure - locally, bazel test //src/test/shell/bazel:bazel_java17_test passes. Any idea what could be up with that?

fmeum · 2022-10-25T20:38:03Z

I changed this commit so that the flag is injected in JavaCommon rather than JavaBuilder. That does require duplicating it in two places though.

tools/java/runfiles/AutoBazelRepositoryProcessor.java

fmeum · 2022-10-28T21:37:23Z

tools/java/runfiles/AutoBazelRepositoryProcessor.java

+      out.printf("   * The canonical name of the repository containing the Bazel target that\n");
+      out.printf("   * compiled {@link %s}.\n", annotatedClass.getQualifiedName().toString());
+      out.printf("   */\n");
+      out.printf("  static final String BAZEL_REPOSITORY = \"%s\";\n", repositoryName);


@cushon Given that the class is called AutoBazelRepository_X, should I maybe replace this with NAME? More concise and less repetitive.

Did this in a separate commit because it just seemed better, let me know if you disagree and I can revert.

Wyverald

looks good!

src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java

comius · 2022-11-04T07:59:21Z

tools/java/runfiles/AutoBazelRepositoryProcessor.java

+  private void emitClass(TypeElement annotatedClass) {
+    // This option is always provided by JavaBuilder.
+    String repositoryName = processingEnv.getOptions()
+        .getOrDefault("bazel.repository", "for testing only");


Wouldn't it be better, that this results in an error? User wants to get repository, but they get a string "for testing only". This can only result in a hard to debug problems.

Made this a hard error.

tools/java/runfiles/testing/AutoBazelRepositoryTest.java

fmeum · 2022-11-07T19:01:56Z

@bazel-io flag

Wyverald · 2022-11-07T19:10:38Z

@bazel-io fork 6.0.0

src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java

Java targets depending on `@bazel_tools//tools/java/runfiles` can add the new `@AutoBazelRepository` to a class to have an annotation processor generate a companion class with a `BAZEL_REPOSITORY` constant containing the repository name of the target that compiled the class.

src/main/java/com/google/devtools/build/lib/rules/java/JavaInfoBuildHelper.java

Java targets depending on `@bazel_tools//tools/java/runfiles` can add the new `@AutoBazelRepository` to a class to have an annotation processor generate a companion class with a `BAZEL_REPOSITORY` constant containing the repository name of the target that compiled the class. This requires a small addition to JavaBuilder to parse the repository name out of the target label and pass it to javac as a processor option. Work towards bazelbuild#16124 Closes bazelbuild#16534. PiperOrigin-RevId: 487573496 Change-Id: Id9b6526ce32268089c91c6d17363d1e7682f64a4

) Java targets depending on `@bazel_tools//tools/java/runfiles` can add the new `@AutoBazelRepository` to a class to have an annotation processor generate a companion class with a `BAZEL_REPOSITORY` constant containing the repository name of the target that compiled the class. This requires a small addition to JavaBuilder to parse the repository name out of the target label and pass it to javac as a processor option. Work towards #16124 Closes #16534. PiperOrigin-RevId: 487573496 Change-Id: Id9b6526ce32268089c91c6d17363d1e7682f64a4

fmeum force-pushed the 16124-auto-bazel-repository branch 4 times, most recently from 2ad9baa to e743290 Compare October 24, 2022 10:13

fmeum changed the title ~~WIP: Implement AutoBazelRepository~~ Expose current repository name to Java with @AutoBazelRepository Oct 24, 2022

fmeum marked this pull request as ready for review October 24, 2022 10:19

fmeum requested a review from a team as a code owner October 24, 2022 10:19

This was referenced Oct 24, 2022

WIP: Add java_current_repository rule #16281

Closed

Implement "Locating runfiles with Bzlmod" proposal #16124

Closed

ShreeM01 added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. awaiting-user-response Awaiting a response from the author labels Oct 24, 2022

fmeum mentioned this pull request Oct 25, 2022

Make Java runfiles library repo mapping aware #16549

Closed

fmeum force-pushed the 16124-auto-bazel-repository branch from 6c63677 to bb84246 Compare October 25, 2022 20:35

fmeum requested a review from lberki as a code owner October 25, 2022 20:35

sgowroji added awaiting-review PR is awaiting review from an assigned reviewer and removed awaiting-user-response Awaiting a response from the author labels Oct 26, 2022

cushon reviewed Oct 28, 2022

View reviewed changes

tools/java/runfiles/AutoBazelRepositoryProcessor.java Outdated Show resolved Hide resolved

fmeum force-pushed the 16124-auto-bazel-repository branch from bb84246 to 30c63c4 Compare October 28, 2022 21:16

fmeum commented Oct 28, 2022

View reviewed changes

Wyverald approved these changes Oct 31, 2022

View reviewed changes

comius self-requested a review November 3, 2022 14:35

comius requested changes Nov 4, 2022

View reviewed changes

comius approved these changes Nov 4, 2022

View reviewed changes

comius added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Nov 4, 2022

bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Nov 7, 2022

bazel-io mentioned this pull request Nov 7, 2022

[6.0.0] Expose current repository name to Java with @AutoBazelRepository #16683

Closed

bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Nov 7, 2022

Wyverald reviewed Nov 9, 2022

View reviewed changes

src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java Outdated Show resolved Hide resolved

fmeum added 4 commits November 9, 2022 21:48

Use NAME instead of BAZEL_REPOSITORY and move test

a9d1206

Make -Abazel.repository mandatory and fix test

9716e25

Only set -Abazel.repository if consumed by the plugin

c4ddafa

fmeum force-pushed the 16124-auto-bazel-repository branch from fd68e9c to c4ddafa Compare November 9, 2022 21:13

Wyverald reviewed Nov 10, 2022

View reviewed changes

src/main/java/com/google/devtools/build/lib/rules/java/JavaInfoBuildHelper.java Outdated Show resolved Hide resolved

Use toSet() rather than toList()

d34d09b

copybara-service bot closed this in 455454a Nov 10, 2022

sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Nov 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose current repository name to Java with @AutoBazelRepository #16534

Expose current repository name to Java with @AutoBazelRepository #16534
fmeum wants to merge 5 commits intobazelbuild:masterfrom
fmeum:16124-auto-bazel-repository

fmeum commented Oct 23, 2022 •

edited

Loading

Uh oh!

fmeum commented Oct 24, 2022

Uh oh!

fmeum commented Oct 24, 2022

Uh oh!

fmeum commented Oct 25, 2022

Uh oh!

Uh oh!

fmeum Oct 28, 2022

Uh oh!

fmeum Oct 31, 2022

Uh oh!

Wyverald left a comment

Uh oh!

Uh oh!

comius Nov 4, 2022

Uh oh!

fmeum Nov 4, 2022

Uh oh!

Uh oh!

fmeum commented Nov 7, 2022

Uh oh!

Wyverald commented Nov 7, 2022

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

fmeum commented Oct 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fmeum commented Oct 24, 2022

Uh oh!

fmeum commented Oct 24, 2022

Uh oh!

fmeum commented Oct 25, 2022

Uh oh!

Uh oh!

fmeum Oct 28, 2022

Choose a reason for hiding this comment

Uh oh!

fmeum Oct 31, 2022

Choose a reason for hiding this comment

Uh oh!

Wyverald left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

comius Nov 4, 2022

Choose a reason for hiding this comment

Uh oh!

fmeum Nov 4, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fmeum commented Nov 7, 2022

Uh oh!

Wyverald commented Nov 7, 2022

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

fmeum commented Oct 23, 2022 •

edited

Loading