zuzukin / whl2conda

Generate conda package from pure python wheel
https://zuzukin.github.io/whl2conda/
Apache License 2.0
6 stars 2 forks source link

Executable permissions are lost when converting to wheel to conda package #135

Closed gilfree closed 7 months ago

gilfree commented 8 months ago

Describe the bug If a wheel file contains a file with executable permissions - they will be lost when converting to a conda package.

To Reproduce Create a python package with a script that has executable permission (chmod +x) and convert to conda.

Expected behavior The executable permissions should be kept.

The root cause of the bug is in here: https://github.com/pypa/wheel/issues/608. To fix it, until wheel is fixed _extract_wheel method need to be updated:

    def _extract_wheel(self, temp_dir: Path) -> Path:
        self.logger.info("Reading %s", self.wheel_path)
        wheel = WheelFile(self.wheel_path)
        wheel_dir = temp_dir / "wheel-files"
        wheel.extractall(wheel_dir)

To something along the lines of:

   wheel_dir = temp_dir / "wheel-files"
    with WheelFile(self.wheel_path) as wf:
        namever = wf.parsed_filename.group("namever")
        for zinfo in wf.filelist:
            wf.extract(zinfo, destination)

            # Set permissions to the same values as they were set in the archive
            # We have to do this manually due to
            # https://github.com/python/cpython/issues/59999
            permissions = zinfo.external_attr >> 16 & 0o777
            wheel_dir.joinpath(zinfo.filename).chmod(permissions)
        if self.logger.getEffectiveLevel() <= logging.DEBUG:
            for wheel_file in wheel_dir.glob("**/*"):
                if wheel_file.is_file():
                    self._debug("Extracted %s", wheel_file.relative_to(wheel_dir))
        return wheel_dir

(Code taken from: https://github.com/pypa/wheel/blob/0a4f40ecf967757b43e2cdfd4b3c52b16d15614a/src/wheel/cli/unpack.py#L24)

Desktop (please complete the following information):

Additional context Add any other context about the problem here.

analog-cbarber commented 8 months ago

Thanks for the report!

Obviously, the lazy solution is to wait for this to get fixed in wheel, but I am open to a PR if you want to submit one.

If you do, I think you should provide an _extract_all function that will act as a drop-in replacement for wheel.extractall and also make sure to provide a test case.

gilfree commented 7 months ago

I'm ok with waiting a few weeks to see what wheel's response will be. If they won't solve it, I'll try to find time to submit a PR.

analog-cbarber commented 7 months ago

One workaround for this would be to convert the wheel to a directory tree instead of a V1 or V2 format package and then manually fix the permissions of the script files and then use cph create to bundle the directory to the desired format. The cph program is from the conda-package-handling package, which is a dependency of whl2conda.

e.g.

$ whl2conda convert foo-1.2.3-py3-none-any.whl --format tree
$ chmod +x foo-1.2.3-py_0/scripts/*.py
$ cph create foo-1.2.3-py_0 foo-1.2.3-py_0.conda
gilfree commented 7 months ago

Seems wheel devs are responsive - https://github.com/pypa/wheel/pull/609.

analog-cbarber commented 7 months ago

I should update the wheel dependency when this fix is available...

agronholm commented 7 months ago

I must point out that wheel does not currently have a public API, so making a tool that imports wheel is currently dubious. Is there no way to use its CLI instead?

analog-cbarber commented 7 months ago

I guess that technically it is dubious but in practice it is highly unlikely that code is actually going to change. The current API has been in place for 6 years already,

Yes, I do believe that I could use wheel unpack instead and throw away the log output (and I guess that would also fix this issue?). It would be a bit slower due to having to make a subprocess call.

analog-cbarber commented 7 months ago

Another issue with wheel unpack is that it unpacks into a subdirectory of the dest dir with name parsed from the wheel file name, so I would have to deal with that extra subdir.

analog-cbarber commented 7 months ago

Another issue with file permissions is that is not really supported on Windows. On windows calling chmod() won't have any effect for the permission bits, so if you run the conversion on Windows it will still throw away the file permissions. I think that fixing that would require actually going into the conda zip compression code and probably would not be practical to fix.

gilfree commented 7 months ago

Thanks!