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

Expose the ability to add a local wheel through the CLI. #1038

Closed
wants to merge 2 commits into from

Conversation

hrfuller
Copy link
Contributor

Pex already has the ability to add distributions from pre downloaded locations. This change will allow users to use wheel resolving and caching strategies outside of the pex, and then build canonically formed PEXes from external requirement sets.

Eric-Arellano
Eric-Arellano previously approved these changes Sep 24, 2020
Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds like a good idea. John gets back tomorrow and will probably have some good thoughts on this.

pex/bin/pex.py Outdated
Comment on lines 640 to 641
help="Add a wheel distribution (.whl) from a local path into the generated .pex file."
" This option can be used multiple times.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be helpful to explain more about the expectation. Can this be a .whl file, or it should be an expanded folder?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Relative path or absolute path?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the fact that the metavar says it should be a file is enough to distinguish that it isn't an expanded folder, and the fact that the .whl suffix is suggested. I'll make a change to indicate it can be relative or absolute, and to canonicalize any relative paths.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the PEXBuilder.add_dist_location method that does the work here already deals with relative paths. I'll change the comment.

@cosmicexplorer
Copy link
Contributor

cosmicexplorer commented Sep 24, 2020

Is this not already possible? e.g. with:

> pex --find-links "$(dirname my-requirement-2.0.0-py2-py3.whl)" my-requirement==2.0.0

@Eric-Arellano
Copy link
Contributor

Good point Danny. See the blue tool tip in https://www.pantsbuild.org/docs/python-third-party-dependencies#basic-setup about how to install from local files in general with Pex.

@Eric-Arellano Eric-Arellano dismissed their stale review September 24, 2020 05:09

(Will re-approve if we find this solves something we can't yet do)

@hrfuller
Copy link
Contributor Author

@Eric-Arellano @cosmicexplorer The main difference I see is that the -w option is supposed to be to stuff some .whl into the pex without invoking the pex resolver. Passing the requirement specifier and a --find-links still has pex run its resolver for that requirement, which I want to avoid. And the pex will fail to build if any of the Requires-Dist in that wheel can't also be found.

@hrfuller
Copy link
Contributor Author

I see the PEP440 direct references in the tooltip you suggested. That combined with --no-transitive works for me. Thanks!

@hrfuller hrfuller closed this Sep 24, 2020
@cosmicexplorer
Copy link
Contributor

Oh -- thanks for pointing that out! Glad that the use case is still already supported ^_^

@jsirois
Copy link
Member

jsirois commented Sep 25, 2020

Thanks to all for realizing this was an existing feature. The feature actually predates using Pip (where Pex gained support for PEP-440). Since Pip supports a non-PEP'd local path just like Pex used to before adopting Pip internally, this always worked without interuption during the Pip transition.

So long ago (two years ago to the day in this example), you could do the simplest thing imaginable:

$ ls -l
total 112
-rw-r--r-- 1 jsirois jsirois 69546 Dec 11  2019 isort-4.3.21.tar.gz
-rw-r--r-- 1 jsirois jsirois 43054 Jul 11 08:04 more_itertools-8.4.0-py3-none-any.whl
$ curl -sSL https://github.com/pantsbuild/pex/releases/download/v1.4.7/pex36 > pex1.4.7.pex
$ chmod +x pex1.4.7.pex 
$ cat << EOF | ./pex1.4.7.pex more_itertools-8.4.0-py3-none-any.whl isort-4.3.21.tar.gz
import isort
import more_itertools

print(f"isrt={isort}\nmore_itertools={more_itertools}")
EOF
Python 3.6.11 (default, Aug  7 2020, 14:31:03) 
[GCC 10.1.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> >>> >>> >>> isrt=<module 'isort' from '/tmp/tmpjvlxwkdp/.deps/isort-4.3.21-py2.py3-none-any.whl/isort/__init__.py'>
more_itertools=<module 'more_itertools' from '/tmp/tmpjvlxwkdp/.deps/more_itertools-8.4.0-py3-none-any.whl/more_itertools/__init__.py'>
>>> 
now exiting InteractiveConsole...

Just like you can today:

$ pex --version
pex 2.1.16
$ pex --intransitive more_itertools-8.4.0-py3-none-any.whl isort-4.3.21.tar.gz -- -c 'import isort, more_itertools; print(f"{isort=}\n{more_itertools=}")'
isort=<module 'isort' from '/tmp/tmpftai414k/.deps/isort-4.3.21-py2.py3-none-any.whl/isort/__init__.py'>
more_itertools=<module 'more_itertools' from '/tmp/tmpftai414k/.deps/more_itertools-8.4.0-py3-none-any.whl/more_itertools/__init__.py'>

@hrfuller
Copy link
Contributor Author

Because you were kind enough to provide that excellent description, I want to put it on the record that PEP440 direct references don't support relative paths like pex. So I just resorted to using the example you gave. This issue pypa/pip#6658 seems to be at the core of it and pip hasn't addressed it yet. Hopefully they will and PEX and pip can standardize on direct references.

@jsirois jsirois deleted the hrfuller/add-local-wheels-on-cli branch February 29, 2024 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants