Include classifier in label if present

[gmishkin]

My use case is, I have a bzl file with a function that returns a list of all our artifacts. The function is called for both the artifacts attr of maven_install in the WORKSPACE and in another rule where I pass [artifact(a) for a in get_rje_artifacts()] as inputs to a custom rule we have. I also use strict_visibility

What happened was that for the artifact that has a classifier, the label returned wasn't valid and so my custom rule failed to build.

I was able to fix it like this.

[jin]

Hm, the target labels are generated without the classifier. I'm not sure if this is the right change.

rules_jvm_external/private/dependency_tree_parser.bzl

Line 78 in 08a3160

target_label = escape(strip_packaging_and_classifier_and_version(artifact["coord"]))

rules_jvm_external/private/coursier_utilities.bzl

Line 48 in 08a3160

def strip_packaging_and_classifier_and_version(coord):

[jin]

Do you have a repro example?

[gmishkin]

@jin I'll try to construct a minimal repro, but I think the gist was:

• I have a function that returns the list to pass to maven_install, which includes

def get_rje_artifacts():
  return [
    maven.artifact("com.google.googlejavaformat", "google-java-format", "1.8", classifier = "all-deps")`,
    # ...
  ]

• I have a rule takes in jars that I pass artifact() (from defs.bzl) on the above to

jar_index(
    name = "external_jar_index",
    jars = [artifact(a) for a in get_rje_artifacts()]
)

Without this patch, the label it ends up passing to the jars attr is not valid because it doesn't have the classifier in the labe.

[SanyaLuc]

@gmishkin It is sad that nobody from repo owners followed up with you, but I found that classifier attribute already exists, just slightly not intuitive format https://groups.google.com/g/bazel-discuss/c/ssIF7MBbN-k