rust-lang
uclibc/mips: fixed SA_* mismatched types
The following error occurs when compiling nix crate on the uclibc/mips platform:
error[E0308]: mismatched types
--> /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/nix-0.24.3/src/macros.rs:70:35
|
70 | const $Flag = libc::$Flag $(as $cast)*;
| ^^^^^^^^^^^ expected `u32`, found `i32`
|
::: /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/nix-0.24.3/src/sys/signal.rs:436:1
|
436 | / libc_bitflags!{
437 | | /// Controls the behavior of a [`SigAction`]
438 | | #[cfg_attr(docsrs, doc(cfg(feature = "signal")))]
439 | | pub struct SaFlags: SaFlags_t {
... |
461 | | }
462 | | }
| |_- in this macro invocation
|
= note: this error originates in the macro `libc_bitflags` (in Nightly builds, run with -Z macro-backtrace for more info)
I found the uclibc/arm definition of SA_* using the ulong type: https://github.com/rust-lang/libc/blob/master/src/unix/linux_like/linux/uclibc/arm/mod.rs#L472-L478
Modified to use ulong type uniformly, can compile nix now.
Please review it.
This is a breaking change and I'd recommend deprecating them first to inform users. Read our policy: https://github.com/rust-lang/libc/blob/master/CONTRIBUTING.md#breaking-change-policy
Please do not change their types, also it'd be great if could squash commits into one once it's done!
Yes, we should inform users that we're going to introduce a breaking change before actually doing so. By this users can prepare that change and we can prevent their code from being broken.
Sorry for the absence! I am considering adding a Cargo feature to cfg breaking changes so that we no longer have to misuse the deprecation attribute. Give me some time to make it. Meanwhile, could you do a rebase instead of merging?
@cppcoffee could you squash this so it drops the merge commits? Looks like this
# Assuming this repo (rust-lang, not your fork) is called `upstream`
git fetch upstream
git rebase -i upstream/main
# This command opens an editor. For everything EXCEPT the first line,
# replace `pick` with `f` or `fixup`.
#
# There might not actually be any lines besides the first here, if not
# just close it.
#
# When you close the editor, it will have combined the commits.
# Resolve conflicts if needed, follow the instructions it gives
# Push to this branch with the new history
git push --force-with-lease
Also a couple different ways to do it here https://stackoverflow.com/questions/5189560/how-do-i-squash-my-last-n-commits-together
I will backport only this PR (to target stable, 0.2) but as soon as it merges, you can create a new PR doing the actual fix (which will be in 1.0 when that happens). I opened https://github.com/rust-lang/libc/issues/3887 to make sure we don't forget this.
@rustbot label +stable-nominated
Hey @cppcoffee, thank you for updating but this needs a rebase and squash - not merge (no merge commits policy). Could you try to follow the instructions from my comment? If that doesn't work then let me know and I can help you figure it out (or worst case I can do it for you, but I think you should be able to get it working).
@cppcoffee thank you for getting this up to date. Unfortunately after some digging, it seems like this wouldn't be correct - see https://github.com/rust-lang/libc/issues/3887#issuecomment-2319939888.
What would be correct is to change all the SA_* constants to be ::c_uint on mips ulibc. And then open an issue/PR to nix to do the same. If this sounds right, could you update this PR to do that?
You can just do the actual change without deprecation notices, it just won't get released until 1.0.
LGTM, but please rebase and squash (and update the title/description).
Comment @rustbot review once that is done so it gets back into my queue.