From 9c4b57ac6330da5c6aa795778dd7e0e6c0721d67 Mon Sep 17 00:00:00 2001 From: Ilan Joselevich Date: Sun, 4 Aug 2024 02:30:40 +0300 Subject: [PATCH] fix(third_party/overlays): Patch crate2nix to use mkDerivation for tests The problem with using runCommand and recreating the src directory with lndir is that it changes the file types of individual files, they will now be a symlink instead of a regular file. If you have a crate that tests that a file is of regular type then it will fail inside the crate2nix derivation. Also regenerate Cargo.nix for //tvix as it will be needed in the next commit. Change-Id: I9275602cc17a428f9fdf0e55daf12cd673bbc030 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12131 Autosubmit: Ilan Joselevich Reviewed-by: flokli Tested-by: BuildkiteCI --- ...ation-with-src-instead-of-runCommand.patch | 109 ++++++++++++++++++ third_party/overlays/tvl.nix | 2 + tvix/Cargo.nix | 67 +++++------ 3 files changed, 139 insertions(+), 39 deletions(-) create mode 100644 third_party/overlays/patches/crate2nix-0001-Fix-Use-mkDerivation-with-src-instead-of-runCommand.patch diff --git a/third_party/overlays/patches/crate2nix-0001-Fix-Use-mkDerivation-with-src-instead-of-runCommand.patch b/third_party/overlays/patches/crate2nix-0001-Fix-Use-mkDerivation-with-src-instead-of-runCommand.patch new file mode 100644 index 000000000..fbc18860a --- /dev/null +++ b/third_party/overlays/patches/crate2nix-0001-Fix-Use-mkDerivation-with-src-instead-of-runCommand.patch @@ -0,0 +1,109 @@ +From 96f66ec32e003c6c215aa2a644281289a71dae7d Mon Sep 17 00:00:00 2001 +From: Ilan Joselevich +Date: Sun, 4 Aug 2024 02:35:27 +0300 +Subject: [PATCH] Fix: Use mkDerivation with src instead of runCommand for test + derivation + +The problem with using runCommand and recreating the src directory with +lndir is that it changes the file types of individual files, they will +now be a symlink instead of a regular file. If you have a crate that tests +that a file is of regular type then it will fail inside the crate2nix derivation. +--- + templates/nix/crate2nix/default.nix | 81 ++++++++----------- + 1 file changed, 35 insertions(+), 46 deletions(-) + +diff --git a/templates/nix/crate2nix/default.nix b/templates/nix/crate2nix/default.nix +index c53925e..90e10c6 100644 +--- a/templates/nix/crate2nix/default.nix ++++ b/templates/nix/crate2nix/default.nix +@@ -120,52 +120,41 @@ rec { + testPostRun + ]); + in +- pkgs.runCommand "run-tests-${testCrate.name}" +- { +- inherit testCrateFlags; +- buildInputs = testInputs; +- } '' +- set -e +- +- export RUST_BACKTRACE=1 +- +- # recreate a file hierarchy as when running tests with cargo +- +- # the source for test data +- # It's necessary to locate the source in $NIX_BUILD_TOP/source/ +- # instead of $NIX_BUILD_TOP/ +- # because we compiled those test binaries in the former and not the latter. +- # So all paths will expect source tree to be there and not in the build top directly. +- # For example: $NIX_BUILD_TOP := /build in general, if you ask yourself. +- # NOTE: There could be edge cases if `crate.sourceRoot` does exist but +- # it's very hard to reason about them. +- # Open a bug if you run into this! +- mkdir -p source/ +- cd source/ +- +- ${pkgs.buildPackages.xorg.lndir}/bin/lndir ${crate.src} +- +- # build outputs +- testRoot=target/debug +- mkdir -p $testRoot +- +- # executables of the crate +- # we copy to prevent std::env::current_exe() to resolve to a store location +- for i in ${crate}/bin/*; do +- cp "$i" "$testRoot" +- done +- chmod +w -R . +- +- # test harness executables are suffixed with a hash, like cargo does +- # this allows to prevent name collision with the main +- # executables of the crate +- hash=$(basename $out) +- for file in ${drv}/tests/*; do +- f=$testRoot/$(basename $file)-$hash +- cp $file $f +- ${testCommand} +- done +- ''; ++ pkgs.stdenvNoCC.mkDerivation { ++ name = "run-tests-${testCrate.name}"; ++ ++ inherit (crate) src; ++ ++ inherit testCrateFlags; ++ ++ buildInputs = testInputs; ++ ++ buildPhase = '' ++ set -e ++ export RUST_BACKTRACE=1 ++ ++ # build outputs ++ testRoot=target/debug ++ mkdir -p $testRoot ++ ++ # executables of the crate ++ # we copy to prevent std::env::current_exe() to resolve to a store location ++ for i in ${crate}/bin/*; do ++ cp "$i" "$testRoot" ++ done ++ chmod +w -R . ++ ++ # test harness executables are suffixed with a hash, like cargo does ++ # this allows to prevent name collision with the main ++ # executables of the crate ++ hash=$(basename $out) ++ for file in ${drv}/tests/*; do ++ f=$testRoot/$(basename $file)-$hash ++ cp $file $f ++ ${testCommand} ++ done ++ ''; ++ }; + in + pkgs.runCommand "${crate.name}-linked" + { +-- +2.44.0 + diff --git a/third_party/overlays/tvl.nix b/third_party/overlays/tvl.nix index 217d2017f..8d541f103 100644 --- a/third_party/overlays/tvl.nix +++ b/third_party/overlays/tvl.nix @@ -99,6 +99,8 @@ depot.nix.readTree.drvTargets { crate2nix = super.crate2nix.overrideAttrs (old: { patches = old.patches or [ ] ++ [ + # TODO(Kranzes): Remove in next release. + ./patches/crate2nix-0001-Fix-Use-mkDerivation-with-src-instead-of-runCommand.patch # https://github.com/nix-community/crate2nix/pull/301 ./patches/crate2nix-tests-debug.patch ]; diff --git a/tvix/Cargo.nix b/tvix/Cargo.nix index 96101a926..9df3c8af8 100644 --- a/tvix/Cargo.nix +++ b/tvix/Cargo.nix @@ -20245,52 +20245,41 @@ rec { testPostRun ]); in - pkgs.runCommand "run-tests-${testCrate.name}" - { - inherit testCrateFlags; - buildInputs = testInputs; - } '' - set -e + pkgs.stdenvNoCC.mkDerivation { + name = "run-tests-${testCrate.name}"; - export RUST_BACKTRACE=1 + inherit (crate) src; - # recreate a file hierarchy as when running tests with cargo + inherit testCrateFlags; - # the source for test data - # It's necessary to locate the source in $NIX_BUILD_TOP/source/ - # instead of $NIX_BUILD_TOP/ - # because we compiled those test binaries in the former and not the latter. - # So all paths will expect source tree to be there and not in the build top directly. - # For example: $NIX_BUILD_TOP := /build in general, if you ask yourself. - # NOTE: There could be edge cases if `crate.sourceRoot` does exist but - # it's very hard to reason about them. - # Open a bug if you run into this! - mkdir -p source/ - cd source/ + buildInputs = testInputs; - ${pkgs.buildPackages.xorg.lndir}/bin/lndir ${crate.src} + buildPhase = '' + set -e + export RUST_BACKTRACE=1 - # build outputs - testRoot=target/debug - mkdir -p $testRoot + # build outputs + testRoot=target/debug + mkdir -p $testRoot - # executables of the crate - # we copy to prevent std::env::current_exe() to resolve to a store location - for i in ${crate}/bin/*; do - cp "$i" "$testRoot" - done - chmod +w -R . + # executables of the crate + # we copy to prevent std::env::current_exe() to resolve to a store location + for i in ${crate}/bin/*; do + cp "$i" "$testRoot" + done + chmod +w -R . - # test harness executables are suffixed with a hash, like cargo does - # this allows to prevent name collision with the main - # executables of the crate - hash=$(basename $out) - for file in ${drv}/tests/*; do - f=$testRoot/$(basename $file)-$hash - cp $file $f - ${testCommand} - done - ''; + # test harness executables are suffixed with a hash, like cargo does + # this allows to prevent name collision with the main + # executables of the crate + hash=$(basename $out) + for file in ${drv}/tests/*; do + f=$testRoot/$(basename $file)-$hash + cp $file $f + ${testCommand} + done + ''; + }; in pkgs.runCommand "${crate.name}-linked" {