Use 'libc' directly instead of through rustix

[SergioBenitez]

As the title suggests, this PR goes from using rustix to check if a stream is a terminal on Unix to using libc directly. The benefit is a decrease in compile-times by ~50% in debug mode and ~35% in release mode, on my machine, as well a decrease in transitive dependencies from 5 to 2. The detriment is an added use of unsafe, albeit a very well understood one.

# with rustix, before
$ cargo build
   Compiling libc v0.2.144
   Compiling io-lifetimes v1.0.11
   Compiling rustix v0.37.19
   Compiling bitflags v1.3.2
   Compiling errno v0.3.1
   Compiling is-terminal v0.4.7
    Finished dev [unoptimized + debuginfo] target(s) in 1.11s

# without rustix, after
$ cargo build
   Compiling libc v0.2.144
   Compiling io-lifetimes v1.0.11
   Compiling is-terminal v0.4.7
    Finished dev [unoptimized + debuginfo] target(s) in 0.63s

[sunfishcode]

This PR removes no_std support, which is used by some users.

Fortunately though, Rust 1.70 will be released in a week with the IsTerminal trait in the standard library, so you'll soon be able to have zero dependencies.

[SergioBenitez]

This PR removes no_std support, which is used by some users.

So as far as I could track it, using rustix would result in invoking the exact same functions from std. Doesn't that imply that the no_std wasn't actually doing anything, since rustix pulled it in anyway? Or is there something I'm missing?

Fortunately though, Rust 1.70 will be released in a week with the IsTerminal trait in the standard library, so you'll soon be able to have zero dependencies.

Yeah! This is awesome! But I'd still love to be able to support older toolchains with fewer dependencies.

[SergioBenitez]

@sunfishcode Checking in.

[sunfishcode]

Thanks. I agree we should do this. I'd like to figure out the no_std situation though. I expect it can be fixed using libc, as the libc crate is no_std.

[SergioBenitez]

The io-lifetimes crate isn't no_std, however:

https://github.com/sunfishcode/io-lifetimes/blob/fd37c31a03d93419553d50a9d970d5ccb637b3b6/src/lib.rs#L47-L57

#[cfg(target_os = "hermit")]
pub use std::os::hermit::io::{AsFd, BorrowedFd, OwnedFd};
#[cfg(unix)]
pub use std::os::unix::io::{AsFd, BorrowedFd, OwnedFd};
#[cfg(target_os = "wasi")]
pub use std::os::wasi::io::{AsFd, BorrowedFd, OwnedFd};
#[cfg(windows)]
pub use std::os::windows::io::{
    AsHandle, AsSocket, BorrowedHandle, BorrowedSocket, HandleOrInvalid, InvalidHandleError,
    NullHandleError, OwnedHandle, OwnedSocket,
};

That would need to be addressed first.

In all, I don't think this crate was ever really no_std. Merging this PR should be okay because as far as I can tell, either this crate didn't compile on a platform because that platform was none of hermit, unix, wasi, or windows, or it did but pulled in std.

[SergioBenitez]

Ah, apparently the io-lifetimes dep was dropped already. Great!

But, I still think the library actually uses std for every platform it supports. As far as I can tell, we have:

• windows (use std::os::windows)
• unix, wasi (use std::os::fd via rustix, now directly)
• hermit (use std::os::hermit)
• unknown (impl IsTerminal for std::io::*)

In any case, I'll push an up-to-date version that does what the crate was previously doing.

[faern]

rustix does not compile for ESP32 targets (riscv32imc-esp-espidf). This is currently stopping me from using any dependency that relies on is-terminal (for example env_logger now when they switched from atty).

error[E0425]: cannot find value `RTLD_DEFAULT` in crate `libc`
   --> /home/faern/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustix-0.38.3/src/weak.rs:143:23
    |
143 |     libc::dlsym(libc::RTLD_DEFAULT, name.as_ptr().cast())
    |                       ^^^^^^^^^^^^ not found in `libc`

error[E0425]: cannot find value `EADV` in module `c`
  --> /home/faern/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustix-0.38.3/src/backend/libc/io/errno.rs:34:35
   |
34 |     pub const ADV: Self = Self(c::EADV);
   |                                   ^^^^ not found in `c`
....

This PR makes is-terminal build for riscv32imc-esp-espidf again, so I would love to see it merged.

[sunfishcode]

@faern I just released rustix 0.38.4 which fixes the build errors on riscv32imc-esp-espidf.

Also, I've now posted #31 which makes is-terminal use libc by default, but adds a cargo feature to use rustix.

[SergioBenitez]

Still hoping we can get this through.

[kayabaNerve]

Having read the above, I don't see any actual reason this shouldn't be merged. #31 does have reasons it shouldn't be however. Am I missing something?

[sunfishcode]

@SergioBenitez I now think this PR makes sense. Would you mind rebasing it to fix the merge conflict?

[SergioBenitez]

Sure, will do.

[SergioBenitez]

Done.

[SergioBenitez]

@sunfishcode Can we get this merged and released?

[sunfishcode]

This is now released in is-terminal 0.4.11.