Remove with_artifact(), with_sources(), and with_docs() from JarDepen…

…dency

Part of the refactoring of Ivy described in https://docs.google.com/document/d/1sEMXUmj7v-YCBZ_wHLpCFjkHOeWjsc1NR1hRIJ9uCZ8/edit?usp=sharing

This removes some methods from JarDependency target, which is aliased to jar() in BUILD files.
These methods have been deprecated since 0.0.29 (early February 2015).  The next step will be to remove
the array of artifacts from JarDependency completely so that there is a 1:1 mapping betwen a JarDependency request
and a resolved jar.

Testing Done:
CI is green at https://travis-ci.org/pantsbuild/pants/builds/76769043

Bugs closed: 2048

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

examples/src/java/org/pantsbuild/example/3rdparty_jvm.md

@@ -172,38 +172,6 @@ transitive dependencies from JVM targets:
       ]
     )
 
-**If you notice a missing dependency**, check for a naming conflict.
-When bringing in multiple jars with the same org, name, and version,
-only the first reference will win, and subsequent references will be
-silently discarded. One way that this can occur is with dependencies
-that use a classifier to differentiate themselves. Consider this
-example:
-
-    :::python
-    jar_library(name = 'stanford-corenlp',
-      jars = [
-        jar(org = 'edu.stanford.nlp', name = 'stanford-corenlp', rev = '3.3.1').with_sources(),
-        jar(org = 'edu.stanford.nlp', name = 'stanford-corenlp', rev = '3.3.1', classifier='models')
-      ]
-    )
-
-In the above example, the
-`edu.stanford.nlp.stanford-corenlp-3.3.1-models.jar` will be silently
-skipped by pants. To bring both jars in, use the `.with_artifacts()`
-method of the bdict\_jar. Using this method, the above example would be
-transformed into:
-
-    :::python
-    jar_library(name = 'stanford-corenlp',
-      jars = [
-        jar(org = 'edu.stanford.nlp', name = 'stanford-corenlp', rev = '3.3.1')
-        .with_sources().with_artifact(classifier='models').with_artifact(classifier=''),
-      ]
-    )
-
-And as a result, both jars will now be brought into the target's
-classpath.
-
 <a pantsmark="test_3rdparty_jvm_snapshot"> </a>
 
 Using a SNAPSHOT JVM Dependency

src/python/pants/CHANGELOG.rst

@@ -1,6 +1,17 @@
 RELEASE HISTORY
 ===============
 
+0.0.45 (??/??/2015)
+-------------------
+
+Release Notes
+~~~~~~~~~~~~~
+
+In this release, the methods `with_sources()`, `with_docs()` and `with_artifact()`
+were removed from the jar() syntax in BUILD files.   They have been deprecated since
+Pants version 0.0.29.
+
+
 0.0.44 (8/21/2015)
 ------------------
 

src/python/pants/backend/jvm/targets/BUILD

@@ -34,6 +34,8 @@ python_library(
     'src/python/pants/base:build_environment',
     'src/python/pants/base:build_manual',
     'src/python/pants/base:exceptions',
+    'src/python/pants/base:deprecated',
+
     'src/python/pants/base:payload',
     'src/python/pants/base:payload_field',
     'src/python/pants/base:target',

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

@@ -5,11 +5,9 @@
 from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
                         unicode_literals, with_statement)
 
-import sys
-from textwrap import dedent
-
 from pants.backend.jvm.targets.exclude import Exclude
 from pants.base.build_manual import manual
+from pants.base.deprecated import deprecated
 from pants.base.payload_field import PayloadField, stable_json_sha1
 from pants.base.validation import assert_list
 
@@ -33,7 +31,19 @@ class IvyArtifact(PayloadField):
   )
 
   def __init__(self, name, type_=None, ext=None, conf=None, url=None, classifier=None):
-    """ See the arguments in JarDependency.with_artifact(). """
+    """Declares a dependency on an artifact to be resolved externally.
+
+    :param name: The name of the published artifact. This name must not include revision.
+    :param type_: The type of the published artifact. It's usually the same as the artifact's file
+      extension, but not necessarily. For instance, ivy files are of type 'ivy' but have 'xml' as
+      their file extension.
+    :param ext: The file extension of the published artifact.
+    :param url: The url at which this artifact can be found if it isn't located at the standard
+      location in the repository.
+    :param configuration: The public configuration in which this artifact is published. The '*' wildcard can
+      be used to designate all public configurations.
+    :param classifier: The maven classifier of this artifact.
+    """
     self.name = name
     self.type_ = type_ or 'jar'
     self.ext = ext
@@ -88,8 +98,6 @@ def __init__(self, org, name, rev=None, force=False, ext=None, url=None, apidocs
       will properly hyperlink {\ @link}s.
     :param string type_: Artifact packaging type.
     :param string classifier: Classifier specifying the artifact variant to use.
-      Use multiple ``with_artifact`` statements to include multiple artifacts of the same org.name,
-      but with different classifiers.
     :param boolean mutable: Inhibit caching of this mutable artifact. A common use is for
       Maven -SNAPSHOT style artifacts in an active development/integration cycle.
     :param list artifacts: A list of additional IvyArtifacts
@@ -104,7 +112,16 @@ def __init__(self, org, name, rev=None, force=False, ext=None, url=None, apidocs
     self.transitive = not intransitive
     self.apidocs = apidocs
     self.mutable = mutable
-    self.artifacts = tuple(assert_list(artifacts, expected_type=IvyArtifact, key_arg='artifacts'))
+
+    @deprecated('0.0.48',
+                hint_message='JarDependency now only specifies a single artifact, so the artifacts argument will be removed.')
+    def make_artifacts():
+      return tuple(assert_list(artifacts, expected_type=IvyArtifact, key_arg='artifacts'))
+
+    if artifacts:
+      self.artifacts = make_artifacts()
+    else:
+      self.artifacts = ()
 
     if ext or url or type_ or classifier:
       self.append_artifact(name,
@@ -115,11 +132,6 @@ def __init__(self, org, name, rev=None, force=False, ext=None, url=None, apidocs
 
     self._configurations = ('default',)
 
-    # Support legacy method names
-    # TODO(John Sirois): introduce a deprecation cycle for these and then kill
-    self.withSources = self.with_sources
-    self.withDocs = self.with_docs
-
     if classifier:
       self.classifier = classifier
     elif len(self.artifacts) == 1:
@@ -135,9 +147,17 @@ def __init__(self, org, name, rev=None, force=False, ext=None, url=None, apidocs
       raise ValueError('Cannot determine classifier. No explicit classifier is set and this jar '
                        'has more than 1 artifact: {}\n\t{}'.format(self, '\n\t'.join(map(str, self.artifacts))))
 
+
   def append_artifact(self, name, type_=None, ext=None, conf=None, url=None, classifier=None):
     """Append a new IvyArtifact to the list of artifacts for this jar."""
-    self.artifacts += (IvyArtifact(name, type_=type_, ext=ext, conf=conf, url=url, classifier=classifier), )
+    @deprecated('0.0.48',
+                hint_message='JarDependency now only specifies a single artifact, {} defines more than one.'.format(name))
+    def add_more_artifacts():
+      return self.artifacts + (IvyArtifact(name, type_=type_, ext=ext, conf=conf, url=url, classifier=classifier), )
+    if self.artifacts:
+      self.artifacts = add_more_artifacts()
+    else:
+      self.artifacts = (IvyArtifact(name, type_=type_, ext=ext, conf=conf, url=url, classifier=classifier), )
 
   @manual.builddict()
   def exclude(self, org, name=None):
@@ -146,60 +166,6 @@ def exclude(self, org, name=None):
     self.excludes += (Exclude(org, name),)
     return self
 
-  @manual.builddict()
-  def with_sources(self):
-    """This historically requested the artifact have its source jar fetched.
-    (This implies there *is* a source jar to fetch.) Used in contexts
-    that can use source jars (as of 2014, just eclipse and idea goals)."""
-    print("jar dependency org={org} name={name}:  with_sources() is now a noop and is deprecated."
-          .format(org=self.org, name=self.name), file=sys.stderr)
-    return self
-
-  @manual.builddict()
-  def with_docs(self):
-    """This historically requested the artifact have its javadoc jar fetched.
-    (This implies there *is* a javadoc jar to fetch.) Used in contexts
-    that can use source jars (as of 2014, just eclipse and idea goals)."""
-    print("jar dependency org={org} name={name}:  with_docs() is now a noop and is deprecated."
-          .format(org=self.org, name=self.name), file=sys.stderr)
-    return self
-
-  @manual.builddict()
-  def with_artifact(self, name=None, type_=None, ext=None, url=None, configuration=None,
-                    classifier=None):
-    """
-    Sets an alternative artifact to fetch or adds additional artifacts if called multiple times.
-
-    :param name: The name of the published artifact. This name must not include revision.
-    :param type_: The type of the published artifact. It's usually the same as the artifact's file
-      extension, but not necessarily. For instance, ivy files are of type 'ivy' but have 'xml' as
-      their file extension.
-    :param ext: The file extension of the published artifact.
-    :param url: The url at which this artifact can be found if it isn't located at the standard
-      location in the repository.
-    :param configuration: The public configuration in which this artifact is published. The '*' wildcard can
-      be used to designate all public configurations.
-    :param classifier: The maven classifier of this artifact.
-    """
-    print(dedent("""
-    jar(...).with_artifact(...) is deprecated. Instead, use:
-    jar(..., artifacts=[ivy_artifact(...), ...]).  You'll need to supply a name= argument
-    to ivy_artifact.""").strip(),
-          file=sys.stderr)
-    return self._with_artifact(name=name, type_=type_, ext=ext, url=url, configuration=configuration,
-                               classifier=classifier)
-
-  def _with_artifact(self, name=None, type_=None, ext=None, url=None, configuration=None,
-                    classifier=None):
-    artifact = IvyArtifact(name or self.name,
-                           type_=type_,
-                           ext=ext,
-                           url=url,
-                           conf=configuration,
-                           classifier=classifier)
-    self.artifacts += (artifact,)
-    return self
-
   def __eq__(self, other):
     return self._coordinates == other._coordinates
 

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

@@ -29,13 +29,6 @@ def alias_groups(self):
 
 class IvyUtilsGenerateIvyTest(IvyUtilsTestBase):
 
-  # TODO(John Sirois): increase coverage.
-  # Some examples:
-  # + multiple confs - via with_sources and with_docs for example
-  # + excludes
-  # + classifiers
-  # + with_artifact
-
   def setUp(self):
     super(IvyUtilsGenerateIvyTest, self).setUp()
 

tests/python/pants_test/base/test_payload_field.py

@@ -38,96 +38,6 @@ def test_jars_field_order(self):
       JarsField([jar2, jar1]).fingerprint(),
     )
 
-  def test_jars_field_artifacts(self):
-    jar1 = JarDependency('com', 'foo', '1.0.0').with_artifact('com', 'baz')
-    jar2 = JarDependency('com', 'foo', '1.0.0')
-
-    self.assertNotEqual(
-      JarsField([jar1]).fingerprint(),
-      JarsField([jar2]).fingerprint(),
-    )
-
-  def test_jars_field_artifacts_arg(self):
-    jar1 = JarDependency('com', 'foo', '1.0.0', artifacts=[IvyArtifact('com', 'baz')])
-    jar2 = JarDependency('com', 'foo', '1.0.0')
-
-    self.assertNotEqual(
-      JarsField([jar1]).fingerprint(),
-      JarsField([jar2]).fingerprint(),
-    )
-
-  def test_jars_field_artifacts_arg_vs_method(self):
-    jar1 = JarDependency('com', 'foo', '1.0.0', artifacts=[IvyArtifact('com', 'baz')])
-    jar2 = JarDependency('com', 'foo', '1.0.0').with_artifact('com', 'baz')
-
-    self.assertEqual(
-      JarsField([jar1]).fingerprint(),
-      JarsField([jar2]).fingerprint(),
-    )
-
-  def test_jars_field_artifacts(self):
-    jar1 = (JarDependency('com', 'foo', '1.0.0')
-              .with_artifact('com', 'baz')
-              .with_artifact('org', 'bat'))
-    jar2 = (JarDependency('com', 'foo', '1.0.0')
-              .with_artifact('org', 'bat')
-              .with_artifact('com', 'baz'))
-    jar3 = (JarDependency('com', 'foo', '1.0.0')
-              .with_artifact('org', 'bat'))
-
-    jar4 = JarDependency('com', 'foo', '1.0.0')
-
-    self.assertEqual(
-      JarsField([jar1]).fingerprint(),
-      JarsField([jar2]).fingerprint(),
-    )
-    self.assertNotEqual(
-      JarsField([jar1]).fingerprint(),
-      JarsField([jar3]).fingerprint(),
-    )
-    self.assertNotEqual(
-      JarsField([jar1]).fingerprint(),
-      JarsField([jar4]).fingerprint(),
-    )
-    self.assertNotEqual(
-      JarsField([jar3]).fingerprint(),
-      JarsField([jar4]).fingerprint(),
-    )
-
-  def test_jars_field_artifacts_ordering(self):
-    """JarDependencies throw away ordering information about their artifacts in the cache key.
-
-    But they do not throw it away in their internal representation!  In the future, this should be
-    fixed: either they should sort them as they are added and keep a canonical representation, or
-    the order information should be preserved.
-    """
-
-    jar1 = (JarDependency('com', 'foo', '1.0.0')
-              .with_artifact('com', 'baz')
-              .with_artifact('org', 'bat'))
-    jar2 = (JarDependency('com', 'foo', '1.0.0')
-              .with_artifact('org', 'bat')
-              .with_artifact('com', 'baz'))
-
-    self.assertEqual(
-      JarsField([jar1]).fingerprint(),
-      JarsField([jar2]).fingerprint(),
-    )
-
-  def test_deprecated_jars_field_methods(self):
-    """with_sources() and with_docs() are now no-ops.  This test shows they don't affect
-    fingerprinting.
-    """
-    jar1 = (JarDependency('com', 'foo', '1.0.0'))
-    jar2 = (JarDependency('com', 'foo', '1.0.0')
-            .with_sources()
-            .with_docs())
-
-    self.assertEqual(
-      JarsField([jar1]).fingerprint(),
-      JarsField([jar2]).fingerprint(),
-      )
-
   def test_jars_field_apidocs(self):
     """apidocs are not properly rolled into the cache key right now.  Is this intentional?"""