Two changes that affect invalidation and artifact caching.

Review Request #2269 — Created May 26, 2015 and submitted

benjyw
pants
1595
0a02cf5...
pants-reviews
jsirois
1. Don't use the task class name in cache paths. This name may not
   be stable across runs because we generate synthetic subclasses at runtime,
   and we make no guarantees about their names (e.g., in tests we inject a
   uuid into the synthetic subclass's name). Instead we introduce a
   stable name, based on the non-synthetic, authored class's name.
   Note that this will all go away once we no longer have any class-level
   task state at runtime.

2. Don't use the registration key or option scope in the cache keys of
   JVM tools. They aren't needed - those are just how the tool is accessed
   by code, they have no effect on which tool jars are resolved or how
   those are shaded.

Both these changes will enable us to get huge test performance speedups by having
tests share shaded JVM tool jars via the local artifact cache.

Unit tests pass locally.
CI passes: https://travis-ci.org/pantsbuild/pants/builds/64289095.

  • 0
  • 0
  • 1
  • 0
  • 1
Description From Last Updated
BE
  1. John - please double-check my assertion that the key and scope are not needed in the fingerprint.

  2. 
      
JS
  1. 
      
  2. This gives all tasks a stable name of pants.backaned.core.tasks.task.  Thats obviously a problem because its degenerate but there is also a problem when that issue is fixed since __name__ (of the derived class) does not permit 2 tasks defined in the same file.  Whether or not we think that's a good idea, it'll happen and be hard to debug.
    1. Yes, I noticed that when a test failed. I had thought that __name__ in that context would be the class name, but apparently not.
    2. To clarify, the intent is that it be the name of the class, which is what it was in the past, and will be again, once we no longer have to synthesize these nutty subclasses.

  3. ACK - these can go, we just care about (classpath, main, custom_rules) and IvyResolveFingerprintStrategy handles the classpath bit.  If 2 different tasks bootstrap the same classpath + main + custom_rules they should get the same tool.
  4. 
      
BE
JS
  1. 
      
  2. src/python/pants/backend/core/tasks/task.py (Diff revisions 1 - 2)
     
     
    While you're at it - this should really be `cls.__module__ + '.' + cls.__name__` - its an old bug waiting to happen, although its unlikelyish we'll get 2 leaf classes with the same name, it's definitely allowed.  I can imagine writing a JavaCompile plugin utside this repo that uses the new java 9 incremental compilation support say.
    1. You are make execellent point. Fixing. Thanks for the quick review!

  3. 
      
BE
JS
  1. Ship It!
  2. 
      
BE
JS
  1. Ship It!
  2. 
      
BE
BE
Review request changed

Status: Closed (submitted)

Change Summary:

Thanks John! Submitted as ea5dd35cc46de159082d423eaa2aeec7ae8f401c.

Loading...