Skip to content

Clean libc-test for apple targets #1274

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

Merged
merged 1 commit into from
Feb 22, 2019
Merged

Conversation

gnzlbg
Copy link
Contributor

@gnzlbg gnzlbg commented Feb 22, 2019

This cleans up the build.rs of libc-test for apple targets.

I wanted to update the docker containers of some targets so that we can start testing newer currently-skipped APIs properly, but it is impossible to figure out which headers and APIs are skipped for each target, which has to change if we update the glibc version in one Linux container but not the other (updating them all at once is just madness).

This PR separates the testing of apple targets into its own self-contained function. This allows seeing exactly which headers are included, and which items are skipped. A lot of work will be required to separate the testing of all major platforms and make the script reasonable.

During the clean up, I discovered that, at least for apple targets, deprecated but not removed APIs are not tested. I re-enabled testing for those, and fixed daemon, which was not properly linking its symbol. I also added the #[deprecated] attribute to the #[deprecated] APIs of the apple targets. The attribute is available since Rust 1.9.0 and the min. Rust version we support is Rust 1.13.0.

Many other APIs are also currently not tested "because they are weird" which I interpret as "the test failed for an unknown reason", as a consequence:

This probably explains why some other things, like the sa_handler/sa_sigaction fields of sigaction were skipped. The field is actually a union, which can be either a sig_t for the sa_handler field, or some other type for the sa_sigaction field, but because the distinction was not made, the field was not checked.

The latest ctest version can check volatile pointers, so a couple of skipped tests are now tested using this feature.

@rust-highfive
Copy link

@gnzlbg: no appropriate reviewer found, use r? to override

This cleans up the build.rs of `libc-test` for apple targets.

I wanted to update the docker containers of some targets so that we can start
testing newer currently-skipped APIs properly, but it is impossible to figure
out which headers and APIs are skipped for each target.

This PR separates the testing of apple targets into its own self-contained
function. This allows seeing exactly which headers are included, and which items
are skipped. A lot of work will be required to separate the testing of all major
platforms and make the script reasonable.

During the clean up, I discovered that, at least for apple targets, deprecated
but not removed APIs are not tested. I re-enabled testing for those, and fixed
`daemon`, which was not properly linking its symbol. I also added the
`#[deprecated]` attribute to the `#[deprecated]` APIs of the apple targets. The
attribute is available since Rust 1.9.0 and the min. Rust version we support is
Rust 1.13.0.

Many other APIs are also currently not tested "because they are weird" which I
interpret as "the test failed for an unknown reason", as a consequence:

* the signatures of execv, execve, and execvp are incorrect (see
  rust-lang#1272)

* the `sig_t` type is called `sighandler_t` in libc for some reason:
  rust-lang#1273

This probably explains why some other things, like the
`sa_handler`/`sa_sigaction` fields of `sigaction` were skipped. The field is
actually a union, which can be either a `sig_t` for the `sa_handler` field, or
some other type for the `sa_sigaction` field, but because the distinction was
not made, the field was not checked.

The latest ctest version can check volatile pointers, so a couple of skipped
tests are now tested using this feature.
@gnzlbg gnzlbg force-pushed the clean_libctest_apple branch from 29fb3f8 to f5cbdbc Compare February 22, 2019 18:18
@gnzlbg
Copy link
Contributor Author

gnzlbg commented Feb 22, 2019

@bors: r+

@bors
Copy link
Contributor

bors commented Feb 22, 2019

📌 Commit f5cbdbc has been approved by gnzlbg

@bors
Copy link
Contributor

bors commented Feb 22, 2019

⌛ Testing commit f5cbdbc with merge 68701e7...

bors added a commit that referenced this pull request Feb 22, 2019
Clean libc-test for apple targets

This cleans up the build.rs of `libc-test` for apple targets.

I wanted to update the docker containers of some targets so that we can start testing newer currently-skipped APIs properly, but it is impossible to figure out which headers and APIs are skipped for each target, which has to change if we update the glibc version in one Linux container but not the other (updating them all at once is just madness).

This PR separates the testing of apple targets into its own self-contained function. This allows seeing exactly which headers are included, and which items are skipped. A lot of work will be required to separate the testing of all major platforms and make the script reasonable.

During the clean up, I discovered that, at least for apple targets, deprecated but not removed APIs are not tested. I re-enabled testing for those, and fixed `daemon`, which was not properly linking its symbol. I also added the `#[deprecated]` attribute to the `#[deprecated]` APIs of the apple targets. The attribute is available since Rust 1.9.0 and the min. Rust version we support is Rust 1.13.0.

Many other APIs are also currently not tested "because they are weird" which I interpret as "the test failed for an unknown reason", as a consequence:

* the signatures of execv, execve, and execvp are incorrect (see #1272)

* the `sig_t` type is called `sighandler_t` in libc for some reason: #1273

This probably explains why some other things, like the `sa_handler`/`sa_sigaction` fields of `sigaction` were skipped. The field is actually a union, which can be either a `sig_t` for the `sa_handler` field, or some other type for the `sa_sigaction` field, but because the distinction was not made, the field was not checked.

The latest ctest version can check volatile pointers, so a couple of skipped tests are now tested using this feature.
@bors
Copy link
Contributor

bors commented Feb 22, 2019

☀️ Test successful - checks-cirrus, checks-travis, status-appveyor
Approved by: gnzlbg
Pushing 68701e7 to master...

@bors bors merged commit f5cbdbc into rust-lang:master Feb 22, 2019
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.

3 participants