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

Support compilation to wasm target #15

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

mdrokz
Copy link

@mdrokz mdrokz commented Dec 22, 2021

Changes

@svartalf
Copy link
Owner

svartalf commented Jan 5, 2022

Hi, @mdrokz , thank you for your PR!

Yet, I do not understand the purpose of those changes: it does not returns actual hostname (due to linkage issue you mentioned) and set() function is a stub.
How can it be useful for crate users?

@mdrokz
Copy link
Author

mdrokz commented Jan 7, 2022

Hi, @mdrokz , thank you for your PR!

Yet, I do not understand the purpose of those changes: it does not returns actual hostname (due to linkage issue you mentioned) and set() function is a stub. How can it be useful for crate users?

Hi,
this just adds support for wasm target there are lot of crates that depend on this one, for example the lettre and lettre_email crate if i want to compile lettre for wasm then the compilation would fail because this crate doesnt support wasm target.

@svartalf
Copy link
Owner

Hey!

I'm not sure if that is the approach I want to follow: if some particular target is not supported, I personally would prefer to learn about that fact during the compilation.

I can imagine it will be very confusing to figure out in runtime why returned value is very different from the actual system hostname. Even returning some blank error like std::io::Error with std::io::ErrorKind::Unsupported all the time might not fit all use cases.

On the other hand, with compilation failure each of the crate consumers can choose what approach to use: not provide support for this target at all or use some "default" value instead with conditional compilation like in the following pseudo-code (sorry, writing from my head, might have some naive mistakes):

#[cfg(any(target_family="unix", target_family="windows"))]
use hostname::get as get_hostname;

#[cfg(not(any(target_family="unix", target_family="windows")))]
fn get_hostname() -> io::Result<OsString> {
    "unknown".into()
}

Anyway, so far I'm inclined not to accept this PR, but I'm open for further discussion, of course, and having a wasm support would also be great.

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

Successfully merging this pull request may close these issues.

2 participants