Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Current bzip2-sys fails to build with current crane #411

Closed
ISibboI opened this issue Oct 2, 2023 · 27 comments · Fixed by #413
Closed

Current bzip2-sys fails to build with current crane #411

ISibboI opened this issue Oct 2, 2023 · 27 comments · Fixed by #413

Comments

@ISibboI
Copy link
Contributor

ISibboI commented Oct 2, 2023

I created a minimal example here (execute nix build in the repo): https://github.com/ISibboI/crane-bug/tree/03ab836782e10dce38425b3c644c9b564b47d729

Here is what happens:

@nix { "action": "setPhase", "phase": "unpackPhase" }
unpacking sources
unpacking source archive /nix/store/mb2lfsf811nfaps6r96pxwg64nlc0rb8-source
source root is source
@nix { "action": "setPhase", "phase": "patchPhase" }
patching sources
Executing configureCargoCommonVars
copying cargo artifacts from /nix/store/7fa54fs0jydjdxrqra9wfxj11h0wz1n2-crane-bug-deps-0.1.0/target to target
@nix { "action": "setPhase", "phase": "updateAutotoolsGnuConfigScriptsPhase" }
updateAutotoolsGnuConfigScriptsPhase
@nix { "action": "setPhase", "phase": "configurePhase" }
configuring
will append /build/source/.cargo-home/config.toml with contents of /nix/store/31m598m7nr3l0fc9siw9ji31yyccs24a-vendor-cargo-deps/config.toml
default configurePhase, nothing to do
@nix { "action": "setPhase", "phase": "buildPhase" }
building
++ command cargo --version
cargo 1.72.1 (103a7ff2e 2023-08-15)
++ command cargo build --locked --profile release --message-format json-render-diagnostics --bin crane-bug
   Compiling bzip2-sys v0.1.11+1.0.8
error: output file /build/source/target/release/deps/libbzip2_sys-1bf62c2d35b58421.rmeta is not writeable -- check its permissions

error: could not compile `bzip2-sys` (lib) due to previous error
@dpc
Copy link
Contributor

dpc commented Oct 2, 2023

Switch to use-zstd as a fix/workaround. @ISibboI

@ISibboI
Copy link
Contributor Author

ISibboI commented Oct 2, 2023

Are you sure? In the docs it says that "installCargoArtifactsMode will be set to "use-zstd" if not specified."
And I did not specify it? Or should I use this parameter somewhere else?

@newAM
Copy link

newAM commented Oct 2, 2023

This occurs for many libraries at the moment - openssl-sys, rusb, seems to be anything that links a dynamic library.

@dpc
Copy link
Contributor

dpc commented Oct 2, 2023

Are you sure? In the docs it says that "installCargoArtifactsMode will be set to "use-zstd" if not specified."

Oh. I thought use-symlink was a default since couple of versions ago. I'm not sure. But not writeable files in ./target suggest them being symlinks to /nix/store/

@ISibboI
Copy link
Contributor Author

ISibboI commented Oct 2, 2023

Are you sure? In the docs it says that "installCargoArtifactsMode will be set to "use-zstd" if not specified."

Oh. I thought use-symlink was a default since couple of versions ago. I'm not sure.

I see, thanks for the additional info! I think that maybe the docs are outdated? I will try it when I have time.

@dpc
Copy link
Contributor

dpc commented Oct 2, 2023

This occurs for many libraries at the moment - openssl-sys, rusb, seems to be anything that links a dynamic library.

I'm building openssl in our project every day with use-zstd just fine, FWIW.

@newAM
Copy link

newAM commented Oct 2, 2023

I'm building openssl in our project every day with use-zstd just fine, FWIW.

Here's my reproduction (forked from ISibboI): https://github.com/newAM/crane-bug

Have you updated nixpkgs recently and are you using the unstable channel? This only started occurring recently on the unstable channel.

@ipetkov
Copy link
Owner

ipetkov commented Oct 2, 2023

Are you sure? In the docs it says that "installCargoArtifactsMode will be set to "use-zstd" if not specified." And I did not specify it? Or should I use this parameter somewhere else?

@ISibboI this only applies to using buildPackage without specifying cargoArtifacts. If you are manually setting cargoArtifacts = craneLib.buildDepsOnly { ... }; then you'll need to explicitly set installCargoArtifactsMode there

@ISibboI
Copy link
Contributor Author

ISibboI commented Oct 2, 2023

@ISibboI this only applies to using buildPackage without specifying cargoArtifacts. If you are manually setting cargoArtifacts = craneLib.buildDepsOnly { ... }; then you'll need to explicitly set installCargoArtifactsMode there

Thanks @ipetkov ! I updated the flake.

Now I am getting the following error:

Error log
@nix { "action": "setPhase", "phase": "unpackPhase" }
unpacking sources
unpacking source archive /nix/store/mb2lfsf811nfaps6r96pxwg64nlc0rb8-source
source root is source
@nix { "action": "setPhase", "phase": "patchPhase" }
patching sources
Executing configureCargoCommonVars
decompressing cargo artifacts from /nix/store/5c0xk29v577vlmjsfx7dpkplh261w999-crane-bug-deps-0.1.0/target.tar.zst to target
@nix { "action": "setPhase", "phase": "updateAutotoolsGnuConfigScriptsPhase" }
updateAutotoolsGnuConfigScriptsPhase
@nix { "action": "setPhase", "phase": "configurePhase" }
configuring
will append /build/source/.cargo-home/config.toml with contents of /nix/store/31m598m7nr3l0fc9siw9ji31yyccs24a-vendor-cargo-deps/config.toml
default configurePhase, nothing to do
@nix { "action": "setPhase", "phase": "buildPhase" }
building
++ command cargo --version
cargo 1.72.1 (103a7ff2e 2023-08-15)
++ command cargo build --locked --profile release --message-format json-render-diagnostics --bin crane-bug
   Compiling bzip2-sys v0.1.11+1.0.8
error: failed to run custom build command for `bzip2-sys v0.1.11+1.0.8`

Caused by:
  process didn't exit successfully: `/build/source/target/release/build/bzip2-sys-bd443bb3273b170c/build-script-build` (exit status: 101)
  --- stdout
  cargo:rerun-if-env-changed=BZIP2_NO_PKG_CONFIG
  cargo:rerun-if-env-changed=PKG_CONFIG_x86_64-unknown-linux-gnu
  cargo:rerun-if-env-changed=PKG_CONFIG_x86_64_unknown_linux_gnu
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG
  cargo:rerun-if-env-changed=PKG_CONFIG
  cargo:rerun-if-env-changed=BZIP2_STATIC
  cargo:rerun-if-env-changed=BZIP2_DYNAMIC
  cargo:rerun-if-env-changed=PKG_CONFIG_ALL_STATIC
  cargo:rerun-if-env-changed=PKG_CONFIG_ALL_DYNAMIC
  cargo:rerun-if-env-changed=PKG_CONFIG_PATH_x86_64-unknown-linux-gnu
  cargo:rerun-if-env-changed=PKG_CONFIG_PATH_x86_64_unknown_linux_gnu
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_PATH
  cargo:rerun-if-env-changed=PKG_CONFIG_PATH
  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR_x86_64-unknown-linux-gnu
  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR_x86_64_unknown_linux_gnu
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_LIBDIR
  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR
  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR_x86_64-unknown-linux-gnu
  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR_x86_64_unknown_linux_gnu
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_SYSROOT_DIR
  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR
  TARGET = Some("x86_64-unknown-linux-gnu")
  OPT_LEVEL = Some("3")
  HOST = Some("x86_64-unknown-linux-gnu")
  cargo:rerun-if-env-changed=CC_x86_64-unknown-linux-gnu
  CC_x86_64-unknown-linux-gnu = None
  cargo:rerun-if-env-changed=CC_x86_64_unknown_linux_gnu
  CC_x86_64_unknown_linux_gnu = None
  cargo:rerun-if-env-changed=HOST_CC
  HOST_CC = None
  cargo:rerun-if-env-changed=CC
  CC = Some("gcc")
  cargo:rerun-if-env-changed=CRATE_CC_NO_DEFAULTS
  CRATE_CC_NO_DEFAULTS = None
  DEBUG = Some("false")
  CARGO_CFG_TARGET_FEATURE = Some("fxsr,sse,sse2")
  cargo:rerun-if-env-changed=CFLAGS_x86_64-unknown-linux-gnu
  CFLAGS_x86_64-unknown-linux-gnu = None
  cargo:rerun-if-env-changed=CFLAGS_x86_64_unknown_linux_gnu
  CFLAGS_x86_64_unknown_linux_gnu = None
  cargo:rerun-if-env-changed=HOST_CFLAGS
  HOST_CFLAGS = None
  cargo:rerun-if-env-changed=CFLAGS
  CFLAGS = None
  running: "gcc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-I" "bzip2-1.0.8" "-D_FILE_OFFSET_BITS=64" "-DBZ_NO_STDIO" "-o" "/build/source/target/release/build/bzip2-sys-21887c5f83b913e7/out/lib/bzip2-1.0.8/blocksort.o" "-c" "bzip2-1.0.8/blocksort.c"
  exit status: 0
  running: "gcc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-I" "bzip2-1.0.8" "-D_FILE_OFFSET_BITS=64" "-DBZ_NO_STDIO" "-o" "/build/source/target/release/build/bzip2-sys-21887c5f83b913e7/out/lib/bzip2-1.0.8/huffman.o" "-c" "bzip2-1.0.8/huffman.c"
  exit status: 0
  running: "gcc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-I" "bzip2-1.0.8" "-D_FILE_OFFSET_BITS=64" "-DBZ_NO_STDIO" "-o" "/build/source/target/release/build/bzip2-sys-21887c5f83b913e7/out/lib/bzip2-1.0.8/crctable.o" "-c" "bzip2-1.0.8/crctable.c"
  exit status: 0
  running: "gcc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-I" "bzip2-1.0.8" "-D_FILE_OFFSET_BITS=64" "-DBZ_NO_STDIO" "-o" "/build/source/target/release/build/bzip2-sys-21887c5f83b913e7/out/lib/bzip2-1.0.8/randtable.o" "-c" "bzip2-1.0.8/randtable.c"
  exit status: 0
  running: "gcc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-I" "bzip2-1.0.8" "-D_FILE_OFFSET_BITS=64" "-DBZ_NO_STDIO" "-o" "/build/source/target/release/build/bzip2-sys-21887c5f83b913e7/out/lib/bzip2-1.0.8/compress.o" "-c" "bzip2-1.0.8/compress.c"
  exit status: 0
  running: "gcc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-I" "bzip2-1.0.8" "-D_FILE_OFFSET_BITS=64" "-DBZ_NO_STDIO" "-o" "/build/source/target/release/build/bzip2-sys-21887c5f83b913e7/out/lib/bzip2-1.0.8/decompress.o" "-c" "bzip2-1.0.8/decompress.c"
  exit status: 0
  running: "gcc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-I" "bzip2-1.0.8" "-D_FILE_OFFSET_BITS=64" "-DBZ_NO_STDIO" "-o" "/build/source/target/release/build/bzip2-sys-21887c5f83b913e7/out/lib/bzip2-1.0.8/bzlib.o" "-c" "bzip2-1.0.8/bzlib.c"
  exit status: 0
  cargo:rerun-if-env-changed=AR_x86_64-unknown-linux-gnu
  AR_x86_64-unknown-linux-gnu = None
  cargo:rerun-if-env-changed=AR_x86_64_unknown_linux_gnu
  AR_x86_64_unknown_linux_gnu = None
  cargo:rerun-if-env-changed=HOST_AR
  HOST_AR = None
  cargo:rerun-if-env-changed=AR
  AR = Some("ar")
  cargo:rerun-if-env-changed=ARFLAGS_x86_64-unknown-linux-gnu
  ARFLAGS_x86_64-unknown-linux-gnu = None
  cargo:rerun-if-env-changed=ARFLAGS_x86_64_unknown_linux_gnu
  ARFLAGS_x86_64_unknown_linux_gnu = None
  cargo:rerun-if-env-changed=HOST_ARFLAGS
  HOST_ARFLAGS = None
  cargo:rerun-if-env-changed=ARFLAGS
  ARFLAGS = None
  running: ZERO_AR_DATE="1" "ar" "cq" "/build/source/target/release/build/bzip2-sys-21887c5f83b913e7/out/lib/libbz2.a" "/build/source/target/release/build/bzip2-sys-21887c5f83b913e7/out/lib/bzip2-1.0.8/blocksort.o" "/build/source/target/release/build/bzip2-sys-21887c5f83b913e7/out/lib/bzip2-1.0.8/huffman.o" "/build/source/target/release/build/bzip2-sys-21887c5f83b913e7/out/lib/bzip2-1.0.8/crctable.o" "/build/source/target/release/build/bzip2-sys-21887c5f83b913e7/out/lib/bzip2-1.0.8/randtable.o" "/build/source/target/release/build/bzip2-sys-21887c5f83b913e7/out/lib/bzip2-1.0.8/compress.o" "/build/source/target/release/build/bzip2-sys-21887c5f83b913e7/out/lib/bzip2-1.0.8/decompress.o" "/build/source/target/release/build/bzip2-sys-21887c5f83b913e7/out/lib/bzip2-1.0.8/bzlib.o"
  exit status: 0
  running: "ar" "s" "/build/source/target/release/build/bzip2-sys-21887c5f83b913e7/out/lib/libbz2.a"
  exit status: 0
  cargo:rustc-link-lib=static=bz2
  cargo:rustc-link-search=native=/build/source/target/release/build/bzip2-sys-21887c5f83b913e7/out/lib

  --- stderr
  thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 13, kind: PermissionDenied, message: "Permission denied" }', /nix/store/31m598m7nr3l0fc9siw9ji31yyccs24a-vendor-cargo-deps/c19b7c6f923b580ac259164a89f2577984ad5ab09ee9d583b888f934adbbe8d0/bzip2-sys-0.1.11+1.0.8/build.rs:44:64
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@ipetkov
Copy link
Owner

ipetkov commented Oct 2, 2023

The build script for bzip2-sys is trying to write to a source directory instead of using OUT_DIR as set by cargo: https://github.com/alexcrichton/bzip2-rs/blob/3032f3790742bffda521e54d14429f459e078eba/bzip2-sys/build.rs#L44C1-L44C73

crane will symlink vendored sources and the build script's behavior is running afoul of that

@ipetkov
Copy link
Owner

ipetkov commented Oct 2, 2023

My first recommendation is to submit a fix for it upstream.

My second recommendation (which I hesitate to suggest due to performance penalties) is documented here: https://github.com/ipetkov/crane/blob/581245bf1233d6f621ce3b6cb99224a948c3a37f/docs/faq/sandbox-unfriendly-build-scripts.md

@ISibboI
Copy link
Contributor Author

ISibboI commented Oct 2, 2023

The build script for bzip2-sys is trying to write to a source directory instead of using OUT_DIR as set by cargo: https://github.com/alexcrichton/bzip2-rs/blob/3032f3790742bffda521e54d14429f459e078eba/bzip2-sys/build.rs#L44C1-L44C73

crane will symlink vendored sources and the build script's behavior is running afoul of that

Thanks! It seems though that the destination of the copy is inside OUT_DIR, as set in this line: https://github.com/alexcrichton/bzip2-rs/blob/3032f3790742bffda521e54d14429f459e078eba/bzip2-sys/build.rs#L26

Unless the first argument of fs::copy is the destination?

@ipetkov
Copy link
Owner

ipetkov commented Oct 2, 2023

Hmm good catch I read the script too fast 🤔

Maybe the fs::copy is hitting permission issues? Specifically the source files are in the Nix store and they are owned by root (world readable). I wonder if fs::copy is trying to maintain those permissions, but failing since the builder runs as non-root

Maybe the script should create a symlink to the file instead of copying it...

@dpc
Copy link
Contributor

dpc commented Oct 2, 2023

Are you building on MacOS or Linux? There are differences in mode preserving behavior between the too (it's basically a bug).

@ISibboI
Copy link
Contributor Author

ISibboI commented Oct 2, 2023

I am building on Linux.

@dpc
Copy link
Contributor

dpc commented Oct 2, 2023

--keep-failing and post-fail analysis might help.

@ISibboI
Copy link
Contributor Author

ISibboI commented Oct 4, 2023

Thanks, I did that now. What seems to be happening is that the file already exists. May be a problem with the unwanted rebuilds? Anyways, here is the state:

/t/n/s/t/r/b/bzip2-sys-21887c5f83b913e7 ❱ ls -lAh out/include/bzlib.h
-r--r--r-- 1 nixbld1 nixbld 6.1K Jan  1  1970 out/include/bzlib.h
/t/n/s/t/r/b/bzip2-sys-21887c5f83b913e7 ❱ file out/include/bzlib.h
out/include/bzlib.h: C source, ASCII text

Since it has no write permissions, it probably cannot be overwritten. Maybe the build directory requires a umask that makes all files created in there rwx for everyone? Or at least the artifacts that are copied in before the build starts should be chmodded to ugo+rwx?

Edit: Adding the fix for #370 did not help. It builds fine in the buildDepsOnly phase, but fails when rebuilding in the buildPackage phase. Still the same issue.

@ISibboI
Copy link
Contributor Author

ISibboI commented Oct 4, 2023

Okay, I have a proposal. Here and here, crane sets chmod u+w. But not when use-zstd is set. Seems like a simple oversight. Even though I cannot explain of why the permissions would change to read-only, because when cargo creates these files, it should not set them read-only itself, right?

Anyways, I will try this out, and make a pull request.

@dpc
Copy link
Contributor

dpc commented Oct 4, 2023

I don't like changing the permissions. There's a possibility it will cause more problems elsewhere.

Are all files -w or just this one? Possibly it's the only one because std::fs::copy copies the permissions from the original, and the original file is in vendored in the /nix/store somewhere.

This suggest that you are doing a rebuild of this package in the first place, so the root solution would be to fix that.

A patch to upstream to remove the file first might be a good idea as well. It can be explained that if the source is vendored, they might be read only.

@dpc
Copy link
Contributor

dpc commented Oct 4, 2023

Symlink mode is doing chmods because it's generally a bit ... hacky. :D . One of many benefits of use-zstd is that it should be very close to normal build.

@ISibboI
Copy link
Contributor Author

ISibboI commented Oct 4, 2023

This suggest that you are doing a rebuild of this package in the first place, so the root solution would be to fix that.

Should I make a separate issue for that? Since I don't like using the complete sandbox, but would hope that with use-zstd rebuilds would be a thing of the past.

Otherwise, here is a minimal example where bzip2-sys rebuilds on a separate branch without any modifications to crane.

@dpc
Copy link
Contributor

dpc commented Oct 4, 2023

            src = ./.; # The original, unfiltered source

Using raw ./. is not a good practice.

inherit cargoDebugArtifacts;

Not what you want.

            installCargoArtifactsMode = "use-zstd";

should be in commonArgs.

@dpc
Copy link
Contributor

dpc commented Oct 4, 2023

I disabled everything unnecessary and added:

+            cargoExtraArgs = "-v";

Then I get:

crane-test-crate-x>        Dirty bzip2-sys v0.1.11+1.0.8: the env variable PKG_CONFIG_PATH changed

@dpc
Copy link
Contributor

dpc commented Oct 4, 2023

Works with

strictDeps = true;     

Coincidentally: #403

so it's possibly crane introducing an extra dep somewhere, that changes PKG_CONFIG_PATH @ipetkov

Got to go now, but I hope this helps.

ISibboI added a commit to ISibboI/vocabulary-learning-application that referenced this issue Oct 8, 2023
@ISibboI
Copy link
Contributor Author

ISibboI commented Oct 8, 2023

Amazing, thanks @dpc ! Now my build works without rebuilds, even in the more complex project that I created this example from! Thank you so much!

@ipetkov
Copy link
Owner

ipetkov commented Oct 8, 2023

so it's possibly crane introducing an extra dep somewhere, that changes PKG_CONFIG_PATH

Interesting, have we narrowed it down to what might be causing the issue?

FWIW I did see buildInputs = with pkgs; [rustToolchain openssl]; in the original flake (crane will explicitly add cargo to the derivation but not the entire rustToolchain in case that's the culprit

@ipetkov
Copy link
Owner

ipetkov commented Oct 8, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants