Replace term with termcolor

[aloucks]

Fixes #1818 Fixes #2292 CC #1826

[aloucks]

The build is failing due to the output being sent to the real stderr and stdout, which causes some test failures. The currentprocess testing framework sets global state and makes things a little difficult.

I haven't yet determined the least invasive way to fix the tests. Feedback is welcome!

[rbtcollins]

The build is failing due to the output being sent to the real stderr and stdout, which causes some test failures

What sends the output there? If termcolor does it, then termcolor needs to be used in a way that it doesn't do that. stderr and stdout are global state and thus prevent concurrent testing, though there is some horrid capture glue to pretend they aren't in core.

currentprocess lets us fully isolate each test and run an entire CLI style program or components of one, in-process without forking: a massive boost to test efficiency and reliability particularly on Windows.

All it requires is that there is no implicit global state: that things are concurrent instance safe.

[aloucks]

@rbtcollins Is there any interest in this or should I close it?

[rbtcollins]

Sorry, been very busy with life recently; yes I think there is interest. In terms of output - rustup isn't very chatty for users - deliberately, but if you use RUSTUP_DEBUG=1 rustup -v it will output the most it can - which is a couple hundred lines right now. I'm pretty sure in the past we had per-file-debug!() calls, and may again in future too.

[aloucks]

@rbtcollins I've pushed a change to cache the terminal. I can resolve the merge conflicts if we're good to move forward.

[rbtcollins]

I'll try to get to this over the xmas break

[kinnison]

I'll try to get to this over the xmas break

@rbtcollins That sounds fantastic, thank you.

[rbtcollins]

Ok finally got there.

I've fixed the merge conflicts so you don't have to.

There are two issues I see here.

Firstly, this is failing on windows:

test cli::self_update::windows::tests::windows_doesnt_mess_with_a_non_string_path ... FAILED

I'm not sure why.

Secondly, looking at how the caching has been done I need to think about it a bit.

Should we cache as globals, or as state on OSProcess. Probably what you've done is right, but its a thing I need to page in.

[aloucks]

I think the terminal locking needs to be refactored to work closer to how it did before. I think there needs to be something like SharedTerminal::lock() -> SharedTerminalGuard and then have a Write impl for the guard instead (and move all attribute functions to the guard). I'm not sure if this is related to the test failures.

[rbtcollins]

I don't think the test failure is due to the lack of locking: if you look at TestProcess you can see there is a Vec that all writes get stored to, and there is another test default_toolchain_is_stable which succeeds. (Thus my saying I'm not sure why the failure is happening :P)

The locking at the CurrentProcess level was there originally solely to match the stdlibs semantics around stdout/err, so we could pass instances into trait-bound calls that expect that.