Add classifier_artifacts parameter to java_export to allow users to publish extra artifacts #926
Conversation
thirtyseven
changed the title
thirtyseven
force-pushed
the
classifier-artifacts
branch
from
73c1908 to
df8d68f
Compare
private/rules/java_export.bzl
Outdated
| @@ -235,13 +239,16 @@ def maven_export( | |||
| testonly = testonly, | |||
| ) | |||
|
|
|||
| classifier_artifacts = dict(classifier_artifacts) # unfreeze | |||
| classifier_artifacts.setdefault("sources", ":%s-maven-source" % name) | |||
| classifier_artifacts.setdefault("javadoc", docs_jar) | |||
There was a problem hiding this comment.
Neither sources not javadoc are mandatory in some cases (eg. when publishing a BOM), so please check that these values are set before adding them to the dict.
There was a problem hiding this comment.
The javadoc case was already addressed implicitly on line 251 with if v in the dict comprehension, but I've made it more explicit in the latest commit. Not sure how to address the sources case, it's created unconditionally in the current implementation of the java_export, maven_bom is a separate macro not addressed by this PR.
private/rules/maven_publish.bzl
Outdated
|
|
||
| classifier_artifacts_dict = {} | ||
| for target, classifier in ctx.attr.classifier_artifacts.items(): | ||
| file = target.files.to_list()[0] |
There was a problem hiding this comment.
Hmmm... this makes me nervous. Could we check that there's only one file and print a warning if there's more than one? I've a vague memory that (maybe) the kotlin rules have more than one output sometimes....
There was a problem hiding this comment.
Added some checks here.
|
@thirtyseven, I hate to ask for more after the delay in getting this review finished, but could you please rebase? |
Done |
|
Fantastic. Thank you! Merged! :) |
This PR addresses the remaining issues in #805 which got pretty close to being mergeable. I also renamed "extra_artifacts" to "classifier_artifacts" to stick closer to Maven terminology. Also, since javadocs and source jars are special cases of classifier artifacts, I refactored them to use the same code path.