Skip to content

Disable all (implicit) uses of OsStr(ing)Ext in HSTRING and PCWSTR for WSL/Linux support #1862

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

Closed
wants to merge 1 commit into from

Conversation

MarijnS95
Copy link
Contributor

Fixes #1860

On non-Windows targets - such as Linux and WSL - the std::os::windows::ffi::OsStrExt and std::os::windows::ffi::OsStringExt extension traits are not available.

To support compiling and running on such non-Windows targets (not just cross-compiling from them, for a Windows target) anything using these traits has to be guarded in the same way using a #[cfg(windows)].

If needed fallbacks can be implemented manually, but that likely requires a bit more thought since wide-strings on these platforms typically have 32-bit wchar_t sizes, with Windows' 16-bit being the exception.

…TR` for WSL/Linux support

On non-Windows targets - such as Linux and WSL - the
`std::os::windows::ffi::OsStrExt` and
`std::os::windows::ffi::OsStringExt` extension traits are not available.

To support compiling and running _on_ such non-Windows targets (not just
cross-compiling _from_ them, for a Windows target) anything using these
traits has to be guarded in the same way using a `#[cfg(windows)]`.

If needed fallbacks can be implemented manually, but that likely
requires a bit more thought since wide-strings on these platforms
typically have [32-bit `wchar_t` sizes, with Windows' 16-bit being the
exception].

[32-bit `wchar_t` sizes, with Windows' 16-bit being the exception]: https://en.cppreference.com/w/cpp/language/types#Character%20types:~:text=wchar_t,a%20distinct%20type
@riverar riverar requested review from kennykerr and rylev July 1, 2022 18:23
@kennykerr
Copy link
Collaborator

We need a non-windows CI test to validate that this is correct.

@MarijnS95
Copy link
Contributor Author

MarijnS95 commented Jul 1, 2022

@kennykerr I mentioned in another PR that I'd do that once this compiles, and I already pushed some preliminary revision ~20 minutes ago to my fork. I can open a PR that pulls in both #1861 and #1862 and showcase that it builds, but those PRs need to be merged in first before such a CI would succeed.

@kennykerr
Copy link
Collaborator

Yep, just left the comment as a reminder to get the CI updated with or before this PR.

@MarijnS95
Copy link
Contributor Author

MarijnS95 commented Jul 1, 2022

Ah, I thought you wanted to get it over with directly in this PR! I might as well open a draft PR once it compiles, had some fights with tool_yml and "wrong" indentation for arrays :)

And branches: master prevents it from running on a fork :)

@kennykerr
Copy link
Collaborator

yml is not my favorite language... 😜

@MarijnS95
Copy link
Contributor Author

Draft PR open at #1863!

@kennykerr
Copy link
Collaborator

Closing in favor of #1863

@kennykerr kennykerr closed this Jul 5, 2022
@MarijnS95 MarijnS95 deleted the guard-osstrext branch July 5, 2022 20:30
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.

Guard windows::ffi::OsStringExt usage behind #[cfg(windows)] for WSL and Linux support
3 participants