pypa / installer

A low-level library for installing from a Python wheel distribution.
https://installer.readthedocs.io/
MIT License
126 stars 51 forks source link

Ignore existing script files #170

Closed flode closed 1 year ago

flode commented 1 year ago

When a wheel has an entry_points.txt script and another script with the same name. Then installer fails to install the package and complains about an already existing file.

Repro: python3 -m installer --destdir tmp conjur_client-0.1.1-py3-none-any.whl

That wheel has a file conjur_client-0.1.1.data/scripts/conjur-cli that gets copied as well as the autogenerated script from entry_points.txt

[console_scripts]
conjur-cli = conjur:Cli.launch

Pip seems to not mind that and just ignore the script file. This commit does the same.

wuch-g2v commented 1 year ago

I have the same issue #182

eli-schwartz commented 1 year ago

This is, technically, an invalid wheel. At the very least I'd expect a warning.

I don't think that bug for bug compatibility with pip is necessarily an objective, although probably pip should be saying something also...

wuch-g2v commented 1 year ago

This is, technically, an invalid wheel. At the very least I'd expect a warning.

You mean that it is issue with wheel module? 🤔

pradyunsg commented 1 year ago

Hi there! Thanks for filing this but this isn't a change we're going to be making in this project.

The corresponding behaviour is considered a bug in pip (https://github.com/pypa/pip/issues/4625) and mimicking it is not intended in this library.

If you want to, you can write your own write_to_fs and mimick pip bug-for-bug.

https://github.com/python-poetry/poetry/blob/3804a131925854644731923d5c37946b215a71ec/src/poetry/installation/wheel_installer.py#L28 does this for example.

You mean that it is issue with wheel module?

No, rather that conjur_client-0.1.1-py3-none-any.whl is a wheel that has inappropriate contents, since it has two files that are somehow supposed to end up in the same location. This obviously should not work, but pip permits this as noted above.

wuch-g2v commented 1 year ago

You mean that it is issue with wheel module?

No, rather that conjur_client-0.1.1-py3-none-any.whl is a wheel that has inappropriate contents, since it has two files that are somehow supposed to end up in the same location. This obviously should not work, but pip permits this as noted above.

I'm not 100% sure do I understand correctly above .. If that is true IMO it means that wheel module has the issue because it allows form .whl archive which such clash and it should detect that such situations and end with error. Am I right?

wuch-g2v commented 1 year ago

BTW just tested this PR and allowed me to install pecan but after that on testing with all my +1.k rpm packages in which packaging procedure I'm using installer on unpacking .whl to destir I found that it caused that pdm test suite started failing 😞

```console =================================== FAILURES =================================== _________________ test_install_wheel_with_data_scripts[False] __________________ project = use_install_cache = False @pytest.mark.parametrize("use_install_cache", [False, True]) def test_install_wheel_with_data_scripts(project, use_install_cache): req = parse_requirement("jmespath") candidate = Candidate( req, link=Link("http://fixtures.test/artifacts/jmespath-0.10.0-py2.py3-none-any.whl"), ) installer = InstallManager(project.environment, use_install_cache=use_install_cache) > installer.install(candidate) tests/test_installer.py:160: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ ../../BUILDROOT/python-pdm-2.5.3-2.fc39.x86_64/usr/lib/python3.8/site-packages/pdm/installers/manager.py:33: in install installer(str(prepared.build()), self.environment, prepared.direct_url()) ../../BUILDROOT/python-pdm-2.5.3-2.fc39.x86_64/usr/lib/python3.8/site-packages/pdm/installers/installers.py:172: in install_wheel _install_wheel(wheel=wheel, destination=destination, additional_metadata=additional_metadata) ../../BUILDROOT/python-pdm-2.5.3-2.fc39.x86_64/usr/lib/python3.8/site-packages/pdm/installers/installers.py:268: in _install_wheel install(source, destination, additional_metadata=additional_metadata or {}) /usr/lib/python3.8/site-packages/installer/_core.py:109: in install record = destination.write_file( _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = scheme = 'scripts', path = 'jp.py', stream = is_executable = True def write_file( self, scheme: Scheme, path: Union[str, "os.PathLike[str]"], stream: BinaryIO, is_executable: bool, ) -> RecordEntry: """Write a file to correct ``path`` within the ``scheme``. :param scheme: scheme to write the file in (like "purelib", "platlib" etc). :param path: path within that scheme :param stream: contents of the file :param is_executable: whether the file should be made executable - Changes the shebang for files in the "scripts" scheme. - Uses :py:meth:`SchemeDictionaryDestination.write_to_fs` for the filesystem interaction. """ path_ = os.fspath(path) if scheme == "scripts": with fix_shebang(stream, self.interpreter) as stream_with_different_shebang: > return self.write_to_fs( scheme, path_, stream_with_different_shebang, is_executable, True ) E TypeError: write_to_fs() takes 5 positional arguments but 6 were given /usr/lib/python3.8/site-packages/installer/destinations.py:204: TypeError ---------------------------- Captured stdout setup ----------------------------- Changes are written to pyproject.toml. Changes are written to pyproject.toml. ------------------------------ Captured log call ------------------------------- INFO unearth.preparer:preparer.py:309 Downloading (size unknown) __________________ test_install_wheel_with_data_scripts[True] __________________ project = use_install_cache = True @pytest.mark.parametrize("use_install_cache", [False, True]) def test_install_wheel_with_data_scripts(project, use_install_cache): req = parse_requirement("jmespath") candidate = Candidate( req, link=Link("http://fixtures.test/artifacts/jmespath-0.10.0-py2.py3-none-any.whl"), ) installer = InstallManager(project.environment, use_install_cache=use_install_cache) > installer.install(candidate) tests/test_installer.py:160: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ ../../BUILDROOT/python-pdm-2.5.3-2.fc39.x86_64/usr/lib/python3.8/site-packages/pdm/installers/manager.py:33: in install installer(str(prepared.build()), self.environment, prepared.direct_url()) ../../BUILDROOT/python-pdm-2.5.3-2.fc39.x86_64/usr/lib/python3.8/site-packages/pdm/installers/installers.py:195: in install_wheel_with_cache _install_wheel(wheel=wheel, destination=destination) ../../BUILDROOT/python-pdm-2.5.3-2.fc39.x86_64/usr/lib/python3.8/site-packages/pdm/installers/installers.py:268: in _install_wheel install(source, destination, additional_metadata=additional_metadata or {}) /usr/lib/python3.8/site-packages/installer/_core.py:109: in install record = destination.write_file( _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = scheme = 'scripts', path = 'jp.py', stream = is_executable = True def write_file( self, scheme: Scheme, path: Union[str, "os.PathLike[str]"], stream: BinaryIO, is_executable: bool, ) -> RecordEntry: """Write a file to correct ``path`` within the ``scheme``. :param scheme: scheme to write the file in (like "purelib", "platlib" etc). :param path: path within that scheme :param stream: contents of the file :param is_executable: whether the file should be made executable - Changes the shebang for files in the "scripts" scheme. - Uses :py:meth:`SchemeDictionaryDestination.write_to_fs` for the filesystem interaction. """ path_ = os.fspath(path) if scheme == "scripts": with fix_shebang(stream, self.interpreter) as stream_with_different_shebang: > return self.write_to_fs( scheme, path_, stream_with_different_shebang, is_executable, True ) E TypeError: write_to_fs() takes 5 positional arguments but 6 were given /usr/lib/python3.8/site-packages/installer/destinations.py:204: TypeError ---------------------------- Captured stdout setup ----------------------------- Changes are written to pyproject.toml. Changes are written to pyproject.toml. ------------------------------ Captured log call ------------------------------- INFO unearth.preparer:preparer.py:309 Downloading (size unknown) INFO pdm.termui:installers.py:188 Installing wheel into cached location /tmp/pytest-of-tkloczko/pytest-123/test_install_wheel_with_data_s1/caches/packages/jmespath-0.10.0-py2.py3-none-any =========================== short test summary info ============================ ```

So as @pradyunsg wrote this fix is incorrect.

pradyunsg commented 1 year ago

If that is true IMO it means that wheel module has the issue because it allows form .whl archive which such clash and it should detect that such situations and end with error. Am I right?

I'm not sure if it's wheel or setuptools or something else.

The underlying reasoning for this issue is that the wheel is composed inappropriately. How and why that's been created is something I'm not going to be investigating. 😅

wuch-g2v commented 1 year ago

IMO it is wheel bug because it should not generate .whl end exit with non-zero exit code if it is overlapping script and [console_scripts] like it is now in pecan https://github.com/pecan/pecan/blob/cdb385ae8418f94b17be7b4ac1763a1ae7984b40/setup.py#L85-L100

PS. I must sent PR to fix that in pecan