feat: do not copy libraries in site-packages#392
feat: do not copy libraries in site-packages#392
Conversation
31183ae to
3b91821
Compare
|
@mayeut any thoughts? |
…ages for `auditwheel repair`
3b91821 to
7d89650
Compare
|
Could a maintainer allow running the workflow for this? NumPy and SciPy would like to create an openblas wheel, and this is a necessary component to not vendor the shared object into the NumPy/SciPy wheels. |
|
started |
|
builds are red sadly |
fd2fede to
eab8094
Compare
eab8094 to
6c9df74
Compare
Codecov ReportBase: 92.42% // Head: 91.62% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #392 +/- ##
==========================================
- Coverage 92.42% 91.62% -0.80%
==========================================
Files 23 23
Lines 1267 1290 +23
Branches 311 318 +7
==========================================
+ Hits 1171 1182 +11
- Misses 55 63 +8
- Partials 41 45 +4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| rpath_set = OrderedDict([(old_rpath, "") for old_rpath in rpaths]) | ||
| if rpath is not None: | ||
| rpath_set[rpath] = "" | ||
| rpaths = list(OrderedDict.fromkeys(rpaths)) |
There was a problem hiding this comment.
can we just use dict instead here?
There was a problem hiding this comment.
Since Python 3.6, the dict keys are kept in insertion order. But I think explicitly using OrderedDict improves readability.
|
Tests are needed to show this works. Perhaps you could parameterize the failing tests in
|
This comment was marked as abuse.
This comment was marked as abuse.
|
The tests passed for commit But failed after commit: |
e2aa465 to
3570012
Compare
ccca6c3 to
a691a55
Compare
|
The tests failed for grouping duplicate libraries by real soname. I removed the grouping mechanism. Tests passed on my local machine. However, there may be duplicate libraries copied into the wheel file. |
|
It has been a while @mayeut can we get some eyes on this? It would help move openblas packaging as a wheel forward. |
| % soname | ||
| ) | ||
|
|
||
| if not copy_site_libs and "site-packages" in str(src_path).split( |
There was a problem hiding this comment.
There's no guarantee for the site-packages folder to have that name (in fact, it doesn't on debian python, its name is dist-packages).
| action="store_false", | ||
| help=( | ||
| "Do not copy libraries located in the site-packages directory from " | ||
| "other packages. Just update the `rpath` only. The developer will " |
There was a problem hiding this comment.
unfortunately, just updating the rpath is not enough.
|
As mentioned in #391 (comment), it's not as easy as just updating the Given Given specific steps are required to ensure proper loading of shared object, I'm not sure we want to silently filter out all shared objects in @lkollar, any thoughts ? |
|
Closing this PR in favor of #368 |
Add a new option
--no-copy-site-libsto skip libraries in site-packages forauditwheel repair.This feature will significantly reduce the wheel file size if the referenced libraries are shipped with other PyPI packages.
NOTE: The developers may have their own risks in using this feature (e.g. they need to add the skipped library as an install dependency in
setup.py).Resolves #391