https://pantsbuild.org/ logo
Join Slack
Powered by
Curious to understand the choice around relying on...
# general
l
Curious to understand the choice around relying on pyenv for hermetic python vs sourcing it from python-build-standalone? Some team members hit various issues with missing deps like a cc toolchain/make when we flipped this on, and it seems like relying on the prebuilt versions from p-b-s would be simpler
https://www.pantsbuild.org/stable/docs/getting-started/installing-pants it seems like scie-pants already will fetch a python-build-standalone python, so perhaps relatedly is there a way for the main pants build to use this python exclusively?
d
python-build-standalone as a python provider was recently merged https://github.com/pantsbuild/pants/pull/21422
🔥 1
l
Sweet looks like 
2.24.0.dev1
will have this - excited to try it out 🙏
Hrm, just grabbed this version and hit an interesting failure. I'm using macos and looks like these: https://github.com/pantsbuild/pants/pull/21422/files#diff-d10ffa36cbcf52dfde6de9c5671dce5b97def766cd1deb4472d05cc183d1be44R212-R213 are incompatible w/ 
cp
on mac. I'm happy to make a contribution to fix this, but first want to understand if there's a preference to rely on system binaries vs something like 
shutil.copytree
(Since there may be many potential cases to handle with different choices of flags / introducing platform-specific code paths is always a pain)
(cc @curved-manchester-66006 @bitter-ability-32190 if y'all also have thoughts)
h
Huh, yeah, the tests for those should have been annotated with 
@pytest.mark.platform_specific_behavior
so they’d be run on macos
Which would have caught this
At first glance I think this could have been done in pure python via 
copytree
or similar. @curved-manchester-66006, any special reason why it is via spawned cp process? Especially since we have to jump through a hoop to make sure the process isn’t cached.
c
I think that line was from before the "resurrection". I could guess at some plausible optimizations around --no-clobber or reflinks but I have no measurements for that. Regardless, it would be much more useful it if ran on macos!
l
Cool! Something like this should do the trick, though I'm not sure if there are any particulars to be aware of re: caching etc (though this doesn't seem like it's changing any behavior since we're ensuring the process isn't cached anyways) https://github.com/pantsbuild/pants/pull/21551
@curved-manchester-66006 re: https://github.com/pantsbuild/pants/pull/21551#discussion_r1807027623, shouldn't the behavior already be sync?
I added a generic sync-to-async util that wraps submitting any sync function to run in the current event loop's executor, and used the wrapped 
shutil.copytree
here: https://github.com/pantsbuild/pants/pull/21551/commits/4d4f432bd05c2774a09e322bc1d160b386c56d12 - hopefully that addresses your feedback!
@happy-kitchen-89482 or someone else - can a maintainer approve the CI run?
c
or someone else - can a maintainer approve the CI run?
I think (from the UI) CI is running; but let me know if you think a step is missing.
l
No that happened, it looked like someone needed to approve it though before it did so
h
Yeah, I ran it
l
@happy-kitchen-89482 thanks for the callout re: the event loop, I just unwound that commit. Mind retriggering CI when you get a sec?
✅ 1
@curved-manchester-66006 saw your comment on the PR, I'd previously had an issue getting my local dev env setup on an arm mac but just realized I had an x86_64 py3.9 installed vs the universal distro, will thankfully be able to iterate locally now to resolve this
Hm, it seems like there's an issue w/ naively using 
shutil.copytree
- it looks like the sandbox isn't accessible from the python side (is it possible due to the chroot)?
Copy code
FileNotFoundError: [Errno 2] No such file or directory: '/private/tmp/pants-sandbox-vdbcPU'
(this error is from the python side) Perhaps the best option here is to implement the intrinsic for copytree. what do you both think?
h
Oh, well, right… So, you have to capture the files into the process’s output Digest and materialize them somewhere, using the existing intrinsic.
I blanked on the fact that there’s a sandbox involved
So you already have 
downloaded_python.digest
as the relevant digest, you just need to materialize it
But I’m not actually sure how to do that. To write into the Workspace you need a Workspace instance, which you can only have in a @goal_rule, to ensure that such side-effects are applied correctly. But this isn’t a goal rule.
Hmm, so that 
cp
process was actually (whether knowingly or not) a way to bypass this issue and generate a side-effect in the workspace (I think - the copy target is 
BUILD_ROOT/.python_build_standalone
right?)
OH, I see, it’s not copying to the workspace, it’s copying to 
.python_build_standalone
in the 
cp
process’s sandbox, and that is mapped to an append_only_cache (which lives outside the workspace), so this is not a side effect.
👀 1
OK, this makes more sense now.
And yeah, that process is the only robust way to do this. So alas, I think we will need to revert to using a 
cp
process, and set its arguments appropriately for each platform. Can that be done?
l
Yeah, that could be done - is there a common util for detecting the platform within the pants codebase, or is something like the following reasonable:
Copy code
>>> system, _, _, _, _, machine = platform.uname()
>>> system
'Darwin'
>>> machine
'arm'
>>>
h
There is a standard way: you request a 
pants.engine.platform.Platform
as an argument to your @rule, and the engine will fill it in with the correct value
✅ 1
l
Ok great, thanks! Hoping to get this fixed/fully tested by tomorrow
11 Views
Open in Slack
PreviousNext
Powered by