Skip to content

openldap: do strip, with proper fix#18132

Closed
layus wants to merge 1 commit intoNixOS:masterfrom
layus:openldap-do-strip
Closed

openldap: do strip, with proper fix#18132
layus wants to merge 1 commit intoNixOS:masterfrom
layus:openldap-do-strip

Conversation

@layus
Copy link
Copy Markdown
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
Copy Markdown

@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 layus force-pushed the openldap-do-strip branch from e165da2 to 136b4cf Compare August 30, 2016 21:33
@layus
Copy link
Copy Markdown
Member Author

layus commented Aug 30, 2016

also /cc @dezgeg @nshalman

@layus
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

3 participants