Build progressbar interleaved with rustc errors

[bjorn3]

[...]
error[E0308]: mismatched types
   --> /Users/bjorn/.cargo/git/checkouts/miri-f9dcdd0c0ec69fb1/9143a69/src/fn_call.rs:202:83
    |
202 |                     let ptr = self.memory.allocate(Size::from_bytes(size), align, Some(MemoryKind::C.into()))?;
    |                                                                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `rustc_mir::interpret::MemoryKind`, found enum `std::option::Option`
    |
    = note: expected type `rustc_mir::interpret::MemoryKind<memory::MemoryKind>`
               found type `std::option::Option<_>`

error[E0308]: mismatched types===============================>       ] 122/137: serde, hyper, serde_derive, miri, ring                               
   --> /Users/bjorn/.cargo/git/checkouts/miri-f9dcdd0c0ec69fb1/9143a69/src/fn_call.rs:398:25
    |
398 |                         Some(MemoryKind::Env.into()),
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `rustc_mir::interpret::MemoryKind`, found enum `std::option::Option`
    |
    = note: expected type `rustc_mir::interpret::MemoryKind<memory::MemoryKind>`
               found type `std::option::Option<_>`

error[E0308]: mismatched types===============================>       ] 122/137: serde, hyper, serde_derive, miri, ring                               
   --> /Users/bjorn/.cargo/git/checkouts/miri-f9dcdd0c0ec69fb1/9143a69/src/fn_call.rs:659:48
    |
659 |                                                Some(MemoryKind::Rust.into()))?;
    |                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `rustc_mir::interpret::MemoryKind`, found enum `std::option::Option`
    |
    = note: expected type `rustc_mir::interpret::MemoryKind<memory::MemoryKind>`
               found type `std::option::Option<_>`

error[E0308]: mismatched types================================>      ] 123/137: serde, hyper, serde_derive, miri, ring                               
   --> /Users/bjorn/.cargo/git/checkouts/miri-f9dcdd0c0ec69fb1/9143a69/src/fn_call.rs:673:48
    |
673 |                                                Some(MemoryKind::Rust.into()))?;
    |                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `rustc_mir::interpret::MemoryKind`, found enum `std::option::Option`
    |
    = note: expected type `rustc_mir::interpret::MemoryKind<memory::MemoryKind>`
               found type `std::option::Option<_>`

   Compiling toml v0.4.6                                                                                                                             
   Compiling bincode v1.0.1                                                                                                                          
error: aborting due to 9 previous errors======================>      ] 124/137: hyper, serde_derive, miri, ring, toml, bincode 

[alexcrichton]

Thanks for the report!

cc @kennytm, I had forgotten about this interaction :(

I'm not sure how to solve this without capturing rustc's errors which we don't currently have the ability to do without losing formatting :(

[Eh2406]

Silly question, what prevents multiple rustc invocations from having their output mix? Perhaps we can join in that system.

[ehuss]

@Eh2406 it depends on the platform. On Windows there is a global lock between rustc processes. On other platforms, the only coordination is to rely on the operating system to flush data in atomic blocks. In POSIX this is typically PIPE_BUF in size. On linux this is 4k which is pretty safe for most rust messages. On macos it is 512 bytes, which can cause problems on some large messages.

[alexcrichton]

It's true though! We might be able to create a protocol between Cargo and rustc about how errors are coordinated and that could buy Cargo more flexibility in this regard

[euclio]

I seem to remember a discussion about extracting rustc's error formatting machinery into a reusable crate (though I can't find that discussion, sorry!). If that happened, would using rustc's JSON output be enough, as cargo could use that crate to recreate the errors?

[alexcrichton]

Indeed! If that happened we could safely capture rustc's output and render it to the console in Cargo as rustc would.

[ishitatsuyuki]

Regarding error buffering, I don't think we need to parse rustc's JSON output? I think my idea here is a more generic way to achieve this.

[kennytm]

@ishitatsuyuki Cargo currently needs to know the terminal size for the progress bar. This is done by ioctl(STDERR_FILENO, TIOCGWINSZ) on Unix and GetConsoleScreenBufferInfo(GetStdHandle(STD_ERROR_HANDLE)) on Windows. While rustc currently doesn't need to know the terminal size, I cannot deny the possibility that it might be used in the future. Do you know if it is possible to transmit these info to the child process while piping/teeing their stdout/stderr?

[ishitatsuyuki]

@kennytm I believe that the solution here is to buffer all rustc output until the invocation ends, and rustc should never be interactive or display a progress indicator as it's not a compiler's job but the build tool's.

Though, if we ever need to RPC with rustc, we can change the implementation when we do. I'd say that there's not much motivation for it to be done in the near future, though.

[bjorn3]

[...] buffer all rustc output until the invocation ends [...]

Please don't, That would drastically increase the time before the first errors are printed in some cases.

[ishitatsuyuki]

@bjorn3 To address that, we can stream the output in the case that only one rustc is running. Otherwise, buffering benefits more because you don't want the error contexts to be mixed up anyway.

[kennytm]

I tried (line-)buffering all output of rustc with --color always forwarded. It works fine on Unix and MSYS2 but is totally broken on Windows 7:

It is using ANSI code despite cmd.exe does not support it.

[ishitatsuyuki]

@kennytm Yes, this is expected. As I wrote before, we should parse all ANSI escapes (limiting to colours) and feed them again into termcolor, making it a portable solution.

[vorner]

Just throwing a vague idea in, I'm not sure if that would work or be portable. I think the ncurses (and likely other curses) libraries allow to split the terminal into multiple windows that scroll independently. If the progress bar lived in one and rustcs (and other possible things, liku build scripts) run in the other, they should not interact.

Or how do things like screen or tmux work? Do they create a pty for the child processes and process that?

[ishitatsuyuki]

@vorner What are you suggesting that is not portable? The width detecting progress bar or the buffering approach?

I think tmux creates a pty for each pane indeed.

[vorner]

I didn't say it is not portable. I only said I didn't do any research so I don't know if it is or not. And I was talking about the ncurses windows and PTYs.