17 files changed, 248 insertions, 154 deletions

diff --git a/nihil.posix/CMakeLists.txt b/nihil.posix/CMakeLists.txt
index 62f6aaf..b04018e 100644
--- a/nihil.posix/CMakeLists.txt
+++ b/nihil.posix/CMakeLists.txt
@@ -11,12 +11,16 @@ target_sources(nihil.posix
 	argv.ccm
 	ensure_dir.ccm
 	exec.ccm
+	execlp.ccm
 	execv.ccm
+	execvp.ccm
 	executor.ccm
 	fd.ccm
+	fexecv.ccm
 	find_in_path.ccm
 	getenv.ccm
 	open.ccm
+	open_in_path.ccm
 	process.ccm
 	read_file.ccm
 	rename.ccm
@@ -29,10 +33,12 @@ target_sources(nihil.posix
 	ensure_dir.cc
 	exec.cc
 	execv.cc
+	execvp.cc
 	getenv.cc
 	fd.cc
 	find_in_path.cc
 	open.cc
+	open_in_path.cc
 	process.cc
 	rename.cc
 	tempfile.cc
@@ -53,8 +59,6 @@ if(NIHIL_TESTS)
 		Catch2::Catch2WithMain
 	)
 
-	find_package(Catch2 REQUIRED)
-
 	include(CTest)
 	include(Catch)
 	catch_discover_tests(nihil.posix.test)
diff --git a/nihil.posix/exec.cc b/nihil.posix/exec.cc
index 5bdcbf7..b4e8732 100644
--- a/nihil.posix/exec.cc
+++ b/nihil.posix/exec.cc
@@ -4,17 +4,8 @@
 
 module;
 
-#include <coroutine>
 #include <expected>
 #include <format>
-#include <string>
-#include <utility>
-
-#include <err.h>
-#include <fcntl.h>
-#include <unistd.h>
-
-extern char **environ;
 
 module nihil.posix;
 
@@ -23,47 +14,7 @@ import nihil.monad;
 
 namespace nihil {
 
-fexecv::fexecv(fd &&execfd, argv &&args) noexcept
-	: m_execfd(std::move(execfd))
-	, m_args(std::move(args))
-{
-}
-
-auto fexecv::exec(this fexecv &self)
-	-> std::expected<void, error>
-{
-	::fexecve(self.m_execfd.get(), self.m_args.data(), environ);
-	return std::unexpected(error("fexecve failed",
-				     error(std::errc(errno))));
-}
-	
-fexecv::fexecv(fexecv &&) noexcept = default;
-auto fexecv::operator=(this fexecv &, fexecv &&) noexcept -> fexecv& = default;
-	
-auto execv(std::string_view path, argv &&argv)
-	-> std::expected<fexecv, error>
-{
-	auto file = co_await open(path, O_EXEC)
-		.transform_error([&] (error cause) {
-			return error(std::format("could not open {}", path),
-				     std::move(cause));
-		});
-
-	co_return fexecv(std::move(file), std::move(argv));
-}
-
-auto execvp(std::string_view file, argv &&argv)
-	-> std::expected<fexecv, error>
-{
-	auto execfd = find_in_path(file);
-	if (!execfd)
-		return std::unexpected(error(
-			std::format("executable not found in path: {}", file)));
-	return fexecv(std::move(*execfd), std::move(argv));
-}
-
-auto shell(std::string_view const &command)
-	-> std::expected<fexecv, error>
+auto shell(std::string_view const &command) -> std::expected<base_executor_type, error>
 {
 	return execl("/bin/sh", "sh", "-c", command);
 }
diff --git a/nihil.posix/exec.ccm b/nihil.posix/exec.ccm
index 6098318..cd03117 100644
--- a/nihil.posix/exec.ccm
+++ b/nihil.posix/exec.ccm
@@ -11,95 +11,43 @@ module;
 #include <expected>
 #include <string>
 
+#include "nihil.hh"
+
 export module nihil.posix:exec;
 
 import nihil.error;
+import :execv;
+import :fexecv;
 import :argv;
 import :fd;
 
 namespace nihil {
 
 /*
- * A concept to mark spawn executors.
- */
-
-export struct exec_tag{};
-
-export template<typename T>
-concept executor =
-	requires (T e) {
-		std::same_as<exec_tag, typename std::remove_cvref_t<T>::tag>;
-		{ e.exec() };
-	};
-
-/*
- * fexecv: use a file descriptor and an argument vector to call ::fexecve().
- * This is the lowest-level executor which all others are implemented
- * in terms of.
- *
- * TODO: Should have a way to pass the environment (envp).
- */
-export struct fexecv final {
-	using tag = exec_tag;
-
-	fexecv(fd &&execfd, argv &&args) noexcept;
-
-	[[nodiscard]] auto exec(this fexecv &self) 
-		-> std::expected<void, error>;
-
-	// Movable
-	fexecv(fexecv &&) noexcept;
-	auto operator=(this fexecv &, fexecv &&) noexcept -> fexecv&;
-	
-	// Not copyable (because we hold the open fd object)
-	fexecv(fexecv const &) = delete;
-	auto operator=(this fexecv &, fexecv const &) -> fexecv& = delete;
-
-private:
-	fd	m_execfd;
-	argv	m_args;
-};
-
-/*
- * execv: equivalent to fexecv(), except the command is passed as
- * a pathname instead of a file descriptor.  Does not search $PATH.
- */
-export [[nodiscard]] auto execv(std::string_view path, argv &&argv)
-	-> std::expected<fexecv, error>;
-
-/*
- * execvp: equivalent to fexecv(), except the command is passed as
- * a filename instead of a file descriptor.  If the filename is not
- * an absolute path, it will be searched for in $PATH.
+ * The lowest-level executor type, which is returned by other executors.
+ * Prefer fexecve() if it's available, otherwise use execve().
  */
-export [[nodiscard]] auto execvp(std::string_view file, argv &&argv)
-	-> std::expected<fexecv, error>;
+#ifdef NIHIL_HAVE_FEXECVE
+using base_executor_type = fexecv;
+#else
+using base_executor_type = execv;
+#endif
 
 /*
  * execl: equivalent to execv, except the arguments are passed as a
  * variadic pack of string-like objects.
  */
 export [[nodiscard]] auto execl(std::string_view path, auto &&...args)
-	-> std::expected<fexecv, error>
+	-> std::expected<base_executor_type, error>
 {
 	return execv(path, argv({std::string_view(args)...}));
 }
 
 /*
- * execlp: equivalent to execvp, except the arguments are passed as a
- * variadic pack of string-like objects.
- */
-export [[nodiscard]] auto execlp(std::string_view file, auto &&...args)
-	-> std::expected<fexecv, error>
-{
-	return execvp(file, argv({std::string_view(args)...}));
-}
-
-/*
  * shell: run the process by invoking /bin/sh -c with the single argument,
  * equivalent to system(3).
  */
 export [[nodiscard]] auto shell(std::string_view const &command)
-	-> std::expected<fexecv, error>;
+	-> std::expected<base_executor_type, error>;
 
 } // namespace nihil
diff --git a/nihil.posix/execlp.ccm b/nihil.posix/execlp.ccm
new file mode 100644
index 0000000..d0d88d5
--- /dev/null
+++ b/nihil.posix/execlp.ccm
@@ -0,0 +1,30 @@
+/*
+ * This source code is released into the public domain.
+ */
+
+module;
+
+#include <string>
+#include <expected>
+#include <format>
+
+export module nihil.posix:execlp;
+
+import nihil.error;
+import :exec;
+import :argv;
+import :execvp;
+
+namespace nihil {
+
+/*
+ * execlp: equivalent to execvp, except the arguments are passed as a
+ * variadic pack of string-like objects.
+ */
+export [[nodiscard]] auto execlp(std::string_view file, auto &&...args)
+	-> std::expected<base_executor_type, error>
+{
+	return execvp(file, argv({std::string_view(args)...}));
+}
+
+} // namespace nihil
diff --git a/nihil.posix/execv.cc b/nihil.posix/execv.cc
index 63f9698..752a96d 100644
--- a/nihil.posix/execv.cc
+++ b/nihil.posix/execv.cc
@@ -6,6 +6,7 @@ module;
 
 #include <coroutine>
 #include <expected>
+#include <filesystem>
 #include <format>
 #include <string>
 #include <utility>
@@ -36,8 +37,6 @@ auto execv::exec(this execv &self) -> std::expected<void, error>
 }
 
 execv::execv(execv &&) noexcept = default;
-execv::execv(execv const &) = default;
-auto execv::operator=(this execv &, execv &&) -> execv & = default;
-auto execv::operator=(this execv &, execv const &) -> execv & = default;
+auto execv::operator=(this execv &, execv &&) noexcept -> execv & = default;
 
 } // namespace nihil
diff --git a/nihil.posix/execv.ccm b/nihil.posix/execv.ccm
index ca036a1..d97754e 100644
--- a/nihil.posix/execv.ccm
+++ b/nihil.posix/execv.ccm
@@ -35,9 +35,9 @@ export struct execv final
 	execv(execv &&) noexcept;
 	auto operator=(this execv &, execv &&) noexcept -> execv &;
 
-	// Copyable.
-	execv(execv const &);
-	auto operator=(this execv &, execv const &) -> execv &;
+	// Not copyable (because m_args isn't copyable).
+	execv(execv const &) = delete;
+	auto operator=(this execv &, execv const &) -> execv & = delete;
 
 private:
 	std::filesystem::path	m_path;
diff --git a/nihil.posix/execvp.cc b/nihil.posix/execvp.cc
new file mode 100644
index 0000000..5eac315
--- /dev/null
+++ b/nihil.posix/execvp.cc
@@ -0,0 +1,50 @@
+/*
+ * This source code is released into the public domain.
+ */
+
+module;
+
+#include <coroutine>
+#include <expected>
+#include <filesystem>
+#include <format>
+#include <string>
+#include <utility>
+
+#include <err.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+#include "nihil.hh"
+
+module nihil.posix;
+
+import nihil.error;
+import nihil.monad;
+
+namespace nihil {
+#ifdef NIHIL_HAVE_FEXECVE
+
+auto execvp(std::string_view file, argv &&argv) -> std::expected<fexecv, error>
+{
+	auto execfd = open_in_path(file);
+	if (!execfd)
+		return std::unexpected(error(
+			std::format("executable not found in path: {}", file)));
+	return fexecv(std::move(*execfd), std::move(argv));
+}
+
+#else	// !NIHIL_HAVE_FEXECVE
+
+auto execvp(std::string_view file, nihil::argv &&argv) -> std::expected<nihil::execv, nihil::error>
+{
+	auto filename = nihil::find_in_path(file);
+	if (!filename)
+		return std::unexpected(nihil::error(
+			std::format("executable not found in path: {}", file)));
+	return execv(std::move(*filename), std::move(argv));
+}
+
+#endif	// NIHIL_HAVE_FEXECVE
+
+} // namespace nihil
diff --git a/nihil.posix/execvp.ccm b/nihil.posix/execvp.ccm
new file mode 100644
index 0000000..688ecb6
--- /dev/null
+++ b/nihil.posix/execvp.ccm
@@ -0,0 +1,28 @@
+/*
+ * This source code is released into the public domain.
+ */
+
+module;
+
+#include <string>
+#include <expected>
+#include <format>
+
+export module nihil.posix:execvp;
+
+import nihil.error;
+import :argv;
+import :exec;
+
+namespace nihil {
+
+/*
+ * execvp: equivalent to fexecv(), except the command is passed as
+ * a filename instead of a file descriptor.  If the filename is not
+ * an absolute path, it will be searched for in $PATH.
+ */
+
+export [[nodiscard]] auto execvp(std::string_view file, argv &&argv)
+	-> std::expected<base_executor_type, error>;
+
+} // namespace nihil
diff --git a/nihil.posix/fexecv.cc b/nihil.posix/fexecv.cc
new file mode 100644
index 0000000..ad57d14
--- /dev/null
+++ b/nihil.posix/fexecv.cc
@@ -0,0 +1,47 @@
+/*
+ * This source code is released into the public domain.
+ */
+
+module;
+
+#include <coroutine>
+#include <expected>
+#include <format>
+#include <string>
+#include <utility>
+
+#include <err.h>
+
+#include "nihil.h"
+
+#ifdef NIHIL_HAVE_FEXECVE
+
+extern char **environ;
+
+module nihil.posix;
+
+import nihil.error;
+import nihil.monad;
+
+namespace nihil {
+
+fexecv::fexecv(fd &&execfd, argv &&args) noexcept
+	: m_execfd(std::move(execfd))
+	, m_args(std::move(args))
+{
+}
+
+auto fexecv::exec(this fexecv &self)
+	-> std::expected<void, error>
+{
+	::fexecve(self.m_execfd.get(), self.m_args.data(), environ);
+	return std::unexpected(error("fexecve failed",
+				     error(std::errc(errno))));
+}
+
+fexecv::fexecv(fexecv &&) noexcept = default;
+auto fexecv::operator=(this fexecv &, fexecv &&) noexcept -> fexecv& = default;
+
+} // namespace nihil
+
+#endif	// NIHIL_HAVE_FEXECVE
diff --git a/nihil.posix/fexecv.ccm b/nihil.posix/fexecv.ccm
index 5ad6c62..1fe57a8 100644
--- a/nihil.posix/fexecv.ccm
+++ b/nihil.posix/fexecv.ccm
@@ -9,8 +9,6 @@ module;
 
 #include "nihil.hh"
 
-#ifdef NIHIL_HAVE_FEXECVE
-
 export module nihil.posix:fexecv;
 
 import nihil.error;
@@ -20,6 +18,8 @@ import :fd;
 
 namespace nihil {
 
+#ifdef NIHIL_HAVE_FEXECVE
+
 /*
  * fexecv: use a file descriptor and an argument vector to call ::fexecve().
  * This is the lowest-level executor which all others are implemented
@@ -48,6 +48,6 @@ private:
 	argv	m_args;
 };
 
-} // namespace nihil
+#endif	// NIHIL_HAVE_FEXECVE
 
-#endif	// NIHIL_HAVE_FEXECVE
\ No newline at end of file
+} // namespace nihil
diff --git a/nihil.posix/getenv.cc b/nihil.posix/getenv.cc
index ad93305..c596902 100644
--- a/nihil.posix/getenv.cc
+++ b/nihil.posix/getenv.cc
@@ -44,10 +44,16 @@ auto getenv(std::string_view varname) -> std::expected<std::string, error>
 		return std::unexpected(error(std::errc(errno)));
 	}
 #else // NIHIL_HAVE_GETENV_R
+	errno = 0;
 	auto *v = ::getenv(cvarname.c_str());
-	if (v == nullptr)
+
+	if (v != nullptr)
+		return {std::string(v)};
+
+	if (errno != 0)
 		return std::unexpected(error(std::errc(errno)));
-	return {std::string(v)};
+
+	return std::unexpected(error(std::errc::no_such_file_or_directory));
 #endif // NIHIL_HAVE_GETENV_R
 }
 
diff --git a/nihil.posix/open_in_path.ccm b/nihil.posix/open_in_path.ccm
index 1fae56b..d4c090d 100644
--- a/nihil.posix/open_in_path.ccm
+++ b/nihil.posix/open_in_path.ccm
@@ -7,7 +7,7 @@ module;
 #include <filesystem>
 #include <optional>
 
-export module nihil.posix:find_in_path;
+export module nihil.posix:open_in_path;
 
 import nihil.error;
 import :fd;
diff --git a/nihil.posix/posix.ccm b/nihil.posix/posix.ccm
index ea13f81..2ebc1b8 100644
--- a/nihil.posix/posix.ccm
+++ b/nihil.posix/posix.ccm
@@ -18,18 +18,18 @@ import nihil.error;
 export import :argv;
 export import :ensure_dir;
 export import :exec;
+export import :execlp;
 export import :execv;
+export import :execvp;
 export import :fd;
+export import :fexecv;
 export import :find_in_path;
 export import :getenv;
 export import :open;
+export import :open_in_path;
 export import :process;
 export import :read_file;
 export import :rename;
 export import :spawn;
 export import :tempfile;
 export import :write_file;
-
-#ifdef NIHIL_HAVE_FEXECVE
-export import :fexecv;
-#endif
diff --git a/nihil.posix/process.cc b/nihil.posix/process.cc
index 70e84b7..02642bc 100644
--- a/nihil.posix/process.cc
+++ b/nihil.posix/process.cc
@@ -85,7 +85,7 @@ auto process::pid(this process const &self) noexcept -> ::pid_t
 auto process::wait(this process &&self) -> std::expected<wait_result, error>
 {
 	auto status = int{};
-	auto ret = waitpid(self.m_pid, &status, WEXITED);
+	auto ret = waitpid(self.m_pid, &status, 0);
 	if (ret == -1)
 		return std::unexpected(error(std::errc(errno)));
 
diff --git a/nihil.posix/spawn.ccm b/nihil.posix/spawn.ccm
index 4cce334..9fa24e3 100644
--- a/nihil.posix/spawn.ccm
+++ b/nihil.posix/spawn.ccm
@@ -41,19 +41,23 @@ export inline int constexpr stdout_fileno = STDOUT_FILENO;
 export inline int constexpr stderr_fileno = STDERR_FILENO;
 
 /*
- * fd_pipe: create a pipe with one end in the child and the other in the
- * parent.  The child's side will be dup2()'d to the provided fd number.
- * The parent side fd can be retrieved via parent_fd();
+ * fd_{read,write}_pipe: create a pipe with one end in the child and the other in the parent.
+ * The child's side will be dup2()'d to the provided fd number.
+ * The parent side fd can be retrieved via parent_fd().
+ *
+ * fd_read_pipe() puts the reading side in the child, while fd_write_pipe() puts the writing
+ * side in the child.
  */
-export struct fd_pipe final {
-	fd_pipe(int fdno, fd &&child_fd, fd &&parent_fd)
+
+struct fd_pipe_base {
+	fd_pipe_base(int fdno, fd &&child_fd, fd &&parent_fd)
 		: m_fdno(fdno)
 		, m_child_fd(std::move(child_fd))
 		, m_parent_fd(std::move(parent_fd))
 	{
 	}
 
-	auto run_in_child(this fd_pipe &self, process &) -> void
+	auto run_in_child(this fd_pipe_base &self, process &) -> void
 	{
 		auto err = raw_dup(self.m_child_fd, self.m_fdno);
 		if (!err) {
@@ -69,12 +73,12 @@ export struct fd_pipe final {
 		std::ignore = self.m_child_fd.close();
 	}
 
-	auto run_in_parent(this fd_pipe &self, process &) -> void
+	auto run_in_parent(this fd_pipe_base &self, process &) -> void
 	{
 		std::ignore = self.m_child_fd.close();
 	}
 
-	[[nodiscard]] auto parent_fd(this fd_pipe &self) -> fd &
+	[[nodiscard]] auto parent_fd(this fd_pipe_base &self) -> fd &
 	{
 		return self.m_parent_fd;
 	}
@@ -83,13 +87,35 @@ private:
 	int	m_fdno;
 	fd	m_child_fd;
 	fd	m_parent_fd;
+
+};
+
+export struct fd_read_pipe final : fd_pipe_base {
+	fd_read_pipe(int fdno, fd &&read_fd, fd &&write_fd)
+		: fd_pipe_base(fdno, std::move(read_fd), std::move(write_fd))
+	{
+	}
 };
 
+export struct fd_write_pipe final : fd_pipe_base {
+	fd_write_pipe(int fdno, fd &&read_fd, fd &&write_fd)
+		: fd_pipe_base(fdno, std::move(write_fd), std::move(read_fd))
+	{
+	}
+};
+
+export [[nodiscard]] auto
+make_fd_read_pipe(int fdno) -> std::expected<fd_read_pipe, error>
+{
+	auto fds = co_await pipe();
+	co_return fd_read_pipe(fdno, std::move(fds.first), std::move(fds.second));
+}
+
 export [[nodiscard]] auto
-make_fd_pipe(int fdno) -> std::expected<fd_pipe, error>
+make_fd_write_pipe(int fdno) -> std::expected<fd_write_pipe, error>
 {
 	auto fds = co_await pipe();
-	co_return fd_pipe(fdno, std::move(fds.first), std::move(fds.second));
+	co_return fd_write_pipe(fdno, std::move(fds.first), std::move(fds.second));
 }
 
 /*
@@ -161,7 +187,7 @@ stderr_devnull() -> std::expected<fd_file, error>
  */
 export template<std::output_iterator<char> Iterator>
 struct fd_capture final {
-	fd_capture(fd_pipe &&pipe, Iterator it)
+	fd_capture(fd_write_pipe &&pipe, Iterator it)
 		: m_pipe(std::move(pipe))
 		, m_iterator(std::move(it))
 	{
@@ -192,7 +218,7 @@ struct fd_capture final {
 	}
 
 private:
-	fd_pipe		m_pipe;
+	fd_write_pipe	m_pipe;
 	Iterator	m_iterator;
 };
 
@@ -200,7 +226,7 @@ export [[nodiscard]] auto
 make_capture(int fdno, std::output_iterator<char> auto &&it)
 	-> std::expected<fd_capture<decltype(it)>, error>
 {
-	auto pipe = co_await make_fd_pipe(fdno);
+	auto pipe = co_await make_fd_write_pipe(fdno);
 	co_return fd_capture(std::move(pipe),
 			     std::forward<decltype(it)>(it));
 }
@@ -209,7 +235,7 @@ export [[nodiscard]] auto
 make_capture(int fdno, std::string &str)
 	-> std::expected<fd_capture<decltype(std::back_inserter(str))>, error>
 {
-	auto pipe = co_await make_fd_pipe(fdno);
+	auto pipe = co_await make_fd_write_pipe(fdno);
 	co_return fd_capture(std::move(pipe), std::back_inserter(str));
 }
 
diff --git a/nihil.posix/test.fd.cc b/nihil.posix/test.fd.cc
index 6b6394b..5c282af 100644
--- a/nihil.posix/test.fd.cc
+++ b/nihil.posix/test.fd.cc
@@ -184,16 +184,21 @@ TEST_CASE("fd: pipe, read, write", "[fd]") {
 	auto fds = nihil::pipe();
 	REQUIRE(fds);
 
+	/*
+	 * Note: traditionally, the first fd is the reading side, and the second fd
+	 * is the writing side.  Some platforms (e.g., macOS) still behave this way.
+	 */
+
 	auto [fd1, fd2] = std::move(*fds);
 
 	auto constexpr test_string = "test string"sv;
 
-	auto ret = write(fd1, test_string);
+	auto ret = write(fd2, test_string);
 	REQUIRE(ret);
 	REQUIRE(*ret == test_string.size());
 
 	auto readbuf = std::array<char, test_string.size() * 2>{};
-	auto read_buf = read(fd2, readbuf);
+	auto read_buf = read(fd1, readbuf);
 	REQUIRE(read_buf);
 	REQUIRE(std::string_view(*read_buf) == test_string);
 }
diff --git a/nihil.posix/test.spawn.cc b/nihil.posix/test.spawn.cc
index da321ff..ca6c076 100644
--- a/nihil.posix/test.spawn.cc
+++ b/nihil.posix/test.spawn.cc
@@ -32,17 +32,17 @@ TEST_CASE("spawn: execv", "[spawn]") {
 
 	auto args = argv({"sh", "-c", "x=1; echo $x"});
 	auto exec = execv("/bin/sh", std::move(args));
-	REQUIRE(exec);
 
 	auto output = std::string();
 	auto capture = make_capture(stdout_fileno, output);
 	REQUIRE(capture);
 
-	auto proc = spawn(*exec, *capture);
+	auto proc = spawn(exec, *capture);
 	REQUIRE(proc);
 
 	auto status = std::move(*proc).wait();
-	REQUIRE(status);
+	if (!status)
+		FAIL(to_string(status.error()));
 
 	REQUIRE(status->okay());
 	REQUIRE(output == "1\n");