bazel-contrib / rules_jvm_external

Bazel rules to resolve, fetch and export Maven artifacts
Apache License 2.0
336 stars 256 forks source link

Incorrect dependency targets generated by rules_jvm_external #236

Closed borkaehw closed 5 years ago

borkaehw commented 5 years ago

I am seeing a similar issue described in https://github.com/bazelbuild/rules_jvm_external/issues/201, I also left a comment https://github.com/bazelbuild/rules_jvm_external/issues/201#issuecomment-518412423 back then. It seems like https://github.com/bazelbuild/rules_jvm_external/pull/215 didn't fix all the similar issues.

I see an error of

in deps attribute of jvm_import rule @maven//:com_sun_xml_ws_rt: rule '@maven//:org_glassfish_ha_ha_api' does not exist

when trying to build a target that depends on @maven//:com_microsoft_bingads_microsoft_bingads. I can see @maven//:org_glassfish_ha_ha_api is never being generated. The following are my WORKSPACE and maven_install.

WORKSPACE:

workspace(name = "test")

load(
    "@bazel_tools//tools/build_defs/repo:http.bzl",
    "http_archive",
    "http_file",
)

################################################################################
# JVM External
################################################################################

RULES_JVM_EXTERNAL_TAG = "2.7"

http_archive(
    name = "rules_jvm_external",
    sha256 = "f04b1466a00a2845106801e0c5cec96841f49ea4e7d1df88dc8e4bf31523df74",
    strip_prefix = "rules_jvm_external-{}".format(RULES_JVM_EXTERNAL_TAG),
    type = "zip",
    url = "https://github.com/bazelbuild/rules_jvm_external/archive/{}.zip".format(RULES_JVM_EXTERNAL_TAG),
)

################################################################################
# Scala Annex
################################################################################
rules_scala_annex_version = "65ac8b810690dd918c73eceadca2a0acfc506cba"

http_archive(
    name = "rules_scala_annex",
    sha256 = "e97911834d3d7767edd5512d57e98742a15ec95d8e69787ccb588e48b7ce896a",
    strip_prefix = "rules_scala-{}".format(rules_scala_annex_version),
    type = "zip",
    url = "https://github.com/higherkindness/rules_scala/archive/{}.zip".format(rules_scala_annex_version),
)

load(
    "@rules_scala_annex//rules/scala:workspace.bzl",
    "scala_register_toolchains",
    "scala_repositories",
)
load(
    "@rules_scala_annex//rules/scalafmt:workspace.bzl",
    "scalafmt_default_config",
    "scalafmt_repositories",
)
load(
    "@rules_scala_annex//rules/scala_proto:workspace.bzl",
    "scala_proto_register_toolchains",
    "scala_proto_repositories",
)

scala_repositories()

load("@annex//:defs.bzl", annex_pinned_maven_install = "pinned_maven_install")

annex_pinned_maven_install()

scala_register_toolchains()

scalafmt_repositories()

load("@annex_scalafmt//:defs.bzl", annex_scalafmt_pinned_maven_install = "pinned_maven_install")

annex_scalafmt_pinned_maven_install()

scalafmt_default_config()

scala_proto_repositories()

load("@annex_proto//:defs.bzl", annex_proto_pinned_maven_install = "pinned_maven_install")

annex_proto_pinned_maven_install()

scala_proto_register_toolchains()

load(":workspace.bzl", "maven_repositories")

maven_repositories()

load("@maven//:defs.bzl", "pinned_maven_install")

pinned_maven_install()

bind(
    name = "default_scala",
    actual = "@rules_scala_annex//src/main/scala:zinc_2_12_8",
)

################################################################################
# Skylib
################################################################################

skylib_version = "8cecf885c8bf4c51e82fd6b50b9dd68d2c98f757"

http_archive(
    name = "bazel_skylib",
    sha256 = "d54e5372d784ceb365f7d38c3dad7773f73b3b8ebc8fb90d58435a92b6a20256",
    strip_prefix = "bazel-skylib-{}".format(skylib_version),
    type = "zip",
    url = "https://github.com/bazelbuild/bazel-skylib/archive/{}.zip".format(skylib_version),
)

################################################################################
# Protobuf
################################################################################

protobuf_version = "6a59a2ad1f61d9696092f79b6d74368b4d7970a3"

http_archive(
    name = "com_google_protobuf",
    sha256 = "934d04425954cb1b5f7672705aee247103052724b479900ed2b7c861cd10401f",
    strip_prefix = "protobuf-{}".format(protobuf_version),
    type = "zip",
    url = "https://github.com/protocolbuffers/protobuf/archive/{}.zip".format(protobuf_version),
)

load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")

protobuf_deps()

maven_install:

maven_install(
    artifacts = [
        "com.microsoft.bingads:microsoft.bingads:12.13.3",
    ],
    repositories = [
        "https://repo.maven.apache.org/maven2",
    ],
    fetch_sources = True,
    maven_install_json = "//:maven_install.json",
)

I am happy to provide minimal reproducible case if that makes investigation easier. Thanks.

jin commented 5 years ago

I cannot reproduce this with rules_jvm_external on head.

maven_install(
    name = "bingads",
    artifacts = [
        "com.microsoft.bingads:microsoft.bingads:12.13.3",
    ],
    repositories = [
        "https://repo.maven.apache.org/maven2",
    ],
    fetch_sources = True,
    # maven_install_json = "//:maven_install.json",
)
$ bazel query --output=build @bingads//:org_glassfish_ha_ha_api
# /usr/local/google/home/jingwen/.cache/bazel/_bazel_jingwen/8484bc4fff18ee4a905b69a9ddb0e143/external/bingads/BUILD:1372:1
jvm_import(
  name = "org_glassfish_ha_ha_api",
  tags = ["maven_coordinates=org.glassfish.ha:ha-api:3.1.12"],
  jars = ["@bingads//:v1/https/repo.maven.apache.org/maven2/org/glassfish/ha/ha-api/3.1.12/ha-api-3.1.12.jar"],
  srcjar = "@bingads//:v1/https/repo.maven.apache.org/maven2/org/glassfish/ha/ha-api/3.1.12/ha-api-3.1.12-sources.jar",
  deps = [],
)

Same if I use maven_install.json:

maven_install(
    name = "bingads",
    artifacts = [
        "com.microsoft.bingads:microsoft.bingads:12.13.3",
    ],
    repositories = [
        "https://repo.maven.apache.org/maven2",
    ],
    fetch_sources = true,
    maven_install_json = "//:bingads_install.json",
)

load("@bingads//:defs.bzl", "pinned_maven_install")
pinned_maven_install()
$ bazel query --output=build @bingads//:org_glassfish_ha_ha_api
# /usr/local/google/home/jingwen/.cache/bazel/_bazel_jingwen/8484bc4fff18ee4a905b69a9ddb0e143/external/bingads/BUILD:2242:1
jvm_import(
  name = "org_glassfish_ha_ha_api",
  tags = ["maven_coordinates=org.glassfish.ha:ha-api:3.1.12"],
  jars = ["@bingads//:v1/https/repo.maven.apache.org/maven2/org/glassfish/ha/ha-api/3.1.12/ha-api-3.1.12.jar"],
  srcjar = "@bingads//:v1/https/repo.maven.apache.org/maven2/org/glassfish/ha/ha-api/3.1.12/ha-api-3.1.12-sources.jar",
  deps = [],
)
borkaehw commented 5 years ago

@jin Thanks for testing it out. I can confirm that it's not a problem on HEAD, but it's a problem on latest release 2.7. Would you mind to test it again using 2.7? If you can reproduce it and confirm the issue on 2.7, we can simply make a release on HEAD and close this issue, even though I didn't dive in how the recent commits could have fixed this issue.

borkaehw commented 5 years ago

Hold on. By using HEAD, I see other deps are missing.

in deps attribute of jvm_import rule @maven//:com_microsoft_bingads_microsoft_bingads: rule '@maven//:com_sun_xml_ws_release_documentation_zip_docbook' does not exist
in deps attribute of jvm_import rule @maven//:com_microsoft_bingads_microsoft_bingads: rule '@maven//:com_sun_xml_ws_samples_zip' does not exist
in deps attribute of jvm_import rule @maven//:com_microsoft_bingads_microsoft_bingads: rule '@maven//:com_sun_xml_ws_jaxws_ri' does not exist
jin commented 5 years ago

Dug a bit deeper:

com.sun.xml.ws:release-documentation and com.sun.xml.ws:samples are zip packaging types, but Coursier doesn't download them because it doesn't recognize zip as a packaging type. The simplest workaround to is to exclude them in excluded_artifacts.

com.sun.xml.ws:jaxws-ri is a parent pom artifact, but for some reason when Coursier resolves it as a transitive artifact, it stops at that artifact. If you add it as a top level artifact, everything works.

Can you try this configuration at HEAD?

maven_install(
    name = "bingads",
    artifacts = [
        "com.microsoft.bingads:microsoft.bingads:12.13.3",
        "com.sun.xml.ws:jaxws-ri:2.3.2",
    ],
    repositories = [
        "https://repo.maven.apache.org/maven2",
    ],
    fetch_sources = True,
    excluded_artifacts = [
        "com.sun.xml.ws:samples",
        "com.sun.xml.ws:release-documentation",
    ],
)
jin commented 5 years ago

The principled fix here is to iterate through all dependencies of every target, and filter away the ones that don't have a top level Bazel target / mapped jar or aar artifact.

borkaehw commented 5 years ago

Can you try this configuration at HEAD?

Yes, this config works. Thanks.

filter away the ones that don't have a top level Bazel target / mapped jar or aar artifact.

I am fine with the workaround as long as it doesn't cause other issues. Looking forward to principled fix. It would be nice to have a release on HEAD.

jin commented 5 years ago

Done: https://github.com/bazelbuild/rules_jvm_external/releases/tag/2.8

I'll close this for now, since the workaround is OK. I'll revisit this if there are more instances of breakages like this.