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

openldap: do strip, with proper fix #18132

Closed
wants to merge 1 commit into from

Conversation

layus
Copy link
Member

@layus layus commented Aug 30, 2016

This is a proper fix of the build to work with elf stripping.

  • tested executables with ldd on NixOS.

See 366c1e8 for some insight on the issue.

@mention-bot
Copy link

@layus, thanks for your PR! By analyzing the annotation information on this pull request, we identified @rickynils, @lethalman and @edolstra to be potential reviewers

@layus
Copy link
Member Author

layus commented Aug 30, 2016

also /cc @dezgeg @nshalman

@layus
Copy link
Member Author

layus commented Aug 30, 2016

Now, there may be a better fix than this one. The issue resides with the rpath of the executables containing the build path as their first element. When shrinking, that element is kept and the expected /nix/store location removed. I like this solution as it is very straightforward :-).

@dezgeg
Copy link
Contributor

dezgeg commented Aug 31, 2016

The proper fix would be NixOS/patchelf#98. But that is too late for 16.09, so this is a nice solution for the time being.

I will merge this along with #17916 tomorrow (since they rebuild roughly the same stuff).

@dezgeg dezgeg self-assigned this Aug 31, 2016
@dezgeg dezgeg added this to the 16.09 milestone Aug 31, 2016
@dezgeg
Copy link
Contributor

dezgeg commented Sep 1, 2016

Merged, thanks.

@dezgeg dezgeg closed this Sep 1, 2016
dezgeg referenced this pull request Sep 1, 2016
The problem here was that the openldap binaries had /tmp/... in their
RPATH *before* $out/lib, so patchelf --shrink-rpath considered the
$out/lib entry unused.

As a workaround, use NIX_LDFLAGS_BEFORE to ensure a proper order.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants