https://pantsbuild.org/ logo
Powered by
p

polite-garden-50641

03/27/2023, 2:57 PM
looks like the new version on pex (part of 2.16.0 a1 ?) adds spaces to lock files, not sure if this is on purpose or not....
e

enough-analyst-54434

03/27/2023, 3:09 PM
Hrm, no. It just uses stdlib json dump.
I wonder if Pants is doing something weird. It does post-process and add those damn headers.
Let me double-check Pex directly.
Yeah, Pex is not adding those.
Let me go look at Pants possible shenanigans.
I mean, this is also a GH UI. That's certainly worth doubting too.
p

polite-garden-50641

03/27/2023, 3:27 PM
not a GH thing... I ran 
git diff HEAD~1
on my local branch:
e

enough-analyst-54434

03/27/2023, 3:34 PM
@polite-garden-50641 what was your delta to 2.16.0a1? Where you previously using 2.16.0a0 or something else?
p

polite-garden-50641

03/27/2023, 3:35 PM
yes... you can see this PR (sorry for the private link) https://github.com/toolchainlabs/toolchain/pull/17476
e

enough-analyst-54434

03/27/2023, 3:35 PM
Yeah, I was hoping you'd say it publically.
Ok.
I see no changes in the code that adds headers to lockfiles in the last ~year either.
Ballpark idea, but looong since fixed: https://bugs.python.org/issue16333
@polite-garden-50641 what version exactly is the Python you expect is being used to generate this lock file?
Huh, yeah - this has the appearance of Python 2.7 being used. In Python 2.7 the extra space is documented: https://docs.python.org/2/library/json.html#basic-usage
Pex had no changes in interpreter selection in those releases. Pants has had recent activity in Python discovery and usage IIRC. It could be that Pants is picking Python 2.7 to generate the lock file with. Let me check that.
@polite-garden-50641 if you can try 
PANTS_SHA=d800b9240b93395a1e0c651afd9b6fdcc4adfd21
, I'd be grateful. That's the commit before this, which looks relevant to selecting Python 2.7: https://github.com/pantsbuild/pants/pull/18495
👍 1
And if that succeeds and eliminates the trailing ws, then trying once more with 
PANTS_SHA=d3d325777952435186be42443fb28fde6771fae7
will clinch the guess.
It looks like Pex could pass 
separators=(',', ': ')
to force the same output format in JSON dump between 2.7 and 3.5+. I'm happy to add that, but if the investigation turns up Pants picking interpreters differently now to run the Pex CLI with, that seems worthy of scrutiny. There may be other fallout.
p

polite-garden-50641

03/27/2023, 4:39 PM
running this now and will report back shortly.
so this is the diff w/ the 1st SHA (d800b9240b93395a1e0c651afd9b6fdcc4adfd21)
e

enough-analyst-54434

03/27/2023, 4:46 PM
Ok, back to good. I bet the other sha brings back in the trailing ws.
p

polite-garden-50641

03/27/2023, 4:47 PM
after running with the second sha:
so your guess is accutate.
e

enough-analyst-54434

03/27/2023, 4:47 PM
Kill pantsd
Oh, gotcha,
Great - so bug PR is identified. I think it's worth filing an issue and cc'ing Josh.
In this case its just whitespace, but there may be more serious issues this leads to. It's not clear to me at the moment.
Basically a Python not in your ICs is being used which may be a red flag.
Thanks for running the experiments @polite-garden-50641.
p

polite-garden-50641

03/27/2023, 8:25 PM
@bitter-ability-32190
b

bitter-ability-32190

03/27/2023, 8:42 PM
I can't reproduce Can you do me one more debugging step? Mind leaking the sandbox before/after and sending the relevant 
__run.sh
commandline?
e

enough-analyst-54434

03/27/2023, 8:50 PM
@bitter-ability-32190 you have Python 2.7 installed and visible to Pants?
That's a key ingredient.
b

bitter-ability-32190

03/27/2023, 8:53 PM
It shouldn't matter now. 
PexCLI
uses `PexPex`'s 
argv
which comes from 
PexEnvironment
's `bootstrap_python`'s 
path
which in local environments should be 
sys.executable
. So unless someone is running 
pants
in repo with 2.7, which should be impossible...
e

enough-analyst-54434

03/27/2023, 8:58 PM
Ok. You I'll let you and Asher grind out details then.
b

bitter-ability-32190

03/27/2023, 8:58 PM
The sandbox should hold the most useful info.
e

enough-analyst-54434

03/27/2023, 9:05 PM
So, is 
sys.executable
on 
--python-path
?
It must not be. The rest follows if so.
b

bitter-ability-32190

03/27/2023, 9:08 PM
Oh geez. PEX can be a pain sometimes. So the running executable has to be on the python path if provided? In that case comment on the ticket and I can make a PR
e

enough-analyst-54434

03/27/2023, 9:09 PM
Of course. We use the Pex PEX. You run it with a Python you tell it is invalid; so it re-execs itself with a valid Python ASAP.
Pex is certainly not sloppy. I had to clean all that up. It is precise now mod bugs as I can squash them.
b

bitter-ability-32190

03/27/2023, 9:14 PM
I'm not exactly sure how to fix this and not just break more things. Do these variables/flags ever get propagated to built pexes? Or are they just relevant for building/running the PEX CLI?
e

enough-analyst-54434

03/27/2023, 9:20 PM
I can dig, but you have the code you're talking about most fresh. Hopefully I pointed you in the right direction and you can learn what exactly is up by experimenting. That seems time well spent since things landed here without that time.
b

bitter-ability-32190

03/27/2023, 9:24 PM
Not really, this stuff is complicated and unintuitive. Part 1 is likely a product of the python ecosystem. Part 2 is up for debate. Either way this intricacy wasn't caught by reviewers or CI so I don't feel _that_bad
e

enough-analyst-54434

03/27/2023, 9:27 PM
I would if I were you, this is a bit of a pattern. Agreed it is complex, but its not rocket science either.
At any rate, I've put in my ~1hour or more on this today. And I think you're pointed in the right direction.
@polite-garden-50641 https://github.com/pantsbuild/pex/pull/2106 will go out in Pex 2.1.131 tomorrow and that can be used as a work-around. I'll report back here with the 
pants.toml
snippet to use Pex 2.1.131 tomorrow after release.
b

bitter-ability-32190

03/28/2023, 3:33 PM
e

enough-analyst-54434

03/30/2023, 11:46 PM
Ok, Pex 2.1.131 is released and that can be used to work around the trailing white-space issue by upgrading Pants of ~any recent version to Pex 2.1.131 as described here: https://github.com/pantsbuild/pants/pull/18626#issuecomment-1491100039
💯 1
s

sparse-lifeguard-95737

04/03/2023, 8:38 PM
I’m unfortunately seeing what looks like more problems w/ interpreter selection in 2.16.0rc0: https://github.com/pantsbuild/pants/issues/18662 seems like we might have over-corrected and are now using the bootstrap python in places where we shouldn’t be
Powered by