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

std_detect: Don't unnecessarily depend on libc for windows-msvc #1563

Closed
ChrisDenton opened this issue Apr 15, 2024 · 3 comments · Fixed by #1571
Closed

std_detect: Don't unnecessarily depend on libc for windows-msvc #1563

ChrisDenton opened this issue Apr 15, 2024 · 3 comments · Fixed by #1571

Comments

@ChrisDenton
Copy link

I'm trying to reduce std's dependency on libc when it isn't actually needed.

Currently std_detect depends on libc for these features:

std_detect_file_io = [ "libc" ]
std_detect_dlsym_getauxval = [ "libc" ]
std_detect_env_override = [ "libc" ]

However, for windows-msvc, the only feature actually using libc in std_detect is std_detect_env_override

if #[cfg(feature = "std_detect_env_override")] {
#[inline]
fn initialize(mut value: Initializer) -> Initializer {
let env = unsafe {
libc::getenv(b"RUST_STD_DETECT_UNSTABLE\0".as_ptr() as *const libc::c_char)
};
if !env.is_null() {
let len = unsafe { libc::strlen(env) };
let env = unsafe { core::slice::from_raw_parts(env as *const u8, len) };
if let Ok(disable) = core::str::from_utf8(env) {
for v in disable.split(" ") {
let _ = super::Feature::from_str(v).map(|v| value.unset(v as u32));
}
}
}
do_initialize(value);
value
}

The easier way to address this issue is to make it so the on windows-msvc targets, libc is only used for this feature. The harder way is to use the Windows native equivalent of libc::getenv.

@dpaoliello
Copy link
Contributor

@ChrisDenton getenv can be replaced with GetEnvironmentVariableA - would you be ok with a hand-coded binding, or would you want windows-rs pulled in so that the binding can be generated?

@ChrisDenton
Copy link
Author

While this feature isn't enabled by our std builds, it could be, So a hand-coded binding would be better.

@dpaoliello
Copy link
Contributor

While this feature isn't enabled by our std builds, it could be, So a hand-coded binding would be better.

Done: #1571

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 a pull request may close this issue.

2 participants