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: hurd build, stat64.st_fsid was renamed to st_dev #133515

Merged
merged 1 commit into from
Dec 1, 2024

Conversation

SteveLauC
Copy link
Contributor

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:

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

@rustbot
Copy link
Collaborator

rustbot commented Nov 27, 2024

r? @ChrisDenton

rustbot has assigned @ChrisDenton.
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 rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Nov 27, 2024
@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Nov 27, 2024

These commits modify the library/Cargo.lock file. Unintentional changes to library/Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@rust-log-analyzer

This comment has been minimized.

@SteveLauC
Copy link
Contributor Author

Any idea how we can fix the build errors? 🤔 Bumping a dependency to a newer compatible version should not introduce so many errors in my understanding

@ChrisDenton
Copy link
Member

libc did have a bad release recently so I'll cc @tgross35 in case this is being fixed on the libc side

@tgross35
Copy link
Contributor

This was a change to a tier3 target done by the target maintainer, so I think fixing std was the intent. Cc @sthibaul

@ChrisDenton
Copy link
Member

ChrisDenton commented Nov 27, 2024

Right, I'm ok with the hurd change. But look at the CI failures. it looks like rustc_dep_of_std is broken even on tier 1 platforms.

@theemathas
Copy link
Contributor

libc uses #[no_core] when building with the rustc-dep-of-std feature.

The #[derive] attribute is provided by core in the prelude, which I assume is not included when building with #[no_core].

rust-lang/libc#4038 was the PR that added this derive to libc, which presumably broke rustc-dep-of-std.

@tgross35
Copy link
Contributor

tgross35 commented Nov 27, 2024

Oh sorry, I didn't realize there were further failures here. This looks to be from a cleanup that started using the Clone and Copy derives rather than manual implementations. This must only be a problem with rustc-dep-of-std so I don't expect anything else to be broken.

I'll revert that change but I'd prefer not to release for another day since we just bumped the edition.

Separately this is yet another thing that needs to be added to libc's CI.

@theemathas
Copy link
Contributor

Since libc has an extern crate that's named as core, changing

#[derive(Copy, Clone)]

to

#[::core::prelude::v1::derive(::core::marker::Copy, ::core::clone::Clone)]

should probably work.

Playground with similar code

@SteveLauC
Copy link
Contributor Author

Thanks for all the investigations, regarding this PR, I will wait for another libc release, feel free to ping me if you guys need me to do anything 😆

@clubby789 clubby789 added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Nov 27, 2024
@tgross35
Copy link
Contributor

I have a fix in rust-lang/libc#4158, I'll merge that and do a release tomorrow unless anyone has any feedback there.

@SteveLauC
Copy link
Contributor Author

SteveLauC commented Nov 28, 2024

I have a fix in rust-lang/libc#4158

Do you want me to test it here using Rust's CI?

Update: Just realized that will require your branch to be based on the libc-0.2 branch

@tgross35
Copy link
Contributor

You mean just with [patch.crates-io]? That would be great if you don't mind, I created a branch with the backport rust-lang/libc#4167

@SteveLauC
Copy link
Contributor Author

You mean just with [patch.crates-io]? That would be great if you don't mind, I created a branch with the backport rust-lang/libc#4167

Gave it a try in 9d939ba, not sure if I did it in the right way ( not familiar with Rust contribution workflows 😶), building it locally gives me different errors.

@rust-log-analyzer

This comment has been minimized.

@SteveLauC
Copy link
Contributor Author

building it locally gives me different errors.

CI gives the same error I saw locally

@tgross35
Copy link
Contributor

This error looks like something weird with bootstrapping, rustc_const_stable_indirect isn't yet available in all stages. I don't know why this would be the case but I wouldn't worry about it - I was able to reproduce the error within the libc repo and the fix seems to have resolved it, we can just try the real thing tomorrow (but thanks for testing it out!).

@SteveLauC
Copy link
Contributor Author

Get it, then I will discard that testing commit and switch to the next libc release tomorrow

@tgross35
Copy link
Contributor

Released in 0.2.167 https://github.com/rust-lang/libc/releases/tag/0.2.167

@rust-log-analyzer

This comment has been minimized.

@SteveLauC
Copy link
Contributor Author

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

    Checking miniz_oxide v0.7.4
    Checking std_detect v0.1.5 (/checkout/library/stdarch/crates/std_detect)
    Checking object v0.36.5
    Checking addr2line v0.22.0
error: use of deprecated function `libc::_NSGetExecutablePath`: Use the `mach2` crate instead
   --> std/src/sys/pal/unix/os.rs:430:15
    |
430 |         libc::_NSGetExecutablePath(ptr::null_mut(), &mut sz);
    |
    = note: `-D deprecated` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(deprecated)]`


error: use of deprecated function `libc::_NSGetExecutablePath`: Use the `mach2` crate instead
   --> std/src/sys/pal/unix/os.rs:435:25
    |
435 |         let err = libc::_NSGetExecutablePath(v.as_mut_ptr() as *mut i8, &mut sz);

error: could not compile `std` (lib) due to 2 previous errors
Build completed unsuccessfully in 0:01:47
  local time: Fri Nov 29 10:45:55 UTC 2024

mach2 is currently not a dep of std, thoughts on what to do with these warnings?

@tgross35
Copy link
Contributor

tgross35 commented Nov 29, 2024

Just allow the warning, those won't be removed until 1.0 (if at all - I'm not sure an official decision was ever made there, I just added the deprecation warning to be consistent with what's on libc's main).

Edit: better, use #[expect(deprecated)] so we catch it if they wind up undeprecated

@rustbot rustbot added the O-unix Operating system: Unix-like label Nov 29, 2024
@SteveLauC
Copy link
Contributor Author

Hi @ChrisDenton, this patch should be ready for review now:)

@ChrisDenton
Copy link
Member

Great!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 1, 2024

📌 Commit 43ae473 has been approved by ChrisDenton

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Dec 1, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 1, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#128184 (std: refactor `pthread`-based synchronization)
 - rust-lang#132047 (Robustify and genericize return-type-notation resolution in `resolve_bound_vars`)
 - rust-lang#133515 (fix: hurd build, stat64.st_fsid was renamed to st_dev)
 - rust-lang#133602 (fix: fix codeblocks in `PathBuf` example)
 - rust-lang#133622 (update link to "C++ Exceptions under the hood" blog)
 - rust-lang#133660 (Do not create trait object type if missing associated types)
 - rust-lang#133686 (Add diagnostic item for `std::ops::ControlFlow`)
 - rust-lang#133689 (Fixed typos by changing `happend` to `happened`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ec7caab into rust-lang:master Dec 1, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 1, 2024
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
@SteveLauC SteveLauC deleted the fix/hurd branch December 1, 2024 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-unix Operating system: Unix-like S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants