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

Migrate from atty to is-terminal #4382

Merged
merged 5 commits into from
Feb 21, 2023
Merged

Migrate from atty to is-terminal #4382

merged 5 commits into from
Feb 21, 2023

Conversation

souzaguilhermea
Copy link
Contributor

Closes #4377
I did this changes and run the whole test suits, it seems that anything broke.
I would also like to point out that is_terminal / IsTerminal is currently on rust nigthly #98070 so maybe in the future would be great to change (again).

@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/misc/timeout is no longer failing!

@tertsdiepraam
Copy link
Member

Nice! Seems like it created some duplicate dependencies. We have 3 ways of fixing that:

  1. Find an earlier version that matches the transitive dependencies that we already use.
  2. Upgrade other packages to match the versions introduced by is-terminal.
  3. Put exceptions for these in deny.toml.

Could you check whether 1 or 2 are viable? And if not put the exceptions in deny.toml?

@souzaguilhermea
Copy link
Contributor Author

Sure, I will check this.

src/uu/cat/src/cat.rs Outdated Show resolved Hide resolved
@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/misc/tee is no longer failing!
Congrats! The gnu test tests/misc/timeout is no longer failing!

@sylvestre
Copy link
Sponsor Contributor

Cargo deny is still unhappy, could you please fix that? thanks

@souzaguilhermea
Copy link
Contributor Author

Sorry for the delay, I'm from Brazil so we are having Carnival here. I will try to fix this today.

@sylvestre
Copy link
Sponsor Contributor

Obrigado. it can wait until the end of the party :)

@souzaguilhermea
Copy link
Contributor Author

Nice! Seems like it created some duplicate dependencies. We have 3 ways of fixing that:

1. Find an earlier version that matches the transitive dependencies that we already use.

2. Upgrade other packages to match the versions introduced by `is-terminal`.

3. Put exceptions for these in `deny.toml`.

Could you check whether 1 or 2 are viable? And if not put the exceptions in deny.toml?

I did check if 1 or 2 are viable and mostly because of windows-sys which is used by is-terminal and also by procfs rolling back releases or try to update to newer releases didn't work well, it always raise duplicate dependencies.

I think that for now this isn't much of a problem because the best option would be to switch to the std version of is_terminal in the future.

@tertsdiepraam
Copy link
Member

tertsdiepraam commented Feb 19, 2023

Alright, that's what we'll have to go with for now. Thanks for checking! We can do a little bit better if we also run cargo update, then there are just 2 duplicate dependencies left: windows-sys and hermit-abi. But maybe we'll do that separately.

Enjoy the festivities!

Cargo.toml Show resolved Hide resolved
deny.toml Outdated Show resolved Hide resolved
@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/misc/timeout is no longer failing!

@tertsdiepraam
Copy link
Member

Alright, we'll reduce the duplicate dependencies separately. Thanks!

@tertsdiepraam tertsdiepraam merged commit 7d7b9eb into uutils:main Feb 21, 2023
@souzaguilhermea souzaguilhermea deleted the migrate_to_is_terminal branch February 21, 2023 23:22
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.

Migrate from atty to is-terminal
3 participants