https://pantsbuild.org/ logo
Join Slack
Powered by
sooo. we have probably not been running pants from...
# development
w
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
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
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
No, that will pull from PyPI. We must pull from source
w
i’m not sure about that. John’s change made that not the case, i think.
e
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
because it will use 
-m pants
h
I mean pants_requirement. That is how it behaves. It pulls from PyPI because it is a macro for a python_requirement_library
w
@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
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
@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
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
yea, that sounds right.
so, that’s a heck of a lot easier than my existing TODO list item 😃
🎊 1
e
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
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
i’ll push a small fix for #10165, and we can delete the pants.pex in a followup.
❤️ 1
e
Yay!
🎊 1
w
Open in Slack
PreviousNext
Powered by