several illumos fixes, and some additional termios constants #2560

jclulow · 2021-11-22T09:12:56Z

I wanted to add some of the new baud rate constants that we added recently. On the way there, I had to make a variety of fixes to get the tests to pass on illumos systems.

rust-highfive · 2021-11-22T09:12:58Z

r? @JohnTitor

(rust-highfive has picked a reviewer for you, use r? to override)

jclulow · 2021-11-22T09:18:04Z

With this series of commits, the tests now pass again:

$ (cd libc-test && cargo test)
   Compiling libc v0.2.108 (/ws/safari/rust-libc)
   Compiling libc-test v0.2.108 (/ws/safari/rust-libc/libc-test)
    Finished test [unoptimized + debuginfo] target(s) in 11.86s
     Running test/cmsg.rs (/ws/safari/rust-libc/target/debug/deps/cmsg-e194c71fe54543ae)

running 5 tests
test t::test_cmsg_data ... ok
test t::test_cmsg_firsthdr ... ok
test t::test_cmsg_len ... ok
test t::test_cmsg_space ... ok
test t::test_cmsg_nxthdr ... ok

test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.64s

     Running test/errqueue.rs (/ws/safari/rust-libc/target/debug/deps/errqueue-b3e557442ece5dbc)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running test/linux_elf.rs (/ws/safari/rust-libc/target/debug/deps/linux_elf-c4b83707fea8d930)
PASSED 0 tests
     Running test/linux_fcntl.rs (/ws/safari/rust-libc/target/debug/deps/linux_fcntl-cb3b4167f025e27d)
PASSED 0 tests
     Running test/linux_if_arp.rs (/ws/safari/rust-libc/target/debug/deps/linux_if_arp-3b2a8366b990e987)
PASSED 0 tests
     Running test/linux_ipv6.rs (/ws/safari/rust-libc/target/debug/deps/linux_ipv6-fbd30315413a72f0)
PASSED 0 tests
     Running test/linux_strerror_r.rs (/ws/safari/rust-libc/target/debug/deps/linux_strerror_r-bc924bcb42c264e3)
PASSED 0 tests
     Running test/linux_termios.rs (/ws/safari/rust-libc/target/debug/deps/linux_termios-96062aa19befeba0)
PASSED 0 tests
     Running test/main.rs (/ws/safari/rust-libc/target/debug/deps/main-6989d6271949dcc6)
RUNNING ALL TESTS
PASSED 7119 tests
     Running test/semver.rs (/ws/safari/rust-libc/target/debug/deps/semver-0411189b90229d43)
PASSED 1 tests
     Running test/sigrt.rs (/ws/safari/rust-libc/target/debug/deps/sigrt-55024fdaab592c88)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

A few of these issues would have, I suspect, been caught if folks were running the tests before putting up PRs. I recognise that there is not currently a CI builder for illumos systems for this project, but I wonder if it would be possible to CC me on future changes to illumos/Solaris files so that I can make sure the tests get run? Is there a way to volunteer myself and/or perhaps other illumos community members to look at changes that we could help review, etc?

We have also, at Oxide, been experimenting with a GitHub app that can arrange for illumos CI builds of GitHub repositories. It integrates using check suites/runs like other non-Actions based things; e.g., similar to the Cirrus CI being used for FreeBSD here. Is that something we could perhaps try for the libc crate some time soon once it's had a bit more soak time?

JohnTitor · 2021-11-24T07:09:45Z

With this series of commits, the tests now pass again:

Thanks! There are some breaking changes though, can I assume that they currently won't work correctly? If so, I think it's okay as they're not usable anyway.

A few of these issues would have, I suspect, been caught if folks were running the tests before putting up PRs. I recognise that there is not currently a CI builder for illumos systems for this project, but I wonder if it would be possible to CC me on future changes to illumos/Solaris files so that I can make sure the tests get run? Is there a way to volunteer myself and/or perhaps other illumos community members to look at changes that we could help review, etc?

Sure, opened rust-lang/highfive#366. You'll get a ping like #2557 (comment) once it's merged.

We have also, at Oxide, been experimenting with a GitHub app that can arrange for illumos CI builds of GitHub repositories. It integrates using check suites/runs like other non-Actions based things; e.g., similar to the Cirrus CI being used for FreeBSD here. Is that something we could perhaps try for the libc crate some time soon once it's had a bit more soak time?

Yea, definitely! I would love to see it :)

jclulow · 2021-11-24T20:03:37Z

Thanks! There are some breaking changes though, can I assume that they currently won't work correctly? If so, I think it's okay as they're not usable anyway.

Yes I believe the breaking changes I made are all to clean up things that looked mostly right but had subtle ABI-ish problems like the long double thing that the test suite uncovered.

Sure, opened rust-lang/highfive#366. You'll get a ping like #2557 (comment) once it's merged.

Thanks! That's awesome.

Yea, definitely! I would love to see it :)

Great! I'll swing back with a PR once it's ready for public consumption.

JohnTitor · 2021-12-24T02:17:50Z

Ohhh, sorry! I didn't r+ this one even though I checked your comment... @bors r+

bors · 2021-12-24T02:17:51Z

📌 Commit 40db14e has been approved by JohnTitor

bors · 2021-12-24T03:16:33Z

⌛ Testing commit 40db14e with merge 64bac14...

several illumos fixes, and some additional termios constants I wanted to add some of the new baud rate constants that [we added recently](https://www.illumos.org/issues/13975). On the way there, I had to make a variety of fixes to get the tests to pass on illumos systems.

bors · 2021-12-24T03:29:30Z

💔 Test failed - checks-actions

JohnTitor · 2021-12-24T03:31:55Z

Network failure, @bors retry

bors · 2021-12-24T03:32:02Z

⌛ Testing commit 40db14e with merge 0ac761a...

several illumos fixes, and some additional termios constants I wanted to add some of the new baud rate constants that [we added recently](https://www.illumos.org/issues/13975). On the way there, I had to make a variety of fixes to get the tests to pass on illumos systems.

bors · 2021-12-24T04:28:21Z

💔 Test failed - checks-actions

JohnTitor · 2021-12-25T06:18:16Z

There're some implementations that still use the typo'ed field name:

libc/src/unix/solarish/x86_64.rs

Lines 89 to 103 in 5e95cbb

    
           impl PartialEq for mcontext_t { 
        
               fn eq(&self, other: &mcontext_t) -> bool { 
        
                   self.gregs == other.gregs && 
        
                       self.fpgregs == other.fpgregs 
        
               } 
        
           } 
        
           impl Eq for mcontext_t {} 
        
           impl ::fmt::Debug for mcontext_t { 
        
               fn fmt(&self, f:&mut ::fmt::Formatter) -> ::fmt::Result { 
        
                   f.debug_struct("mcontext_t") 
        
                       .field("gregs", &self.gregs) 
        
                       .field("fpgregs", &self.fpgregs) 
        
                       .finish() 
        
               } 
        
           }

jclulow · 2022-01-31T21:10:49Z

@pfmooney did some work to clean this up, and I've included his work in this branch and rebased. The tests now pass again:

$ uname -a
SunOS vulcan 5.11 helios-1.0.20799 i86pc i386 i86pc

$ (cd libc-test && cargo test)
   Compiling libc v0.2.116 (/ws/safari/rust-libc)
   Compiling libc-test v0.2.116 (/ws/safari/rust-libc/libc-test)
    Finished test [unoptimized + debuginfo] target(s) in 23.12s
     Running test/cmsg.rs (/ws/safari/rust-libc/target/debug/deps/cmsg-8121aecb845cbb39)

running 5 tests
test t::test_cmsg_data ... ok
test t::test_cmsg_space ... ok
test t::test_cmsg_len ... ok
test t::test_cmsg_firsthdr ... ok
test t::test_cmsg_nxthdr ... ok

test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.61s

     Running test/errqueue.rs (/ws/safari/rust-libc/target/debug/deps/errqueue-c547c10f8b4f2364)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running test/linux_elf.rs (/ws/safari/rust-libc/target/debug/deps/linux_elf-bd07bbead775f9f6)
PASSED 0 tests
     Running test/linux_fcntl.rs (/ws/safari/rust-libc/target/debug/deps/linux_fcntl-8552d2d6c5d293e5)
PASSED 0 tests
     Running test/linux_if_arp.rs (/ws/safari/rust-libc/target/debug/deps/linux_if_arp-cc3231a8da9b37eb)
PASSED 0 tests
     Running test/linux_ipv6.rs (/ws/safari/rust-libc/target/debug/deps/linux_ipv6-fc3bb8ddc0bc5885)
PASSED 0 tests
     Running test/linux_strerror_r.rs (/ws/safari/rust-libc/target/debug/deps/linux_strerror_r-5cb11314c63e97d5)
PASSED 0 tests
     Running test/linux_termios.rs (/ws/safari/rust-libc/target/debug/deps/linux_termios-e3688047f6791a24)
PASSED 0 tests
     Running test/main.rs (/ws/safari/rust-libc/target/debug/deps/main-f9f9aac89fba0fb8)
RUNNING ALL TESTS
PASSED 7215 tests
     Running test/semver.rs (/ws/safari/rust-libc/target/debug/deps/semver-a5b1ae9ade0f2f3b)
PASSED 1 tests
     Running test/sigrt.rs (/ws/safari/rust-libc/target/debug/deps/sigrt-3c7f8a9f41084872)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

pfmooney · 2022-01-31T21:19:47Z

@pfmooney did some work to clean this up, and I've included his work in this branch and rebased. The tests now pass again:

I checked that the tests also pass when the extra_traits feature is active, which was relevant to the earlier comment about the (now-fixed) lingering "fpgregs" references.

Amanieu · 2022-01-31T21:48:25Z

Nice work! Would you be interested in adding illumos testing to the CI? At the moment we only check that the crate builds.

@bors r+

bors · 2022-01-31T21:48:27Z

📌 Commit c8cc318 has been approved by Amanieu

several illumos fixes, and some additional termios constants I wanted to add some of the new baud rate constants that [we added recently](https://www.illumos.org/issues/13975). On the way there, I had to make a variety of fixes to get the tests to pass on illumos systems.

bors · 2022-01-31T22:16:55Z

⌛ Testing commit c8cc318 with merge 2d36a72...

bors · 2022-01-31T23:17:01Z

💔 Test failed - checks-actions

Amanieu · 2022-02-02T16:40:54Z

CI is failing on older Rust version (1.24)

The test suite flagged that incorrect values for some fcntl(2) constants were added in rust-lang#2083. I have fixed the values so that they are correct for 64-bit programs, which Rust programs always are on illumos.

As per https://illumos.org/man/3EXT/sendfile a separate header and library are required to access sendfile() and sendfilev() on illumos systems.

The semver regression checks in rust-lang#2109 included the "SO_REUSEPORT" constant, which we do not yet have on illumos systems. Move it out to platform-specific files.

Some of the type information in the machine context types, with particular focus on the padding unions, was not quite right. It seems we have used the somewhat baroque "long double" in the system headers, and Rust does not have a type that matches that data layout. I have adjusted the structs to omit that member, but to be explicitly aligned to match the C version. I also gagged a test for the "fp_reg_set" member which is of an anonymous union type. Portions contributed by: Patrick Mooney <pmooney@pfmooney.com>

Various small fixes to the tests to include all the required headers, and to add some constants that are now part of the "unix" semver list, and to drop "sethostid()" which is not something we have on our platform, etc.

We added baud rate constants that are source compatible with Linux systems when used with the cfsetspeed() family, in: https://www.illumos.org/issues/13975

jclulow · 2022-02-10T23:55:49Z

CI is failing on older Rust version (1.24)

Sorry about that! I've changed the align repr directives to be guarded by libc_align, and rebased. Tests still pass for me locally with a current Rust version.

Amanieu · 2022-02-11T12:09:26Z

Let's give it another try!

@bors r+

bors · 2022-02-11T12:09:27Z

📌 Commit 60213e1 has been approved by Amanieu

bors · 2022-02-11T12:09:35Z

⌛ Testing commit 60213e1 with merge b30ec40...

bors · 2022-02-11T13:14:40Z

☀️ Test successful - checks-actions, checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13
Approved by: Amanieu
Pushing b30ec40 to master...

rust-highfive assigned JohnTitor Nov 22, 2021

rust-highfive added the S-waiting-on-review label Nov 22, 2021

JohnTitor mentioned this pull request Nov 24, 2021

Ping some contributors on illumos-related changes rust-lang/highfive#366

Merged

JohnTitor approved these changes Dec 24, 2021

View reviewed changes

bors added S-waiting-on-bors and removed S-waiting-on-review labels Dec 24, 2021

bors added S-waiting-on-review and removed S-waiting-on-bors labels Dec 24, 2021

bors added S-waiting-on-bors and removed S-waiting-on-review labels Dec 24, 2021

bors added S-waiting-on-review and removed S-waiting-on-bors labels Dec 24, 2021

pfmooney mentioned this pull request Jan 30, 2022

solarish lgrp api subset #2661

Merged

jclulow force-pushed the illumos-2021-11-22 branch from 40db14e to c8cc318 Compare January 31, 2022 21:09

bors added S-waiting-on-bors and removed S-waiting-on-review labels Jan 31, 2022

bors added S-waiting-on-review and removed S-waiting-on-bors labels Jan 31, 2022

devnexen mentioned this pull request Feb 1, 2022

solarish systems dl_iterate_phdr support #2665

Merged

jclulow and others added 7 commits February 10, 2022 15:54

illumos: wrong values for F_OFD_*

c14a884

The test suite flagged that incorrect values for some fcntl(2) constants were added in rust-lang#2083. I have fixed the values so that they are correct for 64-bit programs, which Rust programs always are on illumos.

illumos: sendfile(3EXT) is in libsendfile, not libc

4249c68

As per https://illumos.org/man/3EXT/sendfile a separate header and library are required to access sendfile() and sendfilev() on illumos systems.

illumos does not yet have SO_REUSEPORT

96e2e3a

The semver regression checks in rust-lang#2109 included the "SO_REUSEPORT" constant, which we do not yet have on illumos systems. Move it out to platform-specific files.

illumos: fixes to get tests to pass

aee5ce5

Various small fixes to the tests to include all the required headers, and to add some constants that are now part of the "unix" semver list, and to drop "sethostid()" which is not something we have on our platform, etc.

solarish: Fix tests for subsequent additions

0c417e8

illumos: higher baud rates, more termios constants

60213e1

We added baud rate constants that are source compatible with Linux systems when used with the cfsetspeed() family, in: https://www.illumos.org/issues/13975

jclulow force-pushed the illumos-2021-11-22 branch from c8cc318 to 60213e1 Compare February 10, 2022 23:54

bors added S-waiting-on-bors and removed S-waiting-on-review labels Feb 11, 2022

bors merged commit b30ec40 into rust-lang:master Feb 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

several illumos fixes, and some additional termios constants #2560

several illumos fixes, and some additional termios constants #2560

jclulow commented Nov 22, 2021

rust-highfive commented Nov 22, 2021

jclulow commented Nov 22, 2021

JohnTitor commented Nov 24, 2021

jclulow commented Nov 24, 2021

JohnTitor commented Dec 24, 2021

bors commented Dec 24, 2021

bors commented Dec 24, 2021

bors commented Dec 24, 2021

JohnTitor commented Dec 24, 2021

bors commented Dec 24, 2021

bors commented Dec 24, 2021

JohnTitor commented Dec 25, 2021

jclulow commented Jan 31, 2022

pfmooney commented Jan 31, 2022

Amanieu commented Jan 31, 2022

bors commented Jan 31, 2022

bors commented Jan 31, 2022

bors commented Jan 31, 2022

Amanieu commented Feb 2, 2022

jclulow commented Feb 10, 2022

Amanieu commented Feb 11, 2022

bors commented Feb 11, 2022

bors commented Feb 11, 2022

bors commented Feb 11, 2022

several illumos fixes, and some additional termios constants #2560

several illumos fixes, and some additional termios constants #2560

Conversation

jclulow commented Nov 22, 2021

rust-highfive commented Nov 22, 2021

jclulow commented Nov 22, 2021

JohnTitor commented Nov 24, 2021

jclulow commented Nov 24, 2021

JohnTitor commented Dec 24, 2021

bors commented Dec 24, 2021

bors commented Dec 24, 2021

bors commented Dec 24, 2021

JohnTitor commented Dec 24, 2021

bors commented Dec 24, 2021

bors commented Dec 24, 2021

JohnTitor commented Dec 25, 2021

jclulow commented Jan 31, 2022

pfmooney commented Jan 31, 2022

Amanieu commented Jan 31, 2022

bors commented Jan 31, 2022

bors commented Jan 31, 2022

bors commented Jan 31, 2022

Amanieu commented Feb 2, 2022

jclulow commented Feb 10, 2022

Amanieu commented Feb 11, 2022

bors commented Feb 11, 2022

bors commented Feb 11, 2022

bors commented Feb 11, 2022