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.

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

Revive FreeBSD stdenv #254801

Closed
wants to merge 158 commits into from
Closed

Revive FreeBSD stdenv #254801

wants to merge 158 commits into from

Conversation

rhelmot
Copy link
Contributor

@rhelmot rhelmot commented Sep 12, 2023

Description of changes

This allows the stdenv to boot on FreeBSD again. Additionally, many packages have been patched to add FreeBSD support.

Please note: I am very new here! I have never used nix before, I just wanted to learn by making it work on FreeBSD. I am very much open to feedback.

Things done

  • FreeBSD bootstrap files, compilable from linux

  • Entirely new stdenv/freebsd

  • Add missing parameters to let packages build on FreeBSD

  • Built on platform(s)

    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
    • x86_64-freebsd
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)

  • Tested, as applicable:

  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage

  • Tested basic functionality of all binary files (usually in ./result/bin/)

  • 23.11 Release Notes (or backporting 23.05 Release notes)

    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@rhelmot rhelmot requested a review from a user September 12, 2023 18:28
@github-actions github-actions bot added 6.topic: python 6.topic: rust 6.topic: stdenv Standard environment 6.topic: lib The Nixpkgs function library labels Sep 12, 2023
@NixOSInfra NixOSInfra added the 12. first-time contribution This PR is the author's first one; please be gentle! label Sep 12, 2023
@Ericson2314
Copy link
Member

The reason we pulled it out is because these "impure" stdenvs are had to maintain and cannot be reasonably cached by cache.nixos.org

We've improved cross compiling to FreeBSD and NetBSD, and in NixOS/nix#8887 I have added CI for cross compiling Nix itself. The next step is to create a bootstrap tools (which will at least initially be cross compiled) and boot from it natively, purely.

@Ericson2314
Copy link
Member

I know I've been saying this for a while, and thus it appears I need to actually sit down and make it myself or pair with someone on it. I so far have been too busy to do that.

@rhelmot
Copy link
Contributor Author

rhelmot commented Sep 12, 2023

That's great. I think this PR sets up a solid base for being able to do that, since the impure bits are very precisely quarantined. I can try to get a nixos install later today and see what the interaction between your PR and this one looks like.

Is there a channel I should join on matrix to talk to you about this? So far, I've just been dumping progress into #exotic:nixos.org.

@Ericson2314
Copy link
Member

Yeah we can talk a bit. But I am afraid I don't currently envision any overlap between this and the thing I want.

@@ -321,7 +321,7 @@ rec {
# BSDs

x86_64-freebsd = {
config = "x86_64-unknown-freebsd13";
config = "x86_64-freebsd13";
Copy link
Member

@Artturin Artturin Sep 12, 2023

Choose a reason for hiding this comment

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

gnu-config says this used to be correct and is wrong now

	FREEBSD_REL=`echo "$UNAME_RELEASE" | sed -e 's/[-(].*//'`
	GUESS=$UNAME_PROCESSOR-unknown-freebsd$FREEBSD_REL

why change?

@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Sep 20, 2023
+ ln -s $(LIBRHASH_SHARED) $(LIBDIR)/$(LIBRHASH_SO_MAJ) || true
@@ -180,1 +180,1 @@
- ln -s $(LIBRHASH_SHARED) $(LIBRHASH_SO_MAJ)
+ ln -s $(LIBRHASH_SHARED) $(LIBRHASH_SO_MAJ) || true
Copy link
Member

@Artturin Artturin Sep 21, 2023

Choose a reason for hiding this comment

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

does freebsd not support symlinks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a hackfix for an issue that will be fixed in the next version of rhash. I believe this is the relevant issue: rhash/RHash#246

@@ -189,6 +189,10 @@ in lib.makeExtensible (self: ({
nix_2_17 = common {
version = "2.17.0";
hash = "sha256-QMYAkdtU+g9HlZKtoJ+AI6TbWzzovKGnPZJHfZdclc8=";
patches = lib.optionals aws-sdk-cpp.stdenv.hostPlatform.isFreeBSD [
./patches/FreeBSD-tests-restricted.sh.patch
Copy link
Member

Choose a reason for hiding this comment

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

All nix changes should be submitted upstream

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

@Artturin I suppose is more positive, but just to be clear I am still quite against this.

Once/During @emilytrau is done with the bootstrap overhaul, it is a perfect time to simply the other bootstrapping so making a pure FreeBSD boostrap is easier.

@Artturin
Copy link
Member

Artturin commented Sep 23, 2023

@Artturin I suppose is more positive, but just to be clear I am still quite against this.

I agree too that impure bootstraps should not be used, instead cross-compiled bootstrap tools should be used if they're available. Some changes here could apply to cross-compilation too.

I think there's no need to wait for minimal-bootstrap to be ready because the programs needed are likely not going to support *BSD anytime soon

@rhelmot
Copy link
Contributor Author

rhelmot commented Sep 23, 2023

I have been working on a pure bootstrap since reading John's comments. I'll switch this PR to a draft until I've got a proof of concept.

@github-actions github-actions bot added 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: haskell 6.topic: ruby 6.topic: xfce The Xfce Desktop Environment labels Feb 10, 2024
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 10, 2024
@rhelmot
Copy link
Contributor Author

rhelmot commented Feb 10, 2024

Update! Pretty much all of the questions from the previous iteration of this PR have been resolved.

  • The nix changes have been submitted upstream. Add sandboxed building for FreeBSD using jails nix#9968
  • I was able to remove the freebsd version number from the platform tuple. This means that we will have to default to the lowest supported version of the freebsd userland, which I am declaring right now to be 14. I may fix 13 at some point and add it
  • Artemis was able to write a script for automatically pulling in hashes for the freebsd source tree on a variety of branches. This reduces the maintenance burden a lot, we just need to pick a support policy. I'm thinking "two major versions", meaning that we'll support 15 when it comes out and drop 14 when 16 comes out. We can also support only the most recent minor release of the major branch, in addition to the releng branches, where security fixes are applied.
  • I am willing to stay on as a primary support person for this platform. I will also probably be applying to maintain the freebsd ports tree version of nix.

New bits...

  • I went pretty hard on fixing compatibility issues with a variety of nixpkgs entries for freebsd. My goal was to get hydra building, which I succeeded at (may have regressed since a month ago), though tests failed. I also tried to get Gnome and Xfce working, which I failed at, but made some headway. Xterm works, I think I was making my way toward gnome-console last I was trying.
  • CI for all of this is up in the air but can start to come together as soon as we get a) nix merged and released into nixpkgs, and b) hosting. Obviously the second is going to be harder, though if anyone has any pointers on applying for e.g. grants I can give that a shot. If it comes to it I can donate cores from my personal machine.

@artemist
Copy link
Member

artemist commented Feb 10, 2024

My script also pulls the list of "Supported FreeBSD releases" from the FreeBSD Security page and includes that in the versions.json under each version's supported field, so it should not be hard to programmatically look for things like "newest supported version".

Note that supported branches only include stable (working towards the next minor release, e.g. stable/14 is currently an intermediate between 14.0 and 14.1) and releng (security and regression fixes applied on a minor release).

Most of our testing has been on release/14.0.0, a tag which should never change and does not include security patches, but I have confirmed that releng/14.0 compiles and you can boot into a real system using our nixbsd repository.

This PR currently defaults to release/14.0.0 but another version can be selected with:

  • a nixpkgs config option (freebsdBranch)
  • an environment variable on impure builds (NIXPKGS_FREEBSD_BRANCH)
  • building a package for a specific version (e.g. pkgs.freebsd."stable/14".sys)

I would support switching the default to releng/14.0 because any updates tend to Just Work™. Changes in stable branches often break builds and would make responding to security incidents a pain.

Unfortunately, any update will cause a mass rebuild without both ca-derivations and a fair amount of work to remove version numbers from packages.

I would also be willing to support this platform, the port is a huge project and I have been contributing to it for a few months.

@astro
Copy link
Contributor

astro commented Mar 7, 2024

Not sure if truly related, but I have an issue adding an override to the packages in this PR:

diff --git pkgs/os-specific/bsd/freebsd/sys.nix pkgs/os-specific/bsd/freebsd/sys.nix
index fc42e2f0f1b6..74f1ee276d03 100644
--- pkgs/os-specific/bsd/freebsd/sys.nix
+++ pkgs/os-specific/bsd/freebsd/sys.nix
@@ -1,7 +1,6 @@
-{mkDerivation, buildPackages, buildFreebsd, hostArchBsd, patchesRoot, ... }:
-mkDerivation (let
-    cfg = "GENERIC";
-  in rec {
+{mkDerivation, buildPackages, buildFreebsd, hostArchBsd, patchesRoot, cfg ? "GENERIC", ... }:
+if cfg == "GENERIC" then throw "override did not work" else
+  mkDerivation (rec {
     path = "sys";
     extraPaths = ["include"];

With the throw in place I cannot evaluate freebsd.sys.override { cfg = "FIRECRACKER"; }.

My first guess was this additional makeOverridable but removing it does not change anything:

diff --git pkgs/os-specific/bsd/freebsd/make-derivation.nix pkgs/os-specific/bsd/freebsd/make-derivation.nix
index 7a2b4fe3d966..913b8440d3a4 100644
--- pkgs/os-specific/bsd/freebsd/make-derivation.nix
+++ pkgs/os-specific/bsd/freebsd/make-derivation.nix
@@ -1,5 +1,5 @@
 { lib, crossLibcStdenv, stdenv, hostVersion, buildPackages, buildFreebsd, hostArchBsd, compatIfNeeded, filterSource, ... }:
-lib.makeOverridable (attrs: let
+attrs: let
   # the use of crossLibcStdenv in the isStatic case is kind of a misnomer but I think it works
   stdenv' = if (attrs.isStatic or false) then crossLibcStdenv else stdenv;
 in stdenv'.mkDerivation (rec {
@@ -57,4 +57,4 @@ in stdenv'.mkDerivation (rec {
   preBuild = ''
     export NIX_CFLAGS_COMPILE="$NIX_CFLAGS_COMPILE -D_VA_LIST -D_VA_LIST_DECLARED -Dva_list=__builtin_va_list -D_SIZE_T_DECLARED -D_SIZE_T -Dsize_t=__SIZE_TYPE__ -D_WCHAR_T"
   '' + (attrs.preBuild or "");
-}))
+})

@sternenseemann
Copy link
Member

I was able to remove the freebsd version number from the platform tuple. This means that we will have to default to the lowest supported version of the freebsd userland, which I am declaring right now to be 14. I may fix 13 at some point and add it

The version number is intentionally part of the platform tuple and should stay that way. It is probably necessary to adjust Nix' configure.ac to make the population of system a bit smoother on FreeBSD, but that should be pretty easy. This system is much more robust, since the selection of the version via the nixpkgs config does not indicate incompatibilty. GNU config supports the versioned tuples fine.

@rhelmot
Copy link
Contributor Author

rhelmot commented Mar 7, 2024

Can I hear more about that? My experience doing live fire testing of the versionless system has been nothing short of splendid. The only packages that put up a fight were python and gcc, and I have resolved both of them by providing the version number of userland (e.g. libc) as part of the triple even though the system double is unversioned.

The explicit goal of removing the version number from the double was to make it so that we don't have to maintain multiple systems. My understanding was that this is possible because FreeBSD has excellent kernel ABI backward compatibility, a fact that seems to be obscured by decades of misinformation. Rust, for example, distributes only a single unversioned FreeBSD distribution, and that seems to be working fine for them.

@7c6f434c
Copy link
Member

7c6f434c commented Mar 7, 2024

FreeBSD has excellent kernel ABI backward compatibility, a fact that seems to be obscured by decades of misinformation.

I guess it doesn't help that BSDs share some parts of the system construction practices, but are very different in the other parts.

https://wiki.freebsd.org/BinaryCompatibility indeed looks like it targets absolute userland interface stability, and achieves it to the level comparable of Linux…

@sternenseemann
Copy link
Member

we don't have to maintain multiple systems

If we have a switch in the nixpkgs config, we are also maintaining the same amount of complexity.

has excellent kernel ABI backward compatibility

But is it also forwards compatible? The system value would be used to schedule remote builds. If we use the same tuple for all versions it could happen that FreeBSD 14 binaries are executed on a machine that uses an older version of FreeBSD. Would that work in all cases?

@rhelmot
Copy link
Contributor Author

rhelmot commented Mar 8, 2024

It should work for all sane cases, that is, cases that Linux also works for.

Let's say we support FreeBSD versions 13 and 14. Our default userland is therefore going to be 13, and any change to that will change derivation hashes. Therefore, if there is a 14 system servicing builds for a 13 system, it will still be using the 13 userland in its build jail, and so the binaries will link on the 13 system. However, there is always the possibility that the build system probes the kernel directly for capabilities, which can be argued to be acceptable for a non-cross build. I believe that this will also break things on Linux. True forwards compatibility, i.e. the ability for e.g. a static binary built on a new kernel to run on an old kernel, can only exist with extensive compiler or library support (fallbacks!), which is not realistic to expect in all cases.

fwiw, we can also spoof the reported kernel version in the build jail, though this obviously won't affect the actual capabilities available.

@rhelmot
Copy link
Contributor Author

rhelmot commented Mar 17, 2024

Superseded by above. This is so big as to be unmergable :)

@rhelmot rhelmot closed this Mar 17, 2024
@rhelmot rhelmot mentioned this pull request Apr 30, 2024
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: haskell 6.topic: lib The Nixpkgs function library 6.topic: python 6.topic: ruby 6.topic: rust 6.topic: stdenv Standard environment 6.topic: vim 6.topic: xfce The Xfce Desktop Environment 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants