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

Warning for user when Windows default Path Limit is exceeded #10046

Merged
merged 14 commits into from
Jun 11, 2021

Conversation

OBITORASU
Copy link
Contributor

@OBITORASU OBITORASU commented Jun 8, 2021

Description

This PR generates a warning message for the end-user informing them about the potential errors which might be caused if they have Long Paths disabled on their Windows system. I have used:

from pip._internal.utils.compat import WINDOWS

as the means to check the host OS as suggested by the maintainers and added an if statement to check for the 260 default path length limit when Long Paths is disabled. The if statement will successfully trigger a warning using logger.warning in case the host OS is Windows and the error message length (which also contains the path) is greater than 260 characters.

Fixes Issue #10045

@OBITORASU OBITORASU changed the title Warning for user when Windows path limit is exceeded Warning for user when Windows default Path Limit is exceeded Jun 8, 2021
Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

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

Couple of thoughts:

  1. error is an exception, so you need to str() it first.
  2. We should probably check error.errno to make sure we’re catching a FileNotFoundError.
  3. The error message should include a link to some Microsoft documentation. I don’t except people having Long Paths off would understand what set LongPathsEnabled to 1 means.

@OBITORASU
Copy link
Contributor Author

OBITORASU commented Jun 8, 2021

Couple of thoughts:

1. `error` is an exception, so you need to `str()` it first.

2. We should probably check `error.errno` to make sure we’re catching a `FileNotFoundError`.

3. The error message should include a link to some Microsoft documentation. I don’t except people having Long Paths off would understand what _set LongPathsEnabled to 1_ means.

For the second suggestion. Can I go forward with a check like error.errno == errno.ENOENT? Somewhat like this check if error.errno == errno.EACCES: currently being used in the code.

@uranusjr
Copy link
Member

uranusjr commented Jun 8, 2021

Yeah, sounds right to me.

@OBITORASU
Copy link
Contributor Author

OBITORASU commented Jun 8, 2021

Yeah, sounds right to me.

And last question I had for the final suggestion, I am going forward with this microsoft documentation in particular: https://docs.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=cmd#enable-long-paths-in-windows-10-version-1607-and-later
Hope this is fine.

@uranusjr
Copy link
Member

uranusjr commented Jun 8, 2021

Damn, that’s a long one. I guess that’s the best we can have though, since it’s Microsoft.

@OBITORASU
Copy link
Contributor Author

OBITORASU commented Jun 8, 2021

Damn, that’s a long one. I guess that’s the best we can have though, since it’s Microsoft.

Yeah that is the only decent one I found which belonged exclusively to Microsoft. As of now I have updated the PR with all the requested changes, please take a look when it is possible for you. Thanks!

src/pip/_internal/commands/install.py Outdated Show resolved Hide resolved
src/pip/_internal/commands/install.py Outdated Show resolved Hide resolved
src/pip/_internal/commands/install.py Outdated Show resolved Hide resolved
@pradyunsg pradyunsg added the type: enhancement Improvements to functionality label Jun 11, 2021
Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

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

LGTM! Happy to have this merged once CI is happy. :)

@pradyunsg
Copy link
Member

Thanks for your contribution @OBITORASU! ^>^

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: enhancement Improvements to functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants