Nuke venvs in python/clean.sh.

Review Request #327 — Created May 9, 2014 and submitted

benjyw
pants
pants-reviews
jsirois
Also change a var name in that script.


JS
  1. 
      
  2. build-support/python/clean.sh (Diff revision 1)
     
     
    These venvs are supposed to have proper fingerprinting so never need a clean - are you finding evidence that's busted?
    1. I've encountered situations that I don't understand well where nuking those venvs fixes an obscure python requirement error.
      
      In principle shouldn't we never need any of the rest of clean.sh either?
    2. Yes - but this was one corner of the world that was well understood and unbroken.  I'd prefer not to slip back into hand-wave in this corner if we don't need to.
      I won't block but unless you have direct evidence specifically the venv was not updating correctly when requirements files changed, it would be awesome to leave the advancing circle of light in place.
    3. To clarify: I've never seen a problem with the venv not updating correctly when requirements.txt changes. But I have seen the venv get into a bad state for some reason. 
      
      BTW Do we still need to nuke .python in that script? Does anything still use that dir? Also, not sure if we should nuke .pants.d in that script. clean-all does that if needed, and I like the idea of this script just nuking python runtime state, leaving build products intact.
    4. OK - I obviously heavily favor figuring out the "for some reason" over sweeping this under the rug.  But that can be satisfied by refraining from running this script and instead diving in to figure out the real issue or feeling very very guilty when doing so!
      
      .python is not used/created any longer so that clean line can go.
      I agree .pants.d should be left to clean-all.
      
      
  3. 
      
JS
  1. Ship It!
  2. 
      
BE
Review request changed

Status: Closed (submitted)

Loading...