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

allow copying files into an already existing directory #199

Open
mattip opened this issue Feb 5, 2024 · 5 comments
Open

allow copying files into an already existing directory #199

mattip opened this issue Feb 5, 2024 · 5 comments
Labels

Comments

@mattip
Copy link

mattip commented Feb 5, 2024

Describe the bug
A clear and concise description of what the bug is.

I would like to ship a build of OpenBLAS as a wheel. On posix, the build ships a libscipy-openblas.so which depends on libgfortran.so and libquadmath.so. The scipy-openblas shared object is the target bundled in the wheel under lib, then auditwheel/delocate is used to copy libgfortran and libquadmath into the lib directory. On auditwheel I can do this using --lib-sdir /lib. But in delocate, using delocate-wheel -L /lib raises an error here (note there is also a typo in the use of f-string, the f should be outside the quotes):

if copied_libs and lib_path_exists_before_delocate:
raise DelocationError(
"f{lib_path} already exists in wheel but need to copy "
+ "; ".join(copied_libs)

To Reproduce
Steps used to reproduce the behavior, such as the delocate commands used.

Expected behavior
A clear and concise description of what you expected to happen.

Wheels used
If a wheel is involved then consider attaching the original wheel (before being delocated) or linking to the repository where the original wheel can be created.

Platform (please complete the following information):

  • OS version: [e.g. macOS 12, macOS 10.15]
  • Delocate version: [e.g. 0.10.0]

Additional context
Add any other context about the problem here.

What was the reasoning for wanting a clean directory?

@mattip mattip added the bug label Feb 5, 2024
@mattip
Copy link
Author

mattip commented Feb 5, 2024

here is the code in question

@HexDecimal
Copy link
Collaborator

If I recall correctly it was a safety check since I wasn't sure how to handle a situation where a library references an external dependency but that dependency already exists inside the package. Without this check the internal file would've been silently clobbered.

What does auditwheel do in this situation? Does it reuse or clobber the internal file?

@HexDecimal
Copy link
Collaborator

At the very least I could probably change this into a warning instead of an error.

@mattip
Copy link
Author

mattip commented Feb 6, 2024

Does it reuse or clobber the internal file?

I'm not sure. That would be a strange situation, but then maybe the check should be more fine-grained. If I understand correctly the code currently will error out if the directory pre-exists. I could make a PR to check my assumptions, if you think it makes sense.

@HexDecimal
Copy link
Collaborator

You're right. It seems it'd delete the folders contents and start over if this check didn't stop it. I'm wondering if that's actually desirable or what the edge cases are.

Looking into it more, I did not write this other than to mess up converting it to an f-string. This check was here for decades. Original code for comparision.

I suspect it's surviving multiple refactors because everyone including me has been too afraid to touch it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants