Find a replacement for patchelf

[lkollar]

We rely on the patchelf project to repair ELF files but unfortunately it has several bugs and the fixes often take a very long time to land in a release. Releases also seem to be very infrequent.

A few example issues we have faced:

• Teach --replace-needed to update .gnu.version_r table NixOS/patchelf#85 fixed Rename libraries on graft, fixes #24 #25 but the next release which shipped this fix happened only 3 years later.
• Remove zeroing out logic in modifySoname NixOS/patchelf#171 fixes a long standing bug: --set-soname corrupts binary if the original soname string is also used for symbol versioning NixOS/patchelf#155. The PR has not been reviewed yet and it's unclear when/if it would get released.
• Cannot force rpath NixOS/patchelf#94 has been open since 2016 and affects Vendored library cannot be loaded #29. @daa is working around this with Clear rpath before setting #173 which is probably more productive than trying to fix this in patchelf.

I think it would be useful to look at potential alternatives so that we can resolve similar issues quicker. The tasks patchelf implements in audithweel today are:

• Replace DT_NEEDED section
• Change SONAME
• Update RPATH

If we can find a replacement which can do these, has fewer bugs and ideally a healthier release cycle, we could consider replacing patchelf.

One potential viable alternative I found is LIEF. I think it does everything we need, but I have to test it out. It's also a wrapper around a C++ project and it does not seem to ship wheels which is problematic, but we might be able to help with that.

I'm curious if anyone knows of other alternatives we could consider.

[takluyver]

Should we consider forking patchelf to fix issues that affect auditwheel? Or would the maintainers be open to us helping out? Depending on the magnitude of the problems, there's a chance that's easier than modifying auditwheel to use a different tool.

[njsmith]

During the period when we were waiting for NixOS/patchelf#85, we did ship a custom patched build as part of manylinux image.

Another possibility to consider: contacting the patchelf maintainers and asking explicitly if they'd be up for adding some python folks as additional patchelf maintainers.

[lkollar]

I agree, we should definitely consider forking patchelf or helping out maintaining it. I did think of the fork option originally but somehow forgot to include it in the initial issue. I think trying to help out with the project would be best and creating a fork is a less ideal solution, so let's aim for helping out first.

I'm planning to open an issue in patchelf to gauge interest on taking on more maintainers and helping out in other ways to move the project forward. I'm happy to volunteer helping out. If anyone else wants to participate feel free to comment on the issue I'll link here.

[lkollar]

Created NixOS/patchelf#174. Feel free to comment on it if you want to offer help as well.

[lkollar]

I have this patch which uses LIEF instead of patchelf and it passes the test suite. This fixes #29 (patchelf bug we have a workaround for) and quite likely others as well. I should probably check if #159 gets fixed by this too.

The one caveat is that LIEF needs a modern C++ compiler so I had to build it for manylinux1 using a CentOS 5 build of GCC 6.3 with -static-libstdc++. I actually ran repair on the resulting wheel and ended up with a correctly tagged manylinux1 wheel, which could be uploaded to PyPI . I plan to raise this on their issue tracker and contribute this to the build process if they are interested.

Taking ownership in patchelf is a massive overtaking. I would still consider it if we had no alternatives, but I think in light of the success with LIEF we should definitely consider it as an alternative.

Given that this is still a serious change and it can cause unforeseen problems, I was thinking about adding it as an extra, experimental backend to auditwheel. Anyone experiencing issues with the patchelf backend can switch to LIEF and if that solves the issue we can count it as a good sign. At some point we can flip the default to LIEF and if nobody complains we can just remove patchelf. The code itself is fairly simple so it should not add much maintenance work.

[figroc]

It seems that #187 hasn't been merged yet. How can we try out the new patcher?

[lkollar]

LIEF 0.10.0 with proper manylinux1 support is available now so I spent some time updating the PR with the new version. However, I found a problem with RPATH patching which causes some tests to fail as the modified libraries end up containing a null NEEDED entry which triggers an assertion in ld.so. This might be due to a bug in LIEF. I’m planning to look into it soon.

[lkollar]

Unfortunately LIEF also has some stability issues. We cannot land #187 until lief-project/LIEF#374 is fixed, which has been open since December. In the meantime I applied my patch to patchelf in the manylinux images to avoid zeroing the SONAME entry. This fixed #159 for manylinux users. We might have to follow this approach for other patchelf bugs as well and patch our version until a fix is merged.

I will close #187 but I plan to extract the new ELF patcher interface from it and land it separately to make the code a bit more modular. It will also make it eaiser to integrate a different patcher if we eventually find an alternative.

[domenkozar]

I think the main problem is getting regression tests for patchelf, then releases could be more frequent.

[mayeut]

@lkollar,

It would be nice to get the patcher interface from #187 even if not with lief integration.

I was thinking of creating a patchelf python package auditwheel could depend on with patchelf + patches prebuilt for manylinux that respects that interface. This way, it'd be easier to upgrade patchelf on manylinux images (especially with PEP600 coming) and will allow for a wider use of auditwheel without the need to build patchelf (and missing patches).

Any thoughts ?

[JanuszL]

2 cents from my side. I just experimented in our project where we are using patchelf on several MB sized libraries - NVIDIA/DALI#1950 and LIEF is terribly slow Even printing rpath on a big library takes tens of seconds, replacing needed libraries takes several minutes as well (in case of patchelf it is just several seconds).
I think this approach could be no-go for projects with big binaries.

[lkollar]

It would be nice to get the patcher interface from #187 even if not with lief integration.

I opened #240 for this.

I was thinking of creating a patchelf python package auditwheel could depend on with patchelf + patches prebuilt for manylinux that respects that interface. This way, it'd be easier to upgrade patchelf on manylinux images (especially with PEP600 coming) and will allow for a wider use of auditwheel without the need to build patchelf (and missing patches).

I think this is a great idea. This would make it much easier to use the master branch of patchelf with the needed patches for any project, not only from the manylinux images. If you set up a repo for this I'm happy to help out with it.

[mayeut]

@lkollar,
I had time to setup a repo.
It's still work-in-progress (mostly lacking documentation, not sure about the name, shall it install patchelf binary for real?, the setup.py is a bit complicated also...)
If you have feedback, that would be nice.

[lkollar]

Given that we can consider this a fork of patchelf, I agree that the package should be distributed under a different name. I think we should use the original patchelf binary name though as 1. it's unlikely that anyone would want both of these tools on the same system (but if one needs it, it's fairly simple to install the Python package in a different path), 2. I expect we would only apply minor fixes to the upstream package, so it should be fairly safe to use it as a substitute.

If you open issues in the repo for things which need to be done, I can assign myself and start working on them.

[matthew-brett]

But - is there any reason to keep the binary name? Surely it would reduce the possibility for confusion if we change it to something like py-patchelf or aw-patchelf?

[lkollar]

If we installed the binary under the auditwheel_patchelf Python package (as it is done now), I think it's fine to leave it as patchelf as it won't appear on PATH.

[mayeut]

@lkollar,

I created issue https://github.com/mayeut/auditwheel-patchelf/issues/1 to discuss things.
In fact there are only 2 things on the list for now (check what's existing and licensing). Once they are answered, we might be able to move forward (which of course includes pooling resources with existing projects rather than having yet another solution).

Also mentioning @rmcgibbo, @jimporter as authors of packages that could do the same with update. The context is bit clearer in this issue so pinging from here for them to have some context. We can discuss furthermore in the linked issue.

[domenkozar]

Hey folks.

First of all an apology from Nix community that we're neglecting patchelf.

I've opened NixOS/patchelf#199 to help out here a bit.

[domenkozar]

0.11 was just released today with fixes for all the listed patches here.

[lkollar]

That is great news! Much appreciated @domenkozar.

[domenkozar]

With 1.3 released and a new maintainer, are you still seeing bugs?

[lkollar]

I don't recall any recent issues and I think all of the ones I mentioned in the description have been fixed. Now that patchelf development has picked up again I think we can close this issue. Thanks @domenkozar.

[john-science]

Sorry to be late to the party on this one, but I am on Ubuntu 20.04 and I don't seem to be able to install a newer version of patchelf than 0.10 and auditwheel is complaining that I need 0.14 or newer.

This is probably an Ubuntu problem, and not yours. But has anyone hit this problem before?

UPDATE for future readers: I have a server that I would have preferred to keep on Ubuntu 20.04 for 1 more year (until the release of 24.04, the LTS). But I had to abandon that notion. Updating Ubuntu to 22.04 solved the impossible quagmire of patchelf dependency management. I consider this to have been an Ubuntu issue, not a patchelf one.