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

Add support for FreeBSD CURRENT (aka freebsd13) #1440

Merged
merged 21 commits into from Aug 10, 2019

Conversation

pizzamig
Copy link
Contributor

Using the last FreeBSD-CURRENT (development snapshot) the libc build, but tests fail.
This patch detects and supports FreeBSD CURRENT as freebsd13, and reworks the conditional compilation to use the freebsd11 attribute instead of not(freebsd12)
For now, freebsd13 is reusing all freebsd12 definitions, except for ELAST
While here, add a new errnointroduced in freebsd13

AdminXVII and others added 8 commits July 15, 2019 08:21
Currently, libc supports and detects freebsd11 and freebsd13
Unknown versions, like freebsd13, is treated as freebsd11.
This patch solve the issues, detecting freebsd13 and treating it like
freebsd12.
Inverting the logic not(freebsd12) -> freebsd11 where possible
@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 @gnzlbg (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 23, 2019

So this LGTM, @asomers what do you think?

Copy link
Contributor

@asomers asomers left a comment

Choose a reason for hiding this comment

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

The basic change looks good, as in it should allow libc-test to pass on FreeBSD 13. But it doesn't fix the greater underlying problem that libc still can't support multiple versions of FreeBSD, because cross compiles don't know which version to target.

ELAST should not be exported at all, because it's not forward-compatible. Programs that use ELAST won't be able to run correctly on newer versions of FreeBSD. The very few programs that need something like it should use the runtime constant sys_nerr instead.

Are you planning to add new symbols from FreeBSD 13? If so, funlinkat, CNO_RTSDTR, EINTEGRITY, O_BENEATH, AT_BENEATH, FD_NONE, FIOBMAP2, PT_GET_SC_RET should probably all be added.

@pizzamig
Copy link
Contributor Author

This patch has the only goal to allow me to have a libc up and running on FreeBSD 13 and I thought it was a good way to start contributing to this crate.
My following goal is to make cpuset_{get|set}affinity() functions usable from Rust.
However I'm willing to help to improve FreeBSD support in general.

ELAST was already there and I see it changed because of EINTEGRITY (already added with this patch). In general I agree to remove it, but I'm not sure that it can be easily done without breaking the crate retro-compatibility (removing is considered a breaking change)

I can work to add the other FreeBSD13 symbols in a later patch.

For the cross-compile problem, I've not a clear idea of how that could be solved. Maybe adding a feature to the crate, managed by build.rs, to generate appropriate rustc-cfg, with default to freebsd11 ?

@gnzlbg gnzlbg closed this Jul 26, 2019
@gnzlbg gnzlbg reopened this Jul 26, 2019
AdminXVII and others added 4 commits July 26, 2019 13:32
Update wasmtime to the latest master.

The previous wasmtime revision is broken because one of its dependencies, `memoffset` 0.3, was yanked, which appears to make it unavailable. This was fixed by downgrading to `memoffset` 0.2.
Increase Redox & Relibc support

- Add a lot of constants from relibc
- Fix the timezone not found error found previously on Redox
- Wrap WIFEXITED _et al._ in an unsafe block to match the rest of the API
- Add support for the extra_traits feature and Redox
- Fmt

cc @jackpot51
@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 27, 2019

@bors: r+


I think that, as long as freebsd11 and 12 are not broken, if this works for you, it is ok to merge. We should try setting up at least a build job for freebsd 13 to check that things don't stop building, but that can be done later (should be the next step though). At some point once there are freebsd 13 images available on cirrus-ci, we can start running the tests there.

@bors
Copy link
Contributor

bors commented Jul 27, 2019

📌 Commit 72aa226 has been approved by gnzlbg

@bors
Copy link
Contributor

bors commented Jul 27, 2019

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

  1. git checkout invert-freebsd12 (switch to your branch)
  2. git fetch upstream master (retrieve the latest master)
  3. git rebase upstream/master -p (rebase on top of it)
  4. Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
  5. git push self invert-freebsd12 --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message
Auto-merging src/unix/mod.rs
CONFLICT (content): Merge conflict in src/unix/mod.rs
Automatic merge failed; fix conflicts and then commit the result.

@bors
Copy link
Contributor

bors commented Jul 27, 2019

☔ The latest upstream changes (presumably #1438) made this pull request unmergeable. Please resolve the merge conflicts.

@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 27, 2019

This needs to be rebased

bors and others added 3 commits July 27, 2019 11:51
Add support for hexagon-unknown-linux-musl
Currently, libc supports and detects freebsd11 and freebsd13
Unknown versions, like freebsd13, is treated as freebsd11.
This patch solve the issues, detecting freebsd13 and treating it like
freebsd12.
Inverting the logic not(freebsd12) -> freebsd11 where possible
@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 29, 2019

It appears that some ctest dependency is now failing to build on cirrus ci for some reason.

@pizzamig
Copy link
Contributor Author

I ran some tests and it seems that the extprim crate is broken using nightly

@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 29, 2019

Which dependency is pulling it? Maybe we can just remove that dependency somehow (this would be a ctest issue), it is weird that it only triggers on freebsd12 cirrus build job.

@pizzamig
Copy link
Contributor Author

extprim is needed by syntex_syntax2 that's needed by ctest.
Theoretically, extprim is not needed anymore, because rust 1.16 provides u128 and i128

The error is of this form:

error: `X...` range patterns are not supported
   --> /root/.cargo/registry/src/github.com-1ecc6299db9ec823/extprim-1.7.0/src/traits.rs:94:21
    |
94  |                     1 ... $e => u128::new(mantissa) << exp,
    |                     ^^^^^ help: try using the maximum value for the type: `1...MAX`
...
117 | impl_to_extra_primitive_for_float!(f32, 23, 104, 103);
    | ------------------------------------------------------ in this macro invocation

error: expected one of `=>`, `if`, or `|`, found `104`
   --> /root/.cargo/registry/src/github.com-1ecc6299db9ec823/extprim-1.7.0/src/traits.rs:94:27
    |
94  |                     1 ... $e => u128::new(mantissa) << exp,
    |                           ^^ expected one of `=>`, `if`, or `|` here
...
117 | impl_to_extra_primitive_for_float!(f32, 23, 104, 103);
    | ------------------------------------------------------ in this macro invocation

and it seems something related to macro, the $e is not recognized as upper limit.

@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 29, 2019

So syntex_syntax2 requires extprim 1.0, https://github.com/gnzlbg/syntex/blob/master/syntex_syntax/Cargo.toml#L15

@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 29, 2019

I suppose we could patch that to a extprim = "=1.0" to fix it, or maybe extprim 1.6 did not have this issue ?

@pizzamig
Copy link
Contributor Author

1.0 and 1.6 fails to build, they all have the same macro...

@pizzamig
Copy link
Contributor Author

pizzamig commented Aug 2, 2019

Is there something I can do to fix it?

@gnzlbg
Copy link
Contributor

gnzlbg commented Aug 3, 2019

@bors: r+

@bors
Copy link
Contributor

bors commented Aug 3, 2019

💡 This pull request was already approved, no need to approve it again.

  • This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.

@bors
Copy link
Contributor

bors commented Aug 3, 2019

📌 Commit a3681f9 has been approved by gnzlbg

@bors
Copy link
Contributor

bors commented Aug 3, 2019

⌛ Testing commit a3681f9 with merge 7e4dfb8...

bors added a commit that referenced this pull request Aug 3, 2019
Add support for FreeBSD CURRENT (aka freebsd13)

Using the last FreeBSD-CURRENT (development snapshot) the libc build, but tests fail.
This patch detects and supports FreeBSD CURRENT as freebsd13, and reworks the conditional compilation to use the `freebsd11` attribute instead of `not(freebsd12)`
For now, freebsd13 is reusing all freebsd12 definitions, except for `ELAST`
While here, add a new `errno`introduced in freebsd13
@bors
Copy link
Contributor

bors commented Aug 3, 2019

💔 Test failed - status-appveyor

@gnzlbg
Copy link
Contributor

gnzlbg commented Aug 9, 2019

@bors: retry

@mati865
Copy link
Contributor

mati865 commented Aug 9, 2019

Isn't git history a bit messed up here?

@bors
Copy link
Contributor

bors commented Aug 9, 2019

⌛ Testing commit a3681f9 with merge 933b7db...

bors added a commit that referenced this pull request Aug 9, 2019
Add support for FreeBSD CURRENT (aka freebsd13)

Using the last FreeBSD-CURRENT (development snapshot) the libc build, but tests fail.
This patch detects and supports FreeBSD CURRENT as freebsd13, and reworks the conditional compilation to use the `freebsd11` attribute instead of `not(freebsd12)`
For now, freebsd13 is reusing all freebsd12 definitions, except for `ELAST`
While here, add a new `errno`introduced in freebsd13
@bors
Copy link
Contributor

bors commented Aug 9, 2019

💔 Test failed - status-azure

@gnzlbg
Copy link
Contributor

gnzlbg commented Aug 10, 2019

@bors: r+

@bors
Copy link
Contributor

bors commented Aug 10, 2019

📌 Commit 154e58d has been approved by gnzlbg

@bors
Copy link
Contributor

bors commented Aug 10, 2019

⌛ Testing commit 154e58d with merge 245b0f8...

bors added a commit that referenced this pull request Aug 10, 2019
Add support for FreeBSD CURRENT (aka freebsd13)

Using the last FreeBSD-CURRENT (development snapshot) the libc build, but tests fail.
This patch detects and supports FreeBSD CURRENT as freebsd13, and reworks the conditional compilation to use the `freebsd11` attribute instead of `not(freebsd12)`
For now, freebsd13 is reusing all freebsd12 definitions, except for `ELAST`
While here, add a new `errno`introduced in freebsd13
@bors
Copy link
Contributor

bors commented Aug 10, 2019

☀️ Test successful - checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, status-azure
Approved by: gnzlbg
Pushing 245b0f8 to master...

@bors bors merged commit 154e58d into rust-lang:master Aug 10, 2019
@semarie semarie mentioned this pull request Aug 14, 2019
@gnzlbg gnzlbg mentioned this pull request Aug 14, 2019
bors added a commit that referenced this pull request Aug 15, 2019
Fix FreeBSD

#1440 broke FreeBSD by changing `libc` FreeBSD targets to require a `cfg(freebsdXX)` to be defined, but not updating `build.rs` to define `freebsd11` when `LIBC_CI` is not available. Since `LIBC_CI` is always defined on CI, this issue went undetected.

This PR fixes that issue in the `build.rs` and introduces a build task that tests FreeBSD without `LIBC_CI` on FreeBSD11, although I'm  not sure this would have caught the issue in #1466 .

Closes #1466 .
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.

None yet

9 participants