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

build.rs: always use freebsd12 when rustc_dep_of_std is set #3723

Merged
merged 1 commit into from
Jun 15, 2024

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented May 26, 2024

Currently, when rustc_dep_of_std is set, the freebsd version used depends on the return value of which_freebsd():

  • if it is Some(12), it uses freebsd12
  • otherwise it uses freebsd11

That does not seem intended? It is certainly rather odd that when building on FreeBSD 13 we'd get the freebsd11 API, but when building on FreeBSD 12 we get freebsd12. This also causes some "fun" for Miri when cross-building the freebsd std on other targets: which_freebsd() then returns None, so std uses different symbols than it does in the std that is shipped by rustup.

I assume the intention of #3434 was to force std to use freebsd12 when rustc_dep_of_std is set (at least that's what the comments seem to say), so let's implement that.

@rustbot
Copy link
Collaborator

rustbot commented May 26, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @JohnTitor (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@RalfJung RalfJung force-pushed the buildrs-freebsd branch 2 times, most recently from 8dabe1d to b3d5f22 Compare May 26, 2024 18:54
build.rs Outdated
Comment on lines 60 to 61
15 => set_cfg("freebsd15"),
v => panic!("unsupported FreeBSD version: {}", v),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some day there will be a FreeBSD 16, so let's be forwards compatible with that.

Suggested change
15 => set_cfg("freebsd15"),
v => panic!("unsupported FreeBSD version: {}", v),
_ => set_cfg("freebsd15"),

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But isn't FreeBSD not-forward-compatible? It seems wrong to load the freebsd15 module on FreeBSD 16, no?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean backwards-compatible? It is. It will always be safe to use a FreeBSD X ABI on a FreeBSD X+1 system. You may be thinking of OpenBSD, which does not provide that guarantee.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes I may be mixing things up. Sorry for that.

@JohnTitor JohnTitor added this pull request to the merge queue Jun 15, 2024
Merged via the queue into rust-lang:main with commit 399c6ce Jun 15, 2024
41 checks passed
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

4 participants