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

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

Closed
gilfree opened this issue Apr 2, 2024 · 10 comments
Closed
Labels
component: convert defect Something isn't working

Comments

@gilfree
Copy link

gilfree commented Apr 2, 2024

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: pypa/wheel#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):

  • OS: linux
  • whl2conda version 24.1.2

Additional context
Add any other context about the problem here.

@analog-cbarber analog-cbarber added defect Something isn't working component: convert labels Apr 2, 2024
@analog-cbarber
Copy link
Collaborator

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
Copy link
Author

gilfree commented Apr 3, 2024

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
Copy link
Collaborator

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
Copy link
Author

gilfree commented Apr 3, 2024

Seems wheel devs are responsive - pypa/wheel#609.

@analog-cbarber
Copy link
Collaborator

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

@agronholm
Copy link

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Author

gilfree commented Apr 14, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: convert defect Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants