bazelbuild / migration-tooling

Migration tools for Bazel
Apache License 2.0
45 stars 30 forks source link

Aether dependency resolution fails with "Invalid Range Result", generates partial workspace #47

Open Zetten opened 7 years ago

Zetten commented 7 years ago

As the title suggests, the generate_workspace tool after PR #45 ends up omitting some dependencies due to a problem with the new dependency resolution mechanism.

I noticed this problem when one of my dependencies attempted to pull in com.google.inject:guice:4.0 but the resulting generate_workspace.bzl did not contain corresponding maven_jar/java_library rules.

I've created a minimal test project: https://github.com/Zetten/bazel-generate-workspace-error

The pom.xml file describes Guava and Guice dependencies, but if the generate_workspace command is executed against it (may be run easily from the project dir with bazel build :test_generate_workspace) the resulting .bzl file does not contain the Guice dependency (the Guava deps are fine). The build log reports:

Aug 02, 2017 6:55:33 PM com.google.devtools.build.workspace.maven.Resolver traverseDeps
INFO:   Downloading pom for com.example:bazel-generate-error:0.0.0
Aug 02, 2017 6:55:34 PM com.google.devtools.build.workspace.maven.Resolver traverseDeps
INFO:   Downloading pom for com.google.guava:guava:22.0
Aug 02, 2017 6:55:34 PM com.google.devtools.build.workspace.maven.Resolver traverseDeps
INFO:   Downloading pom for com.google.code.findbugs:jsr305:2.0.2
Aug 02, 2017 6:55:34 PM com.google.devtools.build.workspace.maven.Resolver traverseDeps
INFO:   Downloading pom for com.google.errorprone:error_prone_annotations:2.0.18
Aug 02, 2017 6:55:35 PM com.google.devtools.build.workspace.maven.Resolver traverseDeps
INFO:   Downloading pom for com.google.j2objc:j2objc-annotations:1.1
Aug 02, 2017 6:55:35 PM com.google.devtools.build.workspace.maven.Resolver traverseDeps
INFO:   Downloading pom for org.codehaus.mojo:animal-sniffer-annotations:1.14
Aug 02, 2017 6:55:35 PM com.google.devtools.build.workspace.maven.Resolver addDependency
WARNING: Could not resolve dependency com.google.inject:guice:4.0: Unable to find a version for com.google.inject:guice:4.0 due to Invalid Range Result
petroseskinder commented 7 years ago

Thanks for pointing this out.

@kchodorow i investigated. I believe I have found the cause. I logged all the available versions we find when we perform:

bazel run generate_workspace:all -- -a "com.google.inject:guice:[,)"

and I find the following versions

{1.0, 2.0, 2.0-no_aop, 3.0-rc2, 3.0-rc3, 3.0, 4.0-beta, 4.0-beta4, 4.0-beta5}

You'll notice no version 4.0, which is what Zetten requested. We are requesting versions from https://repo1.maven.org/maven2/com/google/inject/guice/, and you'll notice that there is a version 4.0 as well a version 4.1.0. However, notice in the image below that their time stamps are empty.

I believe that because of the empty time stamp, aether is interpreting that as an empty result. I'll eventually play around with this to resolve this. Or I might log all available versions whenever a version is unavailable. Thoughts?

screenshot
petroseskinder commented 7 years ago

@Zetten for a temporary workaround, can you try 3.0 or 4.0-beta5 rather than version 4.0

Zetten commented 7 years ago

@petroseskinder I can confirm these versions work fine.

Guice is actually being pulled in as a transitive dependency so I might have to do some fiddling to get it working in my real project, but I can also add the maven_jar and java_library rules manually to my WORKSPACE/BUILD files. Guice 4.0 is successfully retrieved with a maven_jar, it's just the automatic generation that has an issue.

After checking https://repo1.maven.org/maven2/com/google/inject/guice/maven-metadata.xml version 4.0 is also not listed there, so it seems that Aether is using the repo metadata/index accurately. To me this smells like a repo issue - if it were my own Nexus/Artifactory I'd probably try reindexing from the concrete artifact files, but I'm not sure how to fix such a problem on central.

petroseskinder commented 7 years ago

After checking https://repo1.maven.org/maven2/com/google/inject/guice/maven-metadata.xml version 4.0 is also not listed there, so it seems that Aether is using the repo metadata/index accurately. To me this smells like a repo issue - if it were my own Nexus/Artifactory I'd probably try reindexing from the concrete artifact files, but I'm not sure how to fix such a problem on central.

Thanks for confirming.

Guice is actually being pulled in as a transitive dependency so I might have to do some fiddling to get it working in my real project, but I can also add the maven_jar and java_library rules manually to my WORKSPACE/BUILD files.

@Zetten try to pass guice 4.0 as the first argument to generate_workspace in your list of artifacts. That should circumvent the transitive dependency issue.

Zetten commented 7 years ago

@petroseskinder Passing the artifact directly has the same problem:

$ bazel --batch run @bazel_migration_tooling//generate_workspace -- -a com.google.inject:guice:4.0
INFO: Found 1 target...
Target @bazel_migration_tooling//generate_workspace:generate_workspace up-to-date:
  bazel-bin/external/bazel_migration_tooling/generate_workspace/generate_workspace.jar
  bazel-bin/external/bazel_migration_tooling/generate_workspace/generate_workspace
INFO: Elapsed time: 1.580s, Critical Path: 0.01s

INFO: Running command line: bazel-bin/external/bazel_migration_tooling/generate_workspace/generate_workspace -a com.google.inject:guice:4.0
Aug 02, 2017 9:05:47 PM com.google.devtools.build.workspace.maven.Resolver resolveArtifact
WARNING: Unable to resolve version
Wrote /home/foo/.cache/bazel/_bazel_foo/46f07ee29bf55cd26c1513c218f2ef5a/execroot/__main__/bazel-out/local-fastbuild/bin/external/bazel_migration_tooling/generate_workspace/generate_workspace.runfiles/__main__/generate_workspace.bzl
$ cat /home/foo/.cache/bazel/_bazel_foo/46f07ee29bf55cd26c1513c218f2ef5a/execroot/__main__/bazel-out/local-fastbuild/bin/external/bazel_migration_tooling/generate_workspace/generate_workspace.runfiles/__main__/generate_workspace.bzl
# The following dependencies were calculated from:
#
# generate_workspace -a com.google.inject:guice:4.0

def generated_maven_jars():
  pass

def generated_java_libraries():
  pass
petroseskinder commented 7 years ago

@Zetten yes, this is the behavior I saw before.

What I meant was, try passing a working version at the start of your argument list. These are the working versions:

1.0, 2.0, 2.0-no_aop, 3.0-rc2, 3.0-rc3, 3.0, 4.0-beta, 4.0-beta4, 4.0-beta5

It's not pretty, but it should unblock you, if you are blocked.

nevillelyh commented 6 years ago

I worked around it by not checking version spec if it's a single version. It's hacky though and not sure if worthy of a PR. https://github.com/nevillelyh/migration-tooling/commit/f10e14fd18ad3885c7ec8aa305e4eba266a07ebf

vorburger commented 6 years ago

This issue seems kinda blocker, no? ;-) I learnt more about Bazel as FOSDEM today and read up a bit on the way back home, found https://docs.bazel.build/versions/master/generate-workspace.html, tried it, but immediately hit what seems to be this issue, with the simplest possible pom.xml, inspired by https://github.com/bazelbuild/examples/blob/master/java-maven/WORKSPACE:

<project>
  <modelVersion>4.0.0</modelVersion>

  <groupId>build.bazel.examples</groupId>
  <artifactId>java-maven2bazel</artifactId>
  <version>1.0.0-SNAPSHOT</version>

  <dependencies>
    <dependency>
      <groupId>com.google.guava</groupId>
      <artifactId>guava</artifactId>
      <version>18.0</version>
    </dependency>
    <dependency>
      <groupId>junit</groupId>
      <artifactId>junit</artifactId>
      <version>4.12</version>
    </dependency>
  </dependencies>
</project>

As hitting this isn't terribly encouraging to further pursue exploring Bazel for someone interested in possibly migrating something from Maven, I've just taken the liberty of raising a PR for @nevillelyh on https://github.com/bazelbuild/migration-tooling/pull/80 - are there any active maintainers of this project who would like to merge that? Or review it and say how that's NOK and should be done right to unblock this? Or make me a commiter so that I can press the Merge button? :smiley:

prestonvanloon commented 6 years ago

Same problem with http://central.maven.org/maven2/org/codehaus/jackson/jackson-mapper-asl/