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

Doesn't work for Cargo packages with both a library and binary crate #32

Closed
AlexTMjugador opened this issue Jul 2, 2021 · 7 comments
Closed
Labels

Comments

@AlexTMjugador
Copy link

AlexTMjugador commented Jul 2, 2021

As stated in the title of this issue, winres fails to change the resource data of executables generated by Cargo when compiling a package that has both a library and binary crate (i.e. both a src/main.rs and src/lib.rs). No error happens whatsoever, and both the resource file and Cargo instructions output in the build script are correct, which makes this a pretty sneaky issue.

I've managed to trace the cause down to how Cargo works. Citing the documentation (emphasis mine):

The -l flag is only passed to the library target of the package, unless there is no library target, in which case it is passed to all targets. This is done because all other targets have an implicit dependency on the library target, and the given library to link should only be included once. This means that if a package has both a library and a binary target, the library has access to the symbols from the given lib, and the binary should access them through the library target's public API.

Because winres instructs the linker to link to the resource file via cargo:rustc-link-lib, the binary crate that generates the actual program is not linked against the resource file, and that essentially means that winres ends up doing nothing visible. I've confirmed that this happens when using the GNU toolchain, although for MSVC this is also done as well, so I'd expect MSVC builds to be affected too.

One obvious workaround would be getting rid of the library crate, but this is not necessarily a good practice (separating the application code from the API it uses is usually nice). Another workaround is to set up a workspace and separate the library and binary crates into different packages, but this seems like overkill to just get some metadata linked to a binary crate.

Can winres reliably link the resource file with the binary somehow, even if it shares package with a library crate? If not, can I workaround it in a better way than using workspaces? Thanks!

@AlexTMjugador
Copy link
Author

After some trying, I settled with rcedit and a custom PowerShell script that is invoked after cargo build, which is a better workaround for me than using workspaces. But I think that this issue is still relevant, and if it is not fixable by winres, I think this quirk should at least be documented.

@mxre
Copy link
Owner

mxre commented Oct 3, 2021

winres is intended to be used by the cargo build script and therefor can only do what cargo allows. Please raise that issue with the cargo developers to allow the build script to manipulate individual targets and provide more control over the linker process (see rust-lang/cargo#9554, but that's nightly),
or to allow more than a separate build script for each target.

Currently cargo only allows to supply some flags to the linker and this reliably only works for one target in the crate. If you have multiple artifacts the easiest workaround would be to create a crate for each of them.

I think that this is basically the same problem as with #29

@mxre
Copy link
Owner

mxre commented Oct 17, 2021

I did some testing and using [[bin]] nodes is simply unsupported as the build scripts output is not used to link these executables. There is nothing winres can do.

But having a crate with a [lib] node and using winres to embed resources into a .dll is supported. Just set crate-type = ["cdylib"] so the output of cargo will be a .dll and not a .rlib. Only .dll libraries can have resources.

@mxre mxre added the wontfix label Oct 17, 2021
@mxre
Copy link
Owner

mxre commented Oct 17, 2021

From https://doc.rust-lang.org/cargo/reference/build-scripts.html#cargorustc-link-libkindname

The -l flag is only passed to the library target of the package, unless there is no library target, in which case it is passed to all targets. This is done because all other targets have an implicit dependency on the library target, and the given library to link should only be included once. This means that if a package has both a library and a binary target, the library has access to the symbols from the given lib, and the binary should access them through the library target's public API.

So if there is a library crate the build scripts parameter are only passed to the lib part. Since we generate a static library ourselves (one without symbols only prescribing a .rsrc section for the resulting binary) It gets optimized away in the linking step of the [[bin]] executable.

No way of fixing this because from the perspective of the cargo devs this is all intended.

@mxre mxre closed this as completed Oct 17, 2021
@AlexTMjugador
Copy link
Author

AlexTMjugador commented Oct 17, 2021

It's sad to read that linking a DLL with a resources section does not work either, but I understand that this crate can't do anything about it. I guess I chose the best workaround for my case then. Thanks for your insights.

Anyway, what do you think about adding a small note in the crate documentation about this situation? I initially was clueless about why my Windows executables stopped having a nice icon, until I started digging and bisecting and traced it back to Cargo's design. I think such a note would save time from future crate users in my situation.

@mxre
Copy link
Owner

mxre commented Oct 17, 2021

It's sad to read that linking a DLL with a resources section does not work either

No, that part works see #32 (comment)

Anyway, what do you think about adding a small note in the crate documentation about this situation?

I think I'll detail this situation, apparently this is very confusing for many people

@ITachiLab
Copy link

ITachiLab commented Nov 6, 2022

I too encountered this problem today and I'd really appreciate some warning notice being added to winres documentation. Anyways, the other thing I wanted to share is yet another workaround that worked for me and might also work for others. I added a single line to my build.rs and apparently it did the trick:

fn main() {
    let mut res = winres::WindowsResource::new();
    res.set_icon("resources/icon.ico");
    res.compile().expect("Cannot compile resource bundle.");
    println!("cargo:rustc-link-arg-bins=resource.lib"); // This line
}

What we are doing here is telling the linker to take into account one more file when linking: resource.lib. This file is produced by winres and it sits in a directory which is present in the search path, so linker doesn't have problems with finding it. That additional argument will be used for all binaries, so if you have multiple binaries and want to include the resources only in a couple of them, you can use: cargo:rustc-link-arg-bin=BIN=resource.lib in order to specify the target binaries.

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

No branches or pull requests

3 participants