Cleanup after custom options bootstrapping in reflect.

Review Request #1468 — Created Dec. 11, 2014 and submitted

jsirois
pants
jsirois/reflect/fix
861
26da40d...
pants-reviews
benjyw, lahosken

The custom bootstrapping to get a symbolic buildroot for
doc generation was affecting global state and leaving a
buildroot of <buildroot> laying around for subsequent code
to pick up and unwittingly try to use.

src/python/pants/backend/core/tasks/reflect.py | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)

Locally this was failing before, passed after:

$ PANTS_DEV=1 ./pants goal test tests/python/pants_test/tasks:reflect tests/python/pants_test/backend/python:test_builder

Also generated docs and confirmed the intended symbolic <buildroot> was being interpolated: http://pantsbuild.github.io/jsirois/goals_reference.html#Common

Installed python 3.2.5 after which the following failed but then passed after diff 3:

$ PATH=/home/jsirois/.pyenv/versions/3.2.5/bin:$PATH pants.dev goal test tests/python/pants_test/tasks:reflect tests/python/pants_test/backend/python:test_builder

CI went green here: https://travis-ci.org/pantsbuild/pants/builds/43773742

JS
JS
LA
  1. Ship It!

  2. 
      
DT
  1. Ship It!

  2. 
      
BE
  1. Thanks for fixing! I can't wait to get rid of this unholy hack.

  2. 
      
IT
  1. Ship It!

  2. 
      
JS
  1. Turns out the fix in https://rbcommons.com/s/twitter/r/1463/diff/# is also needed here, more diff coming...

  2. 
      
JS
JS
JS
JS
JS
  1. The prior functionally equivalent CI went green here https://travis-ci.org/pantsbuild/pants/builds/43768256
    I'll wait for the latest CI though @ https://travis-ci.org/pantsbuild/pants/builds/43773742 to go green before submitting.

  2. 
      
JS
JS
  1. Thanks folks - submitted @ https://github.com/pantsbuild/pants/commit/db511a94eb77af9d441a62ac2889cd6e23dce9eb

  2. 
      
JS
Review request changed

Status: Closed (submitted)

JS
  1. Nice. This did not fix OSX CI because ... unlike linux, which has defaulted to relatime for mounts for a while now, OSX defaults to atime and so...:

    cached _binary_stat
    posix.stat_result(st_mode=33261, st_ino=2726319, st_dev=16777218L, st_nlink=2, st_uid=0, st_gid=0, st_size=58608, st_atime=1418359951, st_mtime=1411335907, st_ctime=1411336406)

    current _binary_stat
    posix.stat_result(st_mode=33261, st_ino=2726319, st_dev=16777218L, st_nlink=2, st_uid=0, st_gid=0, st_size=58608, st_atime=1418359791, st_mtime=1411335907, st_ctime=1411336406)

    I'll fixup the cached == current check to use the binary path instead like it used to and send up an RB to pex to change the PythonInterpreter.__eq__ impl - since the atime setting is system dependent I don't think Wickman intended the current behavior.

    1. pex.PythonInterpreter fix: https://rbcommons.com/s/twitter/r/1477/

    2. temporary pantsbuild/pants test_test_builder workaround fix: https://rbcommons.com/s/twitter/r/1478/

    3. Why does atime interfere? Not seeing the connection.

    4. Oh never mind, seeing the other RB now.

  2. 
      
Loading...