Include ivy options in ivy fingerprints

Options like --soft-excludes are not included in the cache key used by the ivy task mixin. Since the cache key is reused as the key for the resolve, this means for instance, that the bundle task might reuse the result of a resolve with --soft-excludes even though it expected excludes to be applied. It also means that associated subsystem options such as the jar dependency management default target are ignored. And, that the task implementation version is ignored.

This converts IvyResolveFingerprintStrategy to a TaskIdentityFingerprintStrategy subclass rather than a FingerprintStrategy subclass. I think it's possible to reduce this to just the ivy mixin related elements, but that started getting a little complicated, so I backed it out.

Testing Done:
CI away on PR. Ran a bundle / resolve pair that previously reused the same cached resolve now correctly have separate resolves.

Bugs closed: 3234

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

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

@@ -31,9 +31,9 @@
 
 
 class ShadedToolFingerprintStrategy(IvyResolveFingerprintStrategy):
-  def __init__(self, main, custom_rules=None):
+  def __init__(self, task, main, custom_rules=None):
     # The bootstrapper uses no custom confs in its resolves.
-    super(ShadedToolFingerprintStrategy, self).__init__(confs=None)
+    super(ShadedToolFingerprintStrategy, self).__init__(task, confs=None)
 
     self._main = main
     self._custom_rules = custom_rules
@@ -235,7 +235,7 @@ def shader(self):
     return Shader.Factory.create(self.context)
 
   def _bootstrap_shaded_jvm_tool(self, jvm_tool, targets):
