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

patchelf: fix golang support #57656

Closed
wants to merge 1 commit into from
Closed

patchelf: fix golang support #57656

wants to merge 1 commit into from

Conversation

booxter
Copy link
Contributor

@booxter booxter commented Mar 14, 2019

Ideally, we would have the fix released in an official tarball and we
would pick it up in nixpkgs but since there was no new release of the
tool in the last three years, and the issue that asks to release a new
version is open for at least two years now, it's probably better to not
wait for this to happen and cherry-pick the fix.

Ideally, we would have the fix released in an official tarball and we
would pick it up in nixpkgs but since there was no new release of the
tool in the last three years, and the issue that asks to release a new
version is open for at least two years now, it's probably better to not
wait for this to happen and cherry-pick the fix.
@infinisil
Copy link
Member

What specifically does this fix in regards to Go?

@booxter
Copy link
Contributor Author

booxter commented Mar 28, 2019

@infinisil patchelf is now capable of patching (some?) binaries it couldn't patch before. For one, this is the patch that helped me with running a binary for functional tests built for kubevirt project. (The binary is built inside a fedora docker container and linked to fedora library paths.)

The upstream bug is: NixOS/patchelf#66

@infinisil
Copy link
Member

I see. If we're going to cause a mass rebuild by updating patchelf, I think it's be a better idea to update it to the master version instead (since there's no new release). Can you open a PR for that instead?

@edolstra Alternatively, could you release a new patchelf version?

@booxter
Copy link
Contributor Author

booxter commented Mar 28, 2019

@infinisil I was trying to stay low profile with the change. :) I can probably propose something for master but I don't know if I need to do any extensive testing for the change when doing it. It's 3 years of development in a core component. I would not be surprised if things break on the way.

@infinisil
Copy link
Member

infinisil commented Mar 28, 2019

nixpkgs is most likely the biggest user of patchelf, so if there's problems with patchelf, the sooner we use the latest version, the sooner we can fix those problems, if there are any.

On the other hand, we probably wouldn't break anything with this simple patch, so we might as well just merge this.. I'd really like to have a newer version of patchelf though. Am conflicted.

@infinisil
Copy link
Member

After talking about it on IRC, @edolstra agreed to make a new patchelf release :)

Can you make a PR for that update?

@booxter
Copy link
Contributor Author

booxter commented Mar 29, 2019

@infinisil I will update with a link to PR that bumps the version. Thanks for checking with upstream.

@booxter
Copy link
Contributor Author

booxter commented Mar 29, 2019

That being said, I don't think we should necessarily switch to latest for release branches, that may make use of a fix for golang. So if you think a backport would be acceptable in release branches, please tell me so, I will then propose separate PRs.

@booxter
Copy link
Contributor Author

booxter commented Mar 29, 2019

@infinisil here is the PR to bump patchelf version: #58518

Again, if you think it's acceptable to go with a backport of a particular patch for release branches to avoid unexpected breakages due to three years of development, please tell me.

@domenkozar domenkozar closed this Mar 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants