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

Suggest removing atty dependency #22

Merged
merged 3 commits into from
Jul 22, 2024

Conversation

EddieWhi
Copy link
Contributor

@EddieWhi EddieWhi commented Jul 18, 2024

atty is unmaintained and has outstanding sec advisory RUSTSEC-2021-0145.

softprops/atty#50

As removal of the dependency is simple, this seems like the obvious solution.

Copied from the atty source here:
https://github.com/softprops/atty/blob/6633c0e1446aa19e6cd00e00e39770da43081bda/src/lib.rs#L40

This should be the exact equivalent but using libc directly.

Hope you don't mind the surprise PR @softprops... this just came up on cargo deny on an internal repo

atty is unmaintained and has outstanding sec advisory
[RUSTSEC-2021-0145](https://rustsec.org/advisories/RUSTSEC-2021-0145).

As removal of the dependency is simple, this seems like the obvious
solution.

Copied from the atty source here:
https://github.com/softprops/atty/blob/6633c0e1446aa19e6cd00e00e39770da43081bda/src/lib.rs#L40

This should be the exact equivilent but using libc direcly.
@softprops
Copy link
Owner

@EddieWhi no worries. I carry the blame on that one but I think there's actually a better way to address this.

atty is technically provides no additional value over use std::io::IsTerminal introduced in rust 1.7.0.

What are your thoughts on using std::io::stdout().is_terminal() to remove the atty dependency? I think this should be roughly equiv and let's the std lib to do the portability work for us.

@EddieWhi
Copy link
Contributor Author

TIL about IsTerminal, thank you 👍

Turns out it is not just roughly equivalent, it's identical: https://docs.rs/is-terminal/latest/src/is_terminal/lib.rs.html#90

Arguably, there was something pleasant about the symmetry of:

    unsafe { isatty(STDOUT_FILENO...
...
    let r = unsafe { ioctl(STDOUT_FILENO...

but really can't turn down a std lib function :)

Updated and suggest adding min supported rust-version to Cargo.toml, WDYT?

@softprops softprops merged commit dfc2e0c into softprops:master Jul 22, 2024
6 checks passed
@softprops
Copy link
Owner

softprops commented Jul 22, 2024

thanks @EddieWhi this should be up on crates.io now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants