uclibc/mips: fixed SA_* mismatched types

[cppcoffee]

use ulong type uniformly.

[rustbot]

r? @JohnTitor

(rustbot has picked a reviewer for you, use r? to override)

[cppcoffee]

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.

[JohnTitor]

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

[cppcoffee]

Hi @JohnTitor , deprecated attribute has been added, please review it.

[JohnTitor]

Please do not change their types, also it'd be great if could squash commits into one once it's done!

[cppcoffee]

Hi @JohnTitor , Does that mean adding only the deprecated attribute?

[JohnTitor]

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.

[cppcoffee]

Hi @JohnTitor , Add deprecated attribute only, please review.

[cppcoffee]

Hi @JohnTitor , change label status to S-waiting-to-review please.

[JohnTitor]

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]

OK, I'll take care of it when I can rebase it.

[tgross35]

@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 #3887 to make sure we don't forget this.

@rustbot label +stable-nominated

[tgross35]

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]

PR rebase done.

[tgross35]

@cppcoffee thank you for getting this up to date. Unfortunately after some digging, it seems like this wouldn't be correct - see #3887 (comment).

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.

[cppcoffee]

Oh, it looks like nix is using the wrong type.

[cppcoffee]

Hi @tgross35, Use the c_uint type for SA_*, changed now.

[tgross35]

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.

[tgross35]

@cppcoffee could you rebase and squash as mentioned above?

[cppcoffee]

rebase and squash done.

[tgross35]

Thanks! There are some CI issues but it's not related to your changes.