https://pantsbuild.org/ logo
Join Slack
Powered by
I opened an <RFC in the form of a PR>. It is to al...
# development
b
I opened an RFC in the form of a PR. It is to allow 
./pants run ...
on a 
python_source
, freeing users from requiring the declaration of a 
pex_binary
for scripts, and frees us to implement 
pex_binary
running as 
package + run
.
c
Interesting. I think I’m in favour of such a change.
b
(Wanna comment on PR 😛 ) Also as an aside: I have roughly 200 binaries I need to migrate totaling several weeks worth of work in manual validation. I'm adding this as an in-repo plugin. Poof there goes the metadata and validation of almost all 200 😂
🙌 1
h
I thinkkkk I'm in favor of this. We do need to keep 
run
working for 
pex_binary
because you might have something like 
isort.pex
using 
script='isort'
. So this would be allowing you to use either approach -- My gut reaction is "But what if a user tries running something that isn't executable!!!" It's important that 
./pants $goal ::
knows which targets are valid and which are not But I think my fear is overblown because...we only allow you to run on one target at a time. And that will always be the case, given 
InteractiveProcess
claims the foreground We could allow you to do 
./pants run ::
and then we run each process sequentially, which is how 
test --debug
works...but imo that would be a UX regression
my other concern is "Are we confident none of those Pex args matter with run?" But if we keep 
pex_binary
still working, then that's not much of a concern. Usually just use `python_source`; use 
pex_binary
if you gotta do something special
Keeping 
pex_binary
around also means this has zero breaking change for users, which is great
Hm I really like the semantic clarity of this. I don't think any of the scripts in 
build-support
should actually be buildable with 
package
! That is not at all our intent to distribute them!
➕ 1
b
My gut reaction is "But what if a user tries running something that isn't executable!!!"
Have no fear! This is only for 
python_source
. IN which case, every Python file is executable as per the language 😉
my other concern is "Are we confident none of those Pex args matter with run?"
If anything, this would fix the current gap we have today. Right now 
./pants run my_pex_binary
doesn't run the Pex! So those args provided to pex aren't considered. In a future world, it would run the PEX and therefore consider the args.
Keeping 
pex_binary
around also means this has zero breaking change for users, which is great
I had to update PR, there is one breaking change 😅 : "Current users can 
./pants run path/to/file.py
today if that file is the 
entry_point
of a 
pex_binary
. With a change like this, it becomes ambiguous."
😢 1
I think after a deprecation cycle, we might be able to remove the 
SecondaryOwnerMixin
of the 
entry_point
field in 
pex_binary
, so that 
./pants run file.py
is unambiguously us running the file.
h
IN which case, every Python file is executable as per the language 😉
Sure, but you probably didn't mean to 
run
strutil.py
. If 
./pants run ::
worked, then this would be a big concern for me. Fortunately, I think it's unlikely we'd want to ever let that work
c
+1 for the general direction of this thread
✅ 3
b
Well I think I got the feedback, then. Expect a googledoc... at some point
👍 1
❤️ 1
w
only potential backwards compatibility hazard is that we currently have a facility in place for 
./pants run $file
to locate an owning 
pex_binary
… presumably this behavior would be a fallback after that lookup?
b
Yeah that's what I was calling out above. It's up for debate what the future state would be. My opinion is giving a file arg always runs the source, to run the binary you must use the address
➕ 1
w
yea, seems reasonable. just have to figure out how to deprecate it
➕ 1
h
I'm generally in favor of this. It helps shed light on what targets are and when they are necessary. This discussion clarifies that 
pex_binary
has two roles: A) enhances a source file with metadata (describing how to build a PEX out of it) and B) selects what 
::
can match on. 
./pants run
is special in that applying it to 
::
makes no sense and is not supported, and we also don't generally need any extra metadata to do a 
./pants run
(as opposed to 
./pants package
). So, yeah, why require a target?
Longer term, I think in general 
./pants <goal> path/to/file.py
should ~always work with sensible defaults. E.g., 
./pants test path/to/file.py
makes sense even with no 
python_tests
target. It's obvious what to do in that case. Targets should be required only when sensible defaults need to be overridden (or are not possible in the first place) and for 
::
to do the right thing.
b
One issue to watch out for (for posterity) is now we'd maybe want to allow 
!!dep
in dependencies since it acts like a leaf in the dependency tree when run. I'm running into this now in my repo 😕
👀 1
It's obvious what to do in that case.
I'm not so sure thats true, actually. Test files aren't themselves meaningfully runnable (unless they have the 
if __name__
suffix) because pytest magic. So did you want to run it vanilla (what if they have a 
__name__ == "__main__"
handler?) or through Pytest? Also running is matched on field sets, so w'd have to special case the test targets in the one run implementation. Not saying it isn't possible or ideal, but pointing out it does get ambiguous
c
Looks good. I’ve not got the bandwidth atm to take a deep dive into it, though 😉
h
I'm not so sure thats true, actually. Test files aren't themselves meaningfully runnable (unless they have the if name suffix) because pytest magic. So did you want to run it vanilla (what if they have a name == "__main__" handler?) or through Pytest?
Through pytest. I think 
./pants test path/to/file.py
unambiguously means "run pytest with that file as input".
Even if 
file.py
isn't a test. We shouldn't get in the way of the user telling us what to do. The fieldsets are for figuring out what 
::
applies to.
I don't think this specific example is urgent, I'm using it to show that there is a general principle we can apply here, and I would like to at least think about how it all fits together. But again, 
run
is special since 
run ::
is disallowed anyway.
b
Ah yeah I misunderstood! ➕
h
Strong thumbs up from me
This is the direction I want us to go in for all goals 🙂
Open in Slack
PreviousNext
Powered by