Add support for musllinux #315
Conversation
Codecov Report
@@ Coverage Diff @@
## master #315 +/- ##
==========================================
- Coverage 91.28% 89.07% -2.22%
==========================================
Files 20 23 +3
Lines 1102 1199 +97
Branches 237 250 +13
==========================================
+ Hits 1006 1068 +62
- Misses 54 82 +28
- Partials 42 49 +7
Continue to review full report at Codecov.
|
Sounds good to me. TBH I wasn't completely happy with how the policy loading ended up looking and I agree that it would be better to come back to this and properly refactor it later. I'll close my PR in favour of this one. |
mayeut
force-pushed
the
musllinux-simple
branch
2 times, most recently
from
0a00272
to
66a0783
Compare
|
@lkollar, I split the PR in 2:
I think it's now good enough for review even though, as stated earlier, a refactor of the whole policy selection/loading shall be reworked later on. |
| assert len(ld_musl) <= 1 | ||
| if len(ld_musl) == 0: | ||
| ldpaths['conf'] = [ | ||
| root + '/lib', |
There was a problem hiding this comment.
I wonder if the prefix should be taken into account here.
There was a problem hiding this comment.
I'm not sure. I guess it won't matter until cross-repairing is a thing.
PS: for glibc, some folders are appended without either root or prefix.
| ldpaths['conf'].append(root + ldpath_stripped) | ||
| else: | ||
| # Load up /etc/ld.so.conf. | ||
| ldpaths['conf'] = parse_ld_so_conf(root + prefix + '/etc/ld.so.conf', |
There was a problem hiding this comment.
| ldpaths['conf'] = parse_ld_so_conf(root + prefix + '/etc/ld.so.conf', | |
| ldpaths['conf'] = parse_ld_so_conf(str(root_prefix / 'etc/ld.so.conf'), |
There was a problem hiding this comment.
I'd rather not touch the glibc code path for now. We can get this cleaned up later.
| ldpath_stripped = ldpath.strip() | ||
| if ldpath_stripped == "": | ||
| continue | ||
| ldpaths['conf'].append(root + ldpath_stripped) |
There was a problem hiding this comment.
Prefix is not taken into account here either but I'm not actually sure how the prefix logic is supposed to work in this function so maybe it's fine.
There was a problem hiding this comment.
This could be:
| ldpaths['conf'].append(root + ldpath_stripped) | |
| ldpaths['conf'].append(str(root_prefix / ldpath_stripped)) |
There was a problem hiding this comment.
Same comment as earlier, it won't matter until cross-repairing is a thing.
It mimics parse_ld_so_conf where only root is taken into account.
The prefix seems to only be used for the configuration on glibc so I did the same on musl for now.
Detect the running libc at startup & select the correct policy file based on this. For musllinux, we select only one musllinux policy depending on the running version of musl.
mayeut
force-pushed
the
musllinux-simple
branch
from
ce52d0a
to
e34bb79
Compare
|
are my answers enough for you to finish the review ? |
This aims to modify less things than #313 at first to ease the review.
Once there are sufficient tests, the added complexity of #313 might be something to look into as it might be a good step forward to cross-repairing/cross-checking wheels.
@lkollar, thanks for the work in #313 (and in manylinux). What do you think of doing this in 2 steps as mentioned above?