Fix target-specific feature flags for rustix dependency

[decathorpe]

The "rustix::process" module is only imported for cfg(target_os = "linux") in Rust code, but the "process" feature of rustix was only enabled if the target_os was not "linux", so the logic in Cargo.toml was reversed from what is actually needed.

This PR changes the target-specific dependency for rustix to only enable the "process" feature if the target_os is "linux", but not otherwise, which seems to have been the intended configuration all along.

[taiki-e]

I thought that separating Linux from the others here was actually useless (#94 (comment)), but is there a case where this is not the case?

[decathorpe]

Yes, it makes a difference if you update to async-process 2.3.1 without also updating to async-signal 0.2.11 (for whatever reason). So either this change or bumping the async-signal dependency to 0.2.11 seems to be necessary here (but the logic in Cargo.toml is still wrong, and should reflect actual code usage IMO) :)

[taiki-e]

Yes, it makes a difference if you update to async-process 2.3.1 without also updating to async-signal 0.2.11 (for whatever reason). So either this change or bumping the async-signal dependency to 0.2.11 seems to be necessary here (but the logic in Cargo.toml is still wrong, and should reflect actual code usage IMO) :)

Does that mean that it will not just compile with the process feature unnecessarily enabled in Linux, but that it will cause some concrete issues? If so, can you give us some information on that issue?

(For example, the reason #94 was initially failing was actually a bug in dependabot that could not properly update dependencies specified in this way (as well as those specified in this PR's way, AFAIK).)

should reflect actual code usage IMO

Personally I prefer to always enable the process feature. (I agree that we should change the current one anyway.)

This is because even if we use process feature without realizing that our Cargo.toml does not have that feature enabled, the issue will likely not be discovered until the async-signal is updated to rustix 2.0.

[zeenix]

Does that mean that it will not just compile with the process feature unnecessarily enabled in Linux, but that it will cause some concrete issues? If so, can you give us some information on that issue?

zbus updating to 2.3.1 fails to build with:

error[E0432]: unresolved import `rustix::process`
   --> /home/zeenix/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/async-process-2.3.1/src/reaper/wait.rs:137:13
    |
137 |         use rustix::process;
    |             ^^^^^^^^^^^^^^^ no `process` in the root
    |
note: found an item that was configured out
   --> /home/zeenix/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/rustix-1.0.7/src/lib.rs:268:9
    |
268 | pub mod process;
    |         ^^^^^^^
note: the item is gated behind the `process` feature
   --> /home/zeenix/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/rustix-1.0.7/src/lib.rs:266:7
    |
266 | #[cfg(feature = "process")]
    |       ^^^^^^^^^^^^^^^^^^^
help: consider importing one of these modules instead
    |
137 -         use rustix::process;
137 +         use std::os::linux::process;
    |
137 -         use rustix::process;
137 +         use std::os::unix::process;
    |
137 -         use rustix::process;
137 +         use std::process;
    |

For more information about this error, try `rustc --explain E0432`.

[zeenix]

LGTM actually.

[zeenix]

LGTM actually.

but will not merge until @taiki-e is satisfied too. :)

[taiki-e]

zbus updating to 2.3.1 fails to build with:

Thanks for the info! I missed the fact that this was a problem that was already happening today, but this seems to be exactly the problem I mentioned in #96 (comment):

Personally I prefer to always enable the process feature.

This is because even if we use process feature without realizing that our Cargo.toml does not have that feature enabled, the issue will likely not be discovered until the async-signal is updated to rustix 2.0.

I think the robust solution here is to always enable the feature that the dependency enables (process feature, in this case), as mentioned above.

(I don't see the need to retain the current approach that doing complex things with hard-to-detect-bug way in order to optimize compile time for the case of there are multiple dependency versions.)

[decathorpe]

Unconditionally depending on "rustix/process" if cfg(unix) would be fine with me too. Should I amend this PR for this change or do you want to push this change yourself?

[zeenix]

I think the robust solution here is to always enable the feature that the dependency enables (process feature, in this case), as mentioned above.

True. Sorry, I was a bit distracted earlier. You're right.