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 NetBSD CI #3965

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Conversation

nathaniel-bennett
Copy link
Contributor

@nathaniel-bennett nathaniel-bennett commented Oct 14, 2024

Description

This PR adds NetBSD to the CI for nightly target and resolves current build/test failures resulting from NetBSD modules.

Fixes #3890
Fixes #3965

@rustbot
Copy link
Collaborator

rustbot commented Oct 14, 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 Oct 14, 2024

Some changes occurred in OpenBSD module

cc @semarie

Some changes occurred in OpenBSD module

cc @semarie

@nathaniel-bennett nathaniel-bennett force-pushed the netbsd-ci branch 3 times, most recently from 7d1927e to f0d2715 Compare October 14, 2024 13:01
@nathaniel-bennett
Copy link
Contributor Author

Looks like this assertion in ctest2 fails during the build process for libc-test:
https://github.com/JohnTitor/ctest2/blob/80f9cc420ed2739631f1604f44744107ad10d9a0/src/lib.rs#L2107

Based on context, it looks like some variadic function in the NetBSD module is tripping up the testing harness. I'm willing to bet it's esetfunc:
https://github.com/rust-lang/libc/blob/fbec9289b33627d03b72701a296095c9e91354fd/src/unix/bsd/netbsdlike/netbsd/mod.rs#L2953C1-L2955C72

...as it's the only function in the entirety of the libc crate that passes a variadic function as a parameter.

Potential solutions:

  1. Refactor ctest2 to handle variadic functions as parameters.
  2. Configure build.rs to ignore esetfunc until (1.) is accomplished.

I've gone ahead and opted for 2. at the moment, since I have no experience with the ctest2 crate and wouldn't be able to perform that refactor.

@nathaniel-bennett
Copy link
Contributor Author

It appears that adding an exception to the libc-test/build.rs script doesn't fix this assertion; removing esetfunc from semver/netbsd.txt similarly fails to fix the issue.

I've temporarily opted to remove the esetfunc function entirely to get past that build error and see if the rest of CI works as normal; obviously this is a breaking change, so we may want to fix ctest2 and re-add it before 1.0 is released.

@nathaniel-bennett
Copy link
Contributor Author

Source for changing sendmmsg/recvmmsg flags parameter from int to unsigned int:

https://github.com/NetBSD/src/blob/da7ff22a4ae89b32ca9e90ca20e7d0d434e8cdd2/sys/sys/socket.h#L651-L654

@nathaniel-bennett
Copy link
Contributor Author

Documentation on kevent now using a void pointer for its udata field:

https://github.com/NetBSD/src/blob/da7ff22a4ae89b32ca9e90ca20e7d0d434e8cdd2/sys/sys/event.h#L66

@nathaniel-bennett
Copy link
Contributor Author

Documentation that uucred using a short type instead of an int for its cr_ngroups field:

https://github.com/NetBSD/src/blob/da7ff22a4ae89b32ca9e90ca20e7d0d434e8cdd2/sys/sys/ucred.h#L48

@nathaniel-bennett
Copy link
Contributor Author

Documentation that sockaddr_dl uses a separate dl_addr struct with an address field of 24 bytes, not 12:

https://github.com/NetBSD/src/blob/da7ff22a4ae89b32ca9e90ca20e7d0d434e8cdd2/sys/net/if_dl.h#L66-L91

@nathaniel-bennett
Copy link
Contributor Author

Documentation of correct utmpx fields (thereby removing incomplete type __exit_status):

https://github.com/NetBSD/src/blob/da7ff22a4ae89b32ca9e90ca20e7d0d434e8cdd2/include/utmpx.h#L103-L118

@nathaniel-bennett
Copy link
Contributor Author

Looks like there are a ton of CI failures scattered across the NetBSD module--too many for me to try to resolve at the moment. I'll try to knock out some more later this week in this PR.

@tgross35
Copy link
Contributor

tgross35 commented Oct 15, 2024

Thanks for taking a look at this, it would be great to have something here. Feel free to put any of the changes into smaller PRs that can be merged on their own, it would be good to get any problems fixed even if we aren't testing it right away.

Any reason to use GHA rather than Cirrus?

Also I don't think we need to test MSRV, we don't do that for other tier2 targets.

@semarie it would probably be best if you get involved here.

Comment on lines +209 to +211
curl https://sh.rustup.rs -sSf --output rustup.sh
sh rustup.sh -y --default-toolchain nightly --profile=minimal
. $HOME/.cargo/env
Copy link
Contributor

Choose a reason for hiding this comment

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

It would probably be best to just adjust ci/install-rust.sh as needed (if at all) to do what is needed here.

@nathaniel-bennett
Copy link
Contributor Author

Any reason to use GHA rather than Cirrus?

Honestly, I mostly used it because I knew how to set it up based on past projects (whereas I don't have any familiarity with Cirrus). I would be perfectly fine if someone wanted to move it to a Cirrus setup; based on my preliminary exploring, they seem to run NetBSD VMs on top of Linux just like GHA does (though I could be wrong there).

@tgross35
Copy link
Contributor

tgross35 commented Nov 6, 2024

There is still some work needed here right?

@rustbot author

@bors
Copy link
Contributor

bors commented Nov 7, 2024

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

@tgross35
Copy link
Contributor

CI now has a test_tier2_vm job based on vmactions for Solaris

test_tier2_vm:
name: Test tier2 VM
needs: [test_tier1, style_check]
runs-on: ubuntu-latest
strategy:
fail-fast: true
matrix:
target:
- x86_64-pc-solaris
steps:
- uses: actions/checkout@v4
- name: test on Solaris
uses: vmactions/solaris-vm@v1
with:
release: "11.4-gcc"
usesh: true
mem: 4096
copyback: false
prepare: |
set -x
source <(curl -s https://raw.githubusercontent.com/psumbera/solaris-rust/refs/heads/main/sh.rust-web-install)
rustc --version
uname -a
run: |
export PATH=$HOME/.rust_solaris/bin:$PATH
./ci/run.sh ${{ matrix.target }}
. If the existing prepare script is moved to install-rust.sh then the job could just take a matrix with target and vm to handle both Solaris and NetBSD

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.

Add NetBSD to CI
4 participants