Skip to content

stdenv/setup: tell libtool about library paths#144749

Merged
alyssais merged 1 commit intoNixOS:stagingfrom
alyssais:libtool-static
Nov 23, 2021
Merged

stdenv/setup: tell libtool about library paths#144749
alyssais merged 1 commit intoNixOS:stagingfrom
alyssais:libtool-static

Conversation

@alyssais
Copy link
Member

@alyssais alyssais commented Nov 5, 2021

Packages that use libtool run it as a wrapper around the linker. Before calling the linker, libtool will determine what libraries would be linked, and check if there's a corresponding libtool archive (libfoo.la) file in the same directory . This file contains extra information about the library. This is especially important for static linking, because static archives don't contain dependency information, so we need libtool to use the .la files to figure out which libraries actually need to be linked against.

But in Nixpkgs, this has never worked. libtool isn't able to find any libraries, because only the compiler wrapper knows how to find them, and the compiler wrapper is opaque to libtool. This is why pkgsStatic.util-linuxMinimal doesn't build prior to this patch — it depends on libpam, which depends on libaudit, and if libtool can't find the .la file, nothing will tell the linker to also link against libaudit when linking libpam. (It was previously possible to build a static util-linux, because linux-pam only recently had the audit dependency added.)

There are a couple of ways we could fix this, so that libtool knows where to look for .la files.

  • Set LD_LIBRARY_PATH/DYLD_LIBRARY_PATH/whatever, which libtool will examine. This would have major side effects though, because the dynamic linker looks at it too.

  • Inject libtool scripts with the appropriate information. That's what I've done here. It was the obvious choice because we're already finding and modifying the libtool scripts, to remove paths outside the Nix store that libtool might check in unsandboxed builds. Instead of emptying out the system paths, we can repopulate it with our own library paths.

(We can't use a wrapper like we do for other tools in Nixpkgs, because libtool scripts are often distributed in source tarballs, so we can't just add a wrapped version of libtool as a dependency. That's why there's already the fixLibtool function in stdenv.)

With this change, libtool is able to discover .la files, and pkgsStatic.util-linuxMinimal can build again, linking correctly against libpam and libaudit.

Packages that use libtool run it as a wrapper around the linker.
Before calling the linker, libtool will determine what libraries would
be linked, and check if there's a corresponding libtool
archive (libfoo.la) file in the same directory .  This file
contains extra information about the library.  This is especially
important for static linking, because static archives don't contain
dependency information, so we need libtool to use the .la files to
figure out which libraries actually need to be linked against.

But in Nixpkgs, this has never worked.  libtool isn't able to find any
libraries, because only the compiler wrapper knows how to find them,
and the compiler wrapper is opaque to libtool.  This is why
pkgsStatic.util-linuxMinimal doesn't build prior to this patch — it
depends on libpam, which depends on libaudit, and if libtool can't
find the .la file, nothing will tell the linker to also link against
libaudit when linking libpam.  (It was previously possible to build a
static util-linux, because linux-pam only recently had the audit
dependency added.)

There are a couple of ways we could fix this, so that libtool knows
where to look for .la files.

 * Set LD_LIBRARY_PATH/DYLD_LIBRARY_PATH/whatever, which libtool will
   examine.  This would have major side effects though, because the
   dynamic linker looks at it too.

 * Inject libtool scripts with the appropriate information.  That's
   what I've done here.  It was the obvious choice because we're
   already finding and modifying the libtool scripts, to remove paths
   outside the Nix store that libtool might check in unsandboxed
   builds.  Instead of emptying out the system paths, we can
   repopulate it with our own library paths.

(We can't use a wrapper like we do for other tools in Nixpkgs, because
libtool scripts are often distributed in source tarballs, so we can't
just add a wrapped version of libtool as a dependency.  That's why
there's already the fixLibtool function in stdenv.)

With this change, libtool is able to discover .la files, and
pkgsStatic.util-linuxMinimal can build again, linking correctly
against libpam and libaudit.
@alyssais alyssais added 6.topic: static Static builds (e.g. pkgsStatic) 6.topic: stdenv Standard environment labels Nov 5, 2021
fixLibtool() {
sed -i -e 's^eval sys_lib_.*search_path=.*^^' "$1"
local search_path
for flag in $NIX_LDFLAGS; do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to account for the role here? If libtool is ever used in depsBuildBuild we need to look into a different variable or does that never happen?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100%, but I don't think so — remember, libtool isn't a dependency we're providing (and when we are providing it, all that it's doing is generating the script this applies to), so I don't think it makes sense to think of it in any role but host. What do you think
?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, the question then becomes: Are there other search paths (for build and target, maybe) we should take care of as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean other paths libtool looks at? I don't think so — when linking a proogram, it looks at two paths specific to the program being linked, sys_lib_search_path (what we're using here), and LD_LIBRARY_PATH.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I see. What's matched by sys_lib_.+search_path?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sys_lib_dlsearch_path. I'm not sure what it does, but it's not used in the same way as the other paths.

@alyssais
Copy link
Member Author

alyssais commented Nov 5, 2021

Not sure what's up with OfBorg — lib/tests/release.nix builds fine for me…

@alyssais
Copy link
Member Author

alyssais commented Nov 5, 2021

OfBorg issue is the same one as #142602 (comment), so not related to changes here.

@cole-h
Copy link
Member

cole-h commented Nov 6, 2021

Not sure what's up with OfBorg — lib/tests/release.nix builds fine for me…

I just rebuilt this on the machine that the lib-tests failed on, and they succeeded this time... Maybe vcunat's assessment is correct? #142602 (comment)

If you see this again, don't be afraid to ping me (or post in the ofborg room on Matrix, or even ping me there too).

Anyways, let's have the bot try it again.

@ofborg eval

EDIT: Guh, it's still happening......... I don't know why. Maybe if you rebase, something will change...? Sorry >.<

@ofborg ofborg bot added 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Nov 6, 2021
@cole-h
Copy link
Member

cole-h commented Nov 6, 2021

Yay, it finally worked! Still don't know why it failed twice, but... yeah.

(This comment not an edit so it doesn't re-trigger ofborg and potentially make it unhappy again -- sorry for the noise.)

Copy link
Contributor

@Mindavi Mindavi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So in general, at least nix build github:alyssais/nixpkgs/libtool-static#pkgsStatic.util-linuxMinimal works. Tested some random tools and they run fine too.

Also built:

  • nix build github:alyssais/nixpkgs/libtool-static#pkgsStatic.sqlite
  • nix build github:alyssais/nixpkgs/libtool-static#pkgsStatic.openldap (failure on cyrus-sasl, undefined reference to openssl symbol OPENSSL_init_crypto)

Still building:

  • nix build github:alyssais/nixpkgs/libtool-static#pkgsStatic.glib (libselinux failed with (I think) a mismatch between shared and static builds)

(I just grepped some random packages that had libtool in them and looked interesting-ish. Not sure how useful it is, but here we are :).)

I guess this is ok, but I don't feel that qualified to validate this. So you might want to get another pair of eyes than me 👍.

@alyssais
Copy link
Member Author

I've been using a Nixpkgs with this in it for a few weeks, and have built a huge number of packages in this time. @sternenseemann gave me the okay on Matrix as well, so I think we should just try it out, especially now 21.11 has been branched.

@alyssais alyssais merged commit 2ebeb02 into NixOS:staging Nov 23, 2021
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/libudev-as-static-library/16272/5

@pennae
Copy link
Contributor

pennae commented Jan 3, 2022

this seems to also affect 21.11. what's the best way of getting this fixed there?

@alyssais
Copy link
Member Author

alyssais commented Mar 9, 2022

@pennae hi, sorry I missed this comment. I'm not sure it's something feasible to fix in 21.11 because I'd be worried about unintended side effects affecting stability.

@pennae
Copy link
Contributor

pennae commented Mar 9, 2022

that's fair. 22.05 isn't far out any more, shouldn't be a problem to just wait a little longer.

(and no worries! we forgot about this too 😅)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: static Static builds (e.g. pkgsStatic) 6.topic: stdenv Standard environment 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants