Enable musl support for ebtables #50242
Conversation
| @@ -30,4 +53,4 @@ stdenv.mkDerivation rec { | |||
| license = licenses.gpl2; | |||
| platforms = platforms.linux; | |||
| }; | |||
| } | |||
| } // (if enableStatic then staticBuildOpts else {})) | |||
There was a problem hiding this comment.
Use optionalAttrs here for clarity.
| @@ -12,16 +23,28 @@ stdenv.mkDerivation rec { | |||
| makeFlags = | |||
| [ "LIBDIR=$(out)/lib" "BINDIR=$(out)/sbin" "MANDIR=$(out)/share/man" | |||
| "ETCDIR=$(out)/etc" "INITDIR=$(TMPDIR)" "SYSCONFIGDIR=$(out)/etc/sysconfig" | |||
| "LOCALSTATEDIR=/var" | |||
| "LOCALSTATEDIR=/var" "CC=${stdenv.cc}/bin/${stdenv.cc.targetPrefix}cc" | |||
There was a problem hiding this comment.
We' have issues with this in the past. Sometimes packages will save $CC in their output for debugging, causing a dependency on the compiler. To avoid that, you should just do CC=${stdenv.cc.targetPrefix}cc.
| "-Du_int8_t=uint8_t" | ||
| "-Du_int32_t=uint32_t" | ||
| ] ++ | ||
| lib.optional enableDebug "-g3" |
There was a problem hiding this comment.
We don't want this. This is something that you would set globally. You can use keepDebugInfo for this purpose:
https://github.com/NixOS/nixpkgs/blob/master/pkgs/stdenv/adapters.nix#L157-L165
There was a problem hiding this comment.
Er.. yeah, that makes sense. You can see my debugging process in this PR :)
| preBuild = | ||
| '' | ||
| substituteInPlace Makefile --replace '-o root -g root' "" | ||
| ''; | ||
|
|
||
| NIX_CFLAGS_COMPILE = "-Wno-error"; | ||
| NIX_CFLAGS_COMPILE = lib.concatStringsSep " " ( |
There was a problem hiding this comment.
NIX_CFLAGS_COMPILE can just be made a list. Nix will do the concating for you.
|
|
||
| stdenv.mkDerivation rec { | ||
| let staticBuildOpts = { |
There was a problem hiding this comment.
I think it would be more clear to put these inside of mkDerivation. For instance:
buildFlags = if enableStatic then "static" else null;
installPhase = if enableStatic then ''
...
'' else null;
|
@GrahamcOfBorg build pkgsCross.aarch64-multiplatform.ebtables |
|
Success on aarch64-linux (full log) Attempted: pkgsCross.aarch64-multiplatform.ebtables Partial log (click to expand)
|
|
Success on x86_64-linux (full log) Attempted: pkgsCross.aarch64-multiplatform.ebtables Partial log (click to expand)
|
| @@ -14143,7 +14143,7 @@ with pkgs; | |||
|
|
|||
| lightum = callPackage ../os-specific/linux/lightum { }; | |||
|
|
|||
| ebtables = callPackage ../os-specific/linux/ebtables { }; | |||
| ebtables = callPackage ../os-specific/linux/ebtables { enableStatic = stdenv.hostPlatform.isMusl; }; | |||
There was a problem hiding this comment.
I would move the default inside the package itself
{
...
enableStatic = stdenv.hostPlatform.isMusl
}
Otherwise people might miss the override from all-packages.nix
|
Failure on x86_64-darwin (full log) Attempted: pkgsCross.aarch64-multiplatform.ebtables Partial log (click to expand)
|
|
I think the last failure is unrelated to this patch? |
|
Well this seems in good shape now! That said, this may not be needed anymore-- iptables now includes I haven't investigated the static/dynamic issue mentioned, but since void-linux has deprecated their ebtables package for iptables it probably already works or can be made to work readily. All that said, as long as we have this package and don't replace it with iptables on all arch's, |
|
@tathougies what do you think about what @dtzWill wrote? |
|
Thank you for your contributions.
|
stale
bot
added
the
2.status: stale
stale
bot
removed
the
2.status: stale
|
I marked this as stale due to inactivity. → More info |
stale
bot
added
the
2.status: stale
|
@tathougies ping |
stale
bot
removed
the
2.status: stale
|
Closing as this PR is really old, has merge conflicts and conditional patches are not nice for maintainability. Also I suspect this needs at least to be partly redone. |
Motivation for this change
By default,
ebtablesdepends on shared object initialization routines which are not supported in the musl dynamic linker.ebtablescan also be built statically, and then works fine with musl. This patch adds an option to enable static builds forebtables, and enables this by default for musl.Additionally, there is some name-shadowing going on with the
ethhdrstruct. I used the C preprocessor to get rid of this, but I'm open to suggestions on how to make this better.Things done
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)