[coursier/m2-coords] update coursier json parsing; use maven's coords (…

…pantsbuild#5475)

This is a multi-tool change. The coursier side is here: coursier/coursier#782

This fixes the issue with coursier resolution where transitive dependencies that have classifiers can't be differentiated. Ie, if you have a classifier on a dependency, you get all the jars even if they have a different classifier.

Additionally this updates the m2 coordinate string representation to bring it inline with maven's coordinate language. This is because I've chosen that language to use in the coursier change as well. I've bumped ivy and coursier's implementation version to invalidate cached artifacts that used the old format.

contrib/node/src/python/pants/contrib/node/subsystems/node_distribution.py

@@ -305,4 +305,4 @@ def eslint_supportdir(self, task_workdir):
       installed = self._binary_util.is_bin_valid(bootstrapped_support_path, binary_file_specs)
       if not installed:
         self._configure_eslinter(bootstrapped_support_path)
-    return (bootstrapped_support_path, configured)
+    return (bootstrapped_support_path, configured)

src/python/pants/backend/jvm/tasks/coursier/coursier_subsystem.py

@@ -36,14 +36,14 @@ def register_options(cls, register):
     register('--fetch-options', type=list, fingerprint=True,
              help='Additional options to pass to coursier fetch. See `coursier fetch --help`')
     register('--bootstrap-jar-url', fingerprint=True,
-             default='https://dl.dropboxusercontent.com/s/a1xr4qxzj5aoait/coursier-cli-1.0.2.3e4a65d5ee66f043c2467972bd6e29f48b570715.jar?dl=0',
+             default='https://dl.dropboxusercontent.com/s/t8zz2nupqkv2sfv/coursier-cli-1.0.2.e943c8069180d22f352878949f9522ad91d89026.jar?dl=0',
              help='Location to download a bootstrap version of Coursier.')
     # TODO(wisechengyi): currently using a custom url for fast iteration.
     # Once the coursier builds are stable, move the logic to binary_util. https://github.com/pantsbuild/pants/issues/5381
     # Ths sha in the version corresponds to the sha in the PR https://github.com/coursier/coursier/pull/774
     # The jar is built by following https://github.com/coursier/coursier/blob/master/DEVELOPMENT.md#build-with-pants
     register('--version', type=str, fingerprint=True,
-             default='1.0.2.3e4a65d5ee66f043c2467972bd6e29f48b570715',
+             default='1.0.2.e943c8069180d22f352878949f9522ad91d89026',
              help='Version paired with --bootstrap-jar-url, in order to invalidate and fetch the new version.')
     register('--bootstrap-fetch-timeout-secs', type=int, advanced=True, default=10,
              help='Timeout the fetch if the connection is idle for longer than this value.')

src/python/pants/backend/jvm/tasks/ivy_task_mixin.py

@@ -98,7 +98,7 @@ def register_options(cls, register):
 
   @classmethod
   def implementation_version(cls):
-    return super(IvyTaskMixin, cls).implementation_version() + [('IvyTaskMixin', 3)]
+    return super(IvyTaskMixin, cls).implementation_version() + [('IvyTaskMixin', 4)]
 
   @memoized_property
   def ivy_cache_dir(self):

src/python/pants/java/jar/jar_dependency_utils.py

@@ -58,10 +58,14 @@ def __init__(self, org, name, rev=None, classifier=None, ext=None):
     self.name = name
     self.rev = rev
     self.classifier = classifier
-    self.ext = ext or 'jar'
+    self._ext = ext
 
     self._id = (self.org, self.name, self.rev, self.classifier, self.ext)
 
+  @property
+  def ext(self):
+    return self._ext or 'jar'
+
   @classmethod
   def create(cls, jar):
     """Creates an actual M2Coordinate from the given M2Coordinate-like object (eg a JarDependency).
@@ -109,8 +113,17 @@ def maybe_compenent(component):
 
   @classmethod
   def from_string(cls, string_coord):
-    org, name, rev, classifier, ext = string_coord.split(':')
-    return M2Coordinate(org, name, rev or None, classifier or None, ext or None)
+    packaging = None
+    classifier = None
+    ct = string_coord.count(':')
+    if ct == 2:
+      org, name, rev = string_coord.split(':')
+    elif ct == 3:
+      org, name, packaging, rev = string_coord.split(':')
+    elif ct == 4:
+      org, name, packaging, classifier, rev = string_coord.split(':')
+    rev = rev or None
+    return M2Coordinate(org=org, name=name, rev=rev, ext=packaging, classifier=classifier)
 
   @property
   def simple_coord(self):
@@ -131,13 +144,27 @@ def __hash__(self):
     return hash(self._id)
 
   def __str__(self):
-    # Doesn't follow https://maven.apache.org/pom.html#Maven_Coordinates
-    # Instead produces an unambiguous string representation of the coordinate
-    # org:name:rev:classifier:type_
-    # if any of the fields are None, it uses ''
-    # for example org=a, name=b, type_=jar -> a:b:::jar
-    return ':'.join((x or '') for x in self._id)
+    # Follows https://maven.apache.org/pom.html#Maven_Coordinates,
+    # with the exception that if rev is missing, it adds an extra ':' at the end.
+    # for example org=a, name=b, type_=jar -> a:b:jar:
+    if self.classifier:
+      components = (self.org, self.name, self.ext or 'jar', self.classifier, self.rev or '')
+    elif self.ext and self.ext != 'jar':
+      components = (self.org, self.name, self.ext, self.rev or '')
+    else:
+      components = (self.org, self.name, self.rev or '')
+
+
+    return ':'.join((x or '') for x in components)
 
   def __repr__(self):
     return ('M2Coordinate(org={!r}, name={!r}, rev={!r}, classifier={!r}, ext={!r})'
             .format(*self._id))
+
+  def copy(self, **replacements):
+    """Returns a clone of this M2Coordinate with the given replacements kwargs overlaid."""
+    cls = type(self)
+    kwargs = {'org': self.org, 'name': self.name, 'ext': self.ext, 'classifier': self.classifier, 'rev': self.rev}
+    for key, val in replacements.items():
+      kwargs[key] = val
+    return cls(**kwargs)

tests/python/pants_test/backend/jvm/tasks/test_classpath_products.py

@@ -277,7 +277,7 @@ def test_jar_missing_pants_path_fails_adding(self):
                                                            cache_path='somewhere',
                                                            pants_path=None)])
     self.assertEqual(
-      'Jar: org:name:::jar has no specified path.',
+      'Jar: org:name: has no specified path.',
       str(cm.exception))
 
   def test_get_product_target_mappings_for_targets_respect_excludes(self):

tests/python/pants_test/backend/jvm/tasks/test_coursier_resolve.py

@@ -17,6 +17,7 @@
 from pants.backend.jvm.targets.java_library import JavaLibrary
 from pants.backend.jvm.targets.jvm_target import JvmTarget
 from pants.backend.jvm.targets.managed_jar_dependencies import ManagedJarDependencies
+from pants.backend.jvm.tasks.classpath_products import ClasspathProducts
 from pants.backend.jvm.tasks.coursier_resolve import (CoursierResolve,
                                                       CoursierResolveFingerprintStrategy)
 from pants.base.exceptions import TaskError
@@ -80,6 +81,25 @@ def test_resolve_with_local_url(self):
       paths = [tup[1] for tup in compile_classpath.get_for_target(dep_with_url_lib)]
       self.assertTrue(any('.pants.d/coursier/cache/absolute/' in path for path in paths))
 
+  def test_resolve_specific_with_sources_javadocs(self):
+    # Create a jar_library with a single dep, and another library with no deps.
+    dep = JarDependency('commons-lang', 'commons-lang', '2.5')
+    jar_lib = self.make_target('//:a', JarLibrary, jars=[dep])
+    scala_lib = self.make_target('//:b', JavaLibrary, sources=[])
+    with self._temp_workdir() as workdir:
+      # Confirm that the deps were added to the appropriate targets.
+      context = self.context(target_roots=[jar_lib, scala_lib])
+      task = self.prepare_execute(context)
+      compile_classpath = context.products.get_data('compile_classpath',
+        init_func=ClasspathProducts.init_func(workdir)
+      )
+      task.resolve([jar_lib, scala_lib], compile_classpath, sources=True, javadoc=True)
+
+      # Both javadoc and sources jars are added to the classpath product
+      self.assertEquals(['default', 'src_doc', 'src_doc'],
+       sorted([c[0] for c in compile_classpath.get_for_target(jar_lib)]))
+      self.assertEquals(0, len(compile_classpath.get_for_target(scala_lib)))
+
   def test_resolve_conflicted(self):
     losing_dep = JarDependency('com.google.guava', 'guava', '16.0')
     winning_dep = JarDependency('com.google.guava', 'guava', '16.0.1')
@@ -98,6 +118,20 @@ def test_resolve_conflicted(self):
     self.assertEqual('default', conf)
     self.assertEqual('guava-16.0.1.jar', os.path.basename(path))
 
+  def test_resolve_ignores_jars_with_rev_left_off(self):
+    """If a resolve jar leaves off the rev, we're supposed to get the latest version,
+       but coursier doesn't currently support that.
+       https://github.com/coursier/coursier/issues/209
+    """
+    with self.assertRaises(TaskError) as cm:
+      jar = JarDependency('com.google.guava', 'guava')
+      lib = self.make_target('//:b', JarLibrary, jars=[jar])
+
+      self.resolve([lib])
+    self.assertEqual(
+      'Undefined revs for jars unsupported by Coursier. "jar(org=u\'com.google.guava\', name=u\'guava\', rev=None, classifier=None, ext=u\'jar\')"',
+      str(cm.exception))
+
   def test_resolve_multiple_artifacts(self):
     def coordinates_for(cp):
       return {resolved_jar.coordinate for conf, resolved_jar in cp}
@@ -224,6 +258,7 @@ def test_when_invalid_coursier_cache_should_trigger_resolve(self):
       #    └─ org.hamcrest:hamcrest-core:1.3
       self.assertEquals(2, len(jar_cp))
 
+
       # Take a sample jar path, remove it, then call the task again, it should invoke coursier again
       conf, path = jar_cp[0]
 
@@ -241,6 +276,16 @@ def test_when_invalid_coursier_cache_should_trigger_resolve(self):
 
       task.runjava.assert_called()
 
+  def test_resolve_jarless_pom(self):
+    jar = JarDependency('org.apache.commons', 'commons-weaver-privilizer-parent', '1.3')
+    lib = self.make_target('//:b', JarLibrary, jars=[jar])
+
+    compile_classpath = self.resolve([lib])
+
+    lib_cp = compile_classpath.get_for_target(lib)
+
+    self.assertEqual(0, len(lib_cp))
+
   def _make_junit_target(self):
     junit_dep = JarDependency('junit', 'junit', rev='4.12')
     junit_jar_lib = self.make_target('//:a', JarLibrary, jars=[junit_dep])

tests/python/pants_test/backend/jvm/test_jar_dependency_utils.py

@@ -12,32 +12,32 @@
 
 class JarDependencyUtilsTest(unittest.TestCase):
   def test_m2_string_representation(self):
-    org_name_ref = M2Coordinate(org='org.example', name='lib', rev='the-ref')
+    org_name_ref = M2Coordinate(org='org.example', name='lib', rev='the-rev')
 
-    self.assertEquals('org.example:lib:the-ref::jar', str(org_name_ref))
+    self.assertEquals('org.example:lib:the-rev', str(org_name_ref))
     self.assertEquals(org_name_ref, M2Coordinate.from_string(str(org_name_ref)))
 
     org_name_ref_classifier = M2Coordinate(org='org.example', name='lib',
-                                           rev='the-ref', classifier='classify')
+                                           rev='the-rev', classifier='classify')
 
-    self.assertEquals('org.example:lib:the-ref:classify:jar', str(org_name_ref_classifier))
+    self.assertEquals('org.example:lib:jar:classify:the-rev', str(org_name_ref_classifier))
     self.assertEquals(org_name_ref_classifier, M2Coordinate.from_string(str(org_name_ref_classifier)))
 
     org_name_classifier = M2Coordinate(org='org.example', name='lib', classifier='classify')
 
-    self.assertEquals('org.example:lib::classify:jar', str(org_name_classifier))
+    self.assertEquals('org.example:lib:jar:classify:', str(org_name_classifier))
     self.assertEquals(org_name_classifier, M2Coordinate.from_string(str(org_name_classifier)))
 
     org_name_type_classifier = M2Coordinate(org='org.example', name='lib',
                                             classifier='classify', ext='zip')
 
-    self.assertEquals('org.example:lib::classify:zip', str(org_name_type_classifier))
+    self.assertEquals('org.example:lib:zip:classify:', str(org_name_type_classifier))
     self.assertEquals(org_name_type_classifier, M2Coordinate.from_string(str(org_name_type_classifier)))
 
     org_name_type_jar_classifier = M2Coordinate(org='org.example', name='lib',
                                                 classifier='classify', ext='jar')
 
-    self.assertEquals('org.example:lib::classify:jar', str(org_name_type_jar_classifier))
+    self.assertEquals('org.example:lib:jar:classify:', str(org_name_type_jar_classifier))
     self.assertEquals(org_name_type_jar_classifier, M2Coordinate.from_string(str(org_name_type_jar_classifier)))
 
   def test_m2_coordinates_with_same_properties(self):