-    fingerprint_strategy = ShadedToolFingerprintStrategy(jvm_tool.main,
+    fingerprint_strategy = ShadedToolFingerprintStrategy(self, jvm_tool.main,
                                                          custom_rules=jvm_tool.custom_rules)
 
     with self.invalidated(targets,

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

@@ -7,14 +7,13 @@
 
 import logging
 import os
-from hashlib import sha1
 
 from pants.backend.jvm.ivy_utils import NO_RESOLVE_RUN_RESULT, IvyFetchStep, IvyResolveStep
 from pants.backend.jvm.subsystems.jar_dependency_management import JarDependencyManagement
 from pants.backend.jvm.targets.jar_library import JarLibrary
 from pants.backend.jvm.targets.jvm_target import JvmTarget
 from pants.base.exceptions import TaskError
-from pants.base.fingerprint_strategy import FingerprintStrategy
+from pants.base.fingerprint_strategy import TaskIdentityFingerprintStrategy
 from pants.invalidation.cache_manager import VersionedTargetSet
 from pants.ivy.ivy_subsystem import IvySubsystem
 from pants.task.task import TaskBase
@@ -24,10 +23,11 @@
 logger = logging.getLogger(__name__)
 
 
-class IvyResolveFingerprintStrategy(FingerprintStrategy):
+# TODO(nh): We could fingerprint just the ivy options and not the task options.
+class IvyResolveFingerprintStrategy(TaskIdentityFingerprintStrategy):
 
-  def __init__(self, confs):
-    super(IvyResolveFingerprintStrategy, self).__init__()
+  def __init__(self, task, confs):
+    super(IvyResolveFingerprintStrategy, self).__init__(task)
     self._confs = sorted(confs or [])
 
   def compute_fingerprint(self, target):
@@ -47,7 +47,8 @@ def compute_fingerprint(self, target):
     if not hash_elements_for_target:
       return None
 
-    hasher = sha1()
+    hasher = self._build_hasher(target)
+
     for conf in self._confs:
       hasher.update(conf)
 
@@ -89,7 +90,7 @@ def register_options(cls, register):
     # TODO: Register an --ivy-jvm-options here and use that, instead of the --jvm-options
     # registered by the task we mix into. That task may have intended those options for some
     # other JVM run than the Ivy one.
-    register('--soft-excludes', type=bool, advanced=True,
+    register('--soft-excludes', type=bool, advanced=True, fingerprint=True,
              help='If a target depends on a jar that is excluded by another target '
                   'resolve this jar anyway')
 
@@ -213,7 +214,7 @@ def _ivy_resolve(self,
 
     confs = confs or ('default',)
 
-    fingerprint_strategy = IvyResolveFingerprintStrategy(confs)
+    fingerprint_strategy = IvyResolveFingerprintStrategy(self, confs)
 
     with self.invalidated(targets,
                           invalidate_dependents=invalidate_dependents,

tests/python/pants_test/backend/jvm/tasks/BUILD

@@ -218,10 +218,12 @@ python_tests(
     'src/python/pants/backend/jvm/tasks:ivy_resolve',
     'src/python/pants/backend/jvm/tasks:ivy_task_mixin',
     'src/python/pants/backend/jvm:ivy_utils',
+    'src/python/pants/task',
     'src/python/pants/util:contextutil',
     'tests/python/pants_test/jvm:jvm_tool_task_test_base',
     'tests/python/pants_test/subsystem:subsystem_utils',
     'tests/python/pants_test:base_test',
+    'tests/python/pants_test/tasks:task_test_base',
   ]
 )
 

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

@@ -22,12 +22,12 @@
 from pants.backend.jvm.tasks.ivy_resolve import IvyResolve
 from pants.backend.jvm.tasks.ivy_task_mixin import IvyResolveFingerprintStrategy
 from pants.ivy.bootstrapper import Bootstrapper
+from pants.task.task import Task
 from pants.util.contextutil import temporary_dir, temporary_file_path
 from pants.util.dirutil import safe_delete
-from pants_test.base_test import BaseTest
 from pants_test.jvm.jvm_tool_task_test_base import JvmToolTaskTestBase
 from pants_test.subsystem.subsystem_util import subsystem_instance
-from pants_test.tasks.task_test_base import ensure_cached
+from pants_test.tasks.task_test_base import TaskTestBase, ensure_cached
 
 
 def strip_workdir(dir, classpath):
@@ -462,7 +462,29 @@ def _assertIsFile(self, path):
                     'Expected {} to exist as a file'.format(path))
 
 
-class IvyResolveFingerprintStrategyTest(BaseTest):
+class EmptyTask(Task):
+  @classmethod
+  def register_options(cls, register):
+    register('--a', type=bool, default=False, fingerprint=True)
+
+  @property
+  def fingerprint(self):
+    # NB: The fake options object doesn't contribute to fingerprinting, so this class redefines
+    #     fingerprint.
+    if self.get_options().a:
+      return "a"
+    else:
+      return "b"
+
+  def execute(self):
+    pass
+
+
+class IvyResolveFingerprintStrategyTest(TaskTestBase):
+
+  @classmethod
+  def task_type(cls):
+    return EmptyTask
 
   def setUp(self):
     super(IvyResolveFingerprintStrategyTest, self).setUp()
@@ -479,23 +501,44 @@ def set_artifact_set_for(self, managed_jar_target, artifact_set):
 
   def test_target_target_is_none(self):
     confs = ()
-    strategy = IvyResolveFingerprintStrategy(confs)
+    strategy = IvyResolveFingerprintStrategy(self.create_task(self.context()),
+                                             confs)
 
     target = self.make_target(':just-target')
 
     self.assertIsNone(strategy.compute_fingerprint(target))
 
+  def test_options_included_in_fingerprint(self):
+    confs = ()
+    jar_library = self.make_target(':jar-library', target_type=JarLibrary,
+                                   jars=[JarDependency('org.some', 'name')])
+
+    self.set_options(a=True)
+    strategy = IvyResolveFingerprintStrategy(self.create_task(self.context()),
+                                             confs)
+    with_true_a = strategy.compute_fingerprint(jar_library)
+
+    self.set_options(a=False)
+    strategy = IvyResolveFingerprintStrategy(self.create_task(self.context()),
+                                             confs)
+
+    with_false_a = strategy.compute_fingerprint(jar_library)
+
+    self.assertNotEqual(with_true_a, with_false_a)
+
   def test_jvm_target_without_excludes_is_none(self):
     confs = ()
-    strategy = IvyResolveFingerprintStrategy(confs)
+    strategy = IvyResolveFingerprintStrategy(self.create_task(self.context()),
+                                             confs)
 
     target_without_excludes = self.make_target(':jvm-target', target_type=JvmTarget)
 
     self.assertIsNone(strategy.compute_fingerprint(target_without_excludes))
 
   def test_jvm_target_with_excludes_is_hashed(self):
     confs = ()
-    strategy = IvyResolveFingerprintStrategy(confs)
+    strategy = IvyResolveFingerprintStrategy(self.create_task(self.context()),
+                                             confs)
 
     target_with_excludes = self.make_target(':jvm-target', target_type=JvmTarget,
                                                excludes=[Exclude('org.some')])
@@ -504,7 +547,8 @@ def test_jvm_target_with_excludes_is_hashed(self):
 
   def test_jar_library_with_one_jar_is_hashed(self):
     confs = ()
-    strategy = IvyResolveFingerprintStrategy(confs)
+    strategy = IvyResolveFingerprintStrategy(self.create_task(self.context()),
+                                             confs)
 
     jar_library = self.make_target(':jar-library', target_type=JarLibrary,
                                    jars=[JarDependency('org.some', 'name')])
@@ -513,7 +557,8 @@ def test_jar_library_with_one_jar_is_hashed(self):
 
   def test_identical_jar_libraries_with_same_jar_dep_management_artifacts_match(self):
     confs = ()
-    strategy = IvyResolveFingerprintStrategy(confs)
+    strategy = IvyResolveFingerprintStrategy(self.create_task(self.context()),
+                                             confs)
 
     managed_jar_deps = self.make_target(':managed', target_type=ManagedJarDependencies,
                                artifacts=[JarDependency('org.some', 'name')])
@@ -533,7 +578,8 @@ def test_identical_jar_libraries_with_same_jar_dep_management_artifacts_match(se
 
   def test_identical_jar_libraries_with_differing_managed_deps_differ(self):
     confs = ()
-    strategy = IvyResolveFingerprintStrategy(confs)
+    strategy = IvyResolveFingerprintStrategy(self.create_task(self.context()),
+                                             confs)
 
     managed_jar_deps = self.make_target(':managed', target_type=ManagedJarDependencies,
                                artifacts=[JarDependency('org.some', 'name')])