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

Support [target.'cfg(...)'.lib] sections #12260

Open
mcclure opened this issue Jun 13, 2023 · 14 comments
Open

Support [target.'cfg(...)'.lib] sections #12260

mcclure opened this issue Jun 13, 2023 · 14 comments
Labels
A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches) A-crate-types Area: crate-type declaration (lib, staticlib, dylib, cdylib, etc.) A-manifest Area: Cargo.toml issues C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` E-hard Experience: Hard S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@mcclure
Copy link

mcclure commented Jun 13, 2023

Problem

I have a simple WebGPU app. (See this repo, commit 19dbf4a0. This is a simplified version that only draws a stripe. The actual repo is an in-progress "video game".) I build this application both to run on Desktop on Windows, and to run embedded in a web page (using wasm-pack; the wasm-pack entry point is fn main).

When building for wasm, you build your executable as a lib. When building for desktop, you build your executable as an exe. However, adding the [lib] section to your Cargo.toml as recommended by the wasm-pack documentation means that when building for desktop you build an exe and a lib, even though you only need the exe, and when building for web you build an exe and a lib even though you only need the lib. This is a waste of cpu time (even if it's only a small one) and can cause ancillary problems (see below).

Proposed Solution

The simplest and most logical fix for this would be to allow me to put in Cargo.toml a

[target.'cfg(target_arch = "wasm32")'.lib]

… and to pair with it, a…

[target.'cfg(not(target_arch = "wasm32"))'.bin]

section, mirroring (and thus completing the user intuition from) what I can already do with dependencies. Currently, either of these sections in Cargo.toml just gets a warning of an "unused manifest key".

Supporting these would not only help with wasm (an increasingly common use case where it is naturally the case that projects that built as executables-only on every other platform, would be built as libraries-only) but any other situation where the set of libraries needed for a project varies between platforms. For example, in the past when writing other languages I have developed for platforms that do not support dynamic libraries at all, or otherwise worked with build scripts that for whatever practical reason needed to build static only on certain platforms; but right now there's no way to say "build rlib+dylib on Windows, but only build rlib on WEIRD_EMBEDDED_PLATFORM".

What ifs

You are about to recommend to me an alternate way of solving my problem. But I have already tried whatever it was

This is a long story and I think it's really a distraction. But, here are some things I looked into while trying to deal with the specific problem above. I found all but one option inadequate and frustrating, and the one option I finally found adequate I feel like I shouldn't have had to do it.

Let me tell you about my ancillary problems.

  • The naive approach

    The first thing I tried was:

      [lib]
      crate-type = ["cdylib", "rlib"]
      path = "src/main.rs"
    

    This horrifies most Rust people I show it to, but the thing is, it is correct for my application. My program has only one entry point (fn main). When it is built as a lib, the only thing called is the fn main entry point.

    This works, but doing it creates a lot of spurious warnings. In particular:

      warning: file `C:\Users\Andi\work\r\wgt\src/main.rs` found to be present in multiple build targets:
        * `lib` target `wgpu-hello`
        * `bin` target `wgpu-hello`
    

    and:

      warning: output filename collision.
      The bin target `wgpu-hello` in package `wgpu-hello v0.1.0 (C:\Users\Andi\work\r\zap)` has the same output filename as the lib target `wgpu-hello` in package `wgpu-hello
       v0.1.0 (C:\Users\Andi\work\r\zap)`.
      Colliding filename is: C:\Users\Andi\work\r\zap\target\debug\deps\wgpu_hello.pdb
      The targets should have unique names.
      Consider changing their names to be unique or compiling them separately.
      This may become a hard error in the future; see <https://github.com/rust-lang/cargo/issues/6313>.
    

    And… yes. I am using main.rs in both the lib and bin file targets. But the thing is, there shouldn't be a lib file target. If I built lib only on wasm and bin only on desktop there wouldn't be a conflict here.

    Oh, in addition to this, when building as an executable on desktop, every symbol in my application produces an "unused symbol" warning. It is a lot of warnings. This is because the lib (which I don't want to build) has no entry points. fn main is decorated with a #[cfg_attr(target_arch="wasm32", wasm_bindgen(start))] which makes it an entry point on wasm, but this doesn't apply on desktop.

  • Add a stub lib.rs that calls into main.rs

    This would trivially, and idiomatically, resolve the "file found to be present in multiple build targets" warning.

    I don't like it, because it means I've got a junk lib.rs file that serves no purpose, and when I tried it it turned out I would have to maintain two sets of mod lists, one in main.rs in and one in lib.rs, and keep them eternally in sync. Yeuch. I admit my objections here are mostly aesthetic, but aesthetically, especially for a minimal or single-file app, I don't like it.

    Also, I think if I'd ever gotten this approach working, I don't think it would actually solve the problem with the many "unused symbol" warnings, because fn fakemain() would still not be an exported symbol on desktop. Because in addition to being aesthetically unpleasant, this approach doesn't solve the problem! It only solves the warning. With this approach I'm still building an unnecessary library, the library is just easier to ignore because it's not emitting warnings.

  • Split my project up into three crates.

    This was recommended to me by several people and is the "correct" way to solve the problem. I would have a "project" crate that builds an rlib, a "bin" crate that builds my Windows exe, and an "lib" crate that builds a dylib for my wasm output. The bin crate contains a minimal main.rs that calls "realmain" in the project crate, the lib crate contains a minimal lib.rs that calls "realmain" in the project crate. This produces no warnings, builds no unnecessary binaries, and is "aesthetic" in the sense that it doesn't require junk files in the project crate or extra "mod" statements.

    This is incredibly unnecessary. It is, simply, overpowered for what I am trying to do. This is the correct way to structure a project that builds a library and also a binary where a user might need to build either or both. But my project builds a library xor a binary. I always know which I need to build and I never need both.

    It is also disruptive to the project, due to the rule that 1 crate = 1 folder. My nice, succinct 1 cargo file + 1 src/main file project is now split across three directories, each with their own Cargo.toml and src/ file, and out of the seven files in the project now the only one I ever edit is project/src/main.rs. I feel I should not have to change my project directory structure to support an additional platform (which remember, is the only reason I'm doing to any of this— to support wasm), especially because directory restructuring can be disruptive to VCS merges— imagine if instead of considering this problem when the project was born, I'd gotten three years into the project before deciding to add wasm support and then done the directory restructure when multiple PRs were live on github.

    Anyway after putting this off for weeks I realized I didn't actually have to do it, because of:

  • cargo run --bin

    This was the closest to a happy solution I found with wasm-pack. Imagine I change my [lib] a little:

      [lib]
      crate-type = ["cdylib"]
      path = "src/main.rs"
    

    Now imagine I want to build my binary. Instead of cargo run I run: cargo run --bin wgpu-hello. Now, by explicit request, I am building only the bin. If I wanted to build only the lib, by comparison, I could build with --lib, or I could use wasm-pack which already avoids building the bin.

    Problem solved, actually! But there are two issues with this approach:

    • It is not discoverable. A couple people recommended this early in my journey, and I tried it, and it didn't work, and they didn't know why it wasn't working for me. It turned out that in order for this approach to work, lib.crate-type has to contain "cdylib" but not "rlib". If rlib is present, then the bin target even when built alone will also build the rlib because the bin target implicitly has the rlib as a dependency. This is deep magic. I understand why it works now, but if the information to understand this is in any piece of documentation, I haven't found it. I figured it out from having a conversation on the Rust internals forum about my problem here. The next person who needs this capability will not go so far as to ask for help on internals.rust-lang and will not figure it out.
    • My cargo invocation, now longer and ungainliner, now requires me to know the name of the --bin target, so it changes from project to project and when I change the exe name I have to update my build instructions. I feel unfair complaining about this, but it's there.
  • Use Trunk.

    After playing with all these possibilities for a long time, I tried switching from wasm-pack to trunk. Trunk just solves the problem. You just configure your Cargo.toml as if for a exe, and run trunk build, and it… works, somehow, I don't know how, it doesn't even need the #[] on fn main. It's great. I should have done this to begin with. All my problems are solved.

    If my problems are solved, why am I still complaining? Because this took me two months, and testing out all these different options above plus a couple I didn't even list (.cargo/?!) all just to make warnings not actually relevant to my project go away, was time I was not working on the actual project code. If I could have simply marked my initial [libs] section as specific to cfg(target_arch = "wasm32"), which it was, I could have solved this on day one (because I actually did try this on day one) and without having to learn the deep Cargo mysteries. Even if the absence of this feature can be worked around, it's simply a reasonable thing to want to do.

"There is some horrible reason, obvious to implementors but not users, why [target.'cfg(...)'.lib] cannot, in general, be allowed"

Okay if this (hypothetically) is true then could you at least special-case [target.'cfg(target_arch=...)'.lib] (and any other cfg() usages that specifically relate to platform) as allowed?

See also

@mcclure mcclure added C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage. labels Jun 13, 2023
@shanecelis
Copy link

The proposed feature [target.'cfg(target_arch = "wasm32")'.lib] is exactly what I tried when facing a similar problem in the hopes that Cargo worked like that; it didn't, but let this comment be a +1 for the proposed feature being "unsurprising" to a new user.

@weihanglo
Copy link
Member

Thanks for the detailed report! This seems mostly a duplicate of #4881 with a specific solution. There is also a proposal trying to scope a package under a set of target platforms from a different angle (#6179).

While these are all valid feature requests, I'd like to recommend --crate-type command line option in cargo rustc. This is a stable feature you can use to override lib.crate-type when building a lib. Here is a read world example of how it speed up compile time for hyper hyperium/hyper#2770.

For the WebAssembly use case, is it possible for folks to remove lib.crate-type field in Cargo.toml, and run cargo rustc --crate-type cdylib whenever you need it? It might be a bit long to type, but you could have short [alias] in .cargo/config.toml to help you through somehow.

@weihanglo weihanglo added S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. A-crate-types Area: crate-type declaration (lib, staticlib, dylib, cdylib, etc.) and removed S-triage Status: This issue is waiting on initial triage. labels Jun 13, 2023
@mcclure
Copy link
Author

mcclure commented Jun 13, 2023

I'd like to recommend --crate-type command line option in cargo rustc

Oh… that's a very useful feature actually.

It does not seem to help in this specific case. I can think of two ways to apply it and neither appear to work in my testing:

  • Remove crate-type=cdylib from Cargo.toml and invoke with --crate-type cdylib when building for wasm

    This hits multiple problems:

    • In general, people building for browser targets don't invoke rustc or even cargo themselves, they call an intermediary tool such as wasm-pack or trunk. Trunk, as described above, does the "expected" thing through means unknown. wasm-pack however appears to be hardcoded to check for the crate type cdylib key, I assume to ensure the user gets a helpful error message early instead of a strange one later. Say I run:

        $ cmd.exe /c "set RUSTFLAGS=--cfg=web_sys_unstable_apis --crate-type=cdylib & wasm-pack.exe build --target web"
      
        Error: crate-type must be cdylib to compile to wasm32-unknown-unknown. Add the following to your Cargo.toml file:
      
        [lib]
        crate-type = ["cdylib", "rlib"]
      

      (Does this look weird? It's because I'm using WSL, which makes setting environment variables harder than it should be.)

    • Even if this successfully introduced a cdylib target in wasm-pack, it would not suppress the bin target, so wasm-pack would still print the warning about "main.rsfound to be present in multiple build targets". Actually, I was still getting this warning even in mycargo run --bin BINNAME` solution described above.

  • Leave crate-type=cdylib in the Cargo.toml, and invoke with --crate-type bin when building for desktop

    This fails catastrophically because, since --crate-type is a flag on rustc, I must foward it to rustc using the RUSTFLAGS variable. But this variable is passed through not only to my project, but also to every single dependency Cargo builds. So I get like two dependencies in and then cfg_if fails to build with multiple big scary errors because it's a a macro crate and a macro crate (reasonably) doesn't know how to get built as --crate-type bin.

Another small note, I mentioned .cargo briefly in the OP, my thoughts on .cargo is I am kinda worried about using .cargo for any "Permanent" (IE, not specific to a particular checkout) solutions to a problem like this, because that implies I would have to check it in to source control, which implies my .cargo/config.toml in source control would then conflict with any single user's attempts to create a local/checkout-specific .cargo/config.toml.

@workingjubilee
Copy link
Member

workingjubilee commented Jun 13, 2023

If you put it in X:\YourFolder\YourProjectRepo\.cargo\config.toml, yes, it will conflict with anyone creating a .cargo/config.toml. It is not unusual, in my experience, to have these in a project if it needs them.

However, you can also put it in ${CARGO_HOME}/config.toml (and ${CARGO_HOME} is usually ${HOME}/.cargo, which will probably be something like C:\User\UserName\.cargo, last I checked), which will apply its configuration to all of your repos. This is useful, especially for convenience features like an [alias] which are not "essential", or for anything concerning "building on this computer at all", and are thus safe to have apply to all instances of cargo.

This may not have much of an impact on whether it is a good idea, your mileage may vary, and so on. I do not necessarily consider this a "solution", merely an observation that the fact Cargo's configuration files can be layered in this way is directly related to why it's not unusual to check in such a configuration file into a repo: they have at least one "out".

@weihanglo
Copy link
Member

Not sure if we are on the same page. cargo rustc --crate-type is a flag for overriding only one compilation unit (your root crate). It's different from environment variable RUSTFLAGS. A user shouldn't set --crate-type again in RUSTFLAGS if it is already passed to cargo rustc.

By removing the entire lib.crate-type from Cargo.toml your package should be back to default to compile bin for main.rs or rlib for lib.rs. If you have multiple Cargo targets, and you want to build only the library target, you could run something like

cargo rustc --lib --crate-type cdylib

If that doesn't work, please share a reproducible example and we can investigate together.

For the wasm-pack, it seems to be the tool could alleviate that by adopting cargo rustc --crate-type, as well as stop recommending setting lib.crate-type to Cargo.toml. I am not familiar with the tool so could be wrong.

@mcclure
Copy link
Author

mcclure commented Jun 13, 2023

If that doesn't work, please share a reproducible example and we can investigate together.

Okay, thank you for explaining. I don't think I know how to apply this even for testing because as explained I don't currently have a situation where I directly invoke cargo while building a lib for wasm.

@weihanglo
Copy link
Member

Triage:
We had a talk on Zulip. No matter which solution comes out, Cargo can help in spreading the words about how wasm-pack and --crate-type play well with each other (and probably some other tools as well?). The possible solution from Cargo seems not easy to me and may involve a certain amount of discussions and RFC, so label this as:

@rustbot label +S-needs-design -S-needs-info +E-hard

Thank you @mcclure and @workingjubilee for your patient and well-written report!

@rustbot rustbot added E-hard Experience: Hard S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. and removed S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. labels Jun 14, 2023
@epage epage added A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches) A-manifest Area: Cargo.toml issues labels Oct 31, 2023
@Pauan
Copy link

Pauan commented Oct 31, 2023

@weihanglo For the WebAssembly use case, is it possible for folks to remove lib.crate-type field in Cargo.toml, and run cargo rustc --crate-type cdylib whenever you need it?

That's exactly what the Rollup Rust Wasm plugin does. It's a very clean solution, since it doesn't require any changes to the user's Cargo.toml, so it reduces boilerplate.

@daxpedda
Copy link
Contributor

daxpedda commented Jul 8, 2024

Its actually not necessary to compile a cdylib to get things running in Wasm, bin works just as well and wasm-bindgen actually supports finding your main() function and executing it without annotating it with #[wasm_bindgen(start)].

The reason why its often recommended to use cdylib is because currently compiling Wasm binaries with Rustc bloats it. But this is a bug and should be fixed, as Wasm doesn't actually have any "executable" definition (except the start symbol) and both cdylib and bin should output the same apart from exporting the main() function with bin (applies to wasm32-unknown-unknown, not sure about WASI here).

However in Android this is not the case, there are actually Android executables, but when trying to build an APK the cdylib target has to be used. Though in this case compiling with cargo rustc --crate-type cdylib should do the trick apart from somehow having to export the main() function, which in the current ecosystem has to be done manually by the user (e.g. #[wasm_bindgen(start)] fn main() with wasm-bindgen or #[no_mangle] fn android_main() with android-activity).

Considering that I believe that cargo rustc --crate-type type should already cover the Wasm and Android use-case, apart from not working for binaries (main.rs), I would also suggest an additional improvement: getting rid of the required manual export of main() when using cargo rustc --crate-type type.

I'm not sure what the best approach is here, but probably introducing a new crate type like cdylib-bin might make sense. This would also require a bit of a redesign in android-activity, which currently expects a function called android_main() and not main() and passing different arguments, but that could be changed to adapt.

I didn't mention iOS because I'm clueless about that platform. I'm also unsure if there are other targets that require different crate-types and are trying to be as code cross-platform compatible as Wasm, Android and iOS.

EDIT: I'm also wondering why cargo rustc --crate-type type was decided to not be compatible with binaries (source), I couldn't find anything in the related issues, PRs and RFC. Enabling that would make this issue largely disappear if one is using build tools, e.g. wasm-pack and cargo apk.

@daxpedda
Copy link
Contributor

Coming from #14225:

I propose allowing cargo rustc --crate-type type to be used for binaries.
This change should be easy to implement and should improve the situation outlined in OP.

@weihanglo
Copy link
Member

@daxpedda If I understand it correctly, the fn main() is in your lib.rs, and you'd like to build a binary crate also from the same lib.rs file, without exporting the fn main() to a separate binary target src/main.rs. Is that correct?

If that is true, I'd like to know why having a separate src/main.rs and importing functions from src/lib.rs don't work. You can always cargo build --bin <bin-name> so it compile the binary which depends on the library.

@daxpedda
Copy link
Contributor

@daxpedda If I understand it correctly, the fn main() is in your lib.rs, and you'd like to build a binary crate also from the same lib.rs file, without exporting the fn main() to a separate binary target src/main.rs. Is that correct?

If that is true, I'd like to know why having a separate src/main.rs and importing functions from src/lib.rs don't work. You can always cargo build --bin <bin-name> so it compile the binary which depends on the library.

This does work indeed. I was just exploring an idea with the same motivation as the OP: not having users required to create multiple cargo targets to compile to multiple platforms and the extra setup that is included.

I believe the proposal would make much more sense if we have a binary target compile to a library instead of the other way around:
Currently we use lib.rs, which allows us to use cargo rustc --crate-type type and need an extra main.rs importing functions from lib.rs exactly as you mentioned.

The other way around we would use a main.rs, call cargo rustc --crate-type type to compile to platforms requiring it with no further setup required. The only thing left is to export fn main() by default when using cargo rustc --crate-type type so we don't even need to adjust the code for these cases.

This is obviously a small improvement that has a very solid workaround right now. It makes more sense to tackle the issue described in OP first.

Just to clarify: the proposal in #12260 (comment) is unrelated to the whole "how to export main() without extra configuration" proposal.

@MarijnS95
Copy link

A note on this "workaround to provide an Android library and a desktop executable from the same crate":

If you effectively have this:

[lib]
crate-type = ["rlib", "cdylib"]

For a lib.rs file that exposes the Android entrypoint behind a cfg, and uses a generic fn internal_main() entrypoint that's called from an src/main.rs for desktop executables, you'll run into an error like the following on Windows because of a PDB filename conflict:

Important

I dug this up from our commit history >2 years ago, it may have been fixed since!

warning: output filename collision.
The bin target `our-app` in package `our-app v0.1.0 (C:\our_workspace\apps\our-app)` has the same output filename as the lib target `our-app` in package `our-app v0.1.0 (C:\our_workspace\apps\our-app)`.
Colliding filename is: C:\our_workspace\target\debug\deps\cs_memcpy.pdb
The targets should have unique names.
Consider changing their names to be unique or compiling them separately.
This may become a hard error in the future; see <https://github.com/rust-lang/cargo/issues/6313>.

@weihanglo weihanglo added I-nominated-to-discuss To be discussed during issue triage on the next Cargo team meeting and removed I-nominated-to-discuss To be discussed during issue triage on the next Cargo team meeting labels Jul 31, 2024
@epage
Copy link
Contributor

epage commented Aug 15, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches) A-crate-types Area: crate-type declaration (lib, staticlib, dylib, cdylib, etc.) A-manifest Area: Cargo.toml issues C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` E-hard Experience: Hard S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

No branches or pull requests

9 participants