Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

pkgsLinux: set crossSystem instead of localSystem #317651

Draft
wants to merge 5 commits into
base: staging
Choose a base branch
from
+26 −10
Draft

pkgsLinux: set crossSystem instead of localSystem #317651

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f58717b
pkgsLinux: set crossSystem instead of localSystem
tie Jun 6, 2024
c1b197d
tpm2-tss: remove shadow dependency
tie Jun 6, 2024
ab8001f
procps: use systemdLibs instead of full systemd
tie Jun 6, 2024
3106c33
yodl: fix build on darwin
tie Jun 6, 2024
8ba1c35
icmake: allow building on darwin
tie Jun 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions pkgs/development/libraries/tpm2-tss/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{ stdenv, lib, fetchFromGitHub
{ stdenv, lib, fetchFromGitHub, runCommand
, autoreconfHook, autoconf-archive, pkg-config, doxygen, perl
, openssl, json_c, curl, libgcrypt
, cmocka, uthash, ibm-sw-tpm2, iproute2, procps, which
, shadow, libuuid
, libuuid
}:
let
# Avoid a circular dependency on Linux systems (systemd depends on tpm2-tss,
Expand All @@ -11,6 +11,20 @@ let
# might not support the withSystemd option.
procpsWithoutSystemd = procps.override { withSystemd = false; };
procps_pkg = if stdenv.isLinux then procpsWithoutSystemd else procps;

# Configure script expects tools from shadow but we are not actually using
# them. Instead of patching the build scripts, just add fake executables
# to native build inputs. These would fail the build if called from both POSIX
# shell and via native exec system call (albeit for different reasons).
# https://github.com/tpm2-software/tpm2-tss/blob/6c46325b466f35d40c2ed1043bfdfcfb8a367a34/configure.ac#L666-L675
# https://github.com/tpm2-software/tpm2-tss/blob/6c46325b466f35d40c2ed1043bfdfcfb8a367a34/Makefile.am#L880-L898
fakeShadow = runCommand "fake-shadow" { } ''
mkdir -p -- "$out"/bin
for executable in "$out"/bin/{useradd,groupadd}; do
printf '#!/bin/sh -\nexit 1' >"$executable"
chmod +x -- "$executable"
done
'';
in

stdenv.mkDerivation rec {
Expand All @@ -28,7 +42,7 @@ stdenv.mkDerivation rec {

nativeBuildInputs = [
autoreconfHook autoconf-archive pkg-config doxygen perl
shadow
fakeShadow
];

buildInputs = [
Expand Down
2 changes: 1 addition & 1 deletion pkgs/development/tools/build-managers/icmake/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,6 @@ stdenv.mkDerivation rec {
homepage = "https://fbb-git.gitlab.io/icmake/";
license = licenses.gpl3;
maintainers = with maintainers; [ pSub ];
platforms = platforms.linux;
platforms = platforms.linux ++ platforms.darwin;
};
}
6 changes: 4 additions & 2 deletions pkgs/development/tools/misc/yodl/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ stdenv.mkDerivation rec {
preConfigure = ''
patchShebangs ./build
patchShebangs scripts/
substituteInPlace INSTALL.im --replace /usr $out
substituteInPlace INSTALL.im \
--replace-fail /usr "$out" \
--replace-fail 'COMPILER = "gcc";' "COMPILER = \"$CC\";"
substituteInPlace macros/rawmacros/startdoc.pl --replace /usr/bin/perl ${perl}/bin/perl
substituteInPlace scripts/yodl2whatever.in --replace getopt ${util-linux}/bin/getopt
'';
Expand All @@ -47,6 +49,6 @@ stdenv.mkDerivation rec {
homepage = "https://fbb-git.gitlab.io/yodl/";
license = licenses.gpl3;
maintainers = with maintainers; [ pSub ];
platforms = platforms.linux;
platforms = platforms.linux ++ platforms.darwin;
};
}
6 changes: 3 additions & 3 deletions pkgs/os-specific/linux/procps-ng/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@

# `ps` with systemd support is able to properly report different
# attributes like unit name, so we want to have it on linux.
, withSystemd ? lib.meta.availableOn stdenv.hostPlatform systemd
, systemd
, withSystemd ? lib.meta.availableOn stdenv.hostPlatform systemdLibs
, systemdLibs

# procps is mostly Linux-only. Most commands require a running Linux
# system (or very similar like that found in Cygwin). The one
Expand Down Expand Up @@ -40,7 +40,7 @@ stdenv.mkDerivation rec {
];

buildInputs = [ ncurses ]
++ lib.optional withSystemd systemd;
++ lib.optional withSystemd systemdLibs;
nativeBuildInputs = [ pkg-config ];

makeFlags = [ "usrbin_execdir=$(out)/bin" ]
Expand Down
2 changes: 1 addition & 1 deletion pkgs/top-level/stage.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ let
if stdenv.hostPlatform.isLinux
then self
else nixpkgsFun {
localSystem = lib.systems.elaborate "${stdenv.hostPlatform.parsed.cpu.name}-linux";
crossSystem = lib.systems.elaborate "${stdenv.hostPlatform.parsed.cpu.name}-linux";
Copy link
Member

Choose a reason for hiding this comment

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

This is meant to be a natively built Linux package set, ie a non-cross build that's potentially done on a different machine. Do you think we non-cross would be a clearer description than natively built in the doc above?
Note that the "machine" is easy to set up with for example linux-builder.

By changing to cross compilation, we make the user experience worse and increase cost.

  • Cross compilation, despite how well it works in Nixpkgs, does not permit tests to be run in package builds
  • We don't have a cache for darwin->linux cross builds, afaik. These builds would be unnecessary extra builds, costing compute and cache storage (ofborg, hydra, cache.nixos.org, etc)

Also note that if you're in a team using a mix of Linux and Darwin and you're doing deployment with cross builds, your performing unnecessary redeployments depending on who's initiating it, causing unnecessary disruptions as system services are "updated" to an equivalent package on a different store path. You could solve this by deploying from dedicated infrastructure, but then you might as well use that for remote builds as well.
More importantly though, non-cross builds are more likely to work.
All in all, non-cross deployments are simpler and more robust, so I would not default to cross compilation anywhere, including here.

Perhaps your goal could be achieved by adding pkgsLinuxCross, but then I still wouldn't make the test framework use that for the reasons above.

Copy link
Member Author

@tie tie Jun 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

That sounds reasonable given the current state of cross-compilation of Nixpkgs (though I wouldn’t say it works well, rather it works for well-tested {local,cross}System pairs, but that doesn’t cover glibc → glibc static cross-build, NixOS built from macOS, and a lot of other cases).

I think we’ve had a similar discussion in NixOS/nix#10291 about this use case. W.r.t. tests, these do not change the resulting package output. So it doesn’t really matter where the tests are run as long the closures are byte-for-byte identical and reproducible. That is, as long as we can test that a certain set of packages is identical independent of the localSystem, it doesn’t really matter which derivation runs the tests (assuming that at least one does). Sure, that requires content-addressed derivations to get this right, but then there are a lot of other places that should be fixed before that.

I understand your point, but I do want to improve the current state of affairs. I’ll open separate PR for package-specific commits I’ve pushed to this branch, without this particular change.

};

# Extend the package set with zero or more overlays. This preserves
Expand Down