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

Fix tty detection for msys2's /dev/ptmx #119664

Merged
merged 1 commit into from
Jan 23, 2024
Merged

Conversation

ChrisDenton
Copy link
Contributor

Our "true negative" detection assumes that if at least one std handle is a Windows console then no other handle will be a msys2 tty pipe. This turns out to be a faulty assumption in the case of redirection to /dev/ptmx in an msys2 shell. Maybe this is an msys2 bug but in any case we should try to make it work.

An alternative to this would be to replace the "true negative" detection with an attempt to detect if we're in an msys environment (e.g. by sniffing environment variables) but that seems like it'd be flaky too.

Fixes #119658

@rustbot
Copy link
Collaborator

rustbot commented Jan 6, 2024

r? @thomcc

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

@rustbot rustbot added O-windows Operating system: Windows S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jan 6, 2024
@bors
Copy link
Contributor

bors commented Jan 13, 2024

☔ The latest upstream changes (presumably #117285) made this pull request unmergeable. Please resolve the merge conflicts.

@thomcc
Copy link
Member

thomcc commented Jan 22, 2024

Seems fine to me. r=me after conflicts fixed.

Our "true negative" detection assumes that if at least one std handle is a Windows console then no other handle will be a msys2 tty pipe. This turns out to be a faulty assumption in the case of  `/dev/ptmx`.
@ChrisDenton
Copy link
Contributor Author

@bors r=thomcc rollup

@bors
Copy link
Contributor

bors commented Jan 22, 2024

📌 Commit e74c667 has been approved by thomcc

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 22, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 22, 2024
Fix tty detection for msys2's `/dev/ptmx`

Our "true negative" detection assumes that if at least one std handle is a Windows console then no other handle will be a msys2 tty pipe. This turns out to be a faulty assumption in the case of redirection to  `/dev/ptmx` in an msys2 shell. Maybe this is an msys2 bug but in any case we should try to make it work.

An alternative to this would be to replace the "true negative" detection with an attempt to detect if we're in an msys environment (e.g. by sniffing environment variables) but that seems like it'd be flaky too.

Fixes rust-lang#119658
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 22, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#119664 (Fix tty detection for msys2's `/dev/ptmx`)
 - rust-lang#120104 (never_patterns: Count `!` bindings as diverging)
 - rust-lang#120109 (Move cmath into `sys`)
 - rust-lang#120143 (Consolidate logic around resolving built-in coroutine trait impls)
 - rust-lang#120159 (Track `verbose` and `verbose_internals`)
 - rust-lang#120188 (compiler: update freebsd and netbsd base specs.)
 - rust-lang#120216 (Fix a `trimmed_def_paths` assertion failure.)
 - rust-lang#120220 (Document `Token{Stream,Tree}::Display` more thoroughly.)
 - rust-lang#120233 (Revert stabilization of trait_upcasting feature)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 22, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#119664 (Fix tty detection for msys2's `/dev/ptmx`)
 - rust-lang#120104 (never_patterns: Count `!` bindings as diverging)
 - rust-lang#120109 (Move cmath into `sys`)
 - rust-lang#120143 (Consolidate logic around resolving built-in coroutine trait impls)
 - rust-lang#120159 (Track `verbose` and `verbose_internals`)
 - rust-lang#120216 (Fix a `trimmed_def_paths` assertion failure.)
 - rust-lang#120220 (Document `Token{Stream,Tree}::Display` more thoroughly.)
 - rust-lang#120233 (Revert stabilization of trait_upcasting feature)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 67d0936 into rust-lang:master Jan 23, 2024
11 checks passed
@rustbot rustbot added this to the 1.77.0 milestone Jan 23, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 23, 2024
Rollup merge of rust-lang#119664 - ChrisDenton:mingw-pty, r=thomcc

Fix tty detection for msys2's `/dev/ptmx`

Our "true negative" detection assumes that if at least one std handle is a Windows console then no other handle will be a msys2 tty pipe. This turns out to be a faulty assumption in the case of redirection to  `/dev/ptmx` in an msys2 shell. Maybe this is an msys2 bug but in any case we should try to make it work.

An alternative to this would be to replace the "true negative" detection with an attempt to detect if we're in an msys environment (e.g. by sniffing environment variables) but that seems like it'd be flaky too.

Fixes rust-lang#119658
DianQK pushed a commit to DianQK/rust that referenced this pull request Jan 23, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#119664 (Fix tty detection for msys2's `/dev/ptmx`)
 - rust-lang#120104 (never_patterns: Count `!` bindings as diverging)
 - rust-lang#120109 (Move cmath into `sys`)
 - rust-lang#120143 (Consolidate logic around resolving built-in coroutine trait impls)
 - rust-lang#120159 (Track `verbose` and `verbose_internals`)
 - rust-lang#120216 (Fix a `trimmed_def_paths` assertion failure.)
 - rust-lang#120220 (Document `Token{Stream,Tree}::Display` more thoroughly.)
 - rust-lang#120233 (Revert stabilization of trait_upcasting feature)

r? `@ghost`
`@rustbot` modify labels: rollup
sunfishcode added a commit to sunfishcode/is-terminal that referenced this pull request Feb 9, 2024
Port rust-lang/rust#119664 to is-terminal. The commit message is:

Fix msys2 tty detection for /dev/ptmx

Our "true negative" detection assumes that if at least one std handle is a Windows console then no other handle will be a msys2 tty pipe. This turns out to be a faulty assumption in the case of  `/dev/ptmx`.
sunfishcode added a commit to sunfishcode/is-terminal that referenced this pull request Feb 9, 2024
* Port rust-lang/rust#119664 to is-terminal.

Port rust-lang/rust#119664 to is-terminal. The commit message is:

Fix msys2 tty detection for /dev/ptmx

Our "true negative" detection assumes that if at least one std handle is a Windows console then no other handle will be a msys2 tty pipe. This turns out to be a faulty assumption in the case of  `/dev/ptmx`.

* Fix unused import warnings.
sunfishcode added a commit to sunfishcode/rustix-is-terminal that referenced this pull request Feb 9, 2024
* Port rust-lang/rust#119664 to is-terminal.

Port rust-lang/rust#119664 to is-terminal. The commit message is:

Fix msys2 tty detection for /dev/ptmx

Our "true negative" detection assumes that if at least one std handle is a Windows console then no other handle will be a msys2 tty pipe. This turns out to be a faulty assumption in the case of  `/dev/ptmx`.

* Fix unused import warnings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-windows Operating system: Windows S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

std: /dev/ptmx in MSYS2 is not recognized as a tty in std::io::IsTerminal
4 participants