Expose `PATH` in hermetic ITs. #6017

jsirois · 2018-06-25T17:26:58Z

Without PATH there is no way for the ./pants script (or a
pants.pex for that matter), to find a python 2.7 interpreter.

Without `PATH` there is no way for the `./pants` script (or a `pants.pex` for that matter), to find a python 2.7 interpreter.

jsirois · 2018-06-25T17:35:53Z

I have no clue how PantsRunIntegrationTests that are hermetic run green on CI. They fail in droves for me like so prior to this change:

$ ./build-support/bin/ci.sh -x -efkmrcjlpt -y 0/2
...
11:27:06 07:44   [test]
11:27:06 07:44     [test-jvm-prep-command]
11:27:06 07:44       [jvm_prep_command]
11:27:06 07:44     [test-prep-command]
11:27:06 07:44     [test]
11:27:06 07:44     [pytest-prep]
                   Invalidated 423 targets.
11:27:06 07:44     [pytest]
                   Invalidated 49 targets.
11:27:07 07:45       [run]
                     ============== test session starts ===============
                     platform linux2 -- Python 2.7.15, pytest-3.6.0, py-1.5.3, pluggy-0.6.0
                     shard: 0 of 2 (0-based shard numbering)
                     rootdir: /home/jsirois/dev/pantsbuild/pants/.pants.d, inifile: /home/jsirois/dev/pantsbuild/pants/.pants.d/test/pytest-prep/CPython-2.7.15/2ec95ab6681897ed1759a0eae4ff508e76a865fa/pytest.ini
                     plugins: timeout-1.2.1, cov-2.4.0
                     Only executing 178 of 356 total tests in shard 0 of 2
                     collected 356 items
                     
                     contrib/avro/tests/python/pants_test/contrib/avro/tasks/test_avro_gen.py . [  0%]
                                                                [  0%]
                     contrib/avro/tests/python/pants_test/contrib/avro/tasks/test_avro_gen_integration.py F [  1%]
                                                                [  1%]
                     contrib/buildgen/tests/python/pants_test/contrib/buildgen/test_build_file_manipulator.py . [  1%]
                     ...                                        [  3%]
                     contrib/buildrefactor/tests/python/pants_test/contrib/buildrefactor/test_buildozer.py . [  3%]
                     ...                                        [  5%]
                     contrib/buildrefactor/tests/python/pants_test/contrib/buildrefactor/test_buildozer_integration.py . [  6%]
                                                                [  6%]
                     contrib/buildrefactor/tests/python/pants_test/contrib/buildrefactor/test_meta_rename.py . [  6%]
                                                                [  6%]
                     contrib/codeanalysis/tests/python/pants_test/contrib/codeanalysis/tasks/test_index_java_integration.py . [  7%]
                                                                [  7%]
                     contrib/cpp/tests/python/pants_test/contrib/cpp/test_cpp_integration.py . [  7%]
                     ..                                         [  8%]
                     contrib/cpp/tests/python/pants_test/contrib/cpp/test_cpp_toolchain.py . [  9%]
                     .                                          [ 10%]
                     contrib/errorprone/tests/python/pants_test/contrib/errorprone/tasks/test_errorprone_integration.py F [ 10%]
                     FF                                         [ 11%]
                     contrib/errorprone/tests/python/pants_test/contrib/errorprone/tasks/test_errorprone.py . [ 12%]
                                                                [ 12%]
                     contrib/findbugs/tests/python/pants_test/contrib/findbugs/tasks/test_findbugs_integration.py F [ 12%]
                     FFF
...

Or, more succinctly, one of those CI shard failing tests run in isolation:

$ ./pants test contrib/avro/tests/python/pants_test/contrib/avro/tasks: -- -vktest_idl_gen
...
11:32:50 00:03   [test]
11:32:50 00:03     [test-jvm-prep-command]
11:32:50 00:03       [jvm_prep_command]
11:32:50 00:03     [test-prep-command]
11:32:50 00:03     [test]
11:32:50 00:03     [pytest-prep]
11:32:50 00:03     [pytest]
                   Invalidated 2 targets.
                   scrubbed PYTHONPATH=/home/jsirois/dev/pantsbuild/jsirois-pants/src/python: from py.test environment
11:32:50 00:03       [run]
                     ============== test session starts ===============
                     platform linux2 -- Python 2.7.15, pytest-3.6.1, py-1.5.3, pluggy-0.6.0 -- /home/jsirois/dev/pantsbuild/jsirois-pants/build-support/pants_dev_deps.venv/bin/python2.7
                     cachedir: .pants.d/.pytest_cache
                     rootdir: /home/jsirois/dev/pantsbuild/jsirois-pants/.pants.d, inifile: /home/jsirois/dev/pantsbuild/jsirois-pants/.pants.d/test/pytest-prep/CPython-2.7.15/0ffcdde9a5e6e5eaa08fccecfef9abc6494566ee/pytest.ini
                     plugins: cov-2.4.0, timeout-1.2.1
                     collecting ... collected 3 items / 2 deselected
                     
                     contrib/avro/tests/python/pants_test/contrib/avro/tasks/test_avro_gen_integration.py::AvroJavaGenTest::test_idl_gen <- pyprep/sources/f5e9b94439d2c4925abf70cf343bd05ba9d5068c/pants_test/contrib/avro/tasks/test_avro_gen_integration.py FAILED [100%]
                     
                     ==================== FAILURES ====================
                     __________ AvroJavaGenTest.test_idl_gen __________
                     
                     self = <pants_test.contrib.avro.tasks.test_avro_gen_integration.AvroJavaGenTest testMethod=test_idl_gen>
                     
                         def test_idl_gen(self):
                           target_spec = self.avro_test_target('simple')
                           with self.pants_results(['gen', target_spec]) as pants_run:
                     >       self.assert_success(pants_run)
                     
                     .pants.d/pyprep/sources/f5e9b94439d2c4925abf70cf343bd05ba9d5068c/pants_test/contrib/avro/tasks/test_avro_gen_integration.py:63: 
                     _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
                     .pants.d/pyprep/sources/f5e9b94439d2c4925abf70cf343bd05ba9d5068c/pants_test/pants_run_integration_test.py:392: in assert_success
                         self.assert_result(pants_run, self.PANTS_SUCCESS_CODE, expected=True, msg=msg)
                     .pants.d/pyprep/sources/f5e9b94439d2c4925abf70cf343bd05ba9d5068c/pants_test/pants_run_integration_test.py:413: in assert_result
                         assertion(value, pants_run.returncode, error_msg)
                     E   AssertionError: /home/jsirois/dev/pantsbuild/jsirois-pants/pants --no-pantsrc --pants-workdir=/home/jsirois/dev/pantsbuild/jsirois-pants/.pants.d/tmp/tmpdVsLfn.pants.d --kill-nailguns --print-exception-stacktrace=True --pants-config-files=[] --no-cache-read --no-cache-write --cache-bootstrap-read --cache-bootstrap-write --pants-config-files=/home/jsirois/dev/pantsbuild/jsirois-pants/.pants.d/tmp/tmpdVsLfn.pants.d/pants.ini gen contrib/avro/tests/avro/org/pantsbuild/contrib/avro:simple
                     E   returncode: 1
                     E   stdout:
                     E   	
                     E   stderr:
                     E   	which: no python2.7 in ((null))
                      generated xml file: /home/jsirois/dev/pantsbuild/jsirois-pants/.pants.d/test/pytest/c08e0284bafe57b869b79254b8eaa8c682e71126/junitxml/TEST-c08e0284bafe57b869b79254b8eaa8c682e71126.xml 
                     ===== 1 failed, 2 deselected in 0.49 seconds =====
                     
                   contrib/avro/tests/python/pants_test/contrib/avro/tasks:avro_java_gen           .....   SUCCESS
                   contrib/avro/tests/python/pants_test/contrib/avro/tasks:avro_java_gen_integration.....   FAILURE
FAILURE: FAILURE


               Waiting for background workers to finish.
11:32:51 00:04   [complete]
               FAILURE

I assume I'm missing something here, but can't see it.

stuhood · 2018-06-25T19:55:11Z

tests/python/pants_test/pants_run_integration_test.py

+
+        # Used in the wrapper script to locate a python 2.7 binary via `which`; also used in
+        # pants.pex for the same purpose via a pex header shebang of `#!/usr/bin/env python2.7`.
+        'PATH',


This feels like a bit of a can of worms...

Would it be possible to filter this down to only path entries containing python or something? This (in theory, if it were working) should be a good defense against the bad practice of having PATH dependencies that are not fetched with binutils.

Agreed about the can of worms, but also its not clear exactly what the whitelist would need to be to handle all use cases (CI, local machine). For example, in the CI case, imagining #6016 is complete, the test stage shards will start with a clean repo and a download of a prebuilt pants.pex to run tests with. In that case the bare ./pants script run in ITs will potentially do a full bootstrap if travis caches are nuked which would need not only python2.7 but also which, touch, curl tar, git and probably more. I think the right answer will be ensuring ./pants is a pex or else that the ITs can respond to an env var that point to the pants binary to run. This would trim things back down to just needing #!/usr/bin/env python2.7 to work, but that still requires PATH afaict. Perhaps using / whitelisting PEX_PYTHON or PEX_PYTHON_PATH could be used to circumvent a /usr/bin/env probe.

.
.
.
All this said: any ideas about how the heck this has ever worked on CI or any machine for that matter since hermetic was introduced?! I'm stumped.

All this said: any ideas about how the heck this has ever worked on CI or any machine for that matter since hermetic was introduced?! I'm stumped.

Heh. @cosmicexplorer not that you're responsible for this in the slightest, but since you've been working on PATH filtering elsewhere: any ideas?

Looking into it now. Anecdotally, unset PATH; /usr/bin/env python2.7 seems to work for bash on my linux machine though (and I can't invoke anything else that was on the PATH after doing that, so I don't believe some weird shell caching or something is involved).

(I have no clue why this would happen, reading up on env next)

The immediate issue is not env but which fwiw: https://github.com/pantsbuild/pants/blob/master/pants#L50

The only thing I can think of is that which might be implemented as a shell builtin? But upon searching I cannot find a single shell that does that.

...and immediately afterward I find that zsh does that by default so I wouldn't be surprised if some others did too (although bash doesn't on this arch linux machine).

And so, to the heart of the matter, I assume my examples fail for you as well.

Yes -- I really cannot trust my memory, but I believe I have almost definitely never been able to run ./pants test :: locally (and specifically, the command line to run the avro gen test you provided does indeed fail, because we explicitly select #!/usr/bin/env bash).

stuhood · 2018-07-15T04:19:06Z

Resolved (for now) via #6101.

Expose PATH in hermetic ITs.

ea6cfd0

Without `PATH` there is no way for the `./pants` script (or a `pants.pex` for that matter), to find a python 2.7 interpreter.

jsirois requested a review from stuhood June 25, 2018 17:35

kwlzn approved these changes Jun 25, 2018

View reviewed changes

stuhood reviewed Jun 25, 2018

View reviewed changes

stuhood mentioned this pull request Jul 12, 2018

Fix local execution of hermetic integration tests #6101

Merged

stuhood closed this Jul 15, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose `PATH` in hermetic ITs. #6017

Expose `PATH` in hermetic ITs. #6017

jsirois commented Jun 25, 2018

jsirois commented Jun 25, 2018

stuhood Jun 25, 2018

jsirois Jun 25, 2018 •

edited

Loading

stuhood Jun 25, 2018

cosmicexplorer Jun 25, 2018 •

edited

Loading

cosmicexplorer Jun 25, 2018

jsirois Jun 25, 2018

cosmicexplorer Jun 28, 2018

cosmicexplorer Jun 28, 2018

jsirois Jun 28, 2018

cosmicexplorer Jun 28, 2018 •

edited

Loading

stuhood commented Jul 15, 2018

Expose PATH in hermetic ITs. #6017

Expose PATH in hermetic ITs. #6017

Conversation

jsirois commented Jun 25, 2018

jsirois commented Jun 25, 2018

stuhood Jun 25, 2018

Choose a reason for hiding this comment

jsirois Jun 25, 2018 • edited Loading

Choose a reason for hiding this comment

stuhood Jun 25, 2018

Choose a reason for hiding this comment

cosmicexplorer Jun 25, 2018 • edited Loading

Choose a reason for hiding this comment

cosmicexplorer Jun 25, 2018

Choose a reason for hiding this comment

jsirois Jun 25, 2018

Choose a reason for hiding this comment

cosmicexplorer Jun 28, 2018

Choose a reason for hiding this comment

cosmicexplorer Jun 28, 2018

Choose a reason for hiding this comment

jsirois Jun 28, 2018

Choose a reason for hiding this comment

cosmicexplorer Jun 28, 2018 • edited Loading

Choose a reason for hiding this comment

stuhood commented Jul 15, 2018

Expose `PATH` in hermetic ITs. #6017

Expose `PATH` in hermetic ITs. #6017

jsirois Jun 25, 2018 •

edited

Loading

cosmicexplorer Jun 25, 2018 •

edited

Loading

cosmicexplorer Jun 28, 2018 •

edited

Loading