Skip to content

Hook to replace auditwheel/delocate/[windows equivalent that doesn't exist yet] #191

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

Closed
zackw opened this issue Nov 5, 2019 · 8 comments · Fixed by #211
Closed

Hook to replace auditwheel/delocate/[windows equivalent that doesn't exist yet] #191

zackw opened this issue Nov 5, 2019 · 8 comments · Fixed by #211

Comments

@zackw
Copy link

zackw commented Nov 5, 2019

I would like a hook that would allow me to run a command of my own in place of the auditwheel or delocate invocation (on Linux or MacOS respectively), or at the point in the process where an equivalent tool would be invoked on Windows, if one existed.

My primary reason for wanting this is that I need the equivalent tool that doesn't yet exist for Windows. My package is simple enough that I can probably kludge together something that works well enough for me, but I need a way to make cibuildwheel run it.

A plausible interface would be: environment variables named CIBW_FIXUP_WHEEL_${PLATFORM} that take the name of a program. This program will be invoked with two arguments: the absolute pathname of a wheel to process, and the absolute pathname to write the processed filename to. On Linux the default is a wrapper for auditwheel, on MacOS the default is a wrapper for delocate, and on Windows it's copy.

@YannickJadoul
Copy link
Member

@zackw I'm a bit in doubt here, because auditwheel and delocate are official tools to create valid, PyPI-uploadable packages, and allowing those to be messed with feels "dangerous".

Could you motivate a bit further why you need it? And why, if your use case is quite unique anyway, you run your script after cibuidwheel, on all the *.whl files that have win32 or win_amd64 in their name?

@zackw
Copy link
Author

zackw commented Nov 6, 2019

Running the hypothetical script after cibuildwheel finishes would not work. It needs to be run on each wheel before it is installed and tested, so that the wheel that gets tested has been fixed up. (Specifically, it should run in between pip wheel and pip install. This is the point where auditwheel and delocate are run, on their respective platforms.) Right now my tests all fail on Windows because the necessary DLLs are not found. I could possibly work around this by setting $PATH but then I wouldn't be testing the same thing I am putting on PyPI.

I don't myself need to override auditwheel or delocate on their respective platforms, but I could see someone working on a tool like those wanting it, and it would be weird to have a hook on Windows only.

@YannickJadoul
Copy link
Member

It needs to be run on each wheel before it is installed and tested,

Right, I should have thought of that possibility; sorry!

I don't myself need to override auditwheel or delocate on their respective platforms, but I could see someone working on a tool like those wanting it

Minor detail is that you would be testing on the same platform as you have built, so maybe cibuidlwheel's not optimal for this kind of tests.

@joerick Any thoughts? A PR shouldn't be too hard, if you have any intuition on where you'd want to go with this.

@mayeut
Copy link
Member

mayeut commented Nov 6, 2019

@zackw,
A quick workaround if you're in a hurry:
For windows, it can already be done by overriding the bdist_wheel command in setup.py, if you're using that to build your wheels.
For macOS/Linux, this trick depends on auditwheel/delocate being no-ops on repaired wheel.

The override of the bdist_wheel command can be made optional by checking for the CIBUILDWHEEL environment variable.

As for a PR proposition, I'd use CIBW_REPAIR_WHEEL_${PLATFORM} that takes a command line and provides {project} (like CIBW_BEFORE_BUILD and CIBW_TEST_COMMAND) but also {wheel} and {dest_dir}

@nsoranzo
Copy link
Contributor

I also need this to allow to specify an option for auditwheel, so I started working on this and I have working code which I just need to clean up.
I will open a PR as soon as possible.

@YannickJadoul
Copy link
Member

@nsoranzo Which options do you need? Maybe they should be settable through cibuildwheel, as we're trying to abstract away the existence/use of auditwheel.

@nsoranzo
Copy link
Contributor

nsoranzo commented Nov 12, 2019

@YannickJadoul I need -L . to fix pysam-developers/pysam#831

@YannickJadoul
Copy link
Member

YannickJadoul commented Nov 12, 2019

@nsoranzo Thanks! :) OK, that's a bit of a corner-case indeed, it seems. A bit funny that setting the rpath correctly or so wouldn't work, but fair enough. Anyhow, in that case, it's maybe not up to cibuildwheel to provide an option to just change that, indeed.

nsoranzo added a commit to nsoranzo/cibuildwheel that referenced this issue Nov 12, 2019
to allow different options to auditwheel/delocate, alternative
commands and a future Windows-equivalent.

Fix pypa#191 .
nsoranzo added a commit to nsoranzo/cibuildwheel that referenced this issue Nov 13, 2019
to allow different options to auditwheel/delocate, alternative
commands and a future Windows-equivalent.

Fix pypa#191 .
nsoranzo added a commit to nsoranzo/cibuildwheel that referenced this issue Nov 13, 2019
to allow different options for auditwheel/delocate, alternative
commands and a future Windows equivalent.

Fix pypa#191 .
nsoranzo added a commit to nsoranzo/cibuildwheel that referenced this issue Nov 13, 2019
to allow different options for auditwheel/delocate, alternative
commands and a future Windows equivalent.

Fix pypa#191 .
nsoranzo added a commit to nsoranzo/cibuildwheel that referenced this issue Nov 14, 2019
to allow different options for auditwheel/delocate, alternative
commands and a future Windows equivalent.

Fix pypa#191 .
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 a pull request may close this issue.

4 participants