Surface --dereference-symlinks flag to task caching level

Review Request #4338 — Created Oct. 27, 2016 and submitted

wisechengyi
pants
2580, 3961, 4004
pants-reviews
jsirois, mateor, nhoward_tw, stuhood, zundel

Certain tasks such resolve.node requires cache tarball to be constructed via different dereference flag, so this change the surface the option up to task level.

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

  • 0
  • 0
  • 4
  • 0
  • 4
Description From Last Updated
  1. 
      
  2. src/python/pants/cache/cache_setup.py (Diff revision 1)
     
     

    How would you feel about using a name like --dereference-symlinks instead? That way it'd communicate what it does rather than how. Symlink handling will be an issue regardless of whether we're building tars or some other type of artifact.

    Also, perhaps this should be declared next to --compression-level, since it affects the tarball generation.

  3. Add docstring for dereference.

  4. docstring dereference

  5. Do you anticipate the extension changing? Maybe move this to the format string.

  6. 
      
  1. This is great, I have several use cases for it. I would love to see this included in 1.2.0 stable release.

    1. At this point (since this is a new feature) I'd rather not hold the 1.2.0 release for it.

      But 1.2.1 is a good target for the next releaser perhaps?

  2. Seems weird to docstring a test as opposed to a resuable function.

    Maybe convert these docstrings to comments.

    1. converted to comment.

  3. Could you add a test where the referenced file is not present in the results_dir?

    Perhaps where the task dropped the file in the task workdir and the symlink is the only thing in the results_dir.

    Bonus points for another test where the referenced file is outside the buildroot, say in /tmp or something.

    1. Added test cases
      test_cache_no_dereference_no_file
      test_cache_dereference_file_outside_results_dir

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

Status: Closed (submitted)

Change Summary:

2f81262027d14118af7eeb779a2c1bf9c9567520 thanks gents!

Loading...