mkDummySrc: fix handling of build scripts (#122)

* Instead of injecting our own dummy `build.rs` file, we will patch the
  `Cargo.toml` files to specify a build script in the Nix store
* This allow cargo to notice the difference (i.e. changed build script
  path) where it could not before (due to nix enforcing that all sources
  always have the same timestamp)

CHANGELOG.md

@@ -36,6 +36,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 * `buildPackage` now has more robust checks to ensure that all references to
   vendored sources are removed after installation (which avoids consumers of the
   final binaries having to download the sources as well)
+* `mkDummySrc` how handles build scripts in a manner which ensures cargo runs
+  the real script later (instead of thinking it has not changed)
 
 ## [0.6.0] - 2022-09-07
 

checks/compilesFresh.nix

@@ -3,9 +3,16 @@
 , jq
 }:
 
-expected: mkDrv: args:
+expectedArg: mkDrv: args:
 let
-  runCargoAndCheckFreshness = cmd: extra: ''
+  runCargoAndCheckFreshness = cmd: extra:
+  let
+    expected = if builtins.isAttrs expectedArg then
+      expectedArg.${cmd} or ""
+    else
+      expectedArg;
+  in
+  ''
     cargo ${cmd} \
       --release \
       --message-format json-diagnostic-short \
@@ -17,6 +24,7 @@ let
 
     # Make sure only the crate needed building
     if [[ "${expected}" != "$builtTargets" ]]; then
+      echo for command ${cmd}
       echo expected \""${expected}"\"
       echo but got  \""$builtTargets"\"
       false

checks/default.nix

@@ -70,6 +70,38 @@ myPkgs // {
     myLib.cargoBuild {
       src = ./overlapping-targets;
     };
+  compilesFreshWithBuildScript = self.compilesFresh
+    {
+      check = (builtins.concatStringsSep "\n" [
+        "build-script-build"
+        "with-build-script"
+      ]);
+      build = (builtins.concatStringsSep "\n" [
+        "with-build-script"
+      ]);
+      test = (builtins.concatStringsSep "\n" [
+        "with-build-script"
+      ]);
+    }
+    myLib.cargoBuild {
+      src = ./with-build-script;
+    };
+  compilesFreshWithBuildScriptCustom = self.compilesFresh
+    {
+      check = (builtins.concatStringsSep "\n" [
+        "build-script-mycustomscript"
+        "with-build-script-custom"
+      ]);
+      build = (builtins.concatStringsSep "\n" [
+        "with-build-script-custom"
+      ]);
+      test = (builtins.concatStringsSep "\n" [
+        "with-build-script-custom"
+      ]);
+    }
+    myLib.cargoBuild {
+      src = ./with-build-script-custom;
+    };
 
   customCargoTargetDirectory =
     let
@@ -251,6 +283,15 @@ myPkgs // {
 
   vendorGitSubset = callPackage ./vendorGitSubset.nix { };
 
+  # https://github.com/ipetkov/crane/issues/117
+  withBuildScript = myLib.buildPackage {
+    src = ./with-build-script;
+  };
+  # https://github.com/ipetkov/crane/issues/117
+  withBuildScriptCustom = myLib.buildPackage {
+    src = ./with-build-script-custom;
+  };
+
   workspace = myLib.buildPackage {
     src = myLib.cleanCargoSource ./workspace;
     pname = "workspace";

checks/mkDummySrcTests/customized/expected/Cargo.toml

@@ -30,6 +30,7 @@ path = "tests/custom.rs"
 name = "foo"
 
 [package]
+build = "cranespecific-dummy.rs"
 edition = "2021"
 name = "mkDummySrcSimple"
 version = "0.1.0"

checks/mkDummySrcTests/customized/expected/build.rs

@@ -1,2 +0,0 @@
-#![allow(dead_code)]
-pub fn main() {}

checks/mkDummySrcTests/default.nix

@@ -5,7 +5,13 @@
 }:
 
 let
-  doCompare = name: expected: actual:
+  doCompare = name: expected: orig_actual:
+    let
+      actual = runCommand "trim-actual-${name}" { } ''
+        cp --recursive ${orig_actual} --no-target-directory $out --no-preserve=mode,ownership
+        find $out -name Cargo.toml | xargs sed -i"" 's!/nix/store/[^-]\+-dummy.rs!cranespecific-dummy.rs!'
+      '';
+    in
     runCommand "compare-${name}" { } ''
       echo ${expected} ${actual}
       diff -r ${expected} ${actual}

checks/mkDummySrcTests/single-alt/expected/Cargo.toml

@@ -3,6 +3,7 @@ name = "foo"
 path = "src/custom.rs"
 
 [package]
+build = "cranespecific-dummy.rs"
 edition = "2021"
 name = "mkDummySrcSimple"
 version = "0.1.0"

checks/mkDummySrcTests/single-alt/expected/build.rs

@@ -1,2 +0,0 @@
-#![allow(dead_code)]
-pub fn main() {}

checks/mkDummySrcTests/single/expected/Cargo.toml

@@ -30,6 +30,7 @@ path = "tests/custom.rs"
 name = "foo"
 
 [package]
+build = "cranespecific-dummy.rs"
 edition = "2021"
 name = "mkDummySrcSimple"
 version = "0.1.0"

checks/mkDummySrcTests/single/expected/build.rs

@@ -1,2 +0,0 @@
-#![allow(dead_code)]
-pub fn main() {}

checks/mkDummySrcTests/workspace/expected/member/foo/bar/baz/Cargo.toml

@@ -30,6 +30,7 @@ path = "tests/custom.rs"
 name = "foo"
 
 [package]
+build = "cranespecific-dummy.rs"
 edition = "2021"
 name = "bar"
 version = "0.1.0"

checks/mkDummySrcTests/workspace/expected/member/foo/bar/baz/build.rs

@@ -1,2 +0,0 @@
-#![allow(dead_code)]
-pub fn main() {}

checks/mkDummySrcTests/workspace/expected/member/qux/Cargo.toml

@@ -33,6 +33,7 @@ path = "../foo/bar/baz"
 name = "foo"
 
 [package]
+build = "cranespecific-dummy.rs"
 edition = "2021"
 name = "qux"
 version = "0.1.0"

checks/mkDummySrcTests/workspace/expected/member/qux/build.rs

@@ -1,2 +0,0 @@
-#![allow(dead_code)]
-pub fn main() {}

checks/with-build-script-custom/Cargo.lock

@@ -0,0 +1,16 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 3
+
+[[package]]
+name = "byteorder"
+version = "1.4.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610"
+
+[[package]]
+name = "with-build-script-custom"
+version = "0.1.0"
+dependencies = [
+ "byteorder",
+]

checks/with-build-script-custom/Cargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "with-build-script-custom"
+version = "0.1.0"
+edition = "2021"
+build = "mycustomscript.rs"
+
+[build-dependencies]
+byteorder = "*"

checks/with-build-script-custom/mycustomscript.rs

@@ -0,0 +1,10 @@
+use std::{
+    fs,
+    io::{self, Write},
+};
+
+fn main() -> io::Result<()> {
+    fs::create_dir_all("./target")?;
+    fs::File::create("./target/mydata")?.write_all(b"hello world!\n")?;
+    Ok(())
+}

checks/with-build-script-custom/src/main.rs

@@ -0,0 +1,4 @@
+const DATA: &str = include_str!("../target/mydata");
+fn main() {
+    println!("{DATA}");
+}

checks/with-build-script/Cargo.lock

@@ -0,0 +1,16 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 3
+
+[[package]]
+name = "byteorder"
+version = "1.4.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610"
+
+[[package]]
+name = "with-build-script"
+version = "0.1.0"
+dependencies = [
+ "byteorder",
+]

checks/with-build-script/Cargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "with-build-script"
+version = "0.1.0"
+edition = "2021"
+
+[build-dependencies]
+byteorder = "*"

checks/with-build-script/build.rs

@@ -0,0 +1,10 @@
+use std::{
+    fs,
+    io::{self, Write},
+};
+
+fn main() -> io::Result<()> {
+    fs::create_dir_all("./target")?;
+    fs::File::create("./target/mydata")?.write_all(b"hello world!\n")?;
+    Ok(())
+}

checks/with-build-script/src/main.rs

@@ -0,0 +1,4 @@
+const DATA: &str = include_str!("../target/mydata");
+fn main() {
+    println!("{DATA}");
+}

lib/mkDummySrc.nix

@@ -20,6 +20,7 @@ let
 
   inherit (lib)
     optionalString
+    recursiveUpdate
     removePrefix;
 
   inherit (lib.strings) concatStrings;
@@ -139,9 +140,30 @@ let
         cargoTomlDest = builtins.unsafeDiscardStringContext (removePrefix cargoTomlsBase (toString p));
         parentDir = "$out/${dirOf cargoTomlDest}";
 
-        trimmedCargoToml = cleanCargoToml {
-          cargoToml = p;
-        };
+        # Override the cleaned Cargo.toml with a build script which points to our dummy
+        # source. We need a build script present to cache build-dependencies, which can be
+        # achieved by dropping a build.rs file in the source directory. Except that is the most
+        # common format to use, and cargo appears to use file timestamps to check for changes
+        # to the build script, yet nix will strip all timestamps when putting the sources in the
+        # store. This results in cargo not realizing that our dummy script and the project's
+        # _real_ script are, in fact, different. So we work around this by having the Cargo.toml
+        # file point directly to our dummy source in the store.
+        # https://github.com/ipetkov/crane/issues/117
+        trimmedCargoToml =
+          let
+            cleanedCargoToml = cleanCargoToml {
+              cargoToml = p;
+            };
+          in
+          # Only update if we have a `package` definition, workspaces Cargo.tomls don't need updating
+          if cleanedCargoToml ? package then
+            recursiveUpdate
+              cleanedCargoToml
+              {
+                package.build = dummyrs;
+              }
+          else
+            cleanedCargoToml;
 
         safeStubLib =
           if hasAttr "lib" trimmedCargoToml
@@ -160,8 +182,6 @@ let
         mkdir -p ${parentDir}
         cp ${writeTOML "Cargo.toml" trimmedCargoToml} $out/${cargoTomlDest}
       '' + optionalString (trimmedCargoToml ? package) ''
-        # To build build-dependencies
-        ${cpDummy parentDir "build.rs"}
         # To build regular and dev dependencies (cargo build + cargo test)
         ${cpDummy parentDir "src/lib.rs"}