https://pantsbuild.org/ logo
#development
Title
# development
w

witty-crayon-22786

07/07/2020, 1:01 AM
sooo. we have probably not been running pants from the
pants.pex
in integration tests since https://github.com/pantsbuild/pants/pull/8265 due in particular to this line: https://github.com/pantsbuild/pants/blob/6839634f94743aaffd5a4dda818123e4cf943200/src/python/pants/testutil/pants_run_integration_test.py#L253
this relates to #10165 in some way, but i’m not sure how yet.
h

hundreds-father-404

07/07/2020, 1:05 AM
huh
what do you think is the more correct behavior?
This would explain why even when I turn
NO_REGEN_PEX
on, the IT still acts like it’s updated..
w

witty-crayon-22786

07/07/2020, 1:06 AM
yep.
… but… i need to do some more research here. if that’s true, i don’t know why it only broke here.
oh. yea, no: i know why. because the PYTHONPATH only needed to include the root before in order for `-m pants`to find that entrypoint. now the code is nested, but the pythonpath is still correct. and the nested pythonpath now includes that file.
so yes. we have not used the pex.
so, the deal here is: this is a pants_run_integration_test specific issue rather than an issue with the change in general. we can work around it by creating a different filename in the pantsd test.
❤️ 1
but i will also bump a TODO list item i’ve had about making the “`pants.pex` in integration tests” script an actual plugin, because i think that that might be the only way to actually make it work in general with v2. i expect that https://github.com/pantsbuild/pants/pull/8265 has never worked with v2 outside of the pantsbuild/pants repo.
👍 1
…given that, i supposed that we could completely remove
pants.pex
in the meantime too…?
@hundreds-father-404: hm. wait. outside of pantsbuild/pants we’d use
pants_requirement
to load the sources… and you had done that recently?
h

hundreds-father-404

07/07/2020, 1:25 AM
No, that will pull from PyPI. We must pull from source
w

witty-crayon-22786

07/07/2020, 1:26 AM
i’m not sure about that. John’s change made that not the case, i think.
e

enough-analyst-54434

07/07/2020, 1:26 AM
What was the point of running from pants.pex anyhow? We publish 2 things, wheels and a pants.pex. Presumably we care about testing pants running from what we will release, ... but that would require running it from wheels and pex if the concern was that grave.
w

witty-crayon-22786

07/07/2020, 1:26 AM
because it will use
-m pants
h

hundreds-father-404

07/07/2020, 1:26 AM
I mean pants_requirement. That is how it behaves. It pulls from PyPI because it is a macro for a python_requirement_library
w

witty-crayon-22786

07/07/2020, 1:27 AM
@hundreds-father-404: right, but that will end up on the PYTHONPATH of the testrunner, which
-m pants
will be able to load
afaict
@enough-analyst-54434: i honestly don’t remember. i think that we didn’t expect loose sources to be fully loadable from integration tests?
h

hundreds-father-404

07/07/2020, 1:28 AM
I don’t follow. We should be using the sources Pants to set up the test, not the latest wheel releases to PyPI, which is what pants_requirement would end up doing
w

witty-crayon-22786

07/07/2020, 1:28 AM
@hundreds-father-404: right, sorry. i meant outside of pantsbuild/pants
👍 1
inside pantsbuild/pants we’d run from sources. outside, we’d run from the pants_requirement
h

hundreds-father-404

07/07/2020, 1:30 AM
For our repo, I think we could maybe remove pants.pex. I added that setup when porting everything to —chroot because I reasoned it was the best way to declare the dependency via a files() Target. For whatever reason, we didn’t think of running it how it’s run now
w

witty-crayon-22786

07/07/2020, 1:30 AM
yea, that sounds right.
so, that’s a heck of a lot easier than my existing TODO list item 😃
🎊 1
e

enough-analyst-54434

07/07/2020, 1:31 AM
If for whatever reason we want the old behavior, instead of sys.executable, using sys.argv[0] would probably do it. PEX patches that up to point to the PEX file if it's a zip.
h

hundreds-father-404

07/07/2020, 1:31 AM
I vaguely remember issues also with the pants script file name conflicting with the stripped folder pants. That is no longer an issue if we no longer strip source roots
👍 1
w

witty-crayon-22786

07/07/2020, 1:32 AM
i’ll push a small fix for #10165, and we can delete the pants.pex in a followup.
❤️ 1
e

enough-analyst-54434

07/07/2020, 1:32 AM
Yay!
🎊 1
w

witty-crayon-22786

07/07/2020, 2:09 AM