Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add target scripts flag #10472

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

tstaig
Copy link

@tstaig tstaig commented Sep 14, 2021

This pull request is to add --target-scripts flag similar to the requested in #3934. It allows to install the scripts of a package in a different location than the libraries, if desired. If the flag is not passed, the old behavior is maintained. This flag can only be called if the --target flag is present.

@pfmoore
Copy link
Member

pfmoore commented Sep 14, 2021

I really don't see the point of this change. In general, the scripts won't work, because the installed libraries won't be in the Python interpreter's sys.path. The use case you give in #3934 seems highly specialised (and I'm honestly not sure I understand it!), so I'm not clear that this patch will be of benefit to anyone apart from you. I'd like to see a better justification for it before taking it any further.

In addition, the change would need tests. We'd need to test that the target script directory is respected for both script files (specified using the setuptools scripts=[...] argument) and script wrappers (specified by entry_points). And given that it's not at all obvious how anyone would write a script wrapper or script that actually worked with the --target-scripts option, I think that the feature would also need some actual documentation explaining how to use it properly.

@tstaig
Copy link
Author

tstaig commented Sep 14, 2021

Well I didn't create the original issue, so there's definitely more people interested in this. The objective of this ticket was to be able to create a "Python directory" structure from the --target flag. For instance:

bin lib include share

Which is prevented by the current --target mechanism as it places everything in the equivalent to "lib/pythonX.Y/site-packages" directory. Basically the package itself would be siblings to bin, include and share (and whatever other data directory).

I agree, however, that for most cases it would be easy to find other ways to achieve their objective. Even in our case, there's a workaround (using 'cp ...' or 'mv ...' in our installation scripts; not very elegant, but does the job). For some people, the changes in 4152 to install all data directories (including bin as a data directory) were more than enough, handling further details or inconveniences by themselves (setting PATH, copy/move of scripts, etc.).

We can live with Pip being locally patched as part of our framework installation, so if this is rejected, we would just have to fallback to that or wait for more flexibility from Pip in a different way (--prefix + purelib/platlib relative/absolute location, etc.) if it ever happens.

I understand you require tests for this, I of course performed manual tests in my environment, but nothing using setuptools and/or script wrappers, just called pip directly:

> pip install --no-deps --target test astropy==2.0.16
...
> ls
test
> ls test
astropy  astropy-2.0.16.dist-info  bin
> ls test/bin
fits2bitmap  fitscheck	fitsdiff  fitsheader  fitsinfo	samp_hub  volint  wcslint
> pip install --no-deps --target test --target-scripts bin astropy==2.0.16
...
> ls
bin  test
> ls test
astropy  astropy-2.0.16.dist-info
> ls bin
fits2bitmap  fitscheck	fitsdiff  fitsheader  fitsinfo	samp_hub  volint  wcslint

I also performed tests using absolute paths (in this case instead of test and bin, I used '~/tmp/test' and '~/tmp/bin'.

If you don't feel comfortable with the change, simply reject it, I only thought we could help the community with the change. If it was just for us, we would have patched it away as we have done with several third party technologies that don't completely fit our needs or are bugged in some way. Thanks for your time anyway.

@tinmarino
Copy link

Brief
I think this feature is a good design pattern separating "binaries" (aka scripts) and libraries as other languages do. In additions, it leaves the user responsible for that separation.

Explanation
The advantage of --target is that a user can install the python module to any directory he will later reference in PYTHONPATH (ex: ~/.local/lib/python3.9/site-packages)
The same goes with --target-scripts to install "binary" files to any directory referenced by PATH (ex: ~/.local/bin ).

Argumentation
This gives a standard solution to have binary files in a user specified directory (and not at ~/.local/lib/python3.9/site-packages/astropy/bin/fitsinfo) without developer custom setup.py.
The developer cannot guest that I (user) want my binaries (not only Python) at ~/.local/bin but, if it is a Python binary, it is up to me to have set a good PYTHONPATH for it to executes and find its modules wherever this file is.

Testing
Installing a python module depending on another containing scripts is enough.
I agree with the required documentation: even more necessary as it is a user-oriented feature.

@github-actions github-actions bot added the needs rebase or merge PR has conflicts with current master label Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs rebase or merge PR has conflicts with current master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants