Skip to content

Add run command #7925

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

Closed
wants to merge 22 commits into from
Closed

Add run command #7925

wants to merge 22 commits into from

Conversation

jaraco
Copy link
Member

@jaraco jaraco commented Mar 29, 2020

This is a resubmit of #3979 as proposed in #3971.

This approach takes a minimally-invasive approach, sharing the implementation with the pip-run. This vendored approach allows pip to adopt the functionality on a provisional basis and for updates to be made and proven rapidly in the pip-run project.

Once the functionality has stabilized in the pip project, I'd fully expect for pip to fully adopt the code and for pip-run to be sunset, probably in 6-9 months.

This command has provided me with immense value over the past four years and I'd like to share this functionality for others. By removing the barrier to first install pip-run, this functionality will become readily available for maintainers, teachers, and experimenters to use for a variety of one-off installation and testing scenarios.

I have not yet been able to run the tests, so this PR may require some massaging, but the basic functionality is working for me.

Blockers

  • Consent from 2+ contributors.
    • pfmoore
    • uranusjr
  • Inline requirements redesign

@pfmoore
Copy link
Member

pfmoore commented Mar 29, 2020

+1 in principle to this addition. IMO it solves an important bootstrapping problem, and offers a straightforward approach for simpler or more adhoc workflows, where setting up and managing a virtualenv is unnecessary overhead.

@jaraco
Copy link
Member Author

jaraco commented Mar 29, 2020

You can use pip-run to see it in action:

~ $ pip-run -q git+https://github.com/jaraco/pip@run-command -- -m pip run requests                                                                                       
Collecting requests
  Using cached requests-2.23.0-py2.py3-none-any.whl (58 kB)
Collecting chardet<4,>=3.0.2
  Using cached chardet-3.0.4-py2.py3-none-any.whl (133 kB)
Collecting certifi>=2017.4.17
  Using cached certifi-2019.11.28-py2.py3-none-any.whl (156 kB)
Collecting urllib3!=1.25.0,!=1.25.1,<1.26,>=1.21.1
  Using cached urllib3-1.25.8-py2.py3-none-any.whl (125 kB)
Collecting idna<3,>=2.5
  Using cached idna-2.9-py2.py3-none-any.whl (58 kB)
Installing collected packages: chardet, certifi, urllib3, idna, requests
Successfully installed certifi-2019.11.28 chardet-3.0.4 idna-2.9 requests-2.23.0 urllib3-1.25.8
Python 3.8.1 (v3.8.1:1b293b6006, Dec 18 2019, 14:08:53) 
[Clang 6.0 (clang-600.0.57)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import requests
>>> print(requests.__version__)
2.23.0

There seems to be a problem with using -- to pass arguments to the underlying process:

~ $ pip-run -q git+https://github.com/jaraco/pip@run-command -- -m pip run requests -- -c "import requests; print(requests.__version__)"                                  
ERROR: Could not open requirements file: [Errno 2] No such file or directory: 'import requests; print(requests.__version__)'
ERROR: Exception:
Traceback (most recent call last):
  File "/var/folders/c6/v7hnmq453xb6p2dbz1gqc6rr0000gn/T/pip-run-eld9q8up/pip/_internal/cli/base_command.py", line 199, in _main
    status = self.run(options, args)
  File "/var/folders/c6/v7hnmq453xb6p2dbz1gqc6rr0000gn/T/pip-run-eld9q8up/pip/_internal/commands/run.py", line 20, in run
    pip_run.run(args)
  File "/var/folders/c6/v7hnmq453xb6p2dbz1gqc6rr0000gn/T/pip-run-eld9q8up/pip/_vendor/pip_run/__init__.py", line 18, in run
    with deps.load(*deps.not_installed(pip_args)) as home:
  File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/contextlib.py", line 113, in __enter__
    return next(self.gen)
  File "/var/folders/c6/v7hnmq453xb6p2dbz1gqc6rr0000gn/T/pip-run-eld9q8up/pip/_vendor/pip_run/deps.py", line 52, in load
    _installable(args) and subprocess.check_call(cmd)
  File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/subprocess.py", line 364, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '('/usr/local/bin/python', '-m', 'pip', 'install', '-t', '/var/folders/c6/v7hnmq453xb6p2dbz1gqc6rr0000gn/T/pip-run-b15ofn04', 'requests', '-c', 'import requests; print(requests.__version__)')' returned non-zero exit status 1.

@jaraco
Copy link
Member Author

jaraco commented Mar 29, 2020

With this latest commit, the behavior is as prescribed:

$ pip-run -q git+https://github.com/jaraco/pip@run-command -- -m pip run requests -- -c "import requests; print(requests.__version__)"
Collecting requests
  Using cached requests-2.23.0-py2.py3-none-any.whl (58 kB)
Collecting chardet<4,>=3.0.2
  Using cached chardet-3.0.4-py2.py3-none-any.whl (133 kB)
Collecting idna<3,>=2.5
  Using cached idna-2.9-py2.py3-none-any.whl (58 kB)
Collecting urllib3!=1.25.0,!=1.25.1,<1.26,>=1.21.1
  Using cached urllib3-1.25.8-py2.py3-none-any.whl (125 kB)
Collecting certifi>=2017.4.17
  Using cached certifi-2019.11.28-py2.py3-none-any.whl (156 kB)
Installing collected packages: chardet, idna, urllib3, certifi, requests
Successfully installed certifi-2019.11.28 chardet-3.0.4 idna-2.9 requests-2.23.0 urllib3-1.25.8
2.23.0

@chrahunt
Copy link
Member

chrahunt commented Mar 29, 2020

I played with this a little bit and I think pip-run is useful. I use pex for similar temporary workflows.

I have several general concerns with including this in pip:

  1. The motivations for pip run are very similar to those for PEP 582. I think providing a command in pip that is unrelated to PEP 582 but tries to solve the same set of problems will send mixed signals.
  2. I don't think pip is the right place to hold non-PEP 582-compatible workflow helpers, even if pip itself supports older Python versions. We have a history of making decisions that create de-facto standards even before being included with Python by default, so approaches based on standardized behavior or where there's broad overwhelming consensus are preferred.
  3. pip actually executing the sub-command opens us up to additional support burden:
    1. Issues trying to execute the command will be misconstrued as being related to pip, similar to build issues today. For example, pip run pkg -- -m pkg-cmd will fail if the actual module is pkg-command). Currently this is unambiguously related to the package and not pip.
    2. Errors generated by the target command will be misconstrued as being related to pip, similar to build issues today. Currently this is unambiguously related to the command being executed, but even still we get issues created in pip for unrelated commands that have failed to execute.
  4. For better or worse, including something in pip raises it up as the "blessed" implementation or workflow and gets it in front of a lot of people. I think that will create a lot of questions around the best way to use the tool, how it interacts with other tools, feature requests, and bug reports. I looked at the number of issues, contributors, and stars in pip-run and downloads in pypistats and pip-run gets some use, but I think we'd have to field a lot of questions around it and develop our own understanding of best practices while supporting it. I would prefer if this was something users were clamoring for that already had a strong set of principles and people to defend against scope creep.
  5. Starting with pip-run as-is brings user-facing behavior (__requires__, __dependency_links__, __index_url__, future_fstrings compatibility) that may be better considered individually starting from the motivating use cases. I think this would be even more true for the fundamental issues that we're trying to address here.
  6. Starting with pip-run as-is brings workarounds and technical debt (IIUC: using sitecustomize since pip doesn't have a compatible installation type; overwriting the user's ~/.pydistutils.cfg (temporarily)) that may be better addressed in pip beforehand in a more general way

@pfmoore
Copy link
Member

pfmoore commented Mar 30, 2020

I was thinking had about this last night, and I have come to the conclusion that I am strongly in favour of this change (in principle, at least - we may need to iterate a bit on the implementation). I'll explain my motivation first, and then provide some responses to @chrahunt's points below.

Why do we need this feature?

Writing a simple script in Python should be easy (I hope this can be taken as given). Certainly a lot of Python code is part of complex applications with full development processes, etc., but there's also a huge amount of code that is "just" simple automation scripts, adhoc utilities, or similar. I'd also assume that we can take as given that we want using packages from PyPI to be simple.

For such simple scripts, there's a significant step change in complexity as soon as you depend on a 3rd party package. Instead of just running the script using Python, you need to decide whether to install the dependencies into your system Python, whether to use a virtualenv, etc. This is often a lot more complexity than a simple one-off1 script deserves. Personally, I have often written scripts that avoided 3rd party packages even when they would have made my code significantly simpler, precisely for this reason. That's not healthy for Python, or for 3rd party packages.

A pip run command designed to give an easy way to invoke a script with its dependencies made available, smooths over this step change, allowing adhoc scripts to freely use 3rd party dependencies.

Why include this feature in pip?

This functionality needs to be available to all Python installations. Otherwise there's a bootstrapping issue - it's hard to run scripts because you need to install stuff, so install stuff to make it easy to run scripts...? Experience with many tools (pip-run and pipx being good examples) shows that the one place where such tools struggle is in bootstrapping - the typical advice is to install them globally (using --user, possibly), even though this is contrary to a lot of "good practice" advice, and contrary to the philosophy of the tools themselves.

We can't add this to core Python - it fundamentally depends on package installation, and pip (the main package installer) isn't part of core Python (it's just bundled). We can't add it as a separate tool - as has been pointed out, that involves a bootstrapping issue, and pip-run has demonstrated that that hinders adoption. So really, the only place that is left is pip. You can't have a problem of dependency management without having pip available, so having the solution be part of pip as well, seems perfectly reasonable to me.

I could elaborate further here, but I'm trying to be as concise as I can, and I want to address @chrahunt's points, so I'll leave it at this for now. I will say, though, that I have examples where I would use this functionality, multiple times, every day (and yes, I've tried pip-run, and the fact that it needs to be installed is a sufficient issue that it doesn't work for me as a solution).

Responses to Chris' points

The motivations for pip run are very similar to those for PEP 582

I don't think this is true. PEP 582 offers another way of bundling dependencies with a script. The approach is meant to be simpler than virtual environments, but the idea is still to install the dependencies along with the script, and distribute the whole lot. pip run is about not bundling the dependencies, just making them available for the duration of the one run of the script. The mechanism behind pip run is largely irrelevant - it could be PYTHONPATH changes (as it presently is), or a virtualenv, or PEP 582 (__pypackages__). The functionality would remain the same.

I don't think pip is the right place to hold non-PEP 582-compatible workflow helpers

I've explained above why I think that pip is the only place that can provide a helper for this use case. And I don't see what you intend as a "PEP 582 compatible workflow helper" that would address this use case2, so I'm not sure your comment is relevant.

pip actually executing the sub-command opens us up to additional support burden:

This is valid. But we already have many cases of mis-targeted issues, and I hope no-one is suggesting that we block any features which could trigger such issues. The new resolver will definitely result in issues where people get what they thing is "the wrong" thing installed, which will be down to project metadata. Should we not add a new resolver? I believe that the extra support cost of such issues is justified by the value of the feature. Also, if we have serious concerns here, we could look at pip-run and see how many of its support issues have been of this nature (we could also look at similar projects like pipx).

For better or worse, including something in pip raises it up as the "blessed" implementation or workflow and gets it in front of a lot of people.

Maybe. But is that such a bad thing? At the moment, there is no good solution3. Even if the proposed pip run isn't perfect, making it available gets people using it and allows us to iterate on the design.

Starting with pip-run as-is brings user-facing behavior (requires, dependency_links, index_url, future_fstrings compatibility) that may be better considered individually starting from the motivating use cases.

This, I agree with (although I don't know what you mean by "future_fstrings compatibility", it's not something that is described as user-facing in the docs). The common factor here seems to be "how scripts specify metadata that's needed to run them". That seems to me to be something that should be standardised, for interoperability reasons (other tools may want that data). So I'd fully support a proposal to standardise those items. I'd also be OK with removing them from the initial implementation, or marking them as unsupported/experimental somehow, subject to a standard mechanism being agreed.

But I don't think we need to discard the feature altogether just because of some details that need to be reviewed/amended. The functionality would still be more than adequate for the basic use case without them:

$ python -m pip run -q requests html5lib -- my_scraper.py

Clearly, things could be even simpler - python -m pip run -q -- my_scraper.py is an improvement, and python -m pip run my_scraper.py better still. I'm happy to iterate on the best form here, either as part of PR review, or post-implementation, as we get more experience of what the ideal UI should be. But dumping the whole feature because the UI is suboptimal seems a bit extreme.

Starting with pip-run as-is brings workarounds and technical debt

No more than vendoring any other package does. If, as suggested, we decide at a future date to migrate the functionality into core pip, then we can do so in a way that cleans up the code and the mechanisms. But in terms of this PR, any issues would just be "issue with a vendored library, please report downstream", just as with any other library we vendor.


1 As we all know, one-off scripts will be run many times 😉, so we need a longer-term answer to these questions.
2 Specifically, running a single .py file, possibly one of many in the same directory, without needing to "prepare" it for execution in any way.
3 Feel free to point me at solutions. If there's one that I've not tried and found wanting, I'll happily come back here and say so. At the moment, my personal experience is that pip-run is the nearest to the solution I'd want, but its biggest issue is that it's not available by default (i.e., in core Python or in pip).

@pfmoore
Copy link
Member

pfmoore commented Mar 30, 2020

One further point I should add. I would like to see a bit more thought put into the UI of the pip run command. I don't think that the pip-run UI is necessarily the best approach, nor is it particularly future-proof (in the sense that if we expose the full pip-run UI, we will have more trouble changing the UI than if we start with a more limited version).

@jaraco
Copy link
Member Author

jaraco commented Mar 30, 2020

Thank you @chrahunt for the thoughtful and thorough review. And thanks Paul for addressing some of them. Allow me to provide some clarification.

Use cases

The design of pip-run allows for several use-cases and even some not specifically called out in the docs.

Script executor

In my experience, the "script executor" construct, while useful, is less than 20% of the value of the design. Most of the time, I'm relying on the functionality for one of the other use cases.

Multi-version interactions

pip-run by its design satisfies a number of one-off use-cases. Consider for example this comment in which I'm able to illustrate in a single command the serialization of an object with one version of jsonpickle and then pipe that as input to another environment with a different version of jsonpickle to illustrate the incompatibility of a serialization format across versions.

I don't believe this behavior is possible with any other tool.

Installer troubleshooting

Often a person will file a bug report stating that "installing X produces unexpected outcome Y". Because pip-run doesn't alter any environment permanently, a maintainer can rapidly attempt to replicate the installer scenario by running pip-run X (or pip-run X -- -c pass) and either replicate or fail to replicate the user's experience. If it succeeds, there is nothing to clean up. Moreover, said maintainer may paste the result to the report as confirming or disconfirming evidence.

Flexibility

The fact that pip-run specifically attempts not to own the syntax for the arguments before -- (which are passed to pip install) or after -- (which are passed to sys.executable), means that this design supports use-cases for future versions of pip and CPython. For example, pip-run has supported --use-pep-517 from the moment that pip did (and even before, by way of pip-run git+https://github.com/pypa/pip@master -- -m pip-run ...).

It's for this reason that pip run my_script.py may not be a good user experience, because it conflates pip install my_script.py and python my_script.py. It's possible that these two uses could be mechanically disambiguated, but I'd prefer to require the -- in pip run -- my_script.py to be explicit about the execution.

Adapting the design

Some of the advice above could be applied directly to pip-run and I argue they should be. That is, the implementation of pip-run has been undergoing refinement and evaluation for the past several years. If there's a compelling reason to adapt the design of the interface that pip-run provides, I'd like to focus those suggestions and considerations into separate issues in that project. So, if for example, pip run my_script.py is a useful user experience, I'd prefer to implement that as pip-run my_script.py (even if backward-incompatible) and then adopt that here. I'd rather this PR focus on the integration concerns.


I owe more attention to the previous responses and will specifically address some of them, but want to provide these examples and concerns for consideration in the interim.

@uranusjr
Copy link
Member

uranusjr commented Mar 31, 2020

I feel the feature this provides is very useful. This would still be true even if PEP 582 (or an alternative “local environment detection”) is implemented, since the two use cases do not align perfectly. Local environments are generally intended to reused across multiple runs like virtual environments, while this feature, has an isolated, one-off emphasis by contrast, which would still be cumbersome to set up with PEP 582.

I feel this would be better clarified if this feature is introduced under a different name, since run is already used by various tools to represent “running a command against the local environment.” Maybe something like pip once?

@jaraco
Copy link
Member Author

jaraco commented Apr 5, 2020

Command Name

run is already used by various tools to represent “running a command against the local environment.” Maybe something like pip once?

I like this suggestion, and my initial instinct is to agree. The once command is one character longer, but otherwise communicates pretty well what's happening. And if it were a simple matter of selecting a different name, I'd simply accept the suggestion. My hesitation comes from the fact that pip run was established almost four years ago and that's the namesake of the pip-run project (and command and package) that has already undergone one name change. Even as pip hypothetically adopts the functionality, I'd expect pip-run to remain as a fallback until everything is running that version of pip, so it would probably need to be renamed too. In theory the names could diverge, but it might be awkward for the fallback pip-run to implement pip once. I can say from experience that renaming the project is an involved effort and disruption and shouldn't be taken lightly.

The underlying behavior really does draw together two main behaviors. If length were not a factor, I might propose naming the command "install once and run", capturing the two main things the command does (first install temporarily the requirements, then run Python with those requirements in place). "install" is somewhat implied by "pip", so "once" and "run" both seem equally suitable to me. Choosing "run" implies the "once" and vice-versa.

On balance, the two names seem equally suitable to me except for two factors:

  • run might be confused with other run behaviors.
  • once requires substantial investment to rework the fallback package.

While I appreciate the suggestion, I lean toward sticking with "run", but if others feel strongly, I'll add it to a list of blockers.

@jaraco
Copy link
Member Author

jaraco commented Apr 5, 2020

5\. Starting with `pip-run` as-is brings user-facing behavior (`__requires__`, `__dependency_links__`, `__index_url__`, `future_fstrings` compatibility) that may be better considered individually starting from the motivating use cases.

f-strings support

The f-strings compatibility is somewhat user-facing in the sense that if you try to invoke pip-run against a script on a version of Python older than 3.6 and and that script is using future-fstrings to enable fstrings on that script, it would fail except for the compatibility. Support for this use-case isn't crucial to the implementation, but it's a friendly feature that takes ~10 lines to support and is readily removed when Python < 3.6 support is dropped. I'd request that this support be included, as it provides value.

Inline requirements

Support for inline requirements through things like __requires__ and __index_url__ were born out of the desire to support single-file exchange. I'd like for a user to be able to attach a script or paste a gist or put an example in a repo that's a single file. There are many cases where having more than one file to replicate an issue substantially complicates the workflow for exchange. Supporting inline requirements enables this simple workflow, rather than requiring the requirements to be stored/transmitted in a separate channel (as separate instructions, as run instructions, as a separate requirements.txt file). The mongodb example illustrates the power of this - one can download that script and pip-run it, with little other context.

I'm not wed to the syntax of __requires__, etc., but I chose that format because it follows the long-established pattern that pkg_resources uses to validate requirements of __main__ when running a script, but also because I find the use of Python syntax to be elegant and useful. For example, it allows the script runner to inspect the values as well.

I do agree it would be valuable to either settle on a standard for supporting inline requirements before establishing the implementation. I welcome anyone to take this torch and establish such a standard.

In the meantime, this current implementation acts as a de-facto standard that should be supported for maximal compatibility, even if a new standard were to be adopted. Any new standard would almost necessarily be compatible with the current approach.

I'd be okay with flagging this feature as "provisional" and "best effort", but I don't foresee any challenges with transitioning to a newer standard as available.

Support burden

Speaking to the support burden, I fully expect to be principally supporting this functionality and would consider it my responsibility to address user concerns emerging from this feature.


@chrahunt I want to thank you again for the feedback. I feel like between myself and pfmoore, we've addressed your concerns. If you feel there are still issues remaining that are blockers, would you enumerate those?

@pfmoore
Copy link
Member

pfmoore commented Apr 5, 2020

Just to clarify, I don't like the __requires__ syntax, so I would strongly hope that any final standard for this choose a different syntax. But I'm fine with having it as a provisional approach.

I'm not keen on __index__url__. though - I think the user should include the index URL in the pip command explicitly. I'm also not keen on the __dependency_links__ attribute, for similar reasons (scripts downloading code from locations the user didn't explicitly specify), as well as because it feels like it will get in the way of the "dependency links are no longer supported" message, simply due to the similarity of the name.

But as I said above, I'm OK with them being included in the initial implementation, but subject to change or even removal in later versions. I'm also fine with them being removed. How we handle the removal or provisional nature of these items can be decided in PR review (a warning? documentation only?). Let's get the feature agreed (or not) in principle first.

@pradyunsg
Copy link
Member

pradyunsg commented Apr 5, 2020

Flagging that I don't think this would catch the train for pip 20.1 (i.e. the coming 10 days).

Please let me know if that's not the case. :)

@pradyunsg pradyunsg added state: needs discussion This needs some more discussion type: feature request Request for a new feature labels Apr 5, 2020
@pfmoore
Copy link
Member

pfmoore commented Apr 5, 2020

@pradyunsg Agreed, we shouldn't rush this or put deadline pressure on people. OTOH, I would like to see this resolved one way or another for 20.2. I think 3 months to reach consensus and get it in a state for release is a reasonable timeframe.

@jaraco
Copy link
Member Author

jaraco commented Apr 5, 2020

Flagging that I don't think this would catch the train for pip 20.1 (i.e. the coming 10 days).

I've been working hard on this the past few weeks (also presented it as a project at HackIllinois) to try to get this ready for the 20.1 release. If it can't be included then so-be-it. Agreed this should not be rushed.

I'm also fine with [the inline requirements feature] being removed.

One of my key design goals was to allow a single file to contain the entire implementation, including its dependency declarations. The tool is useful without this feature, but is also dramatically limited without it. Sure, one can write in the comments:

# run this script with pip-run requests boto zope.interface 'pathlib2;python_version<"3.5"' -- script.py

And then require the user to read the script and copy/paste the spec. It seems plainly obvious that it's valuable to the user that the run too honor some form of inline version specification for scripts.

If you think this feature should be removed, would you file a bug with the project describing the reason the feature should be removed and what you expect users of the feature to do instead?

I don't like the __requires__ syntax. I'm not keen on __index_url__.

Isn't a de-facto standard better than some other arbitrary syntax? Are you proposing a different syntax? Would you consider filing a bug with pip-run describing a preferred syntax and motivations?

The user should include the index URL in the pip command explicitly.

The current implementation is that the user may specify an index URL, but there are other cases where the script author would like to specify the index URL. Consider, for example, an internal environment where the resources are on a private PyPI server, and someone has thrown together a script to run that depends on that index server. They can add comments to the file:

# Please always run this with --index-url=https://our.server/
# If you run it against PyPI, you'll get the wrong packages and bad things will happen.

Or they could send the script to a colleague and let people know that when they run the script, they should include --index-url=... in their invocation.

But why create the foot gun? When the author knows that the only useful way to run the script is to specify a particular index URL, there should be a way for the author to specify that meaningful default in the script itself and for the tool to help enforce it.

I agree there are situations where an index URL in the script is bad practice, but that doesn't obviate the legitimate uses that are good practice.

It seems by this logic that --index-url should be removed from requirements.txt files as well.

I'm also not keen on the __dependency_links__ attribute.

I think it's possible that __dependency_links__ are unnecessary. They were added in jaraco/pip-run#24 with the justification that they're required to specify dependencies from VCS links, but I've seen lately that git+https URLs work just fine as simple requirements, so perhaps the __dependency_links__ can be removed now that later versions of pip support these URLs.

How we handle the removal or provisional nature of these items can be decided in PR review (a warning? documentation only?). Let's get the feature agreed (or not) in principle first.

I'm fine with marking the functionality as provisional in the docs of pip-run, especially if there's an open issue to reference explaining why the syntax might change. If it's something likely to change, I'd even go as far as warning when the behavior is triggered.

@pfmoore
Copy link
Member

pfmoore commented Apr 5, 2020

If you think this feature should be removed, would you file a bug with the project describing the reason the feature should be removed and what you expect users of the feature to do instead?

I'm OK with the feature staying, I was just saying that if someone else objected to it, and argued the case successfully, then I personally wouldn't defend it - I assumed that you would, though 😉

Isn't a de-facto standard better than some other arbitrary syntax?

It's not a de-facto standard at the moment, it's an implementation-specific feature of a single tool. Pip, on the other hand, tries hard to be standards-based, and on that basis I'd consider "Syntax for scripts to specify their dependencies" to be a perfect example of something that should be standardised (just like we have a standard for how packages define their dependencies). Personally, I'm willing as an interim measure to use pip-run's syntax, but only subject to the understanding that as and when a standard is developed, we'd switch. And personally, I would not defend the __requires__ syntax - I'm not a great fan of "magic variables", as they are difficult to read without executing the code (yes, I know, pip-run does so, but other code might have different constraints - we've already seen this with introspecting versions).

Are you proposing a different syntax? Would you consider filing a bug with pip-run describing a preferred syntax and motivations?

No, that's why I'm OK with accepting the current syntax as an interim measure. Conversely, are you willing to propose the __requires__ syntax as a standard, and get it documented and agreed as a prerequisite of this feature going in? I'm trying to defer the need for that standardisation process.

there are other cases where the script author would like to specify the index URL

... and I'm saying that this may not be something pip wants to support. Where pip looks for package dependencies is very much in the control of the user, not the package author - that's one of the reasons dependency links was deprecated. Letting scripts define their own index goes against that principle, and I don't think it's something that we should do without due consideration.

Again, please understand that I'm not saying we'd never allow something like this, just that I'd rather add it as an enhancement later, with due consideration of the specifics of that feature, and an open discussion of how the feature would be exposed to the user, rather than have it pulled in as a side-effect of adding a pip run command.

It seems by this logic that --index-url should be removed from requirements.txt files as well.

I don't see that - requirements.txt isn't package metadata. The situations are similar, but not identical. I could be persuaded that it's a legitimate parallel, but are you saying that we have to resolve all these questions in favour of doing what pip-run does, or there's no value in accepting this proposal? Because if that's the case, then I'm a lot less comfortable supporting this proposal.

For me, the key functionality here is for a user who has written a script that uses a "standard" dependency like requests. They want to run that on another machine. At the moment, assuming that they follow the standard principle of not messing with the system Python, they have to do

py -m venv temp_env
temp_env\scripts\pip install requests
temp_env\scripts\python the_script.py
del -rec temp_env

With pip run, that gets reduced to

pip run requests -- the_script.py

Or assuming the script can declare that it depends on requests,

# This usage is why the -- argument looks odd...
pip run -- the_script.py

Frankly, that is the only use case that I view as essential. Anything beyond that is, in my view, at best nice to have. I'm happy to have pip-run as a proposed implementation that we can include, and I'm OK with the principle that the pip run subcommand should look to satisfy the other use cases that pip-run currently covers, but I'm not willing to accept that pip take on pre-made backward compatibility constraints of having to preserve all pip-run behaviour.

Maybe we should initially restrict pip run to just this syntax, with __requires__ as an experimental syntax for requirement specification, subject to standardisation (and possibly change as part of the standardisation process). We can use pip-run as the library to implement this feature set, but we wouldn't expose any other features of pip-run initially (and I appreciate that may mean changes to pip-run to allow disabling of, say __dependency_links__ handling).

At the moment, this discussion is starting to feel uncomfortably "all or nothing" to me... 🙁

@jaraco
Copy link
Member Author

jaraco commented Apr 5, 2020

In jaraco/pip-run#41, I started dropping support for __dependency_links__, but I think I see know where the problem lies. Because __requires__ is a setuptools scheme, and because that field is meant to be parsed using pkg_resources.parse_requirements, it's not allowed to have URLs the way that pip allows in requirements. Therefore, the deprecated separate declaration of the dependency and where to locate that dependency must be honored.

This finding does lead me to agree that a redesign of the "inline requirements" feature should be redesigned and aligned with what "pip install" supports.

@jaraco
Copy link
Member Author

jaraco commented Apr 5, 2020

I'm not willing to accept that pip take on pre-made backward compatibility constraints of having to preserve all pip-run behaviour.

I'm happy to adapt pip-run to support the best implementation of pip run, whatever that needs to be. I'd like pip run to obviate pip-run and to have pip-run serve as a fallback where pip run is unavailable. I really want to avoid having two implementations to support different use-cases and formats. I don't want to maintain two code bases. I don't want the instructions to say "if you have this use-case, use pip-run, but if you have this use case, use pip run". I'm aiming instead to use pip-run as an established baseline on which to iterate in the context of pip run. That was what I meant when I said sharing an implementation in the OP.

That doesn't mean it has to be all or nothing - I'm very willing to remove cruft from pip-run (and have), but would like to support as many of the use-cases as makes sense. If a use-case is not valuable enough to be in pip run, then I argue it the same is true for pip-run, but the converse is also true. If a feature is valuable enough to present in pip-run, then it's valuable enough for pip run. For example, I use the pip-run deps -- -c "..." usage almost daily and frequently when interacting with other users. Although it's not currently a use-case that you find essential, it would be a travesty to this effort if that functionality weren't somehow supported.

Can we start from the assumption that pip run will have the same interface as pip-run and work to make the interface of pip-run suitable for adoption into pip?

@pfmoore
Copy link
Member

pfmoore commented Apr 5, 2020

Can we start from the assumption that pip run will have the same interface as pip-run and work to make the interface of pip-run suitable for adoption into pip?

I'm happy to do that. Can we also start from the assumption that not all of pip-run needs to be included in pip run from day 1?

@jaraco
Copy link
Member Author

jaraco commented Apr 5, 2020

I'm happy to do that. Can we also start from the assumption that not all of pip-run needs to be included in pip run from day 1?

Of course.

@jaraco
Copy link
Member Author

jaraco commented Apr 10, 2020

In the latest commit, I've added two decorator factories, provisional and provisonal_eval which may be applied to any part of the functionality and then applied one to pip_run.scripts.DepsReader.try_read such that pip now warns if that functionality had any effect. To illustrate:

draft $ cat script.py
__requires__ = ['requests']


import requests
draft $ pip-run git+https://github.com/jaraco/pip@ba0063a -q -- -m pip run -- script.py
  WARNING: Did not find branch or tag 'ba0063a', assuming revision or ref.
Support for reading requirements from a script is provisional.

I found this approach didn't quite work on Python 2, so I updated it to unwrap and rewrap the classmethod and now it does.

draft $ python2.7 -m pip-run git+https://github.com/jaraco/pip@276bd846 -q -- -m pip run -- script.py
DEPRECATION: Python 2.7 reached the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 is no longer maintained. A future version of pip will drop support for Python 2.7. More details about Python 2 support in pip, can be found at https://pip.pypa.io/en/latest/development/release-process/#python-2-support
  WARNING: Did not find branch or tag '276bd846', assuming revision or ref.
No handlers could be found for logger "pip._internal.commands.run"

I don't understand why the behavior is different on Python 3 and Python 2 for the logging configuration.

Does this address the need to mark functionality as provisional and limit the supported interface? Are there other outstanding concerns that need to be marked as provisional? What other actions are necessary to move this effort forward?

@BrownTruck
Copy link
Contributor

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 master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Apr 17, 2020
@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label May 2, 2020
@BrownTruck
Copy link
Contributor

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 master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Feb 4, 2021
@pradyunsg
Copy link
Member

I'm going to close this since this hasn't been updated in a while and has bitrotten. If someone does want to push this forward, I'd suggest establishing concensus in #3971 before filing an updated PR!

Thanks @jaraco for the PR; and everyone for the discussion in this PR!

@pradyunsg pradyunsg closed this Mar 5, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs rebase or merge PR has conflicts with current master state: needs discussion This needs some more discussion type: feature request Request for a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants