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

Doc.rs of macOS and Windows aren't built #503

Closed
wusyong opened this issue Feb 19, 2022 · 5 comments · Fixed by #509
Closed

Doc.rs of macOS and Windows aren't built #503

wusyong opened this issue Feb 19, 2022 · 5 comments · Fixed by #509

Comments

@wusyong
Copy link
Member

wusyong commented Feb 19, 2022

The docs of Mac and Windows are missing after 0.13.
This is probably because of thiserror. Its recent patch version only generates linux target.
And on Windows, it's also because of sys-info. Since it's only used for detecting windows 7, we could add our own helper function instead.

@wusyong wusyong added good first issue Good for newcomers platform: macOS platform: Windows priority: medium Great to have and removed good first issue Good for newcomers labels Feb 19, 2022
@chippers
Copy link
Member

The docs of Mac and Windows are missing after 0.13. This is probably because of thiserror. Its recent patch version only generates linux target. And on Windows, it's also because of sys-info. Since it's only used for detecting windows 7, we could add our own helper function instead.

serde and family also use the same settings, it's common if there is no platform-specific behavior because linux is seen as the "default" docs.rs runner. That setting on serde has been set for 2 years

@amrbashir
Copy link
Member

And on Windows, it's also because of sys-info. Since it's only used for detecting windows 7, we could add our own helper function instead.

I already done that in the vibrancy plugin https://github.com/tauri-apps/tauri-plugin-vibrancy/blob/e4ffde2a9f87c666858a74afa15f73de8aaa3ca0/src/platform/windows.rs#L72 and lucas mentioned that we probably need to put all these utils in some sort of a crate.

@wusyong
Copy link
Member Author

wusyong commented Feb 20, 2022

After a few test, I can confirm it's actually caused by objc_excpetion on macOS.
And since it's pretty useless (it basically just print the log from message of objc stack unwinds and the runtime already does that), I think we can just disable that feature.

@lucasfernog
Copy link
Member

@wusyong check with the auditors about that, anyway you could also just disable it when building for docs.rs

@tweidinger
Copy link
Contributor

tweidinger commented Feb 27, 2022

After a few test, I can confirm it's actually caused by objc_excpetion on macOS. And since it's pretty useless (it basically just print the log from message of objc stack unwinds and the runtime already does that), I think we can just disable that feature.

The objc_excpetion feature also wraps all !msg_send calls in a try catch, which tries to reduce/avoid having undefined behaviour introduced from the objc code, which is afaik not handled by the default rust runtime.

By default, if the msg_send! macro causes an exception to be thrown, this will unwind into Rust resulting in unsafe, undefined behavior. However, this crate has an "exception" feature which, when enabled, wraps each msg_send! in a @try/@catch and panics if an exception is caught, preventing Objective-C from unwinding into Rust.

just disable it when building for docs.

Sounds like a reasonable tradeoff.
I think it should be fine to be disabled for doc generation but I would rather avoid completely disabling this feature.

EDIT: Nevermind the comment above :)

wry/Cargo.toml

Line 25 in 327a019

default = [ "file-drop", "objc-exception", "protocol" ]

introduced the exception feature as default.

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

Successfully merging a pull request may close this issue.

5 participants