inject, install accept package spec without --spec option

[itsayellow]

This PR allows the user to specify any valid pip package specification instead of a package name for the main argument to pip install and each dependency argument to pip inject. It removes the --spec option for pip install because now that option is unnecessary. And it enables a package spec for pip inject which formerly wasn't possible.

It takes about 2 extra seconds on my local computer to determine the package name from package spec. If package_spec is a valid pypi name and not a local path, I make a shortcut and assume that it is the package name and don't try to independently determine the name (in which case it takes no extra time).

I tried to add some tests, but there could be more tests added, specifically regarding the many different VCS and other URL methods to specify pip packages. One issue with that is that git+... package specifications need git in the path, which I wasn't sure how to do. (#298 (comment) seems like a good way forward on this.) Additionally, I just wasn't sure where to find good package urls to use to test.

This PR is in reference to issue #147

[itsayellow]

@cs01 feel free to modify code if it seems fun to you. Otherwise I'm happy to do the normal review process...

[itsayellow]

Note, I may be out of communication for the next few days, but I may check in occasionally.

[itsayellow]

Oh and before I forget, I was also thinking about making the Venv member variable _python public, meaning just changing its name to python. Because I use it in commands.py in this PR.

[cs01]

The _python name change is fine with me.

[clbarnes]

For compatibility in the extremely unlikely event that anyone else is using it (was it in any previous releases?), _python could be made a property which throws a DeprecationWarning but still returns the right thing, then it can be removed in a couple of release's time.

[itsayellow]

For compatibility in the extremely unlikely event that anyone else is using it (was it in any previous releases?), _python could be made a property which throws a DeprecationWarning but still returns the right thing, then it can be removed in a couple of release's time.

AFAIK it was just an internal variable inside of Venv (hence the underscore). I assume that the Venv author just assumed it would never be used outside.

I'm guessing that nobody was using it, and they were already "warned" when it had the starting underscore that they shouldn't. 😄

[cs01]

Looks great, just a couple comments

• add animation (see inline comment)
• update docs/examples.md
• update the changelog
• would be nice to update the text in the install animation. This has been existing pipx behavior but I just noticed it. Right now it says "Installing package ", and it displays the spec. It would be better if it said, for example,

installing package 'black' from pip specification 'https://github.com/ambv/black/archive/18.9b0.zip'

so it is always very clear what the package determination was from the pip specification. If this is not added in this PR, that is fine. We can create an issue and do it later.

[itsayellow]

Regarding the animation:

• I unquoted the package name, because other non-animated pipx messages refer to the package name unquoted and I was trying to stay consistent with them. (e.g. black not 'black')
• The package specification is quoted, because that made a lot of sense to me.
• I made a util.py function to return a string of the package name plus specification (if any) so that the format of this could be standardized across multiple different messages.
• Now if the package name needs to be determined, the animation says "resolving {package_or_url!r}"
• I used the abbreviation spec. instead of specification just because during animation we only have one line to work with and a long specification url was often getting chopped off. So I tried to economize the other words in the line a bit.

[itsayellow]

I made install_package error out if package name cannot be determined. I think this should be the wisest thing to do.

[itsayellow]

I revamped the package errors. I subclassed PackageInstallationFailureError from PipxError. Now I'm not sure if there's a value to having the separate PackageInstallationFailureError?

Currently if pip install subprocess fails, it will blab all of the pip errors to stderr. It's a little raw, but I think useful, so I left it. I could try and intercept this and send it to logging.info() if we wanted to clean things up a bit.

[itsayellow]

I removed the specific PackageInstallFailureError because we were never specifically using it instead of the more general PipxError.

[cs01]

Great work, thank you! I will let you merge.