Fish Trophy

stuhood got a fish trophy!

Create and use stable symlinks for the target results dir

Review Request #3553 — Created March 9, 2016 and submitted

stuhood
pants
3029
pants-reviews
gmalmquist, peiyu, wisechengyi, zundel

One of the effects of the unique/immutable results dir change was that the target id for codegen targets contains a hash that changes whenever the codegen target is invalidated. An unstable target id makes it impossible to clean up cache entries on a target-by-target basis (because each new version of the target has a new target id) and makes it necessary to re-load IDE projects whenever codegen inputs change.

  • Changes the vt.results_dir into a stably-named symlink to the unique/unstable vt.current_results_dir (ie, the immutable copy).
  • Uses os.path.realpath in create_canonical_classpath, to stabilize a classpath as it is exported.

https://travis-ci.org/pantsbuild/pants/builds/116689746

  1. ...Alternatively to using os.path.realpath in export, we could have zinc use vt.results_dir directory for builds, but then always publish vt.current_results_dir in runtime_classpath.

  2. 
      
  1. Fantastic! Thanks for doing this.

  2. 
      
  1. 
      
  2. src/python/pants/invalidation/cache_manager.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     

    Is it the case this does not change the current behavior and only creates symlink stable_dir -> current_dir?

  3. Regarding the symlinks, I think it is beneficial use stable_dir instead of realpath(stable_dir) so IntelliJ can always pick up the latest codegen files without a refresh. Sometimes a refresh is still needed, but stable_dir would help minimize.

  1. +1 to resolve to the realpath.

    Can't speak for export-classpath, but even though the way it stands doesn't make a difference for bundle, since dist/-bundle is always recreated, but knowing the links are mutable makes me uncomfortable.

    1. Yea, agreed. I think that having intellij continue to run export-classpath after compiles that it triggers makes the most sense to me, since it should mostly prevent the plugin from missing changes caused by builds triggered outside of intellij (on the cli, for example).

  2. 
      
  1. Ship It!
  2. 
      
Review request changed

Status: Closed (submitted)

Change Summary:

Merged as 4d1796ca7964e7bce89b6f7a8f5d4ca2195e1bd0

  1. FYI: We are seeing errors from zinc after this upload for local development builds

                       compile(core/common/src/main/java:lib) failed: [Errno 1] Operation not permitted: '/Users/zundel/Development/java/.pants.d/compile/zinc/252d64521cf9/core.common.src.main.java.lib/current'
    05:57:16 00:42         [cache] 
                       compile(sharded-jooq/sharded-jooq-annotations/src/main/java:lib) failed: [Errno 1] Operation not permitted: '/Users/zundel/Development/java/.pants.d/compile/zinc/252d64521cf9/sharded-jooq.sharded-jooq-annotations.src.main.java.lib/current'
    

    A ./pants clean-all fixes it. Every dev is having to run this locally.

    No problems in CI where we don't have incremental compile enabled and we bumped the global cache version number.

    1. @Stu. I filed a github issue on this: https://github.com/pantsbuild/pants/issues/3087 in case there is some action you wanted to take to prevent this error. Otherwise you can close the issue.

    2. Thanks Eric: will look.

  2. 
      
Loading...