Merge pull request #262107 from hraban/clisp-sbcl-docheck

sbcl: remove patches, enable checks, allow overriding

2 files changed, 200 insertions, 50 deletions

diff --git a/pkgs/development/compilers/sbcl/default.nix b/pkgs/development/compilers/sbcl/default.nix
index 2f18dcc78d4c9..fbd0970848d1e 100644
--- a/pkgs/development/compilers/sbcl/default.nix
+++ b/pkgs/development/compilers/sbcl/default.nix
@@ -1,4 +1,4 @@
-{ lib, stdenv, callPackage, clisp, fetchurl, fetchpatch, writeText, zstd
+{ lib, stdenv, callPackage, clisp, coreutils, fetchurl, strace, texinfo, which, writeText, zstd
 , threadSupport ? (stdenv.hostPlatform.isx86 || "aarch64-linux" == stdenv.hostPlatform.system || "aarch64-darwin" == stdenv.hostPlatform.system)
 , linkableRuntime ? stdenv.hostPlatform.isx86
 , disableImmobileSpace ? false
@@ -8,8 +8,13 @@
 , purgeNixReferences ? false
 , coreCompression ? lib.versionAtLeast version "2.2.6"
 , markRegionGC ? lib.versionAtLeast version "2.4.0"
-, texinfo
 , version
+  # Set this to a lisp binary to use a custom bootstrap lisp compiler for
+  # SBCL. Leave as null to use the default. This is useful for local development
+  # of SBCL, because you can use your existing stock SBCL as a boostrap. On Hydra
+  # of course we can’t do that because SBCL hasn’t been built yet, so we use
+  # CLISP, but that’s much slower.
+, bootstrapLisp ? null
 }:
 
 let
@@ -62,14 +67,16 @@ let
   sbclBootstrap = callPackage ./bootstrap.nix {
     cfg = bootstrapBinaries.${stdenv.hostPlatform.system};
   };
-  bootstrapLisp =
-    if (builtins.hasAttr stdenv.hostPlatform.system bootstrapBinaries)
+  bootstrapLisp' =
+    if bootstrapLisp != null
+    then bootstrapLisp
+    else if (builtins.hasAttr stdenv.hostPlatform.system bootstrapBinaries)
     then "${sbclBootstrap}/bin/sbcl --disable-debugger --no-userinit --no-sysinit"
     else "${clisp}/bin/clisp -E UTF-8 --silent -norc";
 
 in
 
-stdenv.mkDerivation rec {
+stdenv.mkDerivation (self: rec {
   pname = "sbcl";
   inherit version;
 
@@ -78,48 +85,62 @@ stdenv.mkDerivation rec {
     inherit (versionMap.${version}) sha256;
   };
 
-  nativeBuildInputs = [ texinfo ];
+  nativeBuildInputs = [
+    texinfo
+  ] ++ lib.optionals self.doCheck (
+    [
+      which
+    ] ++ lib.optionals (builtins.elem stdenv.system strace.meta.platforms) [
+      strace
+    ]
+  );
   buildInputs = lib.optionals coreCompression [ zstd ];
 
-  patches = lib.optionals (version == "2.4.0") [
+  patches = [
+    ./search-for-binaries-in-PATH.patch
+  ] ++ lib.optionals (version == "2.4.0") [
     ./fix-2.4.0-aarch64-darwin.patch
   ];
 
-  postPatch = ''
-    echo '"${version}.nixos"' > version.lisp-expr
-
-    # SBCL checks whether files are up-to-date in many places..
-    # Unfortunately, same timestamp is not good enough
-    sed -e 's@> x y@>= x y@' -i contrib/sb-aclrepl/repl.lisp
-    #sed -e '/(date)/i((= date 2208988801) 2208988800)' -i contrib/asdf/asdf.lisp
-    sed -i src/cold/slam.lisp -e \
-      '/file-write-date input/a)'
-    sed -i src/cold/slam.lisp -e \
-      '/file-write-date output/i(or (and (= 2208988801 (file-write-date output)) (= 2208988801 (file-write-date input)))'
-    sed -i src/code/target-load.lisp -e \
-      '/date defaulted-fasl/a)'
-    sed -i src/code/target-load.lisp -e \
-      '/date defaulted-source/i(or (and (= 2208988801 (file-write-date defaulted-source-truename)) (= 2208988801 (file-write-date defaulted-fasl-truename)))'
-
-    # Fix the tests
-    sed -e '5,$d' -i contrib/sb-bsd-sockets/tests.lisp
-    sed -e '5,$d' -i contrib/sb-simple-streams/*test*.lisp
+  # I don’t know why these are failing (on ofBorg), and I’d rather just disable
+  # them and move forward with the succeeding tests than block testing
+  # altogether. One by one hopefully we can fix these (on ofBorg,
+  # upstream--somehow some way) in due time.
+  disabledTestFiles = lib.optionals (builtins.elem stdenv.hostPlatform.system [
+    "x86_64-linux"
+    "aarch64-linux"
+  ]) [
+    "foreign-stack-alignment.impure.lisp"
+    # Floating point tests are fragile
+    # https://sourceforge.net/p/sbcl/mailman/message/58728554/
+    "compiler.pure.lisp"
+    "float.pure.lisp"
+  ] ++ lib.optionals (stdenv.hostPlatform.system == "aarch64-linux") [
+    # This is failing on aarch64-linux on ofBorg. Not on my local machine nor on
+    # a VM on my laptop. Not sure what’s wrong.
+    "traceroot.impure.lisp"
+  ];
+  postPatch = lib.optionalString (self.disabledTestFiles != [ ]) ''
+    (cd tests ; rm -f ${lib.concatStringsSep " " self.disabledTestFiles})
   ''
-  + (if purgeNixReferences
-    then
-      # This is the default location to look for the core; by default in $out/lib/sbcl
-      ''
-        sed 's@^\(#define SBCL_HOME\) .*$@\1 "/no-such-path"@' \
-          -i src/runtime/runtime.c
-      ''
-    else
-      # Fix software version retrieval
-      ''
-        sed -e "s@/bin/uname@$(command -v uname)@g" -i src/code/*-os.lisp \
-          src/code/run-program.lisp
-      ''
-    );
-
+  + lib.optionalString purgeNixReferences ''
+    # This is the default location to look for the core; by default in $out/lib/sbcl
+    sed 's@^\(#define SBCL_HOME\) .*$@\1 "/no-such-path"@' \
+        -i src/runtime/runtime.c
+  ''
+  + ''
+    (
+      shopt -s nullglob
+      # Tests need patching regardless of purging of paths from the final
+      # binary. There are some tricky files in nested directories which should
+      # definitely NOT be patched this way, hence just a single * (and no
+      # globstar).
+      substituteInPlace ${if purgeNixReferences then "tests" else "{tests,src/code}"}/*.{lisp,sh} \
+        --replace-quiet /usr/bin/env "${coreutils}/bin/env" \
+        --replace-quiet /bin/uname "${coreutils}/bin/uname" \
+        --replace-quiet /bin/sh "${stdenv.shell}"
+    )
+  '';
 
   preBuild = ''
     export INSTALL_ROOT=$out
@@ -138,7 +159,16 @@ stdenv.mkDerivation rec {
     optional (!threadSupport) "sb-thread" ++
     optionals disableImmobileSpace [ "immobile-space" "immobile-code" "compact-instance-header" ];
 
-  env.NIX_CFLAGS_COMPILE = toString (lib.optionals (lib.versionOlder version "2.1.10") [
+  buildArgs = [
+    "--prefix=$out"
+    "--xc-host=${lib.escapeShellArg bootstrapLisp'}"
+  ] ++ builtins.map (x: "--with-${x}") self.enableFeatures
+  ++ builtins.map (x: "--without-${x}") self.disableFeatures
+  ++ lib.optionals (stdenv.hostPlatform.system == "aarch64-darwin") [
+    "--arch=arm64"
+  ];
+
+  env.NIX_CFLAGS_COMPILE = toString (lib.optionals (lib.versionOlder self.version "2.1.10") [
     # Workaround build failure on -fno-common toolchains like upstream
     # clang-13. Without the change build fails as:
     #   duplicate symbol '_static_code_space_free_pointer' in: alloc.o traceroot.o
@@ -151,22 +181,32 @@ stdenv.mkDerivation rec {
   buildPhase = ''
     runHook preBuild
 
-    sh make.sh --prefix=$out --xc-host="${bootstrapLisp}" ${
-                  lib.concatStringsSep " "
-                    (builtins.map (x: "--with-${x}") enableFeatures ++
-                     builtins.map (x: "--without-${x}") disableFeatures)
-                } ${lib.optionalString (stdenv.hostPlatform.system == "aarch64-darwin") "--arch=arm64"}
+    sh make.sh ${lib.concatStringsSep " " self.buildArgs}
     (cd doc/manual ; make info)
 
     runHook postBuild
   '';
 
+  # Tests on ofBorg’s x86_64-darwin platforms are so unstable that a random one
+  # will fail every other run. There’s a deeper problem here; we might as well
+  # disable them entirely so at least the other platforms get to benefit from
+  # testing.
+  doCheck = stdenv.hostPlatform.system != "x86_64-darwin";
+
+  # From the INSTALL docs
+  checkPhase = ''
+    runHook preCheck
+
+    (cd tests && sh run-tests.sh)
+
+    runHook postCheck
+  '';
+
   installPhase = ''
     runHook preInstall
 
     INSTALL_ROOT=$out sh install.sh
 
-    runHook postInstall
   ''
   + lib.optionalString (!purgeNixReferences) ''
     cp -r src $out/lib/sbcl
@@ -176,6 +216,8 @@ stdenv.mkDerivation rec {
        '(("SYS:SRC;**;*.*.*" #P"$out/lib/sbcl/src/**/*.*")
          ("SYS:CONTRIB;**;*.*.*" #P"$out/lib/sbcl/contrib/**/*.*")))
     EOF
+  '' + ''
+    runHook postInstall
   '';
 
   setupHook = lib.optional purgeNixReferences (writeText "setupHook.sh" ''
@@ -186,7 +228,7 @@ stdenv.mkDerivation rec {
   '');
 
   meta = with lib; {
-    description = "Lisp compiler";
+    description = "Common Lisp compiler";
     homepage = "https://sbcl.org";
     license = licenses.publicDomain; # and FreeBSD
     maintainers = lib.teams.lisp.members;
@@ -198,4 +240,4 @@ stdenv.mkDerivation rec {
       "aarch64-linux"
     ];
   };
-}
+})
diff --git a/pkgs/development/compilers/sbcl/search-for-binaries-in-PATH.patch b/pkgs/development/compilers/sbcl/search-for-binaries-in-PATH.patch
new file mode 100644
index 0000000000000..95263ec857533
--- /dev/null
+++ b/pkgs/development/compilers/sbcl/search-for-binaries-in-PATH.patch
@@ -0,0 +1,108 @@
+From 35856b09e3606361b17f21225c759632be1cdf34 Mon Sep 17 00:00:00 2001
+From: Hraban Luyat <hraban@0brg.net>
+Date: Wed, 24 Jan 2024 14:58:53 -0500
+Subject: [PATCH] Search for binaries in tests in PATH, not /usr/bin
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Same as 8ed662fbfeb5dde35eb265f390b55b01f79f70c1 but for tests, and for more
+than just ‘cat’. For the same reasons as that diff.
+---
+ tests/run-program.impure.lisp | 18 ++++++++++--------
+ tests/run-program.test.sh     |  9 ++++-----
+ 2 files changed, 14 insertions(+), 13 deletions(-)
+
+diff --git a/tests/run-program.impure.lisp b/tests/run-program.impure.lisp
+index 0eab8884c..b07d1e4fb 100644
+--- a/tests/run-program.impure.lisp
++++ b/tests/run-program.impure.lisp
+@@ -15,7 +15,7 @@
+ 
+ (defun bin-pwd-ignoring-result ()
+   (let ((initially-open-fds (directory "/proc/self/fd/*" :resolve-symlinks nil)))
+-    (sb-ext:run-program "/usr/bin/pwd" nil :input :stream :output :stream :wait nil)
++    (sb-ext:run-program "pwd" nil :search t :input :stream :output :stream :wait nil)
+     (length initially-open-fds)))
+ 
+ (with-test (:name (run-program :autoclose-streams)
+@@ -49,7 +49,7 @@
+ (with-test (:name (run-program :cat 2)
+                   :skipped-on (or (not :sb-thread) :win32))
+   ;; Tests that reading from a FIFO is interruptible.
+-  (let* ((process (run-program "/bin/cat" '()
++  (let* ((process (run-program "cat" '() :search t
+                                :wait nil :output :stream :input :stream))
+          (in (process-input process))
+          (out (process-output process))
+@@ -167,7 +167,7 @@
+   (defparameter *cat-out* (make-synonym-stream '*cat-out-pipe*)))
+ 
+ (with-test (:name (run-program :cat 5) :fails-on :win32)
+-  (let ((cat (run-program "/bin/cat" nil :input *cat-in* :output *cat-out*
++  (let ((cat (run-program "cat" nil :search t :input *cat-in* :output *cat-out*
+                           :wait nil)))
+     (dolist (test '("This is a test!"
+                     "This is another test!"
+@@ -310,14 +310,16 @@
+   (let ((had-error-p nil))
+     (flet ((barf (&optional (format :default))
+              (with-output-to-string (stream)
+-               (run-program #-netbsd "/usr/bin/perl" #+netbsd "/usr/pkg/bin/perl"
++               (run-program #-netbsd "perl" #+netbsd "/usr/pkg/bin/perl"
+                             '("-e" "print \"\\x20\\xfe\\xff\\x0a\"")
++                            :search #-netbsd t #+netbsd nil
+                             :output stream
+                             :external-format format)))
+            (no-barf ()
+              (with-output-to-string (stream)
+-               (run-program "/bin/echo"
++               (run-program "echo"
+                             '("This is a test")
++                            :search t
+                             :output stream))))
+       (handler-case
+           (barf :utf-8)
+@@ -353,9 +355,9 @@
+                ;; If the permitted inputs are :ANY then leave it be
+                (listp (symbol-value 'run-tests::*allowed-inputs*)))
+       (push (namestring file) (symbol-value 'run-tests::*allowed-inputs*)))
+-    (assert (null (run-program "/bin/cat" '() :input file)))
+-    (assert (null (run-program "/bin/cat" '() :output #.(or *compile-file-truename*
+-                                                            *load-truename*)
++    (assert (null (run-program "cat" '() :search t :input file)))
++    (assert (null (run-program "cat" '() :search t :output #.(or *compile-file-truename*
++                                                                 *load-truename*)
+                                       :if-output-exists nil)))))
+ 
+ 
+diff --git a/tests/run-program.test.sh b/tests/run-program.test.sh
+index 48eaef889..c926e5a05 100755
+--- a/tests/run-program.test.sh
++++ b/tests/run-program.test.sh
+@@ -39,9 +39,8 @@ run_sbcl --eval "(defvar *exit-ok* $EXIT_LISP_WIN)" <<'EOF'
+   (assert (not (zerop (sb-ext:process-exit-code
+                        (sb-ext:run-program "false" () :search t :wait t)))))
+   (let ((string (with-output-to-string (stream)
+-                  (our-run-program    "/bin/echo"
+-                                      '("foo" "bar")
+-                                      :output stream))))
++                  (run-program  "echo" '("foo" "bar")
++                                :search t :output stream))))
+     (assert (string= string "foo bar
+ ")))
+   (format t ";;; Smoke tests: PASS~%")
+@@ -103,8 +102,8 @@ run_sbcl --eval "(defvar *exit-ok* $EXIT_LISP_WIN)" <<'EOF'
+   ;; make sure that a stream input argument is basically reasonable.
+   (let ((string (let ((i (make-string-input-stream "abcdef")))
+                   (with-output-to-string (stream)
+-                    (our-run-program "/bin/cat" ()
+-                                        :input i :output stream)))))
++                    (run-program "cat" ()
++                                 :search t :input i :output stream)))))
+     (assert (= (length string) 6))
+     (assert (string= string "abcdef")))
+ 
+-- 
+2.43.0
+