https://pantsbuild.org/ logo
Join Slack
Powered by
Hey friends, In pants 2.18 and 2.19, I used a scr...
# general
r
Hey friends, In pants 2.18 and 2.19, I used a script to manipulate my 
pants roots
like this:
Copy code
import subprocess

roots_output = subprocess.check_output(["./pants", "roots"], text=True)
print(roots_output)
And I used to run it using 
pants run name_of_script.py
The issue is that, after I updated my repo to pants 2.20, the 
pants roots
command inside my script is returning:
Copy code
21:57:15.40 [ERROR] 1 Exception encountered:

Engine traceback:
  in `run` goal

NoSourceRootError: No source root found for `.`. See <https://www.pantsbuild.org/2.20/docs/using-pants/key-concepts/source-roots> for how to define source roots.
This only happens if I'm calling my script using 
pants run
. I didn't want to depend on the system's python path as I am not always sure if python is called 
python
or 
python3
or something else. Is this an issue someone else reported or is this intended? Any ideas how to fix this or any workarounds?
f
Why not set a source root as the error message suggests?
Namely the top of your repo might work in this case.
r
I have a source root. If I run 
pants roots
, it returns tons of folders that we're using as sources. The issue is that, for some reason, when I running 
pants roots
inside a script that's being called by 
pants run
, it can't find the root sources. This started happening in pants 2.20.
I'll try to repro it in a separated repo and I'll bring it to the discussion. I think that might be better.
Here's the repo: https://github.com/mikaelsouza/pants-root-issue To reproduce the issue, just run 
PANTS_CONCURRENT=True pants run scripts/generate-env-file.py
using pants 2.18 and then using pants 2.20/2.21. Here's what happens:
f
Pants runs scripts in the execution sandbox. The “interior” call to Pants is not looking at the repository.
(Because its current directory is the transient sandbox directory.)
r
But then this changed recently because it used to work. I can find a workaround for this issue, I just want to understand what happened.
As you can see in the print screen, I ran the same command using both pants 2.18 and 2.21 and it worked on 2.18.
f
Anything relevant in the release notes?
r
I didn't find anything relevant tbh, but I'll check it again.
This change broke a script that automatically created the 
.env
file for vscode users. This relied on the 
pants roots
command. It's not a big deal tbh. I can just write it in bash and it'll probably work fine. I am more interested in knowing if this is a regression or a intentional change.
It seems this stopped working on "2.20.0a0". In Pants "2.20.0.dev7" it was still working .
👍 1
f
git log release_2.20.0.dev7..release_2.20.0a0 -- src/python/pants/backend/python
Untitled
would you be able to git bisect the change? (using 
PANTS_SOURCE
env var to 
pants
invocation to run from Pants sources.)
seems to be some pex/python changes in the commit range you identified
r
Hey, sorry for ignoring your messages. I didn't have much time to check these last days. I'll take a look if I can find what happened in this range later today. Thanks for the support! :DDD
f
No worries. Thanks!
r
So, after doing bisect (first time doing it, didn´t know that existed. pretty good feature), it looks like "the issue" was introduced with this commit:
Copy code
commit 882ffe64f69089b33a6541ba000bbc4638c48d9d (HEAD)
Author: Jacob Floyd <cognifloyd@gmail.com>
Date:   Fri Feb 9 10:20:41 2024 -0600

    Add support for the pex `--executable` argument (#20497)
    
    This PR adds support for `pex --executable <path to script>` added in
    pex [2.1.93](<https://github.com/pantsbuild/pex/releases/tag/v2.1.93>):
    <https://github.com/pantsbuild/pex/pull/1807>
    
    With this change, users should be able to do `pants run
    bin/my-python-script.py`.
    
    Closes #18290
    
    ---------
    
    Co-authored-by: Benjy Weinberger <benjyw@gmail.com>
So, in this commit, there's a change to 
create_python_source_run_request
located in 
src/python/pants/backend/python/goals/
. The PR adds the 
executable=field_set._executable_main()
field to the 
_create_python_source_run_request
call. Removing this brings us back to the previous behavior, but I am not sure what this is doing and if should be changed. I already fixed this issue on my end by running my scripts as shell scripts instead of using pants. If this is a regression, I think this is the place where we should check, but if it's nothing too important to the team, I think we can ignore it 🤷 .
f
cc @proud-dentist-22844 - any insights?
p
That's really weird. Looking at my changes there...
How did your script work before my change? I'm surprised PEX didn't complain that it couldn't import your 
generate-env-file.py
because 
-
is not a valid character for a python identifier. I'm confused in part because I added the executable feature specifically to handle scripts with 
-
. I tried renaming your script to use 
_
instead of 
-
which works by bypassing the new 
pex --executable
feature. Inspecting the sandbox now...
Oddly enough, you can run pants inside the sandbox because of env vars (shown in __run.sh) that tell 
pants
to use a different path: 
PANTS_BUILDROOT_OVERRIDE=/home/cognifloyd/g/github/mikaelsouza/pants-root-issue
I'm not sure yet why going via 
executable
instead of 
entry_point
triggers a source root error.
Here's the rule logic that is triggering the source root lookup: https://github.com/pantsbuild/pants/pull/20497/files#diff-01d55b62882286165f80d38bb3123af1a461ddbaac9863cd93a8822f03059e79R692-R699 It should be stripping the source root from the script's file name... If I add 
/
to the list of source patterns, it gets farther, but stil dies before actually executing the script. 😞
I think I found the issue. https://github.com/pantsbuild/pants/pull/20497/files#r1634119540 The 
pants run
integration test I wrote (uhm. Actually, I copy/paste/modified from another test) for this feature tests running a pex_binary, not a python_source. I didn't realize I missed testing that 😕 Sorry.
@rhythmic-butcher-20315 I will submit a bug fix PR soonish. Until that's merged/released, you can workaround the issue by renaming your script to use 
_
instead of 
-
.
Here's my draft PR. I'll let CI run and we'll see what's missing: https://github.com/pantsbuild/pants/pull/21047
@rhythmic-butcher-20315 could you test https://github.com/pantsbuild/pants/pull/21047 ? I think I've ironed out the bugs 
pex --executable
bugs. I've tagged it to be backported to 2.20 (the version the executable feature was introduced in).
@rhythmic-butcher-20315 the fix has been merged on main, and branches for 2.22.x, 2.21.x, and 2.20.x. So, the next release of each of these versions should include the fix.
😄 1
r
@proud-dentist-22844 thank you so much for taking time to check this!
😁 1
p
These are the latest releases that include the fix 🙂 2.20.3rc1 2.21.1rc1 2.22.0rc0 (latest 2.22.0rc2) 2.23.0.dev2 (latest 2.23.0.dev4)
3 Views
Open in Slack
PreviousNext
Powered by