Migrate from glibc's libcrypt to libxcrypt#181764
Merged
mweinelt merged 117 commits intoNixOS:stagingfrom Oct 9, 2022
Merged
Conversation
mweinelt
force-pushed
the
glibc-without-libcrypt
branch
from
e009ddf to
be4fbc2
Compare
Member
Member
|
(gcc still doesn't build for me) |
Member
Author
|
Yep, I was taking a look at perl yesterday, since libxcrypt requires it and therefore causes an infinite recursion. Two ideas came up:
Outdated informationWent for the second approach this morning, but I currently can't get perl to find `glibc.libcrypt`, probably because to build glibc I require bison, which in turn requires perl. I don't think I grok the dependency chain here.I can build |
mweinelt
changed the title
Member
|
@mweinelt I've continued your efforts and fixed most issues with obvious crypt users and most NixOS tests seem to be working including an expansion of the shadow test to check support of the new hashes. I've rebased and pushed my changes here master...fpletz:nixpkgs:glibc-without-libcrypt. Is it okay if I push to your branch? |
Member
Author
|
Feel free! |
fpletz
force-pushed
the
glibc-without-libcrypt
branch
from
862dda1 to
9907b5a
Compare
github-actions
bot
added
6.topic: nixos
ofborg
bot
added
10.rebuild-darwin-stdenv
mweinelt
requested review from
a team,
FRidh,
dasJ,
matthewbauer,
stigtsp and
zakame
as code owners
mweinelt
commented
Sep 27, 2022
mweinelt
force-pushed
the
glibc-without-libcrypt
branch
from
9ca0833 to
70afcc8
Compare
13 tasks
gefla
pushed a commit
to gefla/nixpkgs
that referenced
this pull request
Nov 30, 2022
ff30c89 made libcrypt support optional (and off-by-default) in glibc, which requires all packages still using it to depend on libxcrypt. See also NixOS#181764
14 tasks
12 tasks
Ma27
added a commit
to Ma27/nixpkgs
that referenced
this pull request
Mar 29, 2023
Closes NixOS#150801 Note: I decided against resuming directly on NixOS#150801 because the conflict was too big (and resolving it seemed too error-prone to me). Also the `this`-refactoring could be done in an easier manner, i.e. by exposing JIT attributes with the correct configuration. More on that below. This patch creates variants of the `postgresql*`-packages with JIT[1] support. Please note that a lot of the work was derived from previous patches filed by other contributors, namely dasJ, andir and abbradar, hence the co-authored-by tags below. Effectively, the following things have changed: * For JIT variants an LLVM-backed stdenv with clang is now used as suggested by dasJ[2]. We need LLVM and CLang[3] anyways to build the JIT-part, so no need to mix this up with GCC's stdenv. Also, using the `dev`-output of LLVM and clang's stdenv for building (and adding llvm libs as build-inputs) seems more cross friendly to me (which will become useful when cross-building for JIT-variants will actually be supported). * Plugins inherit the build flags from the Makefiles in `$out/lib/pgxs/src` (e.g. `-Werror=unguarded-availability-new`). Since some of the flags are clang-specific (and stem from the use of the CLang stdenv) and don't work on gcc, the stdenv of `pkgs.postgresql` is passed to the plugins. I.e., plugins for non-JIT variants are built with a gcc stdenv on Linux and plugins for JIT variants with a clang stdenv. Since `plv8` hard-codes `gcc` as `$CC` in its Makefile[4], I marked it as broken for JIT-variants of postgresql only. * Added a test-matrix to confirm that JIT works fine on each `pkgs.postgresql_*_jit` (thanks Andi for the original test in NixOS#124804!). * For each postgresql version, a new attribute `postgresql_<version>_jit` (and a corresponding `postgresqlPackages<version>JitPackages`) are now exposed for better discoverability and prebuilt artifacts in the binary cache. * In NixOS#150801 the `this`-argument was replaced by an internal recursion. I decided against this approach because it'd blow up the diff even more which makes the readability way harder and also harder to revert this if necessary. Instead, it is made sure that `this` always points to the correct variant of `postgresql` and re-using that in an additional `.override {}`-expression is trivial because the JIT-variant is exposed in `all-packages.nix`. * I think the changes are sufficiently big to actually add myself as maintainer here. * Added `libxcrypt` to `buildInputs` for versions <v13. While building things with an LLVM stdenv, these versions complained that the extern `crypt()` symbol can't be found. Not sure what this is exactly about, but since we want to switch to libxcrypt for `crypt()` usage anyways[5] I decided to add it. For >=13 it's not relevant anymore anyways[6]. * JIT support doesn't work with cross-compilation. It is attempted to build LLVM-bytecode (`%.bc` is the corresponding `make(1)`-rule) for each sub-directory in `backend/` for the JIT apparently, but with a $(CLANG) that can produce binaries for the build, not the host-platform. I managed to get a cross-build with JIT support working with `depsBuildBuild = [ llvmPackages.clang ] ++ buildInputs`, but considering that the resulting LLVM IR isn't platform-independent this doesn't give you much. In fact, I tried to test the result in a VM-test, but as soon as JIT was used to optimize a query, postgres would coredump with `Illegal instruction`. A common concern of the original approach - with llvm as build input - was the massive increase of closure size. With the new approach of using the LLVM stdenv directly and patching out references to the clang drv in `$out` the effective closure size changes are: $ nix path-info -Sh $(nix-build -A postgresql_14) /nix/store/kssxxqycwa3c7kmwmykwxqvspxxa6r1w-postgresql-14.7 306.4M $ nix path-info -Sh $(nix-build -A postgresql_14_jit) /nix/store/xc7qmgqrn4h5yr4vmdwy56gs4bmja9ym-postgresql-14.7 689.2M Most of the increase in closure-size stems from the `lib`-output of LLVM $ nix path-info -Sh /nix/store/5r97sbs5j6mw7qnbg8nhnq1gad9973ap-llvm-11.1.0-lib /nix/store/5r97sbs5j6mw7qnbg8nhnq1gad9973ap-llvm-11.1.0-lib 349.8M which is why this shouldn't be enabled by default. While this is quite much because of LLVM, it's still a massive improvement over the simple approach of adding llvm/clang as build-inputs and building with `--with-llvm`: $ nix path-info -Sh $(nix-build -E ' with import ./. {}; postgresql.overrideAttrs ({ configureFlags ? [], buildInputs ? [], ... }: { configureFlags = configureFlags ++ [ "--with-llvm" ]; buildInputs = buildInputs ++ [ llvm clang ]; })' -j0) /nix/store/i3bd2r21c6c3428xb4gavjnplfqxn27p-postgresql-14.7 1.6G Co-authored-by: Andreas Rammhold <andreas@rammhold.de> Co-authored-by: Janne Heß <janne@hess.ooo> Co-authored-by: Nikolay Amiantov <ab@fmap.me> [1] https://www.postgresql.org/docs/current/jit-reason.html [2] NixOS#124804 (comment) & NixOS#150801 (comment) [3] This fails with the following error otherwise: ``` configure: error: clang not found, but required when compiling --with-llvm, specify with CLANG= ``` [4] https://github.com/plv8/plv8/blob/v3.1.5/Makefile#L14 [5] NixOS#181764 [6] postgres/postgres@c45643d
rtimush
pushed a commit
to rtimush/nixpkgs
that referenced
this pull request
Sep 21, 2023
I guess with NixOS#181764 this might've broken for cross. Perl propagates libxcrypt, but is only listed in nativeBuildInputs. List libxcrypt in buildInputs to ensure it's picked up properly.
tomfitzhenry
added a commit
to tomfitzhenry/nixpkgs
that referenced
this pull request
Apr 28, 2024
Possibly broken during NixOS#181764 Context: https://sourceware.org/legacy-ml/libc-alpha/2017-08/msg01257.html
13 tasks
tm-drtina
pushed a commit
to awakesecurity/nixpkgs
that referenced
this pull request
Jul 23, 2024
Possibly broken during NixOS#181764 Context: https://sourceware.org/legacy-ml/libc-alpha/2017-08/msg01257.html
tm-drtina
pushed a commit
to awakesecurity/nixpkgs
that referenced
this pull request
Jul 29, 2024
Possibly broken during NixOS#181764 Context: https://sourceware.org/legacy-ml/libc-alpha/2017-08/msg01257.html
tm-drtina
pushed a commit
to awakesecurity/nixpkgs
that referenced
this pull request
Aug 13, 2024
Possibly broken during NixOS#181764 Context: https://sourceware.org/legacy-ml/libc-alpha/2017-08/msg01257.html
bacchanalia
pushed a commit
to awakesecurity/nixpkgs
that referenced
this pull request
Aug 27, 2024
Possibly broken during NixOS#181764 Context: https://sourceware.org/legacy-ml/libc-alpha/2017-08/msg01257.html
tm-drtina
pushed a commit
to awakesecurity/nixpkgs
that referenced
this pull request
Dec 16, 2024
Possibly broken during NixOS#181764 Context: https://sourceware.org/legacy-ml/libc-alpha/2017-08/msg01257.html
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR hopefully paves the way for a libxcrypt migration.
For that we disable crypt suppor in glibc and find out what packages break.
On these packages we add libxcrypt and things should build again.
Ideally we can achieve a shorter path to libxcrypt than drawn up in #112371.