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

Fix copy of files from .data dir of wheels if the files already exist #1371

Closed
wants to merge 1 commit into from

Conversation

wiggin15
Copy link

See issue #1362.
From "rename" docs:

On Unix, if dst exists and is a file, it will be replaced silently if the user has permission. [...] On Windows, if dst already exists, OSError will be raised even if it is a file

To make the behavior the same on both platforms, this PR manually removes existing files before rename.

@benoit-pierre
Copy link
Member

I don't agree with this: if the wheel contains duplicate entries, then it's broken. And the failure on Windows is correct and we should instead make sure the installation fails similarly on Linux.

@wiggin15
Copy link
Author

But pip and wheel have no trouble installing such wheel files (e.g. selenium's whl). Since setuptools is the only one that can't, it appears to be a problem with setuptools.

@benoit-pierre
Copy link
Member

Pip will happily overwrite existing file in a number of circumstances, hardly the behavior expected of a package manager.

@wiggin15
Copy link
Author

wiggin15 commented May 17, 2018

Umm.. pip is an widely used package manager... And even the "wheel" package works:

 wget https://files.pythonhosted.org/packages/57/bc/17164fd471ccdf0df3a992c710c0c3c47743462ff41ab72a02c6ede96e90/selenium-3.12.0-py2.py3-none-any.whl
python -c "import wheel.install; wheel.install.WheelFile('selenium-3.12.0-py2.py3-none-any.whl').install()"

So I can't open a ticket to selenium saying their wheel distribution is broken - because it doesn't look like it is.

@pganssle
Copy link
Member

At the very least I think this behavior should raise a warning on any OS where files are overwritten.

@benoit-pierre
Copy link
Member

Pip, setuptools, and wheel have different implementations, so a wheel package can be invalid and still installs successfully with one of those. For example some of the older PyQ5 wheels were created with invalid RECORD metadata (wrong checksum calculation), but since pip does not check those, it has not trouble installing such a wheel (the wheel implementation does check those, we don't either).

@wiggin15
Copy link
Author

Aren't setuptools, pip and wheel all part of the Python packaging alliance? Wasn't the whole point of the wheel format to be well-defined and well-documented (as opposed to eggs)? If there are differences between the reference implementation, the de facto package manager and other implementations - shouldn't these differences be reconciled? What is the correct process to resolve this?

@jaraco
Copy link
Member

jaraco commented May 18, 2018

shouldn't these differences be reconciled? What is the correct process to resolve this?

Yes; First step, maybe ask @ncoghlan if he has an opinion on the matter. It may be an easy call for him to make. If it's not clear-cut and obvious, then the proper thing to do is raise an issue with packaging problems, and it will be settled there.

@ncoghlan
Copy link
Member

I think a packaging-problems issue would be a good option here, as another case where this might come up is with files that differ only by case, which will work on case sensitive file systems, but fail on case insensitive ones.

@wiggin15
Copy link
Author

Thanks. Opened pypa/packaging-problems#152

@jaraco
Copy link
Member

jaraco commented Sep 16, 2018

I'm going to close this for now, but happy to re-open if that's what's determined to be the best course in the upstream discussion.

@jaraco jaraco closed this Sep 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants