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 GNU/Hurd support #2131

Merged
merged 1 commit into from
Dec 8, 2023
Merged

Add GNU/Hurd support #2131

merged 1 commit into from
Dec 8, 2023

Conversation

sthibaul
Copy link
Contributor

No description provided.

@sthibaul sthibaul force-pushed the master branch 2 times, most recently from a65bb09 to d939870 Compare September 21, 2023 22:28
@sthibaul
Copy link
Contributor Author

sthibaul commented Sep 21, 2023

error: this loop never actually loops
    --> test/sys/test_socket.rs:1305:9
     |
1305 | /         for _ in msg.cmsgs() {
1306 | |             panic!("unexpected cmsg");
1307 | |         }
     | |_________^
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#never_loop
     = note: `#[deny(clippy::never_loop)]` on by default

This is unrelated. This actually looks to me like a false positive from clippy: the code does mean not to loop and just panic if we make even just one iteration.

@SteveLauC
Copy link
Member

Hi, thanks for your contribution to nix! Unfortunately, our Cirrus CI is running out of its credits, so I manually stopped the CI for this PR.

After our migration to GitHub action, I will take a look at this PR and re-run the CI job

@sthibaul
Copy link
Contributor Author

sthibaul commented Nov 8, 2023

I have rebased on top of the github action commit

@sthibaul
Copy link
Contributor Author

sthibaul commented Nov 8, 2023

And I have now added i686-unknown-hurd-gnu as tier3 in CI. Obviously, in the meanwhile some errors have popped up, I'll have a look.

@sthibaul sthibaul force-pushed the master branch 5 times, most recently from 2a718c8 to 4775b9d Compare November 9, 2023 02:40
@sthibaul
Copy link
Contributor Author

sthibaul commented Nov 9, 2023

Some fixes need to be done in libc (and wait for a libc release...)

@SteveLauC
Copy link
Member

Some fixes need to be done in libc (and wait for a libc release...)

Ok, then I will label this W-blocking-upstream

@sthibaul sthibaul force-pushed the master branch 3 times, most recently from 848da27 to 11dfa98 Compare November 11, 2023 01:43
src/fcntl.rs Outdated Show resolved Hide resolved
src/sys/resource.rs Outdated Show resolved Hide resolved
test/test_unistd.rs Outdated Show resolved Hide resolved
@asomers
Copy link
Member

asomers commented Nov 12, 2023

You'll have to rebase to fix CI.

@sthibaul
Copy link
Contributor Author

You'll have to rebase to fix CI.

ok, done so!

@asomers
Copy link
Member

asomers commented Nov 12, 2023

@SteveLauC do you need to change the minimum libc version? CI's minver check won't catch an error there, if the error only affects Hurd.

@SteveLauC
Copy link
Member

hi @sthibaul, is this PR blocked by these changes?

These changes haven't been released yet, so I guess we still need to wait for a new libc release

@SteveLauC SteveLauC mentioned this pull request Nov 13, 2023
3 tasks
@SteveLauC
Copy link
Member

Hi @sthibaul, now we are ok to use the libc dependency from git so that we don't have to wait for a libc release.

And, rebasing your branch should make the CI pass since the commit used already has these 2 PRs covered

I am sorry about those merge conflicts, we have switched to cfg alias to make our code clean, and this PR is mostly about cfg, so...

@sthibaul sthibaul force-pushed the master branch 12 times, most recently from 491d766 to dfddc62 Compare December 4, 2023 21:02
@sthibaul
Copy link
Contributor Author

sthibaul commented Dec 4, 2023

Ok, at last it does pass...

src/fcntl.rs Outdated Show resolved Hide resolved
src/sys/mman.rs Outdated Show resolved Hide resolved
src/sys/resource.rs Outdated Show resolved Hide resolved
src/unistd.rs Outdated Show resolved Hide resolved
src/unistd.rs Outdated Show resolved Hide resolved
test/test_unistd.rs Outdated Show resolved Hide resolved
test/test_unistd.rs Outdated Show resolved Hide resolved
Copy link
Member

@SteveLauC SteveLauC left a comment

Choose a reason for hiding this comment

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

Thanks!

@SteveLauC SteveLauC added this pull request to the merge queue Dec 5, 2023
@sthibaul
Copy link
Contributor Author

sthibaul commented Dec 5, 2023

AIUI it is waiting for @asomers ' ack?

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 5, 2023
@SteveLauC
Copy link
Member

AIUI it is waiting for @asomers ' ack?

Yeah, that would be best, let's wait for asomers's review:)

Copy link
Member

@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.

LGTM

@asomers asomers added this pull request to the merge queue Dec 8, 2023
Merged via the queue into nix-rust:master with commit a90b4b2 Dec 8, 2023
35 checks passed
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