Implement rwt as pip run #3979
Implement rwt as pip run #3979
Conversation
|
I should have mentioned #3971 where this discussion originated. |
|
@jaraco btw, what about putting the cached wheels onto python-path for any non-binary wheel file |
|
@RonnyPfannschmidt Perhaps. How is that preferable to invoking pip install? Does pip provide a command to get cached wheel files? |
|
@it removes the need to make a tmpdir So for all pure python packages it would remove the nerd to manage Recent pip is adding commands to interact with cache |
|
I actually don't see why the wheels would have to be non-binary wheels. Setuptools creates binary eggs all the time that are loaded as zip packages. Still, I'll leave it up to the packaging ecosystem to decide which formats are viable. As for linking cached wheels, it's important that there be functionality to resolve a set of requirements to a set of wheels, installed or cached. While attractive, that feature feels a little premature. I'm willing to accept the performance drawbacks until the functionality proves worthwhile. I believe this PR is ready for review. |
|
@jaraco binary eggs need unpacking as well, |
|
Not in my experience: So that's at least one case of a binary package with extensions that's built into a zipped egg, added to PYTHONPATH, and then imported. Based on my past experience, I'd expect that library to be fully functional as well. Can you share what you find doesn't work? |
|
setuptools does have support for eggs with shared objects where it unpacks those parts somewhere dlopen-able automatically at runtime. I've heard that internally at Google they have patched Linux to be able to dlopen an uncompressed shared object stored inside e.g. a zip file. |
|
@jaraco is the key point here It's typically an OS restriction that you can't load shared libraries from a zipfile. Does the egg format have stub files that do something clever like extract the shared library at runtime to a temporary directory, load it from there, and redirect calls into it? I have a vague recollection that something like that exists. On the other hand, wheels don't have that sort of stub, because they explicitly don't take special measures to support "import direct from a wheel" as a use case. |
|
I am -1 on using the wheel cache for this, I don't think we should bake into pip any sort of assumption that wheels are importable if you add them to |
There was a problem hiding this comment.
I'm still not 100% convinced this belongs as a pip subcommand, but I could be swayed.
If it is to be merged, though, there should probably be some tests added to the pip test suite. I know rwt has its own tests, and that pip run simply forwards to rwt but at a minimum we need some tests to confirm that the integration as pip run works correctly.
|
|
||
|
|
||
| class RunCommand(Command): | ||
| """Show help for commands""" |
There was a problem hiding this comment.
That docstring looks incomplete.
More generally, does this patch include suitable documentation for pip run? I know a lot of the docs are auto-generated, but given that this command just defers all its processing to rwt, I guess the auto-generation won't work. Can you add some documentation so that the full syntax for the command (and some examples) show up in the pip documentation?
There was a problem hiding this comment.
Correct. This docstring is a copy-paste artifact (I used pip help command as a template). I'll correct that. This patch does include the docs for the rwt command, and in fact the tests ensure that pip help run outputs the same as pip run --help.
| class RunCommand(Command): | ||
| """Show help for commands""" | ||
| name = 'run' | ||
| usage = rwt.commands.help_doc |
There was a problem hiding this comment.
Possibly this does enough to generate docs, but can you check locally? I'll try to double-check by downloading a local copy of the PR myself, but I don't have the time to do that right now.
There was a problem hiding this comment.
I did confirm, and the tests validate.
There was a problem hiding this comment.
Do you mean the rwt tests? So they test the pip integration as well? Then they definitely should be added to the pip test suite, rather than being external.
We might be talking at cross purposes, though. The other pip subcommands have their own pages (docs/reference/pip_list.rst for example). I was expecting a pip_run.rst alongside these.
There was a problem hiding this comment.
Oh, not the rwt tests, but the pip tests check all commands to ensure that they have a viable docstring. I need to look into the docs situation.
There was a problem hiding this comment.
I've pushed a new commit with docs for the pip run command. I've tested that these docs render properly and without warnings and provide a suitable background within the pip docs. I feel good about the documentation story now.
There was a problem hiding this comment.
To test-build the docs, I ran this command: rwt sphinx -- -m sphinx docs out. I didn't have to set up any virtualenv and I don't have anything to clean up after (except the out dir).
|
|
||
| signal.signal(signal.SIGINT, null_handler) | ||
| cmd = [sys.executable] + params | ||
| subprocess.Popen(cmd, env=_setup_env(target)).wait() |
There was a problem hiding this comment.
Technically this is an issue with rwt rather than with the vendoring or pip run, but this doesn't pass the exit code of the script back to the caller of rwt. It probably should.
There was a problem hiding this comment.
Agreed. I've filed jaraco/pip-run#10
|
I am also not 100% convinced that this should be added (I'm bouncing between -0 and +0) but I think if we do add it, we should probably actually implement it inside of pip itself instead of just wrapping |
|
I agree with @dstufft. We should not use the wheel cache directly. Two reasons:
|
|
I'm not convinced this is quite as bad as the old egg problem. The rwt user is more likely to have tested their published script against the zipped dependencies than the classic egg user, who may have developed everything on the filesystem only to be greeted with the unpleasant surprise of zipped deployment. |
|
But unlike the egg problem, there's no way to say "you have to unpack this wheel to use it". So any such dependencies are straight-up unusable. (And it's way worse because unlike eggs, there's no support in wheels for binaries). Just to note, by the way, I'm perfectly happy with the idea of |
Options are passed directly through to That does mean that passing
I'm considering adding a call that invokes
I see value in keeping rwt decoupled.
I do see advantages of incorporating it natively:
Overall it seems like vendoring is the better approach, but I'm open to native incorporation. May I suggest provisionally accepting the command using the vendored library and defer the potential native incorporation as a subsequent change as demand dictates?
I'd be just as happy with pip requiring rwt rather than vendoring it. I see a small advantage to |
You misunderstood me. by "Incorporating into pip" I meant "having a pip run command that invokes rwt (whether vendored or native)". I'm not too convinced by the bootstrapping argument - having to one-off do I think we need some more compelling justifications as to why this command needs to be spelled |
That's how I understood you. I do see the corollary to virtualenv. The biggest difference is that rwt is meant to be a bootstrapping end-user tool, whereas virtualenv is more of a developer tool. The target audience for rwt has a lot less experience with Python, maybe none, but it gives users like myself the ability to give a one-liner to run this script or that command that an experienced user can just run, without the concerns about privileges and environments and pre-requisites. By making |
|
What is the status of this? I'd love one day to see rwt integrated into pip as |
|
Expanding on what I wrote last, about raising the usability bar, often a user finds himself in an access-limited scenario, where he/she has access to Python/pip but not much more. Installing packages, even rwt, is problemmatic as it's not always obvious where to install. The only real option is |
…ection of purpose.
|
Here's another case where @dstufft might have used pip run. In the recent TLS 1.2 announcement, he gives instructions on how to check TLS versions. The instructions require requests to run. So the instructions are given: In these instructions, we're asking end-users to not only install, but possibly update the version of requests in their system site packages (or maybe implicit user site packages) and not giving any instructions on how to undo the change. In practice, this won't be a huge issue, but how much nicer would it be to give a more concise, single-line command that doesn't mutate the system state: If pip run were something that were generally available, it would enable these one-off, non-invasive usages. |
|
Hello! I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the |
|
As much as I'd like to see the behavior added, it seems others don't see the value, so I'm going to drop it. |
lock
bot
added
the
auto-locked
|
Revived in #7925. |
This PR quite simply vendors rwt and invokes it as pip run. As you can see, the hook is rather minimal (mostly boilerplate).
One thing I'd like to do before accepting this is to stop re-writing sys.argv and instead have rwt.run take the args directly, but that'll take an update to rwt (which I will do later).
I also welcome other feedback. Do you want a changelog entry? Should docs be updated somewhere?
This change is