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

Force loading the Java rules from the rules_java repo #8741

Closed
iirina opened this issue Jun 28, 2019 · 6 comments
Closed

Force loading the Java rules from the rules_java repo #8741

iirina opened this issue Jun 28, 2019 · 6 comments
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-Java Issues for Java rules

Comments

@iirina
Copy link
Contributor

iirina commented Jun 28, 2019

This big breaking change will force users of the Java rules to use the Starlark rules from the rules_java repository. The rules there are currently only wrappers around native rules, but they will soon be rewritten.

To save our users from this big migration after Bazel 1.0, we will implement an incompatible flag and provide migration instructions before 1.0, and we will flip this flag for 1.0.

@iirina iirina self-assigned this Jun 28, 2019
@iirina iirina added bazel 1.0 P2 We'll consider working on this in future. (Assignee optional) team-Rules-Java Issues for Java rules labels Jun 28, 2019
@dslomov dslomov removed the bazel 1.0 label Jul 24, 2019
@davido
Copy link
Contributor

davido commented Jul 27, 2019

Not sure If I should report issues here or rather in #8746 (or open a new issue).

I'm trying to migrate Gerrit (stable-2.14 branch) to this change, using buildifier 0.28.0 with --lint=fix option: [1].

After performing the changes in BUILD and in *.bzl files, build is failing on Bazel 0.28.1 and on Bazel@HEAD (019586a):

 $ bazel build lib/codemirror:codemirror.jar
WARNING: Waiting for server process to terminate (waited 5 seconds, waiting at most 60)
Starting local Bazel server and connecting to it...
INFO: Invocation ID: 10de3546-ba55-4f89-8ce1-461c1caea26e
ERROR: Failed to load Starlark extension '@rules_java//java:defs.bzl'.
Cycle in the workspace file detected. This indicates that a repository is used prior to being defined.
The following chain of repository dependencies lead to the missing definition.
 - @rules_java
This could either mean you have to add the '@rules_java' repository with a statement like `http_archive` in your WORKSPACE file (note that transitive dependencies are not added automatically), or move an existing definition earlier in your WORKSPACE file.
ERROR: cycles detected during target parsing
INFO: Elapsed time: 7.208s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (0 packages loaded)

Update I figured out what change caused this error. It is one single java_import statement in the lib/codemirror/cm.bzl file: [2]. If I revert this change in lib/codemirror/cm.bzl (performed by buildifier) then Gerrit code base can be built (bazel build :release).

@aehlig @laurentlb Any idea how can I further investigate this problem?

[1] https://gerrit-review.googlesource.com/c/gerrit/+/232534
[2] https://gerrit-review.googlesource.com/c/gerrit/+/232534/3/lib/codemirror/cm.bzl#366

bazel-io pushed a commit that referenced this issue Nov 5, 2019
irengrig pushed a commit to irengrig/bazel that referenced this issue Nov 7, 2019
qtprojectorg pushed a commit to qtqa/gerrit that referenced this issue Dec 26, 2019
Force loading the Java rules from the rules_java repo, as pointed out
in: [1]. Note that even with this change, gerrit cannot be built with
--incompatible_load_java_rules_from_bzl option passed yet:

  $ bazel build --incompatible_load_java_rules_from_bzl :release

The other transitive dependencies must be fixed first, most notably
rules_closure: [2], or more generally, Bazel's own: jvm_import_external
and java_import_external rules: [3].

[1] bazelbuild/bazel#8741
[2] bazelbuild/rules_closure#449
[3] bazelbuild/bazel#10046

Bug: Issue 11738
Change-Id: I153b6d3c14d6df465034041c3bf81c245df0aa04
@ittaiz
Copy link
Member

ittaiz commented Jan 17, 2020

I know there were some thoughts on offering automatic migrations for certain flags any chance this flag has this? Alternatively is there a buildozer magic to add a load whenever there’s a usage?
Cc @laurentlb since I think I heard this thought from you

@iirina
Copy link
Contributor Author

iirina commented Jan 28, 2020

You can use buildifier right now to add load statements

buildifier --lint=fix -warnings=native-java -r path/to/your/workspace/root/dir

or do you mean something that runs automatically when the flag is set?

@ittaiz
Copy link
Member

ittaiz commented Jan 28, 2020

Awesome! This is exactly what I meant 😁 thanks!
Any chance you know where I can find a list of the supported fixes?

@iirina
Copy link
Contributor Author

iirina commented Jan 28, 2020

Cool 😄 There's a list of all supported warnings.

@lberki
Copy link
Contributor

lberki commented Oct 6, 2020

Update: We decided not to require everyone to add a load() statement to every single BUILD file because it's too much churn. The current plan is to "pretend" that the eventual C++/Java rules written in Starlark are load()ed everywhere.

We might revisit this decision once they are reasonably stable and once we decide where exactly they need to be load()ed from, but now is not the time.

@lberki lberki closed this as completed Oct 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-Java Issues for Java rules
Projects
None yet
Development

No branches or pull requests

5 participants