audit: Fix build with pkgsMusl. #61574
Conversation
The previous patches no longer applied to the current code. Also declare necessary autoconf, automake, libtool dependencies. Without them, the musl build gets: /build/audit-2.8.5/missing: line 81: aclocal-1.16: command not found configure.ac:16: warning: macro 'AM_PROG_LIBTOOL' not found in library sh: autom4te: not found
There was a problem hiding this comment.
One question, otherwise LGTM.
This command built fine, and seems to make musl-linked binaries:
nix build '(with import (builtins.fetchTarball { url = "https://github.com/nh2/nixpkgs/archive/linxu-audit-fix-musl-build.tar.gz"; }) {}; pkgsMusl.audit)'
I checked that auditctl --help worked but not much else.
| @@ -15,7 +17,7 @@ stdenv.mkDerivation rec { | |||
|
|
|||
| outputs = [ "bin" "dev" "out" "man" ]; | |||
|
|
|||
| depsBuildBuild = [ buildPackages.stdenv.cc ]; | |||
| depsBuildBuild = [ buildPackages.stdenv.cc autoconf automake libtool ]; | |||
There was a problem hiding this comment.
Will the autofoo dependencies go away after the next release? IMHO add a comment here if so.
There was a problem hiding this comment.
I suppose they will not go away; in fact I'm not quite sure why it works without them on glibc.
Given that the current src is a release tarball, it probably has a pre-generated configure script, which should make them unnecessary; but I'm not sure why on musl that one isn't good enough.
Not relying on the pre-generated configure script may be generally preferrable by the way, because it allows swapping out the src to use e.g. a git repository (which often doesn't have pre-generated configure), and is something people often do in overrides to test newer versions.
In any case, they are depsBuildBuild, so build-only and don't make it into the closure.
There was a problem hiding this comment.
That should not be the case. If it is the case, then upstream might be generating configure etc using weird versions of the autotools.
Whether or not every package should depend on autofoo to allow swapping src seems like a higher-level question than this package.
|
Needs to go to staging |
|
Alternatively push the patch and autotools bits into a rebuild-avoiding optionalAttrs, but that's pretty ugly. Nevermind, although it's what I did locally to avoid mass-rebuild when poking at this same issue: https://github.com/dtzWill/nixpkgs/commits/nixos-dtz/pkgs/os-specific/linux/audit (forgive references to 2.8.6, commit messages were written a bit sleepy-minded 😇) Actually looks like I did a hack job of what you've done here ;). I don't think autotools and friends ever really go in I'd guess without the tools the build attempts to use the pre-generated configure which works for glibc but since it won't have the changes from the patch it doesn't for musl... does that fit? As for generating it separately and once regardless of platform details -- in fact, while I don't know it's used much in nixpkgs this is precisely what nix itself does (kinda seen here, details are in plumbing elsewhere: https://github.com/NixOS/nix/blob/master/release.nix#L19 ), it generates a prestine source tarball first (generating the bits) and then uses the result everywhere. I agree such a migration/change shouldn't be started as part of this PR, but perhaps an RFC or issue or something to discuss? If nothing else it'd be good to fix this before such a discussion concludes ;). |
This avoided the rebuild for me nicely: b73f382. EDIT: something is defining the list as empty by default; I don't know when this has started. |
|
I'm almost entirely sure I did not merge this, yet github says I did. Did you merge @vcunat? |
|
Yes. GitHub very often gets this wrong. I had written to their support about this, but that might've been like two years ago. You can retry if you care enough. |
|
Very weird, I would have thought its not too hard to determine who merged. Anyway, as long as I didn't accidentally merge anything that's good enough for me. |
|
I suspect they don't necessarily inspect every push for this, and that's why sometimes merges get attributed to some later pushers on the target branch. In any case, the author and committer of the commit are never changed by GitHub, of course, so they should give a good idea (especially if the committer signed). |
Motivation for this change
The previous patches no longer applied to the current code.
Also declare necessary autoconf, automake, libtool dependencies.
Without them, the musl build gets:
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nix-review --run "nix-review wip"./result/bin/)nix path-info -Sbefore and after)