[PATCH] D31550: Let the OS take care of the current working directory

[Rafael Espíndola via llvm-commits]

On 31 March 2017 at 20:36, Reid Kleckner via Phabricator
<reviews at reviews.llvm.org> wrote:
> rnk added a comment.
>
> chdir isn't thread safe, and lit still supports running the tests with threads.

Oh, I missed this. I thought we were always using multiprocessing these days.

> It looks like there are portability issues with requiring multiprocessing everywhere, so I think this change would require conditionally launching a child python process to implement the internal shell. That also kind of sucks, though, since python process startup isn't fast.

Is the multiprocess module still broken on those platforms? I just
tried deleting all the logic around

-        if jobs != 1 and use_processes and multiprocessing:
-            try:

so that lit always uses multiprocess and all tests pass on freebsd.

Even if we can't use the multiprocess module, we could still use a
process pool, no? Tests using subprocess would still fork a new
python, but that is not the common case.

> I think you're right, if we want to implement subshells and ulimit, we need to create a process and manipulate its environment, cwd, etc. What we have stops just short of that, since I've basically assumed that we will never implement shell variable expansion or subshells. I'd be happy if lit could do more basic environment variable manipulation (export), but subshells felt like a bridge too far. I figured tests that needed this stuff could call bash directly and have a `REQUIRES: bash` line or something.

There is REQUIRES: shell already. We would like to at least be able to
run the posix tests on windows. I was able to do it, except for the
use of ulimit in
compiler-rt/test/asan/TestCases/Posix/deep_call_stack.cc, which is ok
for us since we don't have an direct ulimit equivalent and just have
to take our chances with stuck bots.

> Do you think we can reasonably implement variable expansion? The last time I looked at it, it seemed too hard, and I worked around the problem by adding more lit substitutions. Maybe it would be OK if we just implemented the ${} expansion syntax?

What were the problems with it? I tried implementing export and it
seems to work, but it didn't go very far since it hit "export PATH=
...:$PATH".

So, how about:

* Put this change on hold.
* Try using the multiprocess module everywhere to see if it is still broken.
* Remove some uses of subprocesses.
* Implement export with the current setup.
* Try to implement variable expansion with the current setup.
* Come back to this revision if we find out we can use multiple process.

Cheers,
Rafael