using vendorCargoDeps leads to permission errors when building sqlite-sys

[dpc]

After upgrading crane:

-    crane.url = "github:ipetkov/crane?rev=98894bb39b03bfb379c5e10523cd61160e1ac782"; # https://github.com/ipetkov/crane/releases/tag/v0.11.0
+    crane.url = "github:ipetkov/crane?ref=master&ref=953b70da2813fb882c39890f2514e7db76fc8843";

and with some minor changes that are probably(?) unrelated cross-compiling sqlite is failing with:

client-pkgs-aarch64-android>    Compiling pin-project v1.0.12                                                      
client-pkgs-aarch64-android> error: failed to run custom build command for `libsqlite3-sys v0.24.2`                                                                                                                                    
client-pkgs-aarch64-android> Caused by:                                                                            
client-pkgs-aarch64-android>   process didn't exit successfully: `/build/source/target/release/build/libsqlite3-sys-0323aa547c5cd008/build-script-build` (exit status: 101)                                                            
client-pkgs-aarch64-android>   --- stderr                                                                          
client-pkgs-aarch64-android>   thread 'main' panicked at 'Could not copy bindings to output directory: Os { code: 13, kind: PermissionDenied, message: "Permission denied" }', /nix/store/nmzj27xi1xb0w2jj28gxl7410l32a9z2-vendor-cargo
-deps/c19b7c6f923b580ac259164a89f2577984ad5ab09ee9d583b888f934adbbe8d0/libsqlite3-sys-0.24.2/build.rs:102:18       
client-pkgs-aarch64-android>   note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
client-pkgs-aarch64-android> warning: build failed, waiting for other jobs to finish...
error: builder for '/nix/store/dcv5s7av22jmjkgxxj7cn8pzcvcavs92-client-pkgs-aarch64-android-0.0.1.drv' failed with exit code 101;                                                                                                      
       last 10 log lines:                                                                                          
       >    Compiling tracing v0.1.37                                                                              
       >    Compiling pin-project v1.0.12               
       > error: failed to run custom build command for `libsqlite3-sys v0.24.2`                                    
       >                                                                                                           
       > Caused by:                                                                                                
       >   process didn't exit successfully: `/build/source/target/release/build/libsqlite3-sys-0323aa547c5cd008/build-script-build` (exit status: 101)                                                                                
       >   --- stderr                                                                                              
       >   thread 'main' panicked at 'Could not copy bindings to output directory: Os { code: 13, kind: PermissionDenied, message: "Permission denied" }', /nix/store/nmzj27xi1xb0w2jj28gxl7410l32a9z2-vendor-cargo-deps/c19b7c6f923b58
0ac259164a89f2577984ad5ab09ee9d583b888f934adbbe8d0/libsqlite3-sys-0.24.2/build.rs:102:18

The corresponding line in the build script: https://github.com/rusqlite/rusqlite/blob/9880cdef12c1b624e351ed077dd71a2f77978f77/libsqlite3-sys/build.rs#L101

[dpc]

I bet it's 18ef99b but I don't see how.

[ipetkov]

@dpc Could you please provide a reproduction of the failure?

I use libsqlite3-sys in a few of my personal projects but I wasn't able to reproduce the issue there

[dpc]

Oh dear. :D

Now that you mention it one difference might be that this is a cross-compilation to aarch64-linux-android, but I don't know how this could make it different.

I'll try to minimize at some point, but it might take me a while before I can get to it.

[dpc]

If it is of any help, here is the PR to our project that triggers it: fedimint/fedimint#1720

[ipetkov]

Cross compilation can get quite hairy, but I can try to take a quick look...

@dpc what nix build .#??? invocation do I need to trigger the issue?

[dpc]

@ipetkov nix build -L .#cross.aarch64-android.client-pkgs

And yeah, it is. Though it's hairy outside of crane layer. Mostly setting up all the C stuff to have C deps compile.

[ipetkov]

Fails to build with nix build -L github:dpc/fedimint/crane-rebuilds-fix#cross.aarch64-android.client-pkgs --accept-flake-config --override-input crane github:ipetkov/crane/v0.11.0

I also tried all other v.0.11.x and even some older versions for good measure, but it still fails to build, so I can't tell if this is a recent crane regression.

[dpc]

--override-input crane github:ipetkov/crane/v0.11.0 will not work as some things about src etc. handling had changed. But our master branch uses v0.11.0 and it works.

[ipetkov]

Building with the latest commit on the master branch seems to work fine with v0.11.3

nix build -L github:fedimint/fedimint/4c914914ca46cf02dea501735fe41cc026ef6d49#cross.aarch64-android.client-pkgs --accept-flake-config --override-input crane github:ipetkov/crane/v0.11.3

[dpc]

I don't understand how and why, but that command you're using does work for me, but when I run nix build -L .#cross.aarch64-android.client-pkgs on my branch it fails, and it also does in our CI: https://github.com/fedimint/fedimint/actions/runs/4228754507/jobs/7344488178

If you look at this PR (I just rebased it, but it didn't change anything): https://github.com/fedimint/fedimint/pull/1720/files I'm setting crane input to crane.url = "github:ipetkov/crane?ref=master&ref=953b70da2813fb882c39890f2514e7db76fc8843"; which seems like exactly same commit as v0.11.3 is.

😕

Will investigate more later.

[dpc]

Oh. You're testing master of fedimint with v0.11.3 ... Hmmm... Will check it later.

[dpc]

OK, so the root of the problem is removing src = ./.; and adding cargoVendorDir = craneLib.vendorCargoDeps { src = ./.; }; instead in the buildDepsOnly call. I remove src because now it complains about that buildDepsOnly will ignore src when dummySrc is specified, but after removing src it complains that:

error: unable to find `src` attribute. consider one of the following:
       - set `cargoVendorDir = vendorCargoDeps { cargoLock = ./some/path/to/Cargo.lock; }`
       - set `cargoVendorDir = vendorCargoDeps { src = ./src/containing/cargo/lock/file; }`
       - set `cargoVendorDir = null` to skip vendoring altogether
(use '--show-trace' to show detailed location information)

so I have to add that cargoVendorDir = craneLib.vendorCargoDeps { src = ./.; }; and then compilation fails somehow.

@ipetkov

[dpc]

BTW. set cargoVendorDir = vendorCargoDeps { cargoLock = ./some/path/to/Cargo.lock; } does not seem to be correct as it doesn't work as specified, neither if I try to use cargLock = (builtins.readFile Cargo.lock) (complains about string context then).

[dpc]

I have also confirmed that the problem is there with v0.11.0, so it seems entirely about the usage of vendorCargoDeps.

[ipetkov]

@dpc

• Yes vendorCargoDir needs a src attribute so you will need to explicitly set cargoVendorDir if you are going to be setting dummySrc instead of src
• I don't know what that permissions issue is, but I'll need a minimal repro I can run and look at (there's been lots of debugging in this thread already so maybe a clean slate would be useful)

[dpc]

@ipetkov OK. I think I've figured out the root cause.

First, I've noticed that when I use cargoVendorDir, I get the dependencies being rebuilt again. That is what is causing the permission issue as the file in question is there:

[-r--r--r-- nixbld   nixbld  ]  bindgen.rs

and doesn't have a w.

So the problem is not the permissions, but the fact that ./target/ is there, yet everything is being rebuilt from scratch.

Initially I suspected some timestamps, but that didn't seem like the case.

Turns out the vedored config.toml that is being appended is different:

> diff -u /nix/store/nvhhxpvphziaibzdgwbxa9k17hspp799-vendor-cargo-deps/config.toml /nix/store/xjp9wg7zqzcii8i29shwsijrb1fh3jd7-vendor-cargo-deps/config.toml
--- /nix/store/nvhhxpvphziaibzdgwbxa9k17hspp799-vendor-cargo-deps/config.toml   1969-12-31 16:00:01.000000000 -0800
+++ /nix/store/xjp9wg7zqzcii8i29shwsijrb1fh3jd7-vendor-cargo-deps/config.toml   1969-12-31 16:00:01.000000000 -0800
@@ -1,17 +1,17 @@
 [source.nix-sources-c19b7c6f923b580ac259164a89f2577984ad5ab09ee9d583b888f934adbbe8d0]
-directory = "/nix/store/nvhhxpvphziaibzdgwbxa9k17hspp799-vendor-cargo-deps/c19b7c6f923b580ac259164a89f2577984ad5ab09ee9d583b888f934adbbe8d0"
+directory = "/nix/store/xjp9wg7zqzcii8i29shwsijrb1fh3jd7-vendor-cargo-deps/c19b7c6f923b580ac259164a89f2577984ad5ab09ee9d583b888f934adbbe8d0"
 [source.crates-io]
 registry = "https://github.com/rust-lang/crates.io-index"
 replace-with = "nix-sources-c19b7c6f923b580ac259164a89f2577984ad5ab09ee9d583b888f934adbbe8d0"
 
 [source.nix-sources-49a40141ec3504d7802b09cb1cadde46c2ab4d5c45cc1a77317d06cea31d69ad]
-directory = "/nix/store/nvhhxpvphziaibzdgwbxa9k17hspp799-vendor-cargo-deps/49a40141ec3504d7802b09cb1cadde46c2ab4d5c45cc1a77317d06cea31d69ad"
+directory = "/nix/store/xjp9wg7zqzcii8i29shwsijrb1fh3jd7-vendor-cargo-deps/49a40141ec3504d7802b09cb1cadde46c2ab4d5c45cc1a77317d06cea31d69ad"
 [source.nix-sources-7b3ec600ad985a32a8367f459b9daa32421b08cf3f641c9eeed5b27bb8fe7916]
-directory = "/nix/store/nvhhxpvphziaibzdgwbxa9k17hspp799-vendor-cargo-deps/7b3ec600ad985a32a8367f459b9daa32421b08cf3f641c9eeed5b27bb8fe7916"
+directory = "/nix/store/xjp9wg7zqzcii8i29shwsijrb1fh3jd7-vendor-cargo-deps/7b3ec600ad985a32a8367f459b9daa32421b08cf3f641c9eeed5b27bb8fe7916"
 [source.nix-sources-a68b9b47e08aa92cbe800ee65f8835d0ff8dc152cc7557a9e5dcbcffd188485d]
-directory = "/nix/store/nvhhxpvphziaibzdgwbxa9k17hspp799-vendor-cargo-deps/a68b9b47e08aa92cbe800ee65f8835d0ff8dc152cc7557a9e5dcbcffd188485d"
+directory = "/nix/store/xjp9wg7zqzcii8i29shwsijrb1fh3jd7-vendor-cargo-deps/a68b9b47e08aa92cbe800ee65f8835d0ff8dc152cc7557a9e5dcbcffd188485d"
 [source.nix-sources-c6ac1166e8d5fb3cca134f5488fbc0796e323e56dab7b0dc4a84ae8cb361cdc3]
-directory = "/nix/store/nvhhxpvphziaibzdgwbxa9k17hspp799-vendor-cargo-deps/c6ac1166e8d5fb3cca134f5488fbc0796e323e56dab7b0dc4a84ae8cb361cdc3"
+directory = "/nix/store/xjp9wg7zqzcii8i29shwsijrb1fh3jd7-vendor-cargo-deps/c6ac1166e8d5fb3cca134f5488fbc0796e323e56dab7b0dc4a84ae8cb361cdc3"
 [source."https://github.com/dpc/rust-secp256k1-zkp/?branch=sanket-pr"]
 replace-with = "nix-sources-7b3ec600ad985a32a8367f459b9daa32421b08cf3f641c9eeed5b27bb8fe7916"
 git = "https://github.com/dpc/rust-secp256k1-zkp/"

As you can see the content is roughly the same, but the exact paths where the vendored dependencies are located are different, which (I think) is causing cargo to think that config.toml is different, so the whole project requires a rebuild. Which causes the sqlite-sys to hit a problem overwriting it's own out file (I'll report it to them, it seems that might cause issues).

I don't know why content of that confit.toml is different, but I bet it will be obvious to you.

[dpc]

In addition the fact that this bindgen.rs has lost w seems also caused by crane (or ... nix, I guess?). The original file is in called bindgen_bundled_version.rs and has w set both in git, and in the extracted crate directory , but in the /nix/store it lost it:

> ls -alh /nix/store/9hk4985cliilyq8ly4dzydyl9r2aa5rz-cargo-package-libsqlite3-sys-0.24.2/sqlite3/bindgen_bundled_version.rs 
-r--r--r-- 3 root root 182K Dec 31  1969 /nix/store/9hk4985cliilyq8ly4dzydyl9r2aa5rz-cargo-package-libsqlite3-sys-0.24.2/sqlite3/bindgen_bundled_version.rs

It would probably be safest for crane to preserve it. But it would not be an issue in itself, if it wasn't for this rebuilt.

[ipetkov]

I don't know why content of that confit.toml is different, but I bet it will be obvious to you.

vendorCargoDeps will look at the top level Cargo.lock of the provided source, along with any .cargo/config.toml files which can be found anywhere in the provided source tree. If you are providing a different source than was used before, then the hashes will change if there is a difference (e.g. an extra .cargo/config.toml file was added or removed). I'd probably need to see the code calling vendorCargoDeps before and after the change which led to the rebuild

---

In addition the fact that this bindgen.rs has lost w seems also caused by crane (or ... nix, I guess?).
Crates are vendored in /nix/store which is always read-only, there isn't anything we can "preserve" there.

Build scripts must not change any source files and only make changes to the $OUT_DIR only. The next question is why did the build suddenly start trying to modify source files where it didn't before (maybe a feature flag also changed indirectly enabling a build script path which didn't run before)

[dpc]

If you are providing a different source than was used before, then the hashes will change if there is a difference

I'm uncommenting these 3 lines, and commenting out the one right after:

https://github.com/fedimint/fedimint/blob/51f76884a3d0ec73b5b51a1b94af781ae38b06e5/flake.nix#L336

Build scripts must not change any source files and only make changes to the $OUT_DIR only. The next question is why did the build suddenly start trying to modify source files where it didn't before (maybe a feature flag also changed indirectly enabling a build script path which didn't run before)

Yes. But the build script is trying to overwrite a file that is already there as it's running again (it put this file there in the previous run), and now the file is ro because vendoring in /nix/store turned it into ro.

[ipetkov]

@dpc here's what I noticed:

Setting the following will break running nix build -L .#cross.aarch64-linux-android.client-pkgs

        commonArgsDepsOnly = commonArgsBase // {
          cargoVendorDir = craneLib.vendorCargoDeps {
            src = filterWorkspaceFiles ./.;
          };
          src = null;
          # other args
      };

BUT also setting the following fixes it again:

        commonArgs = commonArgsBase // {
          src = filterWorkspaceFiles ./.;
          cargoVendorDir = craneLib.vendorCargoDeps {
            src = filterWorkspaceFiles ./.;
          };
        };

Clearly there is a difference between the explicit vendorCargoDeps { src = filterWorkspaceFiles ./.; } call and the default vendoring that crane will apply with the provided args (which is what you noticed above given that the hashes changed). My hunch is that maybe it has something to do with the differences between filterWorkspaceFiles and filterModules

As to why this difference manifests in a build script failure I have absolutely no idea. I'd guess that if somehow the different filters are adding/omitting either a .cargo/config.toml or a Cargo.toml which somehow leads to changing the build behavior (e.g. either by changing what feature flags are used or how the dependencies are rebuilt).

If you do manage to come up with a really small reproduction (both in terms of Nix configs and cargo dependencies) I'd love to see it! But until then, I've reached my limits in being able to debug your project given my lack of familiarity and its complexity.

Hope this is useful!

[dpc]

Clearly there is a difference between the explicit vendorCargoDeps { src = filterWorkspaceFiles ./.; } call and the default vendoring that crane will apply with the provided args (which is what you noticed above given that the hashes changed). My hunch is that maybe it has something to do with the differences between filterWorkspaceFiles and filterModules

Well, the easy way to test it is to do:

          cargoVendorDir = craneLib.vendorCargoDeps {
            src = ./.;
          };

(no filtering). And that also fails, all the same. So filtering is not an issue. I assume craneLib.vendorCargoDeps should do whatever filtering it needs.

It seems to me that the difference is between my flake.nix calling craneLib.vendorCargoDeps directly vs crane calling it internally somehow. Isn't that kind of exactly the same issue that was causing #242 (comment) ? Then it was mkDummySrc if IIRC, here is some vendoring.

As to why this difference manifests in a build script failure I have absolutely no idea.

I've described it couple of times, and I think it's important you understand:

cargo build detecting that config.toml change seems to re re-run the whole build from the start while keeping the ./target extracted from deps-only build in place.

The build script of libsqlite3-sys does effectively:

fs::copy(somefile_rs_file_from_src,$out/bingen.rs);

In normal (non nix build ... run) $out/bingen.rs is RW:

> ls -alh (find . -name bindgen.rs)
-rw-r--r-- 1 dpc dpc 182K Feb 24 21:16 ./target/debug/build/libsqlite3-sys-350e0779e242deef/out/bindgen.rs

so overwriting it is not a problem, so the build script doesn't bother detecting if it exists, or deleting it.

But in nix build target/.../out/bindgen.rs is RO as the vendored source code lost the u+w and fs::copy seems to copy the permissions into ./target directory. So fs::copy overwriting the same file that is already there seems to fails.

I don't know if it's necessary for crane to preserve permissions on the vendored source code files, but it is a difference from the original behavior that might trip build script in certain scenarios (maybe?).

[dpc]

But in nix build target/.../out/bindgen.rs is RO as the vendored source code lost the u+w and fs::copy seems to copy the permissions into ./target directory.

Hmmm... just occurred to me that 18ef99b#diff-119bd89c89c2f9eb9fa8a9c1f9a1e98fb14f0d473bfe1d1061122fbb8053c178R42

is doing chmod -R u+w ./target ... so why did I see:

[-r--r--r-- nixbld   nixbld  ]  bindgen.rs

when running recursive tree in a preBuild(?) hook? That does not seem to compute.

[ipetkov]

Alright, I found the issue: you are mixing artifacts from two different
toolchains which causes cargo to rebuild everything. Still no clue why the
libsqlite3-sys permission issue crops up but let's ignore that as a red herring
for now.

---

First apply this change to flake.nix:

        commonArgsDepsOnly = commonArgsBase // {
          cargoVendorDir = craneLib.vendorCargoDeps {
            src = filterWorkspaceFiles ./.;
          };
          src = null;
          # other args
      };

Then we check the vendored dependencies derivation this will create:

nix show-derivation --accept-flake-config .#cross.aarch64-linux-android.client-pkgs | jq 'to_entries[].value.env.cargoVendorDir'
# "/nix/store/cy6498gn79brjz1mmpjp208q10kkk6v0-vendor-cargo-deps"

Next we apply the second change to flake.nix (which "fixes" the build):

        commonArgs = commonArgsBase // {
          src = filterWorkspaceFiles ./.;
          cargoVendorDir = craneLib.vendorCargoDeps {
            src = filterWorkspaceFiles ./.;
          };
        };

Then we check the vendored dependencies derivation this will create:

nix show-derivation --accept-flake-config .#cross.aarch64-linux-android.client-pkgs | jq 'to_entries[].value.env.cargoVendorDir'
# "/nix/store/vwgxblsq4z9c6hf0yaw7y53rfx4ykslb-vendor-cargo-deps"

They are different! Let's compare their differences:

diff -r /nix/store/cy6498gn79brjz1mmpjp208q10kkk6v0-vendor-cargo-deps /nix/store/vwgxblsq4z9c6hf0yaw7y53rfx4ykslb-vendor-cargo-deps

2c2
< directory = "/nix/store/cy6498gn79brjz1mmpjp208q10kkk6v0-vendor-cargo-deps/c19b7c6f923b580ac259164a89f2577984ad5ab09ee9d583b888f934adbbe8d0"
---
> directory = "/nix/store/vwgxblsq4z9c6hf0yaw7y53rfx4ykslb-vendor-cargo-deps/c19b7c6f923b580ac259164a89f2577984ad5ab09ee9d583b888f934adbbe8d0"
8c8
< directory = "/nix/store/cy6498gn79brjz1mmpjp208q10kkk6v0-vendor-cargo-deps/49a40141ec3504d7802b09cb1cadde46c2ab4d5c45cc1a77317d06cea31d69ad"
---
> directory = "/nix/store/vwgxblsq4z9c6hf0yaw7y53rfx4ykslb-vendor-cargo-deps/49a40141ec3504d7802b09cb1cadde46c2ab4d5c45cc1a77317d06cea31d69ad"
10c10
< directory = "/nix/store/cy6498gn79brjz1mmpjp208q10kkk6v0-vendor-cargo-deps/7b3ec600ad985a32a8367f459b9daa32421b08cf3f641c9eeed5b27bb8fe7916"
---
> directory = "/nix/store/vwgxblsq4z9c6hf0yaw7y53rfx4ykslb-vendor-cargo-deps/7b3ec600ad985a32a8367f459b9daa32421b08cf3f641c9eeed5b27bb8fe7916"
12c12
< directory = "/nix/store/cy6498gn79brjz1mmpjp208q10kkk6v0-vendor-cargo-deps/a68b9b47e08aa92cbe800ee65f8835d0ff8dc152cc7557a9e5dcbcffd188485d"
---
> directory = "/nix/store/vwgxblsq4z9c6hf0yaw7y53rfx4ykslb-vendor-cargo-deps/a68b9b47e08aa92cbe800ee65f8835d0ff8dc152cc7557a9e5dcbcffd188485d"
14c14
< directory = "/nix/store/cy6498gn79brjz1mmpjp208q10kkk6v0-vendor-cargo-deps/c6ac1166e8d5fb3cca134f5488fbc0796e323e56dab7b0dc4a84ae8cb361cdc3"
---
> directory = "/nix/store/vwgxblsq4z9c6hf0yaw7y53rfx4ykslb-vendor-cargo-deps/c6ac1166e8d5fb3cca134f5488fbc0796e323e56dab7b0dc4a84ae8cb361cdc3"

We can see that the vendoring is generating the same contents, except just
pointing to different store hashes (which happen to have the same contents).
config.toml differs because it refers to those different hashes.

Let's compare one of those vendored directories in more detail:

diff -u \
<(nix show-derivation $(readlink /nix/store/cy6498gn79brjz1mmpjp208q10kkk6v0-vendor-cargo-deps/49a40141ec3504d7802b09cb1cadde46c2ab4d5c45cc1a77317d06cea31d69ad)) \
<(nix show-derivation $(readlink /nix/store/vwgxblsq4z9c6hf0yaw7y53rfx4ykslb-vendor-cargo-deps/49a40141ec3504d7802b09cb1cadde46c2ab4d5c45cc1a77317d06cea31d69ad))

The interesting thing this shows us is that the cargo-git derivation is
different. Let's look closer:

diff -u \
<(nix show-derivation /nix/store/h735ia59r947c75nca9hvdy74sh0gd0m-cargo-git) \
<(nix show-derivation /nix/store/y1xfjb5nqz1kv1r2jg51n9w50zz35dqy-cargo-git)

Finally we see that one derivation depends on
/nix/store/2f9aqnssah73fypfz59j7hr6wg6lx2py-rust-mixed.drv while the other
depends on
/nix/store/vyij3bkvpw4s5n2z3rpzncgq4imd9f6i-rust-stable-with-components-2023-01-26.drv.

---

This highlights that the buildDepsOnly derivation is probably being built with
one toolchain while the actual buildPackage derivation is built with another
(I see the flake contains craneLib and craneLibCross).
Caching will never properly function if toolchains are mixed, so it's something for
you to fix @dpc

---

As to why vendoring with two different toolchains leads to different hashes
(with the same results): this is because vendoring git deps requires some
processing of Cargo.toml files. This is implemented by the crane-utils
package, which also uses the current toolchain to be built. Perhaps
crane-utils should be built with a fixed toolchain just to avoid confusion as
to why they are different, but this is an independent thing to investigate.

[dpc]

Alright, I found the issue: you are mixing artifacts from two different
toolchains which causes cargo to rebuild everything.

Oh wow. I get it now. Thank you!

Fixing this is going to be very inconvenient, as it requires parametrizing almost everything over craneLib to use.

[ipetkov]

Perhaps
crane-utils should be built with a fixed toolchain just to avoid confusion as
to why they are different, but this is an independent thing to investigate.

Looked into this, although it is possible to build crane-utils with a fixed toolchain, we still need to pull in cargo when vendoring git deps (cargo needs to tell us what crates are part of the workspace so we don't blindly vendor the wrong thing and cause other problems), meaning there isn't a way that we can make vendoring-git-deps not rely on the current toolchain...

I'll document this limitation for the time being, thanks for reporting this!

[dpc]

I fixed our flake.nix after all, it just requires parametrizing bunch of stuff over craneLib to use.

Closing and thanks for help!