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

Ctrl-Z returns from Stdin.read() when reading from the console on Windows #38274

Merged
merged 1 commit into from
Dec 27, 2016

Conversation

elahn
Copy link
Contributor

@elahn elahn commented Dec 10, 2016

Fixes #19914.
Fixes read(), read_to_string(), read_to_end(), etc.

r? @alexcrichton

Windows.
Fixes rust-lang#19914.
Fixes read(), read_to_string(), read_to_end(), etc.
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@alexcrichton
Copy link
Member

Thanks for the PR! The associated issue mentions that Python has a different strategy here, so I'm curious if you've got thoughts on that? Is this essentially an equivalent method of learning about ctrl-z? (I'm relatively naive to this feature of Windows, so sorry if it's otherwise obvious!)

@elahn
Copy link
Contributor Author

elahn commented Dec 10, 2016

c::ERROR_OPERATION_ABORTED => return ErrorKind::TimedOut, read_to_end_uninitialized() (sys_common/io.rs), read_to_end() and append_to_string() (io/mod.rs) would return this error and that isn't happening on my system (Windows 10), so their strategy wouldn't work.

I suppose they used it due to some past windows behavior, so this will need to be tested on all the Windows versions Rust supports before release.

On another note, some code may be using read() directly, expecting 0x1A and working around this issue manually. Depending on how they wrote it, this may break their code, which should be mentioned in the release notes.

This now behaves the same on Windows 10 and Linux, as far as I've thought of tests to check it.

@alexcrichton
Copy link
Member

so this will need to be tested on all the Windows versions Rust supports before release.

Hm this is unfortunately pretty unlikely to happen, so it may be good to do some digging ahead of time? Perhaps Python has configured Windows to return ERROR_OPERATION_ABORTED for these sorts of events and because we're not doing that Windows isn't giving us an error?

It's true yeah that we'll want to mention this in the release notes of course though!

@elahn
Copy link
Contributor Author

elahn commented Dec 20, 2016

Tested and working on Windows 10 (64 gnu/msvc), 7 (64 gnu) and XP SP3 (32 msvc).

Since this method works and doesn't require extra syscalls like the Python method, I don't think it's worth spending more time on trying to figure out why they used it and how to get it to work.

This doesn't fix the issue when run from an MSYS2/MinGW terminal, but it didn't work before anyway, so I don't think that's a reason to block. It seems we'll need something like this which I can do as a separate PR when I get to it.

Is there a way to rebuild just stage 2 libstd? I'd be able to work a lot faster if I didn't have to rebuild everything on my incredibly slow laptop.

@alexcrichton
Copy link
Member

@elahn awesome, thanks for that investigation! Sounds like it's good and ready to land to me.

Could you clarify the MSYS/MinGW issue, though? It looks like the post you linked is just detecting whether we're in a MSYS shell, but do we basically need to have a totally separate branch of logic once we detect that?

Also with an up-to-date master you can use ./x.py build --keep-stage 1 to only rebuild stage2. It may put the build into a wonky state as it's not thoroughly tested yet, but that should hopefully help turnaround times!

@elahn
Copy link
Contributor Author

elahn commented Dec 20, 2016

All I know on the MSYS/MinGW issue is from following @BurntSushi's battle with it. It might be just trapping Ctrl-D instead of Ctrl-Z, but there could be an issue with console/pipe detection. My memory's hazy, but I'll know more once I get into it and re-read that issue thread.

Thanks for the tip on --keep-stage 1!

@alexcrichton
Copy link
Member

Ok well sounds like we're basically fixing MSVC here and MinGW will still be on the table? That sounds ok to me, let's see what bors thinks:

@bors: r+

@bors
Copy link
Contributor

bors commented Dec 26, 2016

📌 Commit f9bca00 has been approved by alexcrichton

@elahn
Copy link
Contributor Author

elahn commented Dec 26, 2016

Well, it fixes MSVC and GNU when the resulting executable is not running in a MinGW shell...fixing that is my next task.

@bors
Copy link
Contributor

bors commented Dec 26, 2016

⌛ Testing commit f9bca00 with merge 023ddd0...

@bors
Copy link
Contributor

bors commented Dec 26, 2016

💔 Test failed - status-travis

@alexcrichton
Copy link
Member

@bors
Copy link
Contributor

bors commented Dec 26, 2016

⌛ Testing commit f9bca00 with merge 66739c4...

@bors
Copy link
Contributor

bors commented Dec 26, 2016

💔 Test failed - status-travis

@alexcrichton
Copy link
Member

alexcrichton commented Dec 26, 2016 via email

@bors
Copy link
Contributor

bors commented Dec 26, 2016

⌛ Testing commit f9bca00 with merge 77f7c7a...

bors added a commit that referenced this pull request Dec 26, 2016
Ctrl-Z returns from Stdin.read() when reading from the console on Windows

Fixes #19914.
Fixes read(), read_to_string(), read_to_end(), etc.

r? @alexcrichton
@bors
Copy link
Contributor

bors commented Dec 27, 2016

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 77f7c7a to master...

@bors bors merged commit f9bca00 into rust-lang:master Dec 27, 2016
jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Mar 20, 2017
Version 1.16.0 (2017-03-16)
===========================

Language
--------

* Lifetimes in statics and consts default to `'static`. [RFC 1623]
* [The compiler's `dead_code` lint now accounts for type aliases][38051].
* [Uninhabitable enums (those without any variants) no longer permit wildcard
  match patterns][38069]
* [Clean up semantics of `self` in an import list][38313]
* [`Self` may appear in `impl` headers][38920]
* [`Self` may appear in struct expressions][39282]

Compiler
--------

* [`rustc` now supports `--emit=metadata`, which causes rustc to emit
  a `.rmeta` file containing only crate metadata][38571]. This can be
  used by tools like the Rust Language Service to perform
  metadata-only builds.
* [Levenshtein based typo suggestions now work in most places, while
  previously they worked only for fields and sometimes for local
  variables][38927]. Together with the overhaul of "no
  resolution"/"unexpected resolution" errors (#[38154]) they result in
  large and systematic improvement in resolution diagnostics.
* [Fix `transmute::<T, U>` where `T` requires a bigger alignment than
  `U`][38670]
* [rustc: use -Xlinker when specifying an rpath with ',' in it][38798]
* [`rustc` no longer attempts to provide "consider using an explicit
  lifetime" suggestions][37057]. They were inaccurate.

Stabilized APIs
---------------

* [`VecDeque::truncate`]
* [`VecDeque::resize`]
* [`String::insert_str`]
* [`Duration::checked_add`]
* [`Duration::checked_sub`]
* [`Duration::checked_div`]
* [`Duration::checked_mul`]
* [`str::replacen`]
* [`str::repeat`]
* [`SocketAddr::is_ipv4`]
* [`SocketAddr::is_ipv6`]
* [`IpAddr::is_ipv4`]
* [`IpAddr::is_ipv6`]
* [`Vec::dedup_by`]
* [`Vec::dedup_by_key`]
* [`Result::unwrap_or_default`]
* [`<*const T>::wrapping_offset`]
* [`<*mut T>::wrapping_offset`]
* `CommandExt::creation_flags`
* [`File::set_permissions`]
* [`String::split_off`]

Libraries
---------

* [`[T]::binary_search` and `[T]::binary_search_by_key` now take
  their argument by `Borrow` parameter][37761]
* [All public types in std implement `Debug`][38006]
* [`IpAddr` implements `From<Ipv4Addr>` and `From<Ipv6Addr>`][38327]
* [`Ipv6Addr` implements `From<[u16; 8]>`][38131]
* [Ctrl-Z returns from `Stdin.read()` when reading from the console on
  Windows][38274]
* [std: Fix partial writes in `LineWriter`][38062]
* [std: Clamp max read/write sizes on Unix][38062]
* [Use more specific panic message for `&str` slicing errors][38066]
* [`TcpListener::set_only_v6` is deprecated][38304]. This
  functionality cannot be achieved in std currently.
* [`writeln!`, like `println!`, now accepts a form with no string
  or formatting arguments, to just print a newline][38469]
* [Implement `iter::Sum` and `iter::Product` for `Result`][38580]
* [Reduce the size of static data in `std_unicode::tables`][38781]
* [`char::EscapeDebug`, `EscapeDefault`, `EscapeUnicode`,
  `CaseMappingIter`, `ToLowercase`, `ToUppercase`, implement
  `Display`][38909]
* [`Duration` implements `Sum`][38712]
* [`String` implements `ToSocketAddrs`][39048]

Cargo
-----

* [The `cargo check` command does a type check of a project without
  building it][cargo/3296]
* [crates.io will display CI badges from Travis and AppVeyor, if
  specified in Cargo.toml][cargo/3546]
* [crates.io will display categories listed in Cargo.toml][cargo/3301]
* [Compilation profiles accept integer values for `debug`, in addition
  to `true` and `false`. These are passed to `rustc` as the value to
  `-C debuginfo`][cargo/3534]
* [Implement `cargo --version --verbose`][cargo/3604]
* [All builds now output 'dep-info' build dependencies compatible with
  make and ninja][cargo/3557]
* [Build all workspace members with `build --all`][cargo/3511]
* [Document all workspace members with `doc --all`][cargo/3515]
* [Path deps outside workspace are not members][cargo/3443]

Misc
----

* [`rustdoc` has a `--sysroot` argument that, like `rustc`, specifies
  the path to the Rust implementation][38589]
* [The `armv7-linux-androideabi` target no longer enables NEON
  extensions, per Google's ABI guide][38413]
* [The stock standard library can be compiled for Redox OS][38401]
* [Rust has initial SPARC support][38726]. Tier 3. No builds
  available.
* [Rust has experimental support for Nvidia PTX][38559]. Tier 3. No
  builds available.
* [Fix backtraces on i686-pc-windows-gnu by disabling FPO][39379]

Compatibility Notes
-------------------

* [Uninhabitable enums (those without any variants) no longer permit wildcard
  match patterns][38069]
* In this release, references to uninhabited types can not be
  pattern-matched. This was accidentally allowed in 1.15.
* [The compiler's `dead_code` lint now accounts for type aliases][38051].
* [Ctrl-Z returns from `Stdin.read()` when reading from the console on
  Windows][38274]
* [Clean up semantics of `self` in an import list][38313]

[37057]: rust-lang/rust#37057
[37761]: rust-lang/rust#37761
[38006]: rust-lang/rust#38006
[38051]: rust-lang/rust#38051
[38062]: rust-lang/rust#38062
[38062]: rust-lang/rust#38622
[38066]: rust-lang/rust#38066
[38069]: rust-lang/rust#38069
[38131]: rust-lang/rust#38131
[38154]: rust-lang/rust#38154
[38274]: rust-lang/rust#38274
[38304]: rust-lang/rust#38304
[38313]: rust-lang/rust#38313
[38314]: rust-lang/rust#38314
[38327]: rust-lang/rust#38327
[38401]: rust-lang/rust#38401
[38413]: rust-lang/rust#38413
[38469]: rust-lang/rust#38469
[38559]: rust-lang/rust#38559
[38571]: rust-lang/rust#38571
[38580]: rust-lang/rust#38580
[38589]: rust-lang/rust#38589
[38670]: rust-lang/rust#38670
[38712]: rust-lang/rust#38712
[38726]: rust-lang/rust#38726
[38781]: rust-lang/rust#38781
[38798]: rust-lang/rust#38798
[38909]: rust-lang/rust#38909
[38920]: rust-lang/rust#38920
[38927]: rust-lang/rust#38927
[39048]: rust-lang/rust#39048
[39282]: rust-lang/rust#39282
[39379]: rust-lang/rust#39379
[`<*const T>::wrapping_offset`]: https://doc.rust-lang.org/std/primitive.pointer.html#method.wrapping_offset
[`<*mut T>::wrapping_offset`]: https://doc.rust-lang.org/std/primitive.pointer.html#method.wrapping_offset
[`Duration::checked_add`]: https://doc.rust-lang.org/std/time/struct.Duration.html#method.checked_add
[`Duration::checked_div`]: https://doc.rust-lang.org/std/time/struct.Duration.html#method.checked_div
[`Duration::checked_mul`]: https://doc.rust-lang.org/std/time/struct.Duration.html#method.checked_mul
[`Duration::checked_sub`]: https://doc.rust-lang.org/std/time/struct.Duration.html#method.checked_sub
[`File::set_permissions`]: https://doc.rust-lang.org/std/fs/struct.File.html#method.set_permissions
[`IpAddr::is_ipv4`]: https://doc.rust-lang.org/std/net/enum.IpAddr.html#method.is_ipv4
[`IpAddr::is_ipv6`]: https://doc.rust-lang.org/std/net/enum.IpAddr.html#method.is_ipv6
[`Result::unwrap_or_default`]: https://doc.rust-lang.org/std/result/enum.Result.html#method.unwrap_or_default
[`SocketAddr::is_ipv4`]: https://doc.rust-lang.org/std/net/enum.SocketAddr.html#method.is_ipv4
[`SocketAddr::is_ipv6`]: https://doc.rust-lang.org/std/net/enum.SocketAddr.html#method.is_ipv6
[`String::insert_str`]: https://doc.rust-lang.org/std/string/struct.String.html#method.insert_str
[`String::split_off`]: https://doc.rust-lang.org/std/string/struct.String.html#method.split_off
[`Vec::dedup_by_key`]: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.dedup_by_key
[`Vec::dedup_by`]: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.dedup_by
[`VecDeque::resize`]:  https://doc.rust-lang.org/std/collections/vec_deque/struct.VecDeque.html#method.resize
[`VecDeque::truncate`]: https://doc.rust-lang.org/std/collections/vec_deque/struct.VecDeque.html#method.truncate
[`str::repeat`]: https://doc.rust-lang.org/std/primitive.str.html#method.repeat
[`str::replacen`]: https://doc.rust-lang.org/std/primitive.str.html#method.replacen
[cargo/3296]: rust-lang/cargo#3296
[cargo/3301]: rust-lang/cargo#3301
[cargo/3443]: rust-lang/cargo#3443
[cargo/3511]: rust-lang/cargo#3511
[cargo/3515]: rust-lang/cargo#3515
[cargo/3534]: rust-lang/cargo#3534
[cargo/3546]: rust-lang/cargo#3546
[cargo/3557]: rust-lang/cargo#3557
[cargo/3604]: rust-lang/cargo#3604
[RFC 1623]: https://github.com/rust-lang/rfcs/blob/master/text/1623-static.md
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.

4 participants