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

freebsd: Correctly detect FreeBSD version #2603

Closed
wants to merge 1 commit into from

Conversation

amshafer
Copy link

This change removes the CI check that applies to freebsd version cfg. This is causing problems where the freebsd version defaults to 11 when CI is not enabled, some types may be incorrect on a > 11 system,
and compilation failures ensue.
A concrete example is libc::dev_t. This was defaulting to u32 on my FreeBSD 13-STABLE desktop, when it should have been u64. I couldn't compile anything that uses libc::devname_r as a result.

This change removes the CI check that applies to freebsd version cfg.
This is causing problems where the freebsd version defaults to 11
when CI is not enabled, some types may be incorrect on a > 11 system,
and compilation failures ensue.

A concrete example is libc::dev_t. This was defaulting to u32 on my
FreeBSD 13-STABLE desktop, when it should have been u64. I couldn't
compile anything that uses libc::devname_r as a result.
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Amanieu (or someone else) soon.

Please see the contribution instructions for more information.

@JohnTitor
Copy link
Member

JohnTitor commented Dec 24, 2021

Note that there's a previous attempt about that, #2581, and it was reverted as it causes an unwanted regression. See the discussion on the mentioned PR.

@valpackett
Copy link
Contributor

couldn't compile anything that uses libc::devname_r as a result

This means we're missing something like

    #[cfg_attr(all(target_os = "freebsd", freebsd11), link_name = "devname_r@FBSD_1.0")]

I really think we should add versioned symbols to ALL functions already.

@JohnTitor
Copy link
Member

I'm going to close this with the above reason, we should discuss how we treat these cfgs first, I think.

@JohnTitor JohnTitor closed this May 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants