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

Fix tests on Solaris #3864

Merged
merged 1 commit into from
Nov 4, 2024
Merged

Fix tests on Solaris #3864

merged 1 commit into from
Nov 4, 2024

Conversation

psumbera
Copy link
Contributor

Tested (libc-test) on both Solaris and Illumos (with latest stable Omnios). All tests are passing.

Illumos changes can be checked against https://github.com/illumos/illumos-gate
Solaris changes can be checked only with installed Solaris CBE release (https://blogs.oracle.com/solaris/post/announcing-the-first-oracle-solaris-114-cbe) or using Solaris 11.4 machine at Cfarm (https://portal.cfarm.net/machines/list/).

@rustbot
Copy link
Collaborator

rustbot commented Aug 21, 2024

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot
Copy link
Collaborator

rustbot commented Aug 21, 2024

Some changes occurred in solarish module

cc @jclulow, @pfmooney

@psumbera
Copy link
Contributor Author

r? @tgross35 @pfmooney

@rustbot
Copy link
Collaborator

rustbot commented Aug 21, 2024

Could not assign reviewer from: tgross35.
User(s) tgross35 are either the PR author, already assigned, or on vacation, and there are no other candidates.
Use r? to specify someone else to assign.

src/unix/solarish/mod.rs Outdated Show resolved Hide resolved
src/unix/solarish/mod.rs Outdated Show resolved Hide resolved
src/unix/solarish/mod.rs Outdated Show resolved Hide resolved
@psumbera
Copy link
Contributor Author

psumbera commented Sep 2, 2024

@tgross can you please help me with the buildbot failure? I have no idea what is wrong and how can I fixt it...

@tgross
Copy link

tgross commented Sep 2, 2024

Hi @psumbera! I think you meant @tgross35 (which is funny because @pfmooney and @jclulow who are at-mentioned here are old colleagues!) 😀

Not really my area of expertise at all but from context it looks like the epoll_event might be missing an implementation of the Debug trait?

src/unix/mod.rs Outdated Show resolved Hide resolved
src/unix/solarish/solaris.rs Outdated Show resolved Hide resolved
@@ -63,3 +65,5 @@ pub const AT_386_FPINFO_NONE: u32 = 0;
pub const AT_386_FPINFO_FXSAVE: u32 = 1;
pub const AT_386_FPINFO_XSAVE: u32 = 2;
pub const AT_386_FPINFO_XSAVE_AMD: u32 = 3;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you fix the indentation?

Comment on lines 1 to 6
cfg_if! {
if #[cfg(target_os = "solaris")] {
use xrs_t;
}
}

Copy link
Contributor

@tgross35 tgross35 Sep 3, 2024

Choose a reason for hiding this comment

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

Could you just use the full path to xrs_t where it is used, rather than having a single cfged import here? I'm not sure this is doing anything anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, not sure what is proper fix. Now I use:

cfg_if! {
    if #[cfg(target_os = "solaris")] {
        use unix::solarish::solaris;
    }
}
..
pub uc_xrs: solaris::xrs_t,

@tgross35
Copy link
Contributor

tgross35 commented Sep 3, 2024

@tgross can you please help me with the buildbot failure? I have no idea what is wrong and how can I fixt it...

I left a comment above but I think xrs_t may be missing implementations. If it can just go in s! to get the automatic derives, rather than s_no_extra_traits!, that is probably easiest. (I'm not actually sure why any of these types are in s_no_extra_traits! when their implementations look near identical to the derives, some of them could move to s!. We have a big cleanup of these traits coming in the near future.)

I'm not sure why FreeBSD failed but it looks spurious.

Are tests currently failing on these platforms? It would be great to add x86_64-unknown-illumos to CI since it is tier 2 with host tools.

@rustbot author based on the above comments. It would probably be good for @pfmooney to take another look at some point too.

@tgross35
Copy link
Contributor

tgross35 commented Sep 3, 2024

Hi @psumbera! I think you meant @tgross35 (which is funny because @pfmooney and @jclulow who are at-mentioned here are old colleagues!) 😀

Out of all the similar GitHub usernames 😄 that really is an unbelievable coincidence

Not really my area of expertise at all but from context it looks like the epoll_event might be missing an implementation of the Debug trait?

Thanks for taking a look :) I think it's a different struct though (mentioned above), this crate is unfortunately pretty macro-heavy so the error messages are a bit obscure.

@psumbera
Copy link
Contributor Author

psumbera commented Sep 3, 2024

Hi @psumbera! I think you meant @tgross35 (which is funny because @pfmooney and @jclulow who are at-mentioned here are old colleagues!) 😀

Out of all the similar GitHub usernames 😄 that really is an unbelievable coincidence

Sorry about that!

@psumbera
Copy link
Contributor Author

psumbera commented Sep 3, 2024

@tgross35 can you please have another look?

Now it fails somewhere else and I don't know how to proceed again...

@tgross35
Copy link
Contributor

tgross35 commented Oct 13, 2024

Sorry, slipped under my radar. I think the error is just saying that door_desc_t__d_data__d_desc needs to move to s_no_extra_traits! rather than s!, before a certain version somewhere in 1.6x you can't derive Debug on packed traits (I suspect you are just testing with a more recent rustc version locally).

Just comment @rustbot review to update the labels once it's working or if it gets stuck again.

@bors
Copy link
Contributor

bors commented Oct 15, 2024

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

@psumbera psumbera force-pushed the solaris-tests branch 2 times, most recently from eab1ea0 to 3bbc12c Compare October 15, 2024 09:20
@tgross35
Copy link
Contributor

I think this looks okay but I am not able to test on the relevant systems. @pfmooney would you be able to double check this?

@bors
Copy link
Contributor

bors commented Oct 24, 2024

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

Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

Sorry I didn't merge this already, I think it should be good with a rebase.

@tgross35 tgross35 changed the title Fixes tests on Solaris Fix tests on Solaris Oct 24, 2024
@tgross35
Copy link
Contributor

Hm, not sure why freebsd is failing but it seems to be happening on everything

@psumbera
Copy link
Contributor Author

Hm, not sure why freebsd is failing but it seems to be happening on everything

Have no idea. Doesn't seem to be related.

@tgross35 tgross35 added this pull request to the merge queue Nov 4, 2024
Merged via the queue into rust-lang:main with commit ddeff5b Nov 4, 2024
41 checks passed
sunfishcode added a commit to bytecodealliance/rustix that referenced this pull request Nov 5, 2024
It appears Solaris doesn't support epoll; only Illumos does. And as
of rust-lang/libc#3864, the libc crate won't declare the epoll libc
interface on Solaris. So disable epoll support on Solaris in rustix
too.
sunfishcode added a commit to bytecodealliance/rustix that referenced this pull request Nov 5, 2024
* Disable epoll support on Solaris.

It appears Solaris doesn't support epoll; only Illumos does. And as
of rust-lang/libc#3864, the libc crate won't declare the epoll libc
interface on Solaris. So disable epoll support on Solaris in rustix
too.

* Add epoll documentation links for illumos.

* Add a documentation reference for ptsname for illumos.

* Enable the epoll tests on illumos and redox.

* Enable the openpty tests on illumos.
Comment on lines -61 to -66
pub const POSIX_FADV_NORMAL: ::c_int = 0;
pub const POSIX_FADV_RANDOM: ::c_int = 1;
pub const POSIX_FADV_SEQUENTIAL: ::c_int = 2;
pub const POSIX_FADV_WILLNEED: ::c_int = 3;
pub const POSIX_FADV_DONTNEED: ::c_int = 4;
pub const POSIX_FADV_NOREUSE: ::c_int = 5;
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like these, plus POLLRDHUP above, were accidentally deleted maybe in a bad merge. I'll roll a fix in for these in #3999.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry about that.

@tgross35 tgross35 added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Nov 7, 2024
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Nov 7, 2024
(backport <rust-lang#3864>)
(cherry picked from commit ec3c338)
@tgross35 tgross35 mentioned this pull request Nov 7, 2024
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Nov 7, 2024
(backport <rust-lang#3864>)
(cherry picked from commit ec3c338)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Nov 7, 2024
(backport <rust-lang#3864>)
(cherry picked from commit ec3c338)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Nov 7, 2024
(backport <rust-lang#3864>)
(cherry picked from commit ec3c338)
@tgross35 tgross35 added stable-applied This PR has been cherry-picked to libc's stable release branch and removed stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Nov 7, 2024
sunfishcode added a commit to bytecodealliance/rustix that referenced this pull request Nov 10, 2024
* Disable epoll support on Solaris.

It appears Solaris doesn't support epoll; only Illumos does. And as
of rust-lang/libc#3864, the libc crate won't declare the epoll libc
interface on Solaris. So disable epoll support on Solaris in rustix
too.

* Add epoll documentation links for illumos.

* Add a documentation reference for ptsname for illumos.

* Enable the epoll tests on illumos and redox.

* Enable the openpty tests on illumos.
sunfishcode added a commit to bytecodealliance/rustix that referenced this pull request Nov 10, 2024
* Disable epoll support on Solaris.

It appears Solaris doesn't support epoll; only Illumos does. And as
of rust-lang/libc#3864, the libc crate won't declare the epoll libc
interface on Solaris. So disable epoll support on Solaris in rustix
too.

* Add epoll documentation links for illumos.

* Add a documentation reference for ptsname for illumos.

* Enable the epoll tests on illumos and redox.

* Enable the openpty tests on illumos.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review stable-applied This PR has been cherry-picked to libc's stable release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants