-
Notifications
You must be signed in to change notification settings - Fork 29
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
Handle .srcjar inputs in Bazel aspect #754
Handle .srcjar inputs in Bazel aspect #754
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏻 Very nice to see more interest in the Bazel aspects, a lot of work went into making those
Thank you very much for taking a look at this, very excited! I decided to build from source and seems to work for most cases. However, it seems to fail when there are 2 java libraries in the same BUILD.bazel that use the same srcjar. Here's an extended example: genrule(
name = "generated-srcjar",
outs = ["sources.srcjar"],
cmd = "echo 'package com.testing; public class Bar {};' > Bar.java && jar cf $(@) Bar.java",
)
java_library(
name = "other_library",
srcs = [
"Baz.java", # create a new file in source at test/Baz.java, alongside test/Foo.java
":generated-srcjar",
],
)
java_library(
name = "testing",
srcs = [
"Foo.java",
":generated-srcjar",
],
)
|
Also, somewhat of an edge case, but we have some srcjars that end up becoming empty due to Reasons, causing the following error: genrule(
name = "empty-srcjar",
outs = ["sources.srcjar"],
cmd = "touch test.txt && zip $(@) test.txt && zip -d $(@) test.txt",
)
java_library(
name = "testing",
srcs = [
"Foo.java",
":empty-srcjar",
],
)
But other than these 2 things, it is looking very promising. Thank you!! |
The aspect will add a new entry to sourceFiles:
and ScipBuildTool will be able to traverse folders and lookup all source files in them.
Together this allows us to avoid unpacking jars in scip-java (which would entail tracking their location and caching, which would be annoying), instead delegating all that to Bazel.
Test plan