Improve Error Messages on EnvironmentErrors when installing

[pradyunsg]

Fixes #5237

[pradyunsg]

Same question as always: Do we want tests?

[pfmoore]

We always want tests :-)

More seriously, a test that we don't say "use -v" when the user is using -v seems reasonable. As does a test that we don't suggest -v on a permission error (and getting that test right on both Windows and Unix should be interesting, given that Windows and Unix have such different permission models - I'm genuinely unsure whether by omitting the EPERM test you've changed the meaning of the message on Windows).

[pv]

Checked against my testcase, it now prints:

Collecting six
  Using cached six-1.11.0-py2.py3-none-any.whl
Installing collected packages: six
Consider using the `--user` option  or  check the permissions

But note that it still doesn't say the installation failed. I think it's important the output contains an explicit statement that the installation was not successful.

[pv]

Not a native speaker, but "increase verbosity by passing --verbose"?

[benoit-pierre]

Not showing the original exception and asking the user to re-run with -v seems backward to me. What if it was a transient error? IMHO, in trying to hard to print a "helpful" message, pip is hiding useful information. Even with -v the output is not that useful:

Installing collected packages: six

Could not install packages due to an error. Consider using the `--user` option or check the permissions. To view error traceback, increase verbosity by passing --verbose.

Cleaning up...

I don't know where pip is installing the package, and the original exception is not shown either, so I have not idea which directory I should check the permissions.

Contrast that with the last line when pip is trying to update its self-check state file:

PermissionError: [Errno 13] Permission denied: '[...]/venv/pip-selfcheck.json'

That one is useful.

[pradyunsg]

Not showing the original exception and asking the user to re-run with -v seems backward to me.

@benoit-pierre I agree. Thanks for your input on this. :)

I'm genuinely unsure whether by omitting the EPERM test you've changed the meaning of the message on Windows

@pfmoore It's trivial to add it back but I think EPERM isn't the right error code here (based on here). None the less, we're still printing the error message so, we should be fine?

As for tests, I'll take a look tomorrow.

---

I've gone ahead and redone this entire logic. I've moved out the code into a separate function -- that should make testing it easier. It's also resulting in either the string representation of the error or the entire Traceback showing up in the terminal (not both). Further, it's also unconditionally mentioning that an error occurred.

[pfmoore]

Why not just show the traceback if we think it's needed, rather than asking the user to rerun? Although honestly I think this is too much hand holding - if there's not enough information in the displayed output, adding --verbose is always the advice, so why call it out specifically here?

But I'd rather someone just make a decision on this - I don't want to debate wording to the point where we don't at least put some information in the user's hands.

[pradyunsg]

I'll add tests in a subsequent PR. Happy to let this go in as is.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.