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

hurd: Fix st_dev name #3785

Merged
merged 1 commit into from
Oct 20, 2024
Merged

hurd: Fix st_dev name #3785

merged 1 commit into from
Oct 20, 2024

Conversation

sthibaul
Copy link
Contributor

Currently struct stat and struct stat64 are not coherent: struct stat is using st_dev, and struct stat64 is using st_fsid.

st_dev is the more commonly-known name, already used by e.g. ~45 rust software in Debian, so better fix st_fsid into st_dev, rather than having to uselessly spend time hand-patching all these software.

@rustbot
Copy link
Collaborator

rustbot commented Jul 25, 2024

r? @JohnTitor

rustbot has assigned @JohnTitor.
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

@sthibaul
Copy link
Contributor Author

I am re-submitting here, since my comments on #3758 don't seem to be getting answers.

@tgross35
Copy link
Contributor

Brought this up for policy in #3839, that should at least give us better guidelines.

@rustbot blocked

@tgross35
Copy link
Contributor

This hasn't been discussed yet, but I am inclined to just merge this to unstick things. Do you have a link to the relevant C headers?

@sthibaul
Copy link
Contributor Author

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.

Thanks, let's get this in.

Currently struct stat and struct stat64 are not coherent: struct stat is
using st_dev, and struct stat64 is using st_fsid.

st_dev is the more commonly-known name, already used by e.g. ~45 rust
software in Debian, so better fix st_fsid into st_dev, rather than having to
uselessly spend time hand-patching all these software.
@tgross35 tgross35 enabled auto-merge October 19, 2024 22:06
@tgross35 tgross35 added stable-nominated This PR should be considered for cherry-pick to libc's stable release branch S-waiting-on-ci and removed S-blocked labels Oct 19, 2024
@tgross35 tgross35 added this pull request to the merge queue Oct 20, 2024
Merged via the queue into rust-lang:main with commit 8d0b3a0 Oct 20, 2024
41 checks passed
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Nov 6, 2024
Currently struct stat and struct stat64 are not coherent: struct stat is
using st_dev, and struct stat64 is using st_fsid.

st_dev is the more commonly-known name, already used by e.g. ~45 rust
software in Debian, so better fix st_fsid into st_dev, rather than having to
uselessly spend time hand-patching all these software.

(backport <rust-lang#3785>)
(cherry picked from commit 043043f)
@tgross35 tgross35 mentioned this pull request Nov 6, 2024
@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 6, 2024
@SteveLauC
Copy link
Contributor

Looks like this PR breaks the std, see Nix CI log:

error[E0609]: no field `st_fsid` on type `&stat64`
   --> /home/runner/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/os/hurd/fs.rs:301:36
    |
301 |         self.as_inner().as_inner().st_fsid as u64
    |                                    ^^^^^^^ unknown field
    |
help: a field with a similar name exists
    |
301 |         self.as_inner().as_inner().st_uid as u64
    |                                    ~~~~~~

@tgross35
Copy link
Contributor

Can you just disable that test in nix for now? Some breakage was expected here (it's tier3 and this PR is authored by the Hurd target maintainer), std just needs to be updated.

@SteveLauC
Copy link
Contributor

std just needs to be updated.

I will send a patch to the std

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 1, 2024
fix: hurd build, stat64.st_fsid was renamed to st_dev

On hurd, `stat64.st_fsid` was renamed to `st_dev` in rust-lang/libc#3785, so if you have a new libc with this patch included, and you build std from source, you get this error:

```sh
error[E0609]: no field `st_fsid` on type `&stat64`
   --> /home/runner/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/os/hurd/fs.rs:301:36
    |
301 |         self.as_inner().as_inner().st_fsid as u64
    |                                    ^^^^^^^ unknown field
    |
help: a field with a similar name exists
    |
301 |         self.as_inner().as_inner().st_uid as u64
    |                                    ~~~~~~
```

Full CI log: https://github.com/nix-rust/nix/actions/runs/12033180710/job/33546728266?pr=2544
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 1, 2024
Rollup merge of rust-lang#133515 - SteveLauC:fix/hurd, r=ChrisDenton

fix: hurd build, stat64.st_fsid was renamed to st_dev

On hurd, `stat64.st_fsid` was renamed to `st_dev` in rust-lang/libc#3785, so if you have a new libc with this patch included, and you build std from source, you get this error:

```sh
error[E0609]: no field `st_fsid` on type `&stat64`
   --> /home/runner/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/os/hurd/fs.rs:301:36
    |
301 |         self.as_inner().as_inner().st_fsid as u64
    |                                    ^^^^^^^ unknown field
    |
help: a field with a similar name exists
    |
301 |         self.as_inner().as_inner().st_uid as u64
    |                                    ~~~~~~
```

Full CI log: https://github.com/nix-rust/nix/actions/runs/12033180710/job/33546728266?pr=2544
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-ci 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.

5 participants