Skip to content

Support different lib.crate-type #227

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.

Sign up for GitHub

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

Open
NickHu wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Support different lib.crate-type #227

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions lib.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ rec
attrs =
# Since we pretend everything is a lib, we remove any mentions
# of binaries
removeAttrs cargotoml [ "bin" "example" "lib" "test" "bench" "default-run" ]
removeAttrs cargotoml [ "bin" "example" "test" "bench" "default-run" ]

@nmattia nmattia Mar 8, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if the project mentions a [lib] that isn't src/lib.rs? Won't cargo try to build that and, not finding any files, fail?

@NickHu NickHu Mar 8, 2022

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, this change in particular is the one I'm not sure about. In order for WASM dependencies to be built, lib.crate-type needs to be retained. This was merely the quickest fix I could think of, I didn't really consider the other effects, and perhaps there's a better way to achieve the same goal.

@nmattia nmattia Mar 8, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see! I believe this will break on cargo tomls that have lib = .... Can you give some example Cargo.tomls that use lib.crate-type? Maybe we can just remove e.g. the path of the lib attribute?

@NickHu NickHu Mar 8, 2022

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any example which uses wasm-bindgen for instance:
https://github.com/rustwasm/wasm-bindgen/tree/main/examples

// lib.optionalAttrs (builtins.hasAttr "package" cargotoml) ({ package = removeAttrs cargotoml.package [ "default-run" ] ; })
;
in
Expand Down Expand Up @@ -209,7 +209,11 @@ rec
pushd $out/$member > /dev/null
mkdir -p src
# Avoid accidentally pulling `std` for no-std crates.
echo '#![no_std]' >src/lib.rs
cat <<EOF >src/lib.rs
#![no_std]

@nmattia nmattia Mar 8, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#![no_std]
#![no_std]
# If you compile with crate-type=cdylib, and also use no_std,
# the compilation will fail because no panic handler has been
# provided (the bit that unwinds the stack trace, usually provided
# by the standard library). This is just a dummy stub to get
# compilation to succeed.
# See https://doc.rust-lang.org/nomicon/panic-handler.html for more details.

#[panic_handler]
fn panic(_info: &core::panic::PanicInfo) -> ! { loop {} }
Comment on lines +214 to +215

@nmattia nmattia Mar 8, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what are these two lines for?

@NickHu NickHu Mar 8, 2022

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you compile with crate-type=cdylib, and also use no_std, the compilation will fail because no panic handler has been provided (the bit that unwinds the stack trace, usually provided by the standard library). This is just a dummy stub to get compilation to succeed. See https://doc.rust-lang.org/nomicon/panic-handler.html for more details.

@nmattia nmattia Mar 8, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Added a code suggestion; worth having the clarification near the code.

EOF
# pretend there's a `build.rs`, otherwise cargo doesn't build
# the `[build-dependencies]`. Custom locations of build scripts
# aren't an issue because we strip the `build` field in
Expand Down