Moved classifier from IvyArtifact to IvyModuleRef

This chaneg doesn't address all what Eric proposed. It's an initial step so futher refactorings will be smaller.

This change simply moves classifier from IvyArtifact to IvyModuleRef.

Testing Done:
https://travis-ci.org/fkorotkov/pants/builds/74449287

Bugs closed: 1489

Reviewed at https://rbcommons.com/s/twitter/r/2579/

src/python/pants/backend/jvm/ivy_utils.py

@@ -28,25 +28,31 @@
 from pants.util.dirutil import safe_mkdir, safe_open
 
 
-IvyArtifact = namedtuple('IvyArtifact', ['path', 'classifier'])
-IvyModule = namedtuple('IvyModule', ['ref', 'artifacts', 'callers'])
+IvyModule = namedtuple('IvyModule', ['ref', 'artifact', 'callers'])
 
 
 logger = logging.getLogger(__name__)
 
 
 class IvyModuleRef(object):
 
-  def __init__(self, org, name, rev):
+  def __init__(self, org, name, rev, classifier=None):
     self.org = org
     self.name = name
     self.rev = rev
+    self.classifier = classifier
 
   def __eq__(self, other):
-    return self.org == other.org and self.name == other.name and self.rev == other.rev
+    return self.org == other.org and \
+           self.name == other.name and \
+           self.rev == other.rev and \
+           self.classifier == other.classifier
 
   def __hash__(self):
-    return hash((self.org, self.name, self.rev))
+    return hash((self.org, self.name, self.rev, self.classifier))
+
+  def __str__(self):
+    return 'IvyModuleRef({})'.format(':'.join([self.org, self.name, self.rev, self.classifier]))
 
   @property
   def unversioned(self):
@@ -59,7 +65,12 @@ def unversioned(self):
     """
 
     # latest.integration is ivy magic meaning "just get the latest version"
-    return IvyModuleRef(name=self.name, org=self.org, rev='latest.integration')
+    return IvyModuleRef(name=self.name, org=self.org, rev='latest.integration', classifier=self.classifier)
+
+  @property
+  def unclassified(self):
+    """This returns an identifier for an IvyModuleRef without classifier information."""
+    return IvyModuleRef(name=self.name, org=self.org, rev=self.rev, classifier=None)
 
 
 class IvyInfo(object):
@@ -73,12 +84,12 @@ def __init__(self):
 
   def add_module(self, module):
     self.modules_by_ref[module.ref] = module
-    if not module.artifacts:
+    if not module.artifact:
       # Module was evicted, so do not record information about it
       return
     for caller in module.callers:
       self._deps_by_caller[caller.unversioned].add(module.ref)
-    self._artifacts_by_ref[module.ref.unversioned].update(module.artifacts)
+    self._artifacts_by_ref[module.ref.unversioned].add(module.artifact)
 
   def traverse_dependency_graph(self, ref, collector, memo=None, visited=None):
     """Traverses module graph, starting with ref, collecting values for each ref into the sets
@@ -114,7 +125,7 @@ def traverse_dependency_graph(self, ref, collector, memo=None, visited=None):
     return acc
 
   def get_artifacts_for_jar_library(self, jar_library, memo=None):
-    """Collects IvyArtifact instances for the passed jar_library.
+    """Collects jars for the passed jar_library.
 
     Because artifacts are only fetched for the "winning" version of a module, the artifacts
     will not always represent the version originally declared by the library.
@@ -125,36 +136,29 @@ def get_artifacts_for_jar_library(self, jar_library, memo=None):
     :param jar_library A JarLibrary to collect the transitive artifacts for.
     :param memo see `traverse_dependency_graph`
     :returns: all the artifacts for all of the jars in this library, including transitive deps
-    :rtype: list of IvyArtifact
+    :rtype: list of str
     """
     artifacts = OrderedSet()
 
     def create_collection(dep):
       return OrderedSet([dep])
     for jar in jar_library.jar_dependencies:
-      jar_module_ref = IvyModuleRef(jar.org, jar.name, jar.rev)
-      valid_classifiers = jar.artifact_classifiers
-      artifacts_for_jar = []
-      for module_ref in self.traverse_dependency_graph(jar_module_ref, create_collection, memo):
-        artifacts_for_jar.extend(
-          artifact for artifact in self._artifacts_by_ref[module_ref.unversioned]
-          if artifact.classifier in valid_classifiers
-        )
-
-      artifacts.update(artifacts_for_jar)
+      for classifier in jar.artifact_classifiers:
+        jar_module_ref = IvyModuleRef(jar.org, jar.name, jar.rev, classifier)
+        for module_ref in self.traverse_dependency_graph(jar_module_ref, create_collection, memo):
+          artifacts.update(self._artifacts_by_ref[module_ref.unversioned])
     return artifacts
 
   def get_jars_for_ivy_module(self, jar, memo=None):
     """Collects dependency references of the passed jar
-    :param jar an IvyModuleRef for a third party dependency.
+    :param jar an JarDependency for a third party dependency.
     :param memo see `traverse_dependency_graph`
     """
 
-    ref = IvyModuleRef(jar.org, jar.name, jar.rev)
-
+    ref = IvyModuleRef(jar.org, jar.name, jar.rev, jar.classifier).unversioned
     def create_collection(dep):
       s = OrderedSet()
-      if ref != dep:
+      if ref != dep.unversioned:
         s.add(dep)
       return s
     return self.traverse_dependency_graph(ref, create_collection, memo)
@@ -300,16 +304,15 @@ def _parse_xml_report(cls, path):
       name = module.get('name')
       for revision in module.findall('revision'):
         rev = revision.get('name')
-        artifacts = []
-        for artifact in revision.findall('artifacts/artifact'):
-          artifacts.append(IvyArtifact(path=artifact.get('location'),
-                                       classifier=artifact.get('extra-classifier')))
         callers = []
         for caller in revision.findall('caller'):
           callers.append(IvyModuleRef(caller.get('organisation'),
                                       caller.get('name'),
                                       caller.get('callerrev')))
-        ret.add_module(IvyModule(IvyModuleRef(org, name, rev), artifacts, callers))
+
+        for artifact in revision.findall('artifacts/artifact'):
+          ivy_module_ref = IvyModuleRef(org, name, rev, artifact.get('extra-classifier'))
+          ret.add_module(IvyModule(ivy_module_ref, artifact.get('location'), callers))
     return ret
 
   @classmethod

src/python/pants/backend/jvm/targets/jar_dependency.py

@@ -64,6 +64,7 @@ class JarDependency(object):
     'org',
     'name',
     'rev',
+    'classifier',
     'force',
     'excludes',
     'transitive',

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

@@ -142,18 +142,17 @@ def execute(self):
       jar_library_targets = [t for t in targets if isinstance(t, JarLibrary)]
       for target in jar_library_targets:
         # Add the artifacts from each dependency module.
-        artifact_paths = []
-        for artifact in ivy_info.get_artifacts_for_jar_library(target, memo=ivy_jar_memo):
-          if artifact.path in symlink_map:
-            key = artifact.path
+        for artifact_location in ivy_info.get_artifacts_for_jar_library(target, memo=ivy_jar_memo):
+          if artifact_location in symlink_map:
+            key = artifact_location
           else:
-            key = os.path.realpath(artifact.path)
+            key = os.path.realpath(artifact_location)
           if key not in symlink_map:
             raise self.UnresolvedJarError(
               'Jar {artifact} in {spec} not resolved to the ivy symlink map in conf {conf}.'.format(
-              spec=target.address.spec, artifact=artifact, conf=conf))
-          artifact_paths.append(symlink_map[key])
-        compile_classpath.add_for_target(target, [(conf, entry) for entry in artifact_paths])
+              spec=target.address.spec, artifact=artifact_location, conf=conf))
+          artifact_location = symlink_map[key]
+          compile_classpath.add_for_target(target, [(conf, artifact_location)])
 
     if self._report:
       self._generate_ivy_report(resolve_hash_name)

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

@@ -61,8 +61,7 @@ class IvyTaskMixin(object):
 
   The creation of the 'ivy_resolve_symlink_map' product is a side effect of
   running `def ivy_resolve`. The product is a map of paths in Ivy's resolve cache to
-  stable locations within the working copy. To consume the map, consume a parsed Ivy
-  report which will give you IvyArtifact instances: the artifact path is key.
+  stable locations within the working copy.
   """
 
   @classmethod

src/python/pants/backend/jvm/tasks/jvm_compile/jvm_dependency_analyzer.py

@@ -88,19 +88,19 @@ def _compute_targets_by_file(self):
       def register_transitive_jars_for_ref(ivyinfo, ref):
         deps_by_ref_memo = {}
 
-        def get_transitive_jars_by_ref(ref1, visited=None):
+        def get_transitive_jars_by_ref(ref1):
           def create_collection(current_ref):
-            return set(ivyinfo.modules_by_ref[current_ref].artifacts)
+            return {ivyinfo.modules_by_ref[current_ref].artifact}
           return ivyinfo.traverse_dependency_graph(ref1, create_collection, memo=deps_by_ref_memo)
 
         target_key = (ref.org, ref.name)
         if target_key in jarlibs_by_id:
           # These targets provide all the jars in ref, and all the jars ref transitively depends on.
           jarlib_targets = jarlibs_by_id[target_key]
 
-          for jar in get_transitive_jars_by_ref(ref):
+          for jar_path in get_transitive_jars_by_ref(ref):
             # Register that each jarlib_target provides jar (via all its symlinks).
-            symlink = all_symlinks_map.get(os.path.realpath(jar.path), None)
+            symlink = all_symlinks_map.get(os.path.realpath(jar_path), None)
             if symlink:
               for jarlib_target in jarlib_targets:
                 targets_by_file[symlink].add(jarlib_target)

src/python/pants/backend/project_info/tasks/export.py

@@ -7,7 +7,7 @@
 
 import json
 import os
-from collections import OrderedDict, defaultdict
+from collections import defaultdict
 
 from twitter.common.collections import OrderedSet
 
@@ -42,7 +42,7 @@ class Export(ConsoleTask):
   #
   # Note format changes in src/python/pants/docs/export.md and update the Changelog section.
   #
-  DEFAULT_EXPORT_VERSION = '1.0.1'
+  DEFAULT_EXPORT_VERSION='1.0.2'
 
   class SourceRootTypes(object):
     """Defines SourceRoot Types Constants"""
@@ -224,24 +224,17 @@ def get_transitive_jars(jar_lib):
   def _resolve_jars_info(self):
     """Consults ivy_jar_products to export the external libraries.
 
-    :return: mapping of jar_id -> { 'default' : [ <jar_files> ],
-                                    'sources' : [ <jar_files> ],
-                                    'javadoc' : [ <jar_files> ],
+    :return: mapping of jar_id -> { 'default'     : <jar_file>,
+                                    'sources'     : <jar_file>,
+                                    'javadoc'     : <jar_file>,
+                                    <other_confs> : <jar_file>,
                                   }
     """
-    mapping = defaultdict(list)
+    mapping = defaultdict(dict)
     jar_data = self.context.products.get_data('ivy_jar_products')
-    jar_infos = get_jar_infos(ivy_products=jar_data, confs=['default', 'sources', 'javadoc'])
-    for ivy_module_ref, paths in jar_infos.iteritems():
-      conf_to_jarfile_map = OrderedDict()
-      for conf, pathlist in paths.iteritems():
-        # TODO(Eric Ayers): pathlist can contain multiple jars in the case where classifiers
-        # to resolve extra artifacts are used.  This only captures the first one, meaning the
-        # export is incomplete. See https://github.com/pantsbuild/pants/issues/1489
-        if pathlist:
-          conf_to_jarfile_map[conf] = pathlist[0]
-
-      mapping[self._jar_id(ivy_module_ref)] = conf_to_jarfile_map
+    jar_infos = get_jar_infos(ivy_products=jar_data)
+    for ivy_module_ref, conf_to_jarfile_map in jar_infos.iteritems():
+      mapping[self._jar_id(ivy_module_ref)].update(conf_to_jarfile_map)
     return mapping
 
   def _get_pants_target_alias(self, pants_target_type):

src/python/pants/backend/project_info/tasks/ide_gen.py

@@ -282,14 +282,14 @@ def map_internal_jars(self, targets):
 
           self._project.internal_jars.add(ClasspathEntry(cp_jar, source_jar=cp_source_jar))
 
-  def copy_jars(self, jar_list, dest_dir):
-    cp_jars = []
-    if jar_list:
-      for jar in jar_list:
-        cp_jar = os.path.join(dest_dir, os.path.basename(jar))
-        shutil.copy(jar, cp_jar)
-        cp_jars.append(cp_jar)
-    return cp_jars
+  @staticmethod
+  def copy_jar(jar, dest_dir):
+    if jar:
+      cp_jar = os.path.join(dest_dir, os.path.basename(jar))
+      shutil.copy(jar, cp_jar)
+      return cp_jar
+    else:
+      return None
 
   def map_external_jars(self):
     external_jar_dir = os.path.join(self.gen_project_workdir, 'external-libs')
@@ -301,27 +301,20 @@ def map_external_jars(self):
     external_javadoc_jar_dir = os.path.join(self.gen_project_workdir, 'external-libjavadoc')
     safe_mkdir(external_javadoc_jar_dir, clean=True)
     jar_products = self.context.products.get_data('ivy_jar_products')
-    jar_paths = get_jar_infos(jar_products, confs=['default', 'sources', 'javadoc'])
+    jar_paths = get_jar_infos(jar_products)
     for entry in jar_paths.values():
-      binary_jars = entry.get('default')
-      sources_jars = entry.get('sources')
-      javadoc_jars = entry.get('javadoc')
-      cp_jars = self.copy_jars(binary_jars, external_jar_dir)
-      cp_source_jars = self.copy_jars(sources_jars, external_source_jar_dir)
-      cp_javadoc_jars = self.copy_jars(javadoc_jars, external_javadoc_jar_dir)
-      for i in range(len(cp_jars)):
-        cp_jar = cp_jars[i]
-        if i < len(cp_source_jars):
-          cp_source_jar = cp_source_jars[i]
-        else:
-          cp_source_jar = None
-        if i < len(cp_javadoc_jars):
-          cp_javadoc_jar = cp_javadoc_jars[i]
-        else:
-          cp_javadoc_jar = None
-        self._project.external_jars.add(ClasspathEntry(cp_jar,
-                                                       source_jar=cp_source_jar,
-                                                       javadoc_jar=cp_javadoc_jar))
+      binary_jar  = self.copy_jar(entry.get('default'), external_jar_dir)
+      sources_jar = self.copy_jar(entry.get('sources'), external_source_jar_dir)
+      javadoc_jar = self.copy_jar(entry.get('javadoc'), external_javadoc_jar_dir)
+      if binary_jar:
+        self._project.external_jars.add(ClasspathEntry(jar=binary_jar,
+                                                       source_jar=sources_jar,
+                                                       javadoc_jar=javadoc_jar))
+      # treat all other jars as binaries
+      for classifier, jar in entry.iteritems():
+        if classifier not in {'default', 'sources', 'javadoc'}:
+          binary_jar  = self.copy_jar(jar, external_jar_dir)
+          self._project.external_jars.add(ClasspathEntry(binary_jar))
 
   def execute(self):
     """Stages IDE project artifacts to a project directory and generates IDE configuration files."""

src/python/pants/backend/project_info/tasks/projectutils.py

@@ -8,28 +8,21 @@
 from collections import defaultdict
 
 
-def get_jar_infos(ivy_products, confs=None):
+def get_jar_infos(ivy_products):
   """Returns a list of dicts containing the paths of various jar file resources.
 
   Keys include 'default' (normal jar path), 'sources' (path to source jar), and 'javadoc'
   (path to doc jar). None of them are guaranteed to be present, but 'sources' and 'javadoc'
   will never be present if 'default' isn't.
 
   :param ivy_products: ivy_jar_products data from a context
-  :param confs: List of key types to return (eg ['default', 'sources']). Just returns 'default' if
-    left unspecified.
-  :returns mapping of IvyModuleRef --> {'default' : [<jar_filenames>],
-                                        'sources' : [<jar_filenames>],
-                                        'javadoc' : [<jar_filenames>]}
+  :returns mapping of unclassified IvyModuleRef --> mapping of <classifier> --> <jar_path>
   """
-  confs = confs or ['default']
   classpath_maps = defaultdict(dict)
   if ivy_products:
-    for conf, info_group in ivy_products.items():
-      if conf not in confs:
-        continue  # We don't care about it.
+    for _, info_group in ivy_products.items():
       for info in info_group:
         for module in info.modules_by_ref.values():
-          if module.artifacts:
-            classpath_maps[module.ref][conf] = [artifact.path for artifact in module.artifacts]
+          if module.artifact:
+            classpath_maps[module.ref.unclassified][module.ref.classifier or 'default'] = module.artifact
   return classpath_maps