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

windows crate can be used on Windows to avoid linking issues #94

Closed
nazar-pc opened this issue Mar 8, 2024 · 4 comments
Closed

windows crate can be used on Windows to avoid linking issues #94

nazar-pc opened this issue Mar 8, 2024 · 4 comments
Labels

Comments

@nazar-pc
Copy link

nazar-pc commented Mar 8, 2024

Microsoft maintains windows crate that should help with all of the linking issues you might encounter. In particular ShellExecuteW is defined here: https://microsoft.github.io/windows-docs-rs/doc/windows/Win32/UI/Shell/fn.ShellExecuteW.html

Yes, the crate is quite large, but I believe it is still worth doing to reduce maintenance burden and get guaranteed correct implementation from upstream.

@Byron
Copy link
Owner

Byron commented Mar 8, 2024

Thanks for bringing this up.

@amrbashir was the one implementing it in a way that avoids additional dependencies, which is important to the tauri plugin that open also serves. After an initial hick-up that caused build failures on some 32 bit builds we now have a solution which works and has no reason to stop working due to the stability guarantees that Windows fortunately has.

Thus I don't think there is value (left) in switching to windows or windows-sys now.

My disposition here is to close this issue as "won't fix" for that reason, but I wanted to leave any action to @amrbashir.

@Byron Byron added the question label Mar 8, 2024
@nazar-pc
Copy link
Author

nazar-pc commented Mar 8, 2024

Interesting, I was not aware of that context.

For Tauri, multiple versions of windows are already in their lock file, so I don't think it makes any difference at all in that context.

The reason I opened this is a big note on optional shellexecute-on-windows feature. It seemed like it can still cause potential linking issues, which is why I suggested to use the crate that Microsoft itself maintains for their OS instead and maybe make that feature non-optional instead.

@amrbashir
Copy link
Contributor

This feature seemed simple enough that could be done with manual linking so I just preferred to not depend on another crate which helps this crate users ofc and tauri as well.

I am fine with both btw, keeping it this way or relying on windows-sys crate, both should be fine and compile fast enough. I know that tauri has a lot of dependencies and multiple windows crate but I try to avoid adding more where possible (and hopefully in the future remove some of these dependencies).

I can't/shouldn't dictate what the best course here should be and would rather leave it up to the crate maintainer and its community.

@Byron
Copy link
Owner

Byron commented Mar 9, 2024

That's great to hear, @nazar-pc , as I indeed didn't update the docs of the feature which made it misleading.

I have updated the doc-string to more clearly reflect that status quo - now the feature is there to assure the behaviour isn't changed which could be breaking in unforeseen ways.

According to my previous argument I think it's fair to close this issue as no further action is needed. Not having windows or windows-sys will only help the downstream to keep their dependency trees slim, which is also a selling-point of open, maybe in comparison to opener.

Thanks everyone for there feedback, it was a pleasure.

@Byron Byron closed this as not planned Won't fix, can't repro, duplicate, stale Mar 9, 2024
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