Fix realpath symlink map

Review Request #2085 — Created April 16, 2015 and discarded

patricklaw
pants
benjyw, jsirois, stuhood, zundel

Strawman fix for the realpath symlink map

I think there is probably a more principled way to go about this, but we've had this patched in internally for a while now and it has kept us unblocked.

  • 1
  • 0
  • 0
  • 0
  • 1
Description From Last Updated
This seems like this is papering over the problem. Why are we ever using realpath anywhere? If we do need ... DT dturner-tw
ZU
  1. Test? We can make a temporary dir, make a second temporary dir with a symlink, then point our buildroot at the path with the symlink.

    1. I'm not even convinced this is the best way to go about this--that's why I'm posting it as a strawman. I'm also very very strapped for time to work on pants for the next month or so, so I can't really take this any further. All I can do is code dump and confirm that it fixed the bug for us internally.

    2. The repro is:

      mkdir ~/realcachedir
      ln -s ~/realcachedir ~/symlinkcachedir
      

      update pants.ini

      [ivy]
      cache_dir: /users/zundle/symlinkcachdir
      

      then try to compile something that requires ivy resolution:

      ./pants compile examples/src/java/org/pantsbuild/example/antlr3
      
  2. never knew this could take a tuple, that's handy.

  3. Maybe we should include both realpath_map[path] and path as keys in the symlink map?

  4. 
      
DT
  1. 
      
  2. This seems like this is papering over the problem.

    1. Why are we ever using realpath anywhere?
    2. If we do need to use realpath, why are we not using it everywhere?
    1. Indeed, that's the issue that was raised in the original discussion about this bug. One thing I can say is this: ivy, for whatever reason, returns canonicalized (real) paths in its report. We'd need a way to reverse those canonicalized paths back to our understanding of where the ivy cache actual lives (in terms of an absolute but not necessarily canonical path) in order to be totally sure. Otherwise, we're stuck with this solution.

      It's fine to raise issues here, but do note I have no intention of upstreaming this. This is a starting point and a holding place for a patch so others can unblock themselves if they wish.

    2. I will create a test for this and see if I can upstream it if you guys are willing to review.

    3. Thanks Eric. I'd be happy to review it. I'm going to close this review as discarded.

  3. 
      
PA
Review request changed

Status: Discarded

Change Summary:

This was a strawman to help demonstrate a working (if kludgy) fix. Eric is taking over the proper fix.

Loading...