search

tauri-apps / winres

Create and set windows icons and metadata for executables with a rust build script
MIT License
7 stars 6 forks source link

refactor: Use embed-resource crate to compile resources #9

Closed FabianLars closed 1 year ago

FabianLars commented 1 year ago

Since there is another winres fork it's hopefully fine to keep this one really specific to tauri.

The reason i did this in the first place is because a community member made me aware of llvm-rc so the ugly mingw workaround i did can be removed again. The only drawback is that this may work less "good" for dyn libs as embed-resource seems to focus on executables (judging by different cargo link flags at least). Coming from the "tauri-specific fork" pov i think this is fine since it still covers our use-case perfectly.

Opened as a draft because i'm still testing os+gnu/msvc combinations:

TODO: Update readme

FabianLars commented 1 year ago

One issue is that we use set_toolkit_path in tauri-build, going to see if setting that via an env var from within winres works

FabianLars commented 1 year ago

@lucasfernog do you think someone will start a riot if i make tauri-build's sdk_dir() no-op out of nowhere?

iirc we added that for gnu people but embed-resource seems to require windres to be in the PATH: https://github.com/nabijaczleweli/rust-embed-resource/blob/master/src/windows_not_msvc.rs#L38

I couldn't produce any errors that would require the use of sdk_dir() so i couldn't really test the consequences of this change.

lucasfernog commented 1 year ago

Can we ask the community member that requested that? Otherwise just pull the trigger and wait for the new issue 😂

FabianLars commented 1 year ago

oh looks like it was added by a msvc user who had registry restrictions https://github.com/tauri-apps/tauri/issues/2871#issuecomment-969191116 but it looks like embed-resource can has a fallback approach that doesn't depend on the registry (vswhom). But according to the docs, embed-resource also reads RC env vars for the msvc target so i guess that would work too.

gnu users have to resort to the PATH env var then which imho is acceptable so i'm going to update the readme and merge it...