change how child processes work

Makefile

@@ -8,7 +8,7 @@ HDRS = \
 	lib/toolchain.h lib/bufio.h
 BUILD = build
 OBJS = $(patsubst %,$(BUILD)/%.o,$(SRCS))
-CFLAGS = -std=c++14 -Wall -Wextra -Wno-unused-parameter -Ilib -O3 -g
+CFLAGS = -std=c++14 -Wall -Wextra -Wpedantic -Wno-unused-parameter -Ilib -O3 -g
 LDLIBS = -lpthread
 
 $(BUILD)/%.cc.o: %.cc $(HDRS)

build.bx

@@ -1,7 +1,7 @@
 target := box
 files := lib cmd
 includes := lib
-warnings := all extra no-unused-parameter
+warnings := all extra pedantic no-unused-parameter
 std := c++14
 optimize := 3
 cxxflags := -g

cmd/main.cc

@@ -102,9 +102,11 @@ static void run(Conf conf) {
 			for (const auto &arg: conf.args2) {
 				argv.push_back(std::move(arg));
 			}
+
 			try {
-				sys::ignoreCtrlC();
-				sys::execute(argv, true);
+				sys::ProcConf conf;
+				conf.forwardSignals = true;
+				sys::execute(argv, conf);
 			} catch (std::exception &ex) {
 				logger::log(ex.what());
 			}

lib/CompileStep.cc

@@ -99,7 +99,9 @@ void CompileStep::doBuild(const std::string &outDir) {
 	BXWriter writer(f);
 	writer.write(newCachedVars);
 
-	sys::execute(command, nullptr, verboseCommand);
+	sys::ProcConf conf;
+	conf.print = verboseCommand;
+	sys::execute(command, conf);
 }
 
 void CompileStep::doWriteCompDB(const std::string &outDir, compdb::Writer &w) {

lib/LinkStep.cc

@@ -56,7 +56,9 @@ void LinkStep::doBuild(const std::string &outDir) {
 	writer.write(newCachedVars);
 
 	sys::mkdirp(dirPath);
-	sys::execute(command, nullptr, global::verbose >= 1);
+	sys::ProcConf conf;
+	conf.print = global::verbose >= 1;
+	sys::execute(command, conf);
 }
 
 std::vector<std::string> &LinkStep::linkCommand(const std::string &outDir) {

lib/sys.cc

@@ -12,6 +12,7 @@
 #include <stdexcept>
 #include <system_error>
 #include <unordered_set>
+#include <atomic>
 
 #include "logger.h"
 #include "globals.h"
@@ -20,6 +21,24 @@ namespace sys {
 
 static thread_local std::unordered_set<std::string> mkdirp_set;
 
+static std::atomic<int> signalCounter(0);
+
+struct SigCtx {
+	SigCtx(bool enabled): enabled(enabled) {
+		if (enabled && signalCounter++ == 0) {
+			signal(SIGINT, SIG_IGN);
+		}
+	}
+
+	~SigCtx() {
+		if (enabled && --signalCounter == 0) {
+			signal(SIGINT, SIG_DFL);
+		}
+	}
+
+	bool enabled;
+};
+
 std::string sanitizePath(const std::string &path) {
 	std::string npath;
 	size_t idx = 0;
@@ -79,7 +98,9 @@ void mkdirp(const std::string &path) {
 	argv.push_back("-p");
 	argv.push_back("--");
 	argv.push_back(path);
-	execute(argv, nullptr, global::verbose >= 2);
+	ProcConf conf;
+	conf.print = global::verbose >= 2;
+	execute(argv, conf);
 }
 
 void rmrf(const std::string &path) {
@@ -90,11 +111,13 @@ void rmrf(const std::string &path) {
 	argv.push_back("-rf");
 	argv.push_back("--");
 	argv.push_back(path);
-	execute(argv, nullptr, global::verbose >= 2);
+	ProcConf conf;
+	conf.print = global::verbose >= 2;
+	execute(argv, conf);
 }
 
-void execute(const std::vector<std::string> &args, std::string *output, bool print) {
-	if (print) {
+void execute(const std::vector<std::string> &args, const ProcConf &conf) {
+	if (conf.print) {
 		std::string str;
 		for (size_t i = 0; i < args.size(); ++i) {
 			if (i != 0) {
@@ -117,18 +140,25 @@ void execute(const std::vector<std::string> &args, std::string *output, bool pri
 	for (size_t i = 1; i < args.size(); ++i) {
 		argv.push_back(args[i].c_str());
 	}
-
 	argv.push_back(nullptr);
 
-	int fds[2];
-	if (pipe(fds) < 0) {
-		throw std::runtime_error(std::string("fork: ") + strerror(errno));
+	int stdoutPipe[2];
+	if (conf.output == nullptr) {
+		stdoutPipe[1] = STDOUT_FILENO;
+	} else {
+		if (pipe(stdoutPipe) < 0) {
+			throw std::runtime_error(std::string("pipe: ") + strerror(errno));
+		}
 	}
 
+	SigCtx sigCtx(conf.forwardSignals);
 	pid_t child = fork();
 	if (child == 0) {
-		close(fds[0]);
-		dup2(fds[1], 1);
+		signal(SIGINT, SIG_DFL);
+		if (conf.output != nullptr) {
+			close(stdoutPipe[0]);
+			dup2(stdoutPipe[1], 1);
+		}
 
 		// So, from what I've read, execvp should never modify its argv; so this should be fine?
 		if (execvp(argv[0], (char *const *)argv.data()) < 0) {
@@ -137,35 +167,32 @@ void execute(const std::vector<std::string> &args, std::string *output, bool pri
 		}
 
 		// This shouldn't happen
-		exit(0);
+		abort();
 	} else if (child < 0) {
 		throw std::runtime_error(std::string("fork: ") + strerror(errno));
 	}
 
-	close(fds[1]);
-
-	if (output == nullptr) {
-		close(fds[0]);
-	} else {
+	if (conf.output != nullptr) {
+		close(stdoutPipe[1]);
 		char buf[1025];
 		while (true) {
-			ssize_t num = read(fds[0], buf, sizeof(buf) - 1);
+			ssize_t num = read(stdoutPipe[0], buf, sizeof(buf) - 1);
 			if (num < 0 && errno != EAGAIN) {
-				close(fds[0]);
+				close(stdoutPipe[0]);
 				throw std::runtime_error(
 					std::string("read: ") + strerror(errno) +
-					" (fd: " + std::to_string(fds[0]) + ")");
+					" (fd: " + std::to_string(stdoutPipe[0]) + ")");
 			} else if (num < 0) {
 				continue;
 			}
 
 			if (num == 0) {
-				close(fds[0]);
+				close(stdoutPipe[0]);
 				break;
 			}
 
 			buf[num] = '\0';
-			*output += buf;
+			*conf.output += buf;
 		}
 	}
 
@@ -181,62 +208,8 @@ void execute(const std::vector<std::string> &args, std::string *output, bool pri
 				" terminated due to " + strsignal(WTERMSIG(wstatus)));
 	}
 
-	if (output != nullptr && output->back() == '\n') {
-		output->pop_back();
-	}
-}
-
-void execute(const std::vector<std::string> &args, bool print) {
-	if (print) {
-		std::string str;
-		for (size_t i = 0; i < args.size(); ++i) {
-			if (i != 0) {
-				str += " ";
-			}
-			str += args[i];
-		}
-		logger::log(str);
-	}
-
-	// argv[0] should be interpreted as a shell command, because being able to run
-	// CC='gcc --sysroot=/blah' is used by some systems.
-	std::string command = std::string(args[0]) + " \"$@\"";
-	static thread_local std::vector<const char *> argv;
-	argv.clear();
-	argv.push_back("/bin/sh"); // TODO: Use $SHELL?
-	argv.push_back("-c");
-	argv.push_back(command.c_str());
-	argv.push_back("--");
-	for (size_t i = 1; i < args.size(); ++i) {
-		argv.push_back(args[i].c_str());
-	}
-
-	argv.push_back(nullptr);
-
-	pid_t child = fork();
-	if (child == 0) {
-		// So, from what I've read, execvp should never modify its argv; so this should be fine?
-		if (execvp(argv[0], (char *const *)argv.data()) < 0) {
-			perror(argv[0]);
-			abort();
-		}
-
-		// This shouldn't happen
-		exit(0);
-	} else if (child < 0) {
-		throw std::runtime_error(std::string("fork: ") + strerror(errno));
-	}
-
-	int wstatus;
-	waitpid(child, &wstatus, 0);
-	if (WIFEXITED(wstatus) && WEXITSTATUS(wstatus) != 0) {
-		throw std::runtime_error(std::string(args[0]) +
-				" exited with code " + std::to_string(WEXITSTATUS(wstatus)));
-	}
-
-	if (WTERMSIG(wstatus)) {
-		throw std::runtime_error(std::string(args[0]) +
-				" terminated due to " + strsignal(WTERMSIG(wstatus)));
+	if (conf.output != nullptr && conf.output->back() == '\n') {
+		conf.output->pop_back();
 	}
 }
 
@@ -289,7 +262,7 @@ void symlink(const std::string &from, const std::string &to) {
 	argv.push_back("--");
 	argv.push_back(from);
 	argv.push_back(to);
-	sys::execute(argv, nullptr, global::verbose > 2);
+	sys::execute(argv, ProcConf{});
 }
 
 std::string dirname(const std::string &path) {
@@ -299,8 +272,4 @@ std::string dirname(const std::string &path) {
 	return std::string(dir);
 }
 
-void ignoreCtrlC() {
-	signal(SIGINT, SIG_IGN);
-}
-
 }

lib/sys.h

@@ -17,18 +17,22 @@ struct FileInfo {
 	}
 };
 
+struct ProcConf {
+	bool print = false;
+	bool forwardSignals = false;
+	std::string *output = nullptr;
+};
+
 std::string sanitizePath(const std::string &path);
 FileInfo fileInfo(const std::string &path);
 bool fileExists(const std::string &path);
 void mkdirp(const std::string &path);
 void rmrf(const std::string &path);
-void execute(const std::vector<std::string> &args, std::string *output, bool print);
-void execute(const std::vector<std::string> &args, bool print);
+void execute(const std::vector<std::string> &args, const ProcConf &conf);
 void readDir(const std::string &path, std::vector<std::string> &files);
 void chdir(const std::string &path);
 std::string cwd();
 void symlink(const std::string &from, const std::string &to);
 std::string dirname(const std::string &path);
-void ignoreCtrlC();
 
 }

lib/toolchain.cc

@@ -141,7 +141,10 @@ void getFlags(const BXVariables &vars, FileType type, std::vector<std::string> &
 
 		// Execute $(PKG_CONFIG) --cflags $(PKGS)
 		std::string output;
-		sys::execute(argv, &output, global::verbose >= 2);
+		sys::ProcConf conf;
+		conf.print = global::verbose >= 2;
+		conf.output = &output;
+		sys::execute(argv, conf);
 		parseWhitespaceSeparated(output, flags);
 	}
 
@@ -201,7 +204,10 @@ void getLDLibs(const BXVariables &vars, std::vector<std::string> &flags) {
 
 		// Execute $(PKG_CONFIG) --cflags $(PKGS)
 		std::string output;
-		sys::execute(argv, &output, global::verbose >= 2);
+		sys::ProcConf conf;
+		conf.print = global::verbose >= 2;
+		conf.output = &output;
+		sys::execute(argv, conf);
 		parseWhitespaceSeparated(output, flags);
 	}
 
@@ -249,7 +255,10 @@ std::vector<std::string> getDependencies(
 
 	// Execute $(compiler) $(flags) -MM $<
 	std::string output;
-	sys::execute(argv, &output, global::verbose >= 2);
+	sys::ProcConf conf;
+	conf.print = global::verbose >= 2;
+	conf.output = &output;
+	sys::execute(argv, conf);
 
 	std::vector<std::string> deps;
 	size_t idx = output.find(':');