Avoid prematurely closing file descriptors when redirecting IO

[pyrmont]

Janet raises an error if os/execute is called with the same stream being used to redirect both STDOUT and STDERR:

(def dn1 (os/open "/dev/null" :w))
(def p1 (os/execute ["ls"] :px {:out dn1})) # will not raise an error

(def dn2 (os/open "/dev/null" :w))
(def p2 (os/execute ["ls"] :px {:out dn2 :err dn2})) # will raise an error

This problem currently affects JPM when installing with --silent (e.g. jpm --silent install judge). This PR prevents this error by avoiding the premature closing of file descriptors in os_execute_impl.

Discussion

In 4a40e57, @bakpakin made changes to os_execute_impl to address #881. For the purposes of this PR, the relevant lines are:

janet/src/core/os.c

Lines 1143 to 1163 in c3f4dc0

	if (pipe_in != JANET_HANDLE_NONE) {
	posix_spawn_file_actions_adddup2(&actions, pipe_in, 0);
	posix_spawn_file_actions_addclose(&actions, pipe_in);
	} else if (new_in != JANET_HANDLE_NONE && new_in != 0) {
	posix_spawn_file_actions_adddup2(&actions, new_in, 0);
	posix_spawn_file_actions_addclose(&actions, new_in);
	}
	if (pipe_out != JANET_HANDLE_NONE) {
	posix_spawn_file_actions_adddup2(&actions, pipe_out, 1);
	posix_spawn_file_actions_addclose(&actions, pipe_out);
	} else if (new_out != JANET_HANDLE_NONE && new_out != 1) {
	posix_spawn_file_actions_adddup2(&actions, new_out, 1);
	posix_spawn_file_actions_addclose(&actions, new_out);
	}
	if (pipe_err != JANET_HANDLE_NONE) {
	posix_spawn_file_actions_adddup2(&actions, pipe_err, 2);
	posix_spawn_file_actions_addclose(&actions, pipe_err);
	} else if (new_err != JANET_HANDLE_NONE && new_err != 2) {
	posix_spawn_file_actions_adddup2(&actions, new_err, 2);
	posix_spawn_file_actions_addclose(&actions, new_err);
	}

The problem is that if new_out and new_err are the same file descriptor, calling posix_spawn_file_actions_addclose for new_out before pos_spawn_file_actions_adddup2 for new_err will cause an error when posix_spawn is later called.

Alternatives

Another possible solution to this problem would be to establish a practice of not using the same file descriptor when redirecting IO. That seems to violate the principle of least surprise and so is not the approach taken by this PR.

[pyrmont]

I don't understand why the 'Build and test on Windows' test failed. It didn't fail on my fork (see here).

[pyrmont]

I don't understand why the 'Build and test on Windows' test failed. It didn't fail on my fork (see here).

This was my mistake. The test has been fixed and CI is now passing on all platforms.

[bakpakin]

LGTM