From 311277204d035558b2eb965a5fd58113d905e4aa Mon Sep 17 00:00:00 2001 From: Elliot Cameron Date: Wed, 10 Jul 2024 17:13:36 -0400 Subject: [PATCH 1/4] krb5: merge krb5 and libkrb5 with krb5.lib output krb5 and libkrb5 are two separate derivations that can easily end up in the same closure. They both provide the same shared libraries and some packages end up getting both copies. Since both copies come from the same source, packages often get lucky in this situation and just use whichever library is found first. Sometimes packages are less fortunate and end up trying to load both. This has gone largely unnoticed in Nixpkgs, likely because Kerberos is not widely used outside of enterprise deployments. This situation seems to have arisen out of a need to break a cycle in `fetchurl -> curl -> krb5 -> fetchurl`. The libkrb5 build was able to avoid depending on bison and libedit, making it easier to break the cycle. However, we can break the cycle without resorting to two variants of krb5. Libedit can be removed with configure flags and byacc can be used instead of bison, allowing a much smaller build closure that can easily be resolved when breaking the cycle. This change also adds a "lib" output to krb5 so that packages depending on krb5 can still benefit from a smaller runtime closure if they only need the shared libraries. A future change will include a tree-wide refactor to switch uses of libkrb5 to krb5. --- pkgs/development/libraries/kerberos/krb5.nix | 89 ++++++++++---------- pkgs/top-level/aliases.nix | 2 +- pkgs/top-level/all-packages.nix | 6 +- 3 files changed, 51 insertions(+), 46 deletions(-) diff --git a/pkgs/development/libraries/kerberos/krb5.nix b/pkgs/development/libraries/kerberos/krb5.nix index 7a51d454cf14..f48a2136ad11 100644 --- a/pkgs/development/libraries/kerberos/krb5.nix +++ b/pkgs/development/libraries/kerberos/krb5.nix @@ -1,5 +1,13 @@ -{ lib, stdenv, fetchurl, pkg-config, perl, bison, bootstrap_cmds -, openssl, openldap, libedit, keyutils, libverto, darwin +{ lib +, stdenv +, fetchurl +, bootstrap_cmds +, byacc # can also use bison, but byacc has fewer dependencies +, darwin +, keyutils +, openssl +, perl +, pkg-config # for passthru.tests , bind @@ -10,27 +18,17 @@ , python3 # Extra Arguments -, type ? "" +, withLdap ? false, openldap +, withLibedit ? true, libedit +, withVerto ? false, libverto + # This is called "staticOnly" because krb5 does not support # builting both static and shared, see below. , staticOnly ? false -, withLdap ? false -, withVerto ? false }: -# Note: this package is used for bootstrapping fetchurl, and thus -# cannot use fetchpatch! All mutable patches (generated by GitHub or -# cgit) that are needed here should be included directly in Nixpkgs as -# files. - -let - libOnly = type == "lib"; -in - -assert withLdap -> !libOnly; - stdenv.mkDerivation rec { - pname = "${type}krb5"; + pname = "krb5"; version = "1.21.3"; src = fetchurl { @@ -38,14 +36,22 @@ stdenv.mkDerivation rec { hash = "sha256-t6TNXq1n+wi5gLIavRUP9yF+heoyDJ7QxtrdMEhArTU="; }; - outputs = [ "out" "dev" ]; + outputs = [ "out" "lib" "dev" ]; - configureFlags = [ "--localstatedir=/var/lib" ] + # While "out" acts as the bin output, most packages only care about the lib output. + # We set prefix such that all the pkg-config configuration stays inside the dev and lib outputs. + # stdenv will take care of overriding bindir, sbindir, etc. such that "out" contains the binaries. + prefix = builtins.placeholder "lib"; + + configureFlags = [ + "--localstatedir=/var/lib" + (lib.withFeature withLdap "ldap") + (lib.withFeature withLibedit "libedit") + (lib.withFeature withVerto "system-verto") + ] # krb5's ./configure does not allow passing --enable-shared and --enable-static at the same time. # See https://bbs.archlinux.org/viewtopic.php?pid=1576737#p1576737 ++ lib.optionals staticOnly [ "--enable-static" "--disable-shared" ] - ++ lib.optional withLdap "--with-ldap" - ++ lib.optional withVerto "--with-system-verto" ++ lib.optional stdenv.isFreeBSD ''WARN_CFLAGS='' ++ lib.optionals (stdenv.buildPlatform != stdenv.hostPlatform) [ "krb5_cv_attr_constructor_destructor=yes,yes" @@ -53,15 +59,14 @@ stdenv.mkDerivation rec { "ac_cv_printf_positional=yes" ]; - nativeBuildInputs = [ pkg-config perl ] - ++ lib.optional (!libOnly) bison + nativeBuildInputs = [ byacc perl pkg-config ] # Provides the mig command used by the build scripts ++ lib.optional stdenv.isDarwin bootstrap_cmds; buildInputs = [ openssl ] ++ lib.optionals (stdenv.hostPlatform.isLinux && stdenv.hostPlatform.libc != "bionic" && !(stdenv.hostPlatform.useLLVM or false)) [ keyutils ] - ++ lib.optionals (!libOnly) [ libedit ] ++ lib.optionals withLdap [ openldap ] + ++ lib.optionals withLibedit [ libedit ] ++ lib.optionals withVerto [ libverto ]; propagatedBuildInputs = lib.optionals stdenv.isDarwin (with darwin.apple_sdk; [ @@ -73,7 +78,7 @@ stdenv.mkDerivation rec { postPatch = '' substituteInPlace config/shlib.conf \ - --replace "'ld " "'${stdenv.cc.targetPrefix}ld " + --replace "'ld " "'${stdenv.cc.targetPrefix}ld " '' # this could be accomplished by updateAutotoolsGnuConfigScriptsHook, but that causes infinite recursion # necessary for FreeBSD code path in configure @@ -83,27 +88,17 @@ stdenv.mkDerivation rec { libFolders = [ "util" "include" "lib" "build-tools" ]; - buildPhase = lib.optionalString libOnly '' - runHook preBuild - - MAKE="make -j $NIX_BUILD_CORES" - for folder in $libFolders; do - $MAKE -C $folder + # To avoid cyclic outputs, we can't let lib depend on out in any way. Unfortunately, the configure + # options don't give us enough granularity to specify that, so we have to override the generated + # Makefiles manually. + postConfigure = '' + find $libFolders -type f -name Makefile -print0 | while IFS= read -rd "" f; do + substituteInPlace "$f" --replace-fail "$out" "$lib" done - - runHook postBuild ''; - installPhase = lib.optionalString libOnly '' - runHook preInstall - - mkdir -p "$out"/{bin,sbin,lib/pkgconfig,share/{et,man/man1}} \ - "$dev"/include/{gssapi,gssrpc,kadm5,krb5} - for folder in $libFolders; do - $MAKE -C $folder install - done - - runHook postInstall + preInstall = '' + mkdir -p "$lib"/{bin,sbin,lib/pkgconfig,share/{et,man/man1}} ''; # not via outputBin, due to reference from libkrb5.so @@ -111,6 +106,14 @@ stdenv.mkDerivation rec { moveToOutput bin/krb5-config "$dev" ''; + # Disable _multioutDocs in stdenv by overriding it to be a no-op. + # We do this because $lib has its own docs and we don't want to squash them into $out. + preFixup = '' + _multioutDocs() { + echo Skipping multioutDocs + } + ''; + enableParallelBuilding = true; doCheck = false; # fails with "No suitable file for testing purposes" diff --git a/pkgs/top-level/aliases.nix b/pkgs/top-level/aliases.nix index 8b15f42c3fa9..007ca15cda24 100644 --- a/pkgs/top-level/aliases.nix +++ b/pkgs/top-level/aliases.nix @@ -718,7 +718,7 @@ mapAliases ({ keepassx = throw "KeePassX is no longer actively developed. Please consider KeePassXC as a maintained alternative."; # Added 2023-02-17 keepassx2 = throw "KeePassX is no longer actively developed. Please consider KeePassXC as a maintained alternative."; # Added 2023-02-17 keepkey_agent = keepkey-agent; # added 2024-01-06 - kerberos = libkrb5; # moved from top-level 2021-03-14 + kerberos = krb5; # moved from top-level 2021-03-14 kexectools = kexec-tools; # Added 2021-09-03 keysmith = libsForQt5.kdeGear.keysmith; # Added 2021-07-14 kfctl = throw "kfctl is broken and has been archived by upstream"; # Added 2023-08-21 diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix index 00afc7a54b0a..634d2fccc14b 100644 --- a/pkgs/top-level/all-packages.nix +++ b/pkgs/top-level/all-packages.nix @@ -1143,9 +1143,11 @@ with pkgs; if stdenv.isDarwin || stdenv.hostPlatform.isWindows then false else old.gssSupport or true; # `? true` is the default - libkrb5 = buildPackages.libkrb5.override { + libkrb5 = buildPackages.krb5.override { fetchurl = stdenv.fetchurlBoot; inherit pkg-config perl openssl; + withLibedit = false; + byacc = buildPackages.byacc.override { fetchurl = stdenv.fetchurlBoot; }; keyutils = buildPackages.keyutils.override { fetchurl = stdenv.fetchurlBoot; }; }; nghttp2 = buildPackages.nghttp2.override { @@ -21079,7 +21081,7 @@ with pkgs; krb5 = callPackage ../development/libraries/kerberos/krb5.nix { inherit (buildPackages.darwin) bootstrap_cmds; }; - libkrb5 = krb5.override { type = "lib"; }; + libkrb5 = krb5; # TODO(de11n) Try to make krb5 reuse libkrb5 as a dependency kronosnet = callPackage ../development/libraries/kronosnet { }; From e6f8f04f9e0e03fe13c61fc3c38f76eabc8f2ca9 Mon Sep 17 00:00:00 2001 From: Elliot Cameron Date: Sat, 17 Aug 2024 00:50:38 -0400 Subject: [PATCH 2/4] openssh: use krb5 dev output --- pkgs/tools/networking/openssh/common.nix | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkgs/tools/networking/openssh/common.nix b/pkgs/tools/networking/openssh/common.nix index 17e228b041f3..6445aa4840ee 100644 --- a/pkgs/tools/networking/openssh/common.nix +++ b/pkgs/tools/networking/openssh/common.nix @@ -24,7 +24,7 @@ , etcDir ? null , withKerberos ? false , withLdns ? true -, libkrb5 +, krb5 , libfido2 , libxcrypt , hostname @@ -60,15 +60,15 @@ stdenv.mkDerivation (finalAttrs: { strictDeps = true; nativeBuildInputs = [ autoreconfHook pkg-config ] - # This is not the same as the libkrb5 from the inputs! pkgs.libkrb5 is + # This is not the same as the krb5 from the inputs! pkgs.krb5 is # needed here to access krb5-config in order to cross compile. See: # https://github.com/NixOS/nixpkgs/pull/107606 - ++ lib.optional withKerberos pkgs.libkrb5 + ++ lib.optional withKerberos pkgs.krb5 ++ extraNativeBuildInputs; buildInputs = [ zlib libedit ] ++ [ (if linkOpenssl then openssl else libxcrypt) ] ++ lib.optional withFIDO libfido2 - ++ lib.optional withKerberos libkrb5 + ++ lib.optional withKerberos krb5 ++ lib.optional withLdns ldns ++ lib.optional withPAM pam; @@ -97,7 +97,7 @@ stdenv.mkDerivation (finalAttrs: { (lib.enableFeature dsaKeysSupport "dsa-keys") ] ++ lib.optional (etcDir != null) "--sysconfdir=${etcDir}" ++ lib.optional withFIDO "--with-security-key-builtin=yes" - ++ lib.optional withKerberos (assert libkrb5 != null; "--with-kerberos5=${libkrb5}") + ++ lib.optional withKerberos (assert krb5 != null; "--with-kerberos5=${lib.getDev krb5}") ++ lib.optional stdenv.isDarwin "--disable-libutil" ++ lib.optional (!linkOpenssl) "--without-openssl" ++ lib.optional withLdns "--with-ldns" From 5f5c2e1f038f795accf9e712951f3ef1169712c2 Mon Sep 17 00:00:00 2001 From: Elliot Cameron Date: Sat, 17 Aug 2024 00:53:19 -0400 Subject: [PATCH 3/4] libgssglue: use krb5 lib output --- pkgs/development/libraries/libgssglue/default.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/development/libraries/libgssglue/default.nix b/pkgs/development/libraries/libgssglue/default.nix index ef9b3d282156..b11c9e3600a5 100644 --- a/pkgs/development/libraries/libgssglue/default.nix +++ b/pkgs/development/libraries/libgssglue/default.nix @@ -3,7 +3,7 @@ stdenv, fetchFromGitLab, autoreconfHook, - libkrb5, + krb5, }: stdenv.mkDerivation (finalAttrs: { @@ -28,7 +28,7 @@ stdenv.mkDerivation (finalAttrs: { postInstall = '' mkdir -p $out/etc cat < $out/etc/gssapi_mech.conf - ${libkrb5}/lib/libgssapi_krb5.so mechglue_internal_krb5_init + ${lib.getLib krb5}/lib/libgssapi_krb5.so mechglue_internal_krb5_init EOF ''; From 28290731fb94876aa1a9fa7c0efbc3373ab41e70 Mon Sep 17 00:00:00 2001 From: Elliot Cameron Date: Sat, 17 Aug 2024 00:56:19 -0400 Subject: [PATCH 4/4] dotnet: use krb5 lib output --- pkgs/development/compilers/dotnet/vmr.nix | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkgs/development/compilers/dotnet/vmr.nix b/pkgs/development/compilers/dotnet/vmr.nix index 8fd7cce1aa94..0a89e558915a 100644 --- a/pkgs/development/compilers/dotnet/vmr.nix +++ b/pkgs/development/compilers/dotnet/vmr.nix @@ -11,7 +11,7 @@ , zlib , icu , lttng-ust_2_12 -, libkrb5 +, krb5 , glibcLocales , ensureNewerSourcesForZipFilesHook , darwin @@ -98,13 +98,13 @@ in stdenv.mkDerivation rec { openssl ] ++ lib.optionals isLinux [ - libkrb5 + krb5 lttng-ust_2_12 ] ++ lib.optionals isDarwin (with apple_sdk.frameworks; [ xcbuild swift - (libkrb5.overrideAttrs (old: { + (krb5.overrideAttrs (old: { # the propagated build inputs break swift compilation buildInputs = old.buildInputs ++ old.propagatedBuildInputs; propagatedBuildInputs = []; @@ -249,7 +249,7 @@ in stdenv.mkDerivation rec { substituteInPlace \ src/runtime/src/native/libs/System.Net.Security.Native/pal_gssapi.c \ - --replace-fail '"libgssapi_krb5.so.2"' '"${libkrb5}/lib/libgssapi_krb5.so.2"' + --replace-fail '"libgssapi_krb5.so.2"' '"${lib.getLib krb5}/lib/libgssapi_krb5.so.2"' substituteInPlace \ src/runtime/src/native/libs/System.Globalization.Native/pal_icushim.c \