Remove the use of process substitution in wine-msvc wrapper

[huangqinjin]

Some tools may redirect stdout/stderr to files and wait for wine-msvc to exit, and then immediately start to process those files. But subprocesses may be still processing data after wine-msvc exits.

Waiting for subprocesses to finish before exiting wine-msvc ensures all data are written to files and the tools get complete data.

Waiting for subprocesses without msvctricks.exe will always hang if wine is not launched once before.

ccache is such a tool that waiting for wine-msvc to exit. I observed that ccache got truncated data when the outputs of /showIncludes are large.

CMake waits for stdout/stderr reaching EOF, so the issue does not happen for CMake.

Edit: Waiting for all process substitution is only available since Bash 5.1. So this commit simply removed the use of process substitution and only relies on FIFO.

[mstorsjo]

Can you explain how this works? Does wait in bash wait for all potential grandchild processes too?

https://www.gnu.org/software/bash/manual/bash.html#index-wait

If no arguments are given, wait waits for all running background jobs

This feels a bit vague - does this mean all background jobs that bash itself has launched, or all grandchild processes that might have been launched?

But I guess if this works for you as expected, then this does wait for all potential grandchild processes, somehow.

Can you give more context about in which concrete cases this issue happens? I guess one potential case is when e.g. cl.exe launches link.exe, but in that case, I wouldn't expect cl.exe to return until link.exe also has finished?

[huangqinjin]

The problem is caused by process substitution launched by wine-msvc.sh.

A simple example to demostrate the problem:

$ sh -c "echo >(sleep 1 && echo hello)" > a.tmp && cat a.tmp && echo "after 1s" && sleep 1 && cat a.tmp
/dev/fd/63
after 1s
/dev/fd/63
hello

The two cat a.tmp have different output. And with wait for process substitution:

$ sh -c "echo >(sleep 1 && echo hello); wait" > a.tmp && cat a.tmp
/dev/fd/63
hello

Back to the case of ccache. It runs equivalently cl /showIncludes 2> a.stderr >a.stdout and wait for cl to exit and then immediately processes a.stdout and a.stderr. The sed process substitution inside cl may be still processing after cl exits. So cl must wait for sed to finish before itself exits.

[huangqinjin]

Regarding the wait command,

If no arguments are given, wait waits for all running background jobs and the last-executed process substitution, if its process id is the same as $!

It only waits for background jobs that the script itself has launched, not including grandchildren. But does wait also wait for process substitution? I tested with different versions of bash.

• Below Bash 5.0, NO.

  $ docker run -it --rm bash:4.4 bash -c "echo >(sleep 10 && echo 10); echo >(sleep 5 && echo 5); wait"
  /dev/fd/63
  /dev/fd/63

• Bash 5.0, only wait for the last-executed process substitution. Same as the referenced document.

  $ docker run -it --rm bash:4.4 bash -c "echo >(sleep 10 && echo 10); echo >(sleep 5 && echo 5); wait"
  /dev/fd/63
  /dev/fd/63
  5

• Bash 5.1+, wait for all process substitution.

  $ docker run -it --rm bash:5.1 bash -c "echo >(sleep 10 && echo 10); echo >(sleep 5 && echo 5); wait"
  /dev/fd/63
  /dev/fd/63
  5
  10

Based on the tests, I would like to remove the elif [ -d /proc/$$/fd ] branch totally and only make use of FIFO. I used to prefer process substitution as it does not require to create tmp files explictly. @mstorsjo what is your opinion?

[mstorsjo]

Based on the tests, I would like to remove the elif [ -d /proc/$$/fd ] branch totally and only make use of FIFO. I used to prefer process substitution as it does not require to create tmp files explictly. @mstorsjo what is your opinion?

That sounds reasonable. The script is already somewhat complex, and the process substitution codepath, while neat, is quite hard to understand in all the minute details. So if we both make the code simpler to understand, and avoid this whole issue, that sounds good to me.

[huangqinjin]

Reworked PR and modified title and commit message.

[mstorsjo]

LGTM, thanks!