runProgram(): Distinguish between empty input and no input
For example, if we call brotli with an empty input, it shouldn't read from the caller's stdin.
This commit is contained in:
		
							parent
							
								
									042975ea8e
								
							
						
					
					
						commit
						25dff2b7db
					
				
					 5 changed files with 30 additions and 15 deletions
				
			
		|  | @ -671,7 +671,7 @@ Path Downloader::downloadCached(ref<Store> store, const string & url_, bool unpa | ||||||
|             Path tmpDir = createTempDir(); |             Path tmpDir = createTempDir(); | ||||||
|             AutoDelete autoDelete(tmpDir, true); |             AutoDelete autoDelete(tmpDir, true); | ||||||
|             // FIXME: this requires GNU tar for decompression.
 |             // FIXME: this requires GNU tar for decompression.
 | ||||||
|             runProgram("tar", true, {"xf", storePath, "-C", tmpDir, "--strip-components", "1"}, ""); |             runProgram("tar", true, {"xf", storePath, "-C", tmpDir, "--strip-components", "1"}); | ||||||
|             unpackedStorePath = store->addToStore(name, tmpDir, true, htSHA256, defaultPathFilter, false); |             unpackedStorePath = store->addToStore(name, tmpDir, true, htSHA256, defaultPathFilter, false); | ||||||
|         } |         } | ||||||
|         replaceSymlink(unpackedStorePath, unpackedLink); |         replaceSymlink(unpackedStorePath, unpackedLink); | ||||||
|  |  | ||||||
|  | @ -92,7 +92,7 @@ static ref<std::string> decompressBzip2(const std::string & in) | ||||||
| static ref<std::string> decompressBrotli(const std::string & in) | static ref<std::string> decompressBrotli(const std::string & in) | ||||||
| { | { | ||||||
|     // FIXME: use libbrotli
 |     // FIXME: use libbrotli
 | ||||||
|     return make_ref<std::string>(runProgram(BRO, true, {"-d"}, in)); |     return make_ref<std::string>(runProgram(BRO, true, {"-d"}, {in})); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| ref<std::string> compress(const std::string & method, const std::string & in) | ref<std::string> compress(const std::string & method, const std::string & in) | ||||||
|  |  | ||||||
|  | @ -1,6 +1,7 @@ | ||||||
| #include "util.hh" | #include "util.hh" | ||||||
| #include "affinity.hh" | #include "affinity.hh" | ||||||
| #include "sync.hh" | #include "sync.hh" | ||||||
|  | #include "finally.hh" | ||||||
| 
 | 
 | ||||||
| #include <cctype> | #include <cctype> | ||||||
| #include <cerrno> | #include <cerrno> | ||||||
|  | @ -10,6 +11,7 @@ | ||||||
| #include <iostream> | #include <iostream> | ||||||
| #include <sstream> | #include <sstream> | ||||||
| #include <thread> | #include <thread> | ||||||
|  | #include <future> | ||||||
| 
 | 
 | ||||||
| #include <sys/wait.h> | #include <sys/wait.h> | ||||||
| #include <unistd.h> | #include <unistd.h> | ||||||
|  | @ -837,23 +839,21 @@ std::vector<char *> stringsToCharPtrs(const Strings & ss) | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
| string runProgram(Path program, bool searchPath, const Strings & args, | string runProgram(Path program, bool searchPath, const Strings & args, | ||||||
|     const string & input) |     const std::experimental::optional<std::string> & input) | ||||||
| { | { | ||||||
|     checkInterrupt(); |     checkInterrupt(); | ||||||
| 
 | 
 | ||||||
|     /* Create a pipe. */ |     /* Create a pipe. */ | ||||||
|     Pipe out, in; |     Pipe out, in; | ||||||
|     out.create(); |     out.create(); | ||||||
|     if (!input.empty()) in.create(); |     if (input) in.create(); | ||||||
| 
 | 
 | ||||||
|     /* Fork. */ |     /* Fork. */ | ||||||
|     Pid pid = startProcess([&]() { |     Pid pid = startProcess([&]() { | ||||||
|         if (dup2(out.writeSide.get(), STDOUT_FILENO) == -1) |         if (dup2(out.writeSide.get(), STDOUT_FILENO) == -1) | ||||||
|             throw SysError("dupping stdout"); |             throw SysError("dupping stdout"); | ||||||
|         if (!input.empty()) { |         if (input && dup2(in.readSide.get(), STDIN_FILENO) == -1) | ||||||
|             if (dup2(in.readSide.get(), STDIN_FILENO) == -1) |  | ||||||
|             throw SysError("dupping stdin"); |             throw SysError("dupping stdin"); | ||||||
|         } |  | ||||||
| 
 | 
 | ||||||
|         Strings args_(args); |         Strings args_(args); | ||||||
|         args_.push_front(program); |         args_.push_front(program); | ||||||
|  | @ -872,10 +872,23 @@ string runProgram(Path program, bool searchPath, const Strings & args, | ||||||
| 
 | 
 | ||||||
|     std::thread writerThread; |     std::thread writerThread; | ||||||
| 
 | 
 | ||||||
|     if (!input.empty()) { |     std::promise<void> promise; | ||||||
|  | 
 | ||||||
|  |     Finally doJoin([&]() { | ||||||
|  |         if (writerThread.joinable()) | ||||||
|  |             writerThread.join(); | ||||||
|  |     }); | ||||||
|  | 
 | ||||||
|  | 
 | ||||||
|  |     if (input) { | ||||||
|         in.readSide = -1; |         in.readSide = -1; | ||||||
|         writerThread = std::thread([&]() { |         writerThread = std::thread([&]() { | ||||||
|             writeFull(in.writeSide.get(), input); |             try { | ||||||
|  |                 writeFull(in.writeSide.get(), *input); | ||||||
|  |                 promise.set_value(); | ||||||
|  |             } catch (...) { | ||||||
|  |                 promise.set_exception(std::current_exception()); | ||||||
|  |             } | ||||||
|             in.writeSide = -1; |             in.writeSide = -1; | ||||||
|         }); |         }); | ||||||
|     } |     } | ||||||
|  | @ -888,8 +901,8 @@ string runProgram(Path program, bool searchPath, const Strings & args, | ||||||
|         throw ExecError(status, format("program ‘%1%’ %2%") |         throw ExecError(status, format("program ‘%1%’ %2%") | ||||||
|             % program % statusToString(status)); |             % program % statusToString(status)); | ||||||
| 
 | 
 | ||||||
|     if (!input.empty()) |     /* Wait for the writer thread to finish. */ | ||||||
|         writerThread.join(); |     if (input) promise.get_future().get(); | ||||||
| 
 | 
 | ||||||
|     return result; |     return result; | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -14,6 +14,7 @@ | ||||||
| #include <cstdio> | #include <cstdio> | ||||||
| #include <map> | #include <map> | ||||||
| #include <sstream> | #include <sstream> | ||||||
|  | #include <experimental/optional> | ||||||
| 
 | 
 | ||||||
| #ifndef HAVE_STRUCT_DIRENT_D_TYPE | #ifndef HAVE_STRUCT_DIRENT_D_TYPE | ||||||
| #define DT_UNKNOWN 0 | #define DT_UNKNOWN 0 | ||||||
|  | @ -232,7 +233,8 @@ pid_t startProcess(std::function<void()> fun, const ProcessOptions & options = P | ||||||
| /* Run a program and return its stdout in a string (i.e., like the
 | /* Run a program and return its stdout in a string (i.e., like the
 | ||||||
|    shell backtick operator). */ |    shell backtick operator). */ | ||||||
| string runProgram(Path program, bool searchPath = false, | string runProgram(Path program, bool searchPath = false, | ||||||
|     const Strings & args = Strings(), const string & input = ""); |     const Strings & args = Strings(), | ||||||
|  |     const std::experimental::optional<std::string> & input = {}); | ||||||
| 
 | 
 | ||||||
| class ExecError : public Error | class ExecError : public Error | ||||||
| { | { | ||||||
|  |  | ||||||
|  | @ -170,10 +170,10 @@ int main(int argc, char * * argv) | ||||||
|                 Path unpacked = (Path) tmpDir + "/unpacked"; |                 Path unpacked = (Path) tmpDir + "/unpacked"; | ||||||
|                 createDirs(unpacked); |                 createDirs(unpacked); | ||||||
|                 if (hasSuffix(baseNameOf(uri), ".zip")) |                 if (hasSuffix(baseNameOf(uri), ".zip")) | ||||||
|                     runProgram("unzip", true, {"-qq", tmpFile, "-d", unpacked}, ""); |                     runProgram("unzip", true, {"-qq", tmpFile, "-d", unpacked}); | ||||||
|                 else |                 else | ||||||
|                     // FIXME: this requires GNU tar for decompression.
 |                     // FIXME: this requires GNU tar for decompression.
 | ||||||
|                     runProgram("tar", true, {"xf", tmpFile, "-C", unpacked}, ""); |                     runProgram("tar", true, {"xf", tmpFile, "-C", unpacked}); | ||||||
| 
 | 
 | ||||||
|                 /* If the archive unpacks to a single file/directory, then use
 |                 /* If the archive unpacks to a single file/directory, then use
 | ||||||
|                    that as the top-level. */ |                    that as the top-level. */ | ||||||
|  |  | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue