Skipping a doc phase should result in transitive deps being skipped a…

…s well.

When `--doc-javadoc-skip` or `--doc-scaladoc-skip` are passed to a transitive publish, products generated by those goals will not be available, and should not be depended on in the Ivy javadoc configuration that JarPublish generates. Otherwise, Ivy will fail to resolve the transitive `javadoc` configuration during publishing.

* Expand existing targets to test transitive publishing of java/scala
* Simplify to-string for jar_coordinate of pushdb entries
* Don`t request docs unless they are available for all transitive targets

Testing Done:
new test, and manual publishing with --doc-scaladoc-skip and --doc-javadoc-skip

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

examples/src/scala/com/pants/example/BUILD

@@ -2,7 +2,9 @@
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
 scala_library(name = 'jvm-run-example-lib',
-  dependencies = [],
+  dependencies = [
+    'examples/src/scala/com/pants/example/hello/welcome',
+  ],
   sources = ['JvmRunExample.scala'],
   provides = artifact(org='com.pants.example',
                       name='jvm-example-lib',

examples/src/scala/com/pants/example/hello/welcome/BUILD

@@ -11,4 +11,7 @@ scala_library(name='welcome',
   resources = [
     'examples/src/resources/com/pants/example/hello',
   ],
+  provides = artifact(org='com.pants.example.hello',
+                      name='welcome',
+                      repo='build-support/ivy:public',),
 )

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

@@ -158,7 +158,7 @@ def write(self, target, path, confs=None):
     internal_codegen = {}
     configurations = set(confs or [])
     for dep in target_internal_dependencies(target):
-      jar = _as_versioned_jar(dep)
+      jar = self._as_versioned_jar(dep)
       dependencies[(jar.org, jar.name)] = self.internaldep(jar, dep)
       if dep.is_codegen:
         internal_codegen[jar.name] = jar.name
@@ -268,6 +268,10 @@ def jar_coordinate(jar, rev=None):
   return coordinate(jar.org, jar.name, rev or jar.rev)
 
 
+def pushdb_coordinate(jar, entry):
+  return jar_coordinate(jar, rev=entry.version().version())
+
+
 def target_internal_dependencies(target):
   return filter(lambda tgt: isinstance(tgt, Jarable), target.dependencies)
 
@@ -570,7 +574,10 @@ def stage_artifacts(tgt, jar, version, changelog):
 
       confs = set(repo['confs'])
       confs.add(IvyWriter.SOURCES_CONFIG)
-      if doc_jar:
+      # don't request docs unless they are available for all transitive targets
+      # TODO: doc products should be checked by an independent jar'ing task, and
+      # conditionally enabled; see https://github.com/pantsbuild/pants/issues/568
+      if doc_jar and self._java_doc(target) and self._scala_doc(target):
         confs.add(IvyWriter.JAVADOC_CONFIG)
       return stage_artifact(tgt, jar, version, changelog, confs)
 
@@ -615,12 +622,12 @@ def stage_artifacts(tgt, jar, version, changelog):
       no_changes = newentry.fingerprint == oldentry.fingerprint
 
       if no_changes:
-        changelog = 'No changes for %s - forced push.\n' % jar_coordinate(jar, oldentry.version())
+        changelog = 'No changes for {0} - forced push.\n'.format(pushdb_coordinate(jar, oldentry))
       else:
         changelog = self.changelog(target, oldentry.sha) or 'Direct dependencies changed.\n'
 
       if no_changes and not self.force:
-        print('No changes for %s' % jar_coordinate(jar, oldentry.version()))
+        print('No changes for {0}'.format(pushdb_coordinate(jar, oldentry)))
         stage_artifacts(target, jar, (newentry.version() if self.force else oldentry.version()).version(), changelog)
       elif skip:
         print('Skipping %s to resume at %s' % (
@@ -649,7 +656,7 @@ def stage_artifacts(tgt, jar, version, changelog):
         ivyxml = stage_artifacts(target, jar, newentry.version().version(), changelog)
 
         if self.dryrun:
-          print('Skipping publish of %s in test mode.' % jar_coordinate(jar, newentry.version()))
+          print('Skipping publish of {0} in test mode.'.format(pushdb_coordinate(jar, newentry)))
         else:
           resolver = repo['resolver']
           path = repo.get('path')
@@ -671,7 +678,7 @@ def publish(ivyxml_path):
             try:
               ivy = Bootstrapper.default_ivy()
             except Bootstrapper.Error as e:
-              raise TaskError('Failed to push %s! %s' % (jar_coordinate(jar, newentry.version()), e))
+              raise TaskError('Failed to push {0}! {1}'.format(pushdb_coordinate(jar, newentry), e))
 
             ivysettings = self.generate_ivysettings(ivy, published, publish_local=path)
             args = [
@@ -695,7 +702,7 @@ def publish(ivyxml_path):
               ivy.execute(jvm_options=jvm_args, args=args,
                           workunit_factory=self.context.new_workunit, workunit_name='jar-publish')
             except Ivy.Error as e:
-              raise TaskError('Failed to push %s! %s' % (jar_coordinate(jar, newentry.version()), e))
+              raise TaskError('Failed to push {0}! {1}'.format(pushdb_coordinate(jar, newentry), e))
 
           publish(ivyxml)
 
@@ -866,9 +873,16 @@ def abs_and_relative_sources(target):
 
     return jar_path
 
+  def _java_doc(self, target):
+    return self.context.products.get('javadoc').get(target)
+
+  def _scala_doc(self, target):
+    return self.context.products.get('scaladoc').get(target)
+
   def create_doc_jar(self, target, open_jar, version):
-    javadoc = self.context.products.get('javadoc').get(target)
-    scaladoc = self.context.products.get('scaladoc').get(target)
+    """Returns a doc jar if either scala or java docs are available for the given target."""
+    javadoc = self._java_doc(target)
+    scaladoc = self._scala_doc(target)
     if javadoc or scaladoc:
       jar_path = self.artifact_path(open_jar, version, suffix='-javadoc')
       with self.open_jar(jar_path, overwrite=True, compressed=True) as open_jar:

tests/python/pants_test/tasks/test_jar_publish_integration.py

@@ -12,45 +12,56 @@
 from pants_test.pants_run_integration_test import PantsRunIntegrationTest
 from pants_test.tasks.test_base import is_exe
 
+def shared_artifacts(version):
+  return {'com/pants/examples/hello-greet/{0}/'.format(version):
+          ['ivy-{0}.xml'.format(version),
+            'hello-greet-{0}.jar'.format(version),
+            'hello-greet-{0}.pom'.format(version),
+            'hello-greet-{0}-sources.jar'.format(version)]}
+
 class JarPublishIntegrationTest(PantsRunIntegrationTest):
   SCALADOC = is_exe('scaladoc')
   JAVADOC = is_exe('javadoc')
 
+
   @pytest.mark.skipif('not JarPublishIntegrationTest.SCALADOC',
                       reason='No scaladoc binary on the PATH.')
   def test_scala_publish(self):
+    unique_artifacts = {'com/pants/example/jvm-example-lib/0.0.1-SNAPSHOT':
+                        ['ivy-0.0.1-SNAPSHOT.xml',
+                         'jvm-example-lib-0.0.1-SNAPSHOT.jar',
+                         'jvm-example-lib-0.0.1-SNAPSHOT.pom',
+                         'jvm-example-lib-0.0.1-SNAPSHOT-sources.jar'],
+                        'com/pants/example/hello/welcome/0.0.1-SNAPSHOT':
+                        ['ivy-0.0.1-SNAPSHOT.xml',
+                         'welcome-0.0.1-SNAPSHOT.jar',
+                         'welcome-0.0.1-SNAPSHOT.pom',
+                         'welcome-0.0.1-SNAPSHOT-sources.jar'],}
     self.publish_test('examples/src/scala/com/pants/example:jvm-run-example-lib',
-                      'com/pants/example/jvm-example-lib/0.0.1-SNAPSHOT',
-                      ['ivy-0.0.1-SNAPSHOT.xml',
-                       'jvm-example-lib-0.0.1-SNAPSHOT.jar',
-                       'jvm-example-lib-0.0.1-SNAPSHOT.pom',
-                       'jvm-example-lib-0.0.1-SNAPSHOT-sources.jar'],
-                      extra_options=['--doc-scaladoc-skip'])
+                      dict(unique_artifacts.items() + shared_artifacts('0.0.1-SNAPSHOT').items()),
+                      extra_options=['--doc-scaladoc-skip'],
+                      expected_primary_artifact_count=3)
 
   @pytest.mark.skipif('not JarPublishIntegrationTest.JAVADOC',
                       reason='No javadoc binary on the PATH.')
   def test_java_publish(self):
     self.publish_test('examples/src/java/com/pants/examples/hello/greet',
-                      'com/pants/examples/hello-greet/0.0.1-SNAPSHOT/',
-                      ['ivy-0.0.1-SNAPSHOT.xml',
-                       'hello-greet-0.0.1-SNAPSHOT.jar',
-                       'hello-greet-0.0.1-SNAPSHOT.pom',
-                       'hello-greet-0.0.1-SNAPSHOT-javadoc.jar',
-                       'hello-greet-0.0.1-SNAPSHOT-sources.jar'])
+                      shared_artifacts('0.0.1-SNAPSHOT'),)
 
   def test_named_snapshot(self):
     name = "abcdef0123456789"
     self.publish_test('examples/src/java/com/pants/examples/hello/greet',
-                      'com/pants/examples/hello-greet/%s/' % name,
-                      ['ivy-%s.xml' % name,
-                       'hello-greet-%s.jar' % name,
-                       'hello-greet-%s.pom' % name,
-                       'hello-greet-%s-javadoc.jar' % name,
-                       'hello-greet-%s-sources.jar' % name],
+                      shared_artifacts(name),
                       extra_options=['--publish-named-snapshot=%s' % name])
 
-  def publish_test(self, target, package_namespace, artifacts, extra_options=None,
+  def publish_test(self, target, artifacts, extra_options=None,
                    expected_primary_artifact_count=1):
+    """Tests that publishing the given target results in the expected output.
+
+    :param target: Target to test
+    :param artifacts: A map from directories to a list of expected filenames
+    :param extra_options: Extra options to the pants run
+    :param expected_primary_artifact_count: Number of artifacts we expect to be published."""
 
     with temporary_dir() as publish_dir:
       options = ['--publish-local=%s' % publish_dir,
@@ -67,7 +78,8 @@ def publish_test(self, target, package_namespace, artifacts, extra_options=None,
                         "got stdout:\n{2}\n".format(pants_run.returncode,
                                                     pants_run.stderr_data,
                                                     pants_run.stdout_data))
-      for artifact in artifacts:
-        artifact_path = os.path.join(publish_dir, package_namespace, artifact)
-        self.assertTrue(os.path.exists(artifact_path))
+      for directory, artifact_list in artifacts.items():
+        for artifact in artifact_list:
+          artifact_path = os.path.join(publish_dir, directory, artifact)
+          self.assertTrue(os.path.exists(artifact_path))