Invalid resource file generated on MSVC

[larsbergstrom]

This came up in the context of Servo:
servo/servo#11969 (comment)

Basically, the issue is that the MSVC codepath uses res to create a .res file, but just renames it to .lib:
https://github.com/mxre/winres/blob/master/lib.rs#L466

Instead, it could either:

1. Call cvtres and then lib to turn the .res into a .lib
2. Pass the .res file directly along to the final link.exe line that is being executed by rustc

Do you have a preference, @mxre?

CC @Coder206

[retep998]

Call cvtres and then lib to turn the .res into a .lib

Just make sure you use kind=dylib for this, not kind=static, otherwise it won't be passed directly to the linker which could ruin things.

Pass the .res file directly along to the final link.exe line that is being executed by rustc

This won't work as a normal build script because rustc forces the extension on input files to be .lib.

[retep998]

If this RFC was accepted and implemented, you'd be able to link the .obj from cvtres.exe directly, instead of having to get a .lib through lib.exe.

[mxre]

Pass the .res file directly along to the final link.exe line that is being executed by rustc

This is actually what I want but cargo would not let me do it. It insists on adding .lib to any name added by the build script.
But since Microsoft's link.exe just plainly ignores any suffixes and treats the while according to their actual context it makes actually no difference if the resource file is names resource.res or resource.lib

Call cvtres and then lib to turn the .res into a .lib

I wanted to avoid calling unnecessary programs. But according to servo/servo#11969 (comment) this seems to be the only solution.

Ideally cargo would let me add arbitrary options to the linker command line from the build script, but this is not possible.

Edit: It could be possible, see rust-lang/rfcs#1766 but it seem highly unlikley this will ever be added in any non nightly versions.

[retep998]

Okay, so after some testing here are the results. Using cargo rustc -- -Clink-arg="bunres.extension" with a simple resource file that sets the icon.

1. bunres.rc: fatal error LNK1107: invalid or corrupt file: cannot read at 0x14 Resource files must be compiled first.
2. bunres.res: Compiled from bunres.rc using rc.exe. Works fine.
3. bunres.obj: Compiled from bunres.res using cvtres.exe. Works fine.
4. bunres.lib: Compiled from bunres.obj using lib.exe. Silently fails.
5. bunres.res.lib: Renamed from bunres.res. Works fine.
6. bunres.obj.lib: Renamed from bunres.obj. Works fine.

In conclusion the only solution at this point in time is to create a .res or .obj file, rename it to have a .lib extension and then pass it off as kind=dylib.

[Coder206]

Does this mean we can still utilize winres for building the icon file?

[mxre]

Well, as @retep998 explained the only way is naming the compiled resource file "*.lib".

And this is exactly what the current version of winres does. Try it out, setting an icon (.ico file) should work perfectly.

[retep998]

println!("cargo:rustc-link-lib=static={}", "resource");
Thus really should be changed to be println!("cargo:rustc-link-lib=dylib={}", "resource");.
static causes rustc to bundled it into the rlib which is incorrect and can cause issues. dylib causes it to be passed directly to the linker.

[larsbergstrom]

Yes, bundling it into the rlib is what was causing us problems. Rust would check that it was actually a .lib, which was breaking the servo build in @Coder206's PR.

[Coder206]

@mxre I am planning on forking winres and modifying itfor functionality with Servo in GNU and MSVC builds according to @retep998's suggestion, did you want me to also open a PR for fixing the issue here?

[mxre]

@Coder206 go ahead, I'd like to have a PR.

As to @larsbergstrom's comment I've never considered that someone would use winres to build an rlib, I've only tested it for the use build scripts of binaries (.exes and .dlls)

[mxre]

I've published version 0.1.1 on crates.io. This version includes the fix proposed by @retep998

[retep998]

Could you perhaps also push the commit with the fix to github?

[Coder206]

Thanks everyone!