From 3f7b5a75e7aafe2a6528172f42475aba229cef54 Mon Sep 17 00:00:00 2001 From: Austin Seipp Date: Wed, 21 Feb 2024 19:50:47 -0600 Subject: [PATCH] github(nix): don't build everything twice When running the `nix build`, the `buildRustPackage` function -- which builds the `jj` crates -- calls `cargo build --release` with flags like `HOST_CXX` set. This is called the `buildPhase`. Then, it runs the `checkPhase`, which calls `cargo nextest`, in our case. However, it does not set `HOST_CXX`, for some reason. The intent of `buildRustPackage` is that the `buildPhase` and `checkPhase` have their compilation options fully aligned so that they reuse the local cargo `target/` cache directory, avoiding a full recompilation. However, the lack of `HOST_CXX` above among others causes a cache miss, and a bunch of cascading recompilations. The net impact is that we compile all of the codebase once in `buildPhase`, then again in `checkPhase`, making the Nix CI build 2x slower on average than the other Linux runners; 2-3 minutes versus 7 minutes, on average. Instead, re-introduce a 'check' into the Flake directly, which overrides the `jujustsu` package, but stubs out the `buildPhase`. This means it will only run `checkPhase`, which is what we want. Then, modify the CI to run `nix flake check` again, like it used to in the past. Unfortunately, this doesn't fix the cache miss when running `nix build` yourself, it recompiles from scratch in both phases still. That needs to be fixed in the future, but it's tolerable for now. This reverts most of 71a3045032f512. Signed-off-by: Austin Seipp --- .github/workflows/nix-linux.yml | 2 +- flake.nix | 21 ++++++++++++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/.github/workflows/nix-linux.yml b/.github/workflows/nix-linux.yml index 771a0adb6..288ef4be2 100644 --- a/.github/workflows/nix-linux.yml +++ b/.github/workflows/nix-linux.yml @@ -19,4 +19,4 @@ jobs: fetch-depth: 0 - uses: DeterminateSystems/nix-installer-action@cd46bde16ab981b0a7b2dce0574509104543276e - uses: DeterminateSystems/magic-nix-cache-action@eeabdb06718ac63a7021c6132129679a8e22d0c7 - - run: nix build --print-build-logs --show-trace + - run: nix flake check -L --show-trace diff --git a/flake.nix b/flake.nix index 264dd1d1f..cdbf1e785 100644 --- a/flake.nix +++ b/flake.nix @@ -67,7 +67,7 @@ version = "unstable-${self.shortRev or "dirty"}"; buildFeatures = [ "packaging" ]; - cargoBuildFlags = ["--bin" "jj"]; # don't build and install the fake editors + cargoBuildFlags = [ "--bin" "jj" ]; # don't build and install the fake editors useNextest = true; src = filterSrc ./. [ ".*\\.nix$" @@ -110,11 +110,30 @@ }; default = self.packages.${system}.jujutsu; }; + apps.default = { type = "app"; program = "${self.packages.${system}.jujutsu}/bin/jj"; }; + formatter = pkgs.nixpkgs-fmt; + + checks.jujutsu = self.packages.${system}.jujutsu.overrideAttrs ({ ... }: { + # FIXME (aseipp): when running `nix flake check`, this will override the + # main package, and nerf the build and installation phases. this is + # because for some inexplicable reason, the cargo cache gets invalidated + # in between buildPhase and checkPhase, causing every nix CI build to be + # 2x as long. + # + # upstream issue: https://github.com/NixOS/nixpkgs/issues/291222 + buildPhase = "true"; + installPhase = "touch $out"; + # NOTE (aseipp): buildRustPackage also, by default, runs `cargo check` + # in `--release` mode, which is far slower; the existing CI builds all + # use the default `test` profile, so we should too. + cargoCheckType = "test"; + }); + devShells.default = pkgs.mkShell { buildInputs = with pkgs; [ # The CI checks against the latest nightly rustfmt, so we should too.