From 4cdc71c64271f4662c53c338fcd05bf62e41ae94 Mon Sep 17 00:00:00 2001
From: Lorenzo Manacorda <lorenzo@mailbox.org>
Date: Mon, 13 Feb 2023 09:59:06 +0100
Subject: [PATCH 1/4] ci: compile benchmarks via nix

---
 .github/workflows/continuous-integration.yml |  8 ++------
 flake.nix                                    | 14 ++++++++++++++
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml
index 544422b35..7f2ea40cf 100644
--- a/.github/workflows/continuous-integration.yml
+++ b/.github/workflows/continuous-integration.yml
@@ -56,9 +56,5 @@ jobs:
       run: |
         nix flake check --print-build-logs
 
-    - name: Compile and typecheck benchmarks
-      run: |
-        nix develop
-        cargo bench --no-run
-
-        find benches -type f -name "*.ncl" -print0 | xargs -0 -I file nix run . -- -f file typecheck
+    - name: Typecheck benchmarks
+      run: find benches -type f -name "*.ncl" -print0 | xargs -0 -I file nix run . -- -f file typecheck
diff --git a/flake.nix b/flake.nix
index 670e22809..9162edde7 100644
--- a/flake.nix
+++ b/flake.nix
@@ -217,6 +217,19 @@
           nickel = buildPackage "nickel-lang";
           lsp-nls = buildPackage "nickel-lang-lsp";
 
+          benchmarks = craneLib.mkCargoDerivation {
+            inherit src cargoArtifacts;
+
+            pnameSuffix = "-bench";
+
+            buildPhaseCargoCommand = ''
+              cargo bench --no-run
+            '';
+
+            doCheck = false;
+            doInstallCargoArtifacts = false;
+          };
+
           rustfmt = craneLib.cargoFmt {
             # Notice that unlike other Crane derivations, we do not pass `cargoArtifacts` to `cargoFmt`, because it does not need access to dependencies to format the code.
             inherit src;
@@ -411,6 +424,7 @@
           nickel
           lsp-nls
           clippy
+          benchmarks
           rustfmt;
         # An optimizing release build is long: eschew optimizations in checks by
         # building a dev profile

From 7c9cb3846bff505c246ab301a4c2e2673faa7a3b Mon Sep 17 00:00:00 2001
From: Lorenzo Manacorda <lorenzo@mailbox.org>
Date: Mon, 13 Feb 2023 10:51:44 +0100
Subject: [PATCH 2/4] flake: separate benchmark compilation and running

This allows compiling them often in CI (since it's fast), and
benchmarking less often.
---
 flake.nix | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/flake.nix b/flake.nix
index 9162edde7..63e387d49 100644
--- a/flake.nix
+++ b/flake.nix
@@ -222,9 +222,7 @@
 
             pnameSuffix = "-bench";
 
-            buildPhaseCargoCommand = ''
-              cargo bench --no-run
-            '';
+            buildPhaseCargoCommand = "cargo bench";
 
             doCheck = false;
             doInstallCargoArtifacts = false;
@@ -393,6 +391,7 @@
       packages = {
         inherit (mkCraneArtifacts { })
           nickel
+          benchmarks
           lsp-nls;
         default = pkgs.buildEnv {
           name = "nickel";
@@ -424,8 +423,10 @@
           nickel
           lsp-nls
           clippy
-          benchmarks
           rustfmt;
+        compileBenchmarks = (mkCraneArtifacts { }).benchmarks.overrideAttrs (old: {
+          buildPhase = "cargo bench --no-run";
+        });
         # An optimizing release build is long: eschew optimizations in checks by
         # building a dev profile
         nickelWasm = buildNickelWasm { profile = "dev"; };

From f55e6f4f4775bdd17b93ce47b942a8eae1847339 Mon Sep 17 00:00:00 2001
From: Lorenzo Manacorda <lorenzo@mailbox.org>
Date: Wed, 15 Feb 2023 18:36:31 +0100
Subject: [PATCH 3/4] flake: clean up benchmarks invocation

Instead of overriding the buildPhase, we use a function argument with optional string appending.
---
 flake.nix | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/flake.nix b/flake.nix
index 63e387d49..4d4b31bd3 100644
--- a/flake.nix
+++ b/flake.nix
@@ -182,7 +182,7 @@
 
       # Given a rust toolchain, provide Nickel's Rust dependencies, Nickel, as
       # well as rust tools (like clippy)
-      mkCraneArtifacts = { rust ? mkRust { } }:
+      mkCraneArtifacts = { rust ? mkRust { }, noRunBench ? false }:
         let
           craneLib = crane.lib.${system}.overrideToolchain rust;
 
@@ -222,7 +222,9 @@
 
             pnameSuffix = "-bench";
 
-            buildPhaseCargoCommand = "cargo bench";
+            buildPhaseCargoCommand = ''
+              cargo bench ${pkgs.lib.optionalString noRunBench "--no-run"}
+            '';
 
             doCheck = false;
             doInstallCargoArtifacts = false;
@@ -419,14 +421,12 @@
       };
 
       checks = {
-        inherit (mkCraneArtifacts { })
-          nickel
-          lsp-nls
+        inherit (mkCraneArtifacts { noRunBench = true; })
+          benchmarks
           clippy
+          lsp-nls
+          nickel
           rustfmt;
-        compileBenchmarks = (mkCraneArtifacts { }).benchmarks.overrideAttrs (old: {
-          buildPhase = "cargo bench --no-run";
-        });
         # An optimizing release build is long: eschew optimizations in checks by
         # building a dev profile
         nickelWasm = buildNickelWasm { profile = "dev"; };

From bd88092b7e8d30a72f86f403374da036e5e9a13a Mon Sep 17 00:00:00 2001
From: Lorenzo Manacorda <lorenzo@mailbox.org>
Date: Wed, 15 Feb 2023 18:38:50 +0100
Subject: [PATCH 4/4] flake: remove doCheck in benchmarks package

The default check phase has no side effects, so no need to disable it.

Cf. https://crane.dev/API.html#optional-attributes-11
---
 flake.nix | 1 -
 1 file changed, 1 deletion(-)

diff --git a/flake.nix b/flake.nix
index 4d4b31bd3..6c6087545 100644
--- a/flake.nix
+++ b/flake.nix
@@ -226,7 +226,6 @@
               cargo bench ${pkgs.lib.optionalString noRunBench "--no-run"}
             '';
 
-            doCheck = false;
             doInstallCargoArtifacts = false;
           };