Run Subproc Refactor

[scopatz]

This is a major overhaul of the way subprocs are executed.

The big idea here is that sub processes are now split into three pieces:

• A process specification, which describes the process to be run and may be manipulated prior to execution
• A process object, like Popen or ProcProxy, responsible for running a single command.
• A command pipeline, which holds all specs and procs and in responsible for making sure that all procs in the pipeline read from and cleaned up correctly.

To enable the streaming and iterating features that we desired, a new PopenThread class has been introduced. This adheres to the Popen API and should be the last non-callable proc in the pipeline. This is responsible for running a regular Popen proc on a separate thread, and reading its output in a non-blocking manner (which was tricky!).

This PR also removes the TeePTY class and related psuedo-terminal handling. This PR should thus enable Storing stdout on all platforms now!

Please bang on this as there are things I have probably missed.

[AstraLuma]

I think the timing issue is resolved well enough until we can fix it properly (I think we'll need some platform-specific work, unfortunately).

The bigger concern to me is the reports that some major programs (like vim) are just up and failing.

Breaking <.5% of calls is a problem, but something fixable. Breaking 100% of a major app? That's a blocker and must not make it to a release.

[scopatz]

@astronouth7303 I have put in a fix for vim. Also, I think this is a good time to merge because it is right after a release. So we can bang on this in master before the next release. I agree that we shouldn't break anything major. But on some level it needs to have more exposure than a PR to find some of the harder to reach corner cases.

[ghost]

vim is broken on this branch for me too.

$ old = tty.tcgetattr(sys.stdout)
$ vim
$ new = tty.tcgetattr(sys.stdout)
$ old == new
False

also i noticed that just running top on this branch, xonsh goes up to +35% cpu usage
and i think i don't see that happening on current master.
can someone confirm this please?

yes i think we should bring this in, even though it's a little unusable for me atm and i'll stay in .4.7 until this gets more stable, that's what the release was all about.

[scopatz]

@laerus - can you try vim now? I was only fixing vi before... sorry!

[ghost]

yep that's fixed, that list will grow.

backgrounding jobs is a little broken too.

[scopatz]

yep that's fixed, that list will grow.

Absolutely.

backgrounding jobs is a little broken too.

It is a little broken on master too :)

[scopatz]

Ok, I have added a new file and updated the docs a bit.

[AstraLuma]

"sued"

[scopatz]

dyslexicai not I am.

[AstraLuma]

So what does !() return now?

[scopatz]

Pipelines. I updated the news to mention this.

[ghost]

just !(ls) sometimes leaks output after printing the pipeline object.

[scopatz]

It doesn't though, right, if you adjust your $XONSH_PROC_FREQUENCY... But I should add that to the news

[scopatz]

Done.

[scopatz]

Are there any other objections? I really do think that I have addressed everything that everyone has brought up that isn't platform specific, a performance or testing issue, or itself a refactor of this. If I am missing something, please let me know!

[gforsyth]

I think this is ready to go -- in the interest of letting people get a word in if they need to, I'll hold off on merging this until noon EST

[gforsyth]

Ok, here we go.

[gforsyth]

Thanks, @scopatz !

🎉🎉🎉

[scopatz]

Thanks @gforsyth! All!