From fb1526f57fe4720aff596b312e6a767502b86e13 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 14 Jan 2016 14:41:09 -0500 Subject: cmake: Change `-E chdir` to pass through stdout/stderr directly Use OUTPUT_PASSTHROUGH instead of OUTPUT_NORMAL in order to avoid buffering the output just to re-print it. --- Source/cmcmd.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/cmcmd.cxx b/Source/cmcmd.cxx index fb7b1f5..7da81bb 100644 --- a/Source/cmcmd.cxx +++ b/Source/cmcmd.cxx @@ -621,7 +621,7 @@ int cmcmd::ExecuteCMakeCommand(std::vector& args) int retval = 0; int timeout = 0; if ( cmSystemTools::RunSingleCommand(command.c_str(), 0, 0, &retval, - directory.c_str(), cmSystemTools::OUTPUT_NORMAL, timeout) ) + directory.c_str(), cmSystemTools::OUTPUT_PASSTHROUGH, timeout) ) { return retval; } -- cgit v0.12 From 92e9bb21758f73266e1f805e366ce90d2cbd688d Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 14 Jan 2016 14:42:11 -0500 Subject: cmcmd.cxx: Remove unused code in __run_iwyu implementation Do not try to capture stderr with OUTPUT_PASSTHROUGH. RunSingleCommand will never populate it. --- Source/cmcmd.cxx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Source/cmcmd.cxx b/Source/cmcmd.cxx index 7da81bb..8dd902b 100644 --- a/Source/cmcmd.cxx +++ b/Source/cmcmd.cxx @@ -361,11 +361,10 @@ int cmcmd::ExecuteCMakeCommand(std::vector& args) } // Now run the real compiler command and return its result value. - if(!cmSystemTools::RunSingleCommand(orig_cmd, 0, &stdErr, &ret, 0, + if(!cmSystemTools::RunSingleCommand(orig_cmd, 0, 0, &ret, 0, cmSystemTools::OUTPUT_PASSTHROUGH)) { - std::cerr << "Error running '" << orig_cmd[0] << "': " - << stdErr << "\n"; + std::cerr << "Error running '" << orig_cmd[0] << "'\n"; return 1; } return ret; -- cgit v0.12 From ffa2a8c967df09c4630e50e7c7339e36b027ecaf Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 14 Jan 2016 14:54:58 -0500 Subject: cmSystemTools: Rename OUTPUT_NORMAL to OUTPUT_FORWARD to clarify its purpose The OUTPUT_NORMAL value is not really "normal" and has only one caller. Rename it to OUTPUT_FORWARD to clarify that we are explicitly forwarding the output. --- Source/cmGlobalGenerator.cxx | 2 +- Source/cmSystemTools.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx index 2126c71..c50bf66 100644 --- a/Source/cmGlobalGenerator.cxx +++ b/Source/cmGlobalGenerator.cxx @@ -1850,7 +1850,7 @@ int cmGlobalGenerator::Build( !makeCommand.empty() && cmSystemTools::LowerCase( cmSystemTools::GetFilenameName(makeCommand[0])) == "vcexpress.exe") { - outputflag = cmSystemTools::OUTPUT_NORMAL; + outputflag = cmSystemTools::OUTPUT_FORWARD; } // should we do a clean first? diff --git a/Source/cmSystemTools.h b/Source/cmSystemTools.h index 9cafbec..f511ae4 100644 --- a/Source/cmSystemTools.h +++ b/Source/cmSystemTools.h @@ -203,7 +203,7 @@ public: * Output is controlled with outputflag. If outputflag is OUTPUT_NONE, no * user-viewable output from the program being run will be generated. * OUTPUT_MERGE is the legacy behaviour where stdout and stderr are merged - * into stdout. OUTPUT_NORMAL passes through the output to stdout/stderr as + * into stdout. OUTPUT_FORWARD copies the output to stdout/stderr as * it was received. OUTPUT_PASSTHROUGH passes through the original handles. * * If timeout is specified, the command will be terminated after @@ -223,7 +223,7 @@ public: { OUTPUT_NONE = 0, OUTPUT_MERGE, - OUTPUT_NORMAL, + OUTPUT_FORWARD, OUTPUT_PASSTHROUGH }; static bool RunSingleCommand(const char* command, -- cgit v0.12 From dc039cc02c857b2c418481533a236dad6a586a7f Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 14 Jan 2016 16:14:25 -0500 Subject: cmSystemTools: Drop redundant condition in RunSingleCommand The output processing loop is already guarded by a condition so we do not need to repeat the condition inside the loop. --- Source/cmSystemTools.cxx | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx index a89a6e0b..2b0c2ed 100644 --- a/Source/cmSystemTools.cxx +++ b/Source/cmSystemTools.cxx @@ -696,19 +696,17 @@ bool cmSystemTools::RunSingleCommand(std::vectorconst& command, { while((pipe = cmsysProcess_WaitForData(cp, &data, &length, 0)) > 0) { - if(captureStdOut || captureStdErr || outputflag != OUTPUT_NONE) + // Translate NULL characters in the output into valid text. + // Visual Studio 7 puts these characters in the output of its + // build process. + for(int i=0; i < length; ++i) { - // Translate NULL characters in the output into valid text. - // Visual Studio 7 puts these characters in the output of its - // build process. - for(int i=0; i < length; ++i) + if(data[i] == '\0') { - if(data[i] == '\0') - { - data[i] = ' '; - } + data[i] = ' '; } } + if(pipe == cmsysProcess_Pipe_STDOUT || (pipe == cmsysProcess_Pipe_STDERR && captureStdOut == captureStdErr)) -- cgit v0.12 From ce3b713baa1c899ae7739c192040e24445368a0a Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 14 Jan 2016 16:11:00 -0500 Subject: cmSystemTools: Simplify RunSingleCommand output string construction Assign to the result strings instead setting to empty and appending. The old approach was left from when we directly buffered output in the strings. --- Source/cmSystemTools.cxx | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx index 2b0c2ed..3a730b2 100644 --- a/Source/cmSystemTools.cxx +++ b/Source/cmSystemTools.cxx @@ -660,14 +660,6 @@ bool cmSystemTools::RunSingleCommand(std::vectorconst& command, argv.push_back(a->c_str()); } argv.push_back(0); - if ( captureStdOut ) - { - *captureStdOut = ""; - } - if (captureStdErr && captureStdErr != captureStdOut) - { - *captureStdErr = ""; - } cmsysProcess* cp = cmsysProcess_New(); cmsysProcess_SetCommand(cp, &*argv.begin()); @@ -745,14 +737,13 @@ bool cmSystemTools::RunSingleCommand(std::vectorconst& command, } cmsysProcess_WaitForExit(cp, 0); - if ( captureStdOut && tempStdOut.begin() != tempStdOut.end()) + if (captureStdOut) { - captureStdOut->append(&*tempStdOut.begin(), tempStdOut.size()); + captureStdOut->assign(tempStdOut.begin(), tempStdOut.end()); } - if ( captureStdErr && captureStdErr != captureStdOut && - tempStdErr.begin() != tempStdErr.end()) + if (captureStdErr && captureStdErr != captureStdOut) { - captureStdErr->append(&*tempStdErr.begin(), tempStdErr.size()); + captureStdErr->assign(tempStdErr.begin(), tempStdErr.end()); } bool result = true; -- cgit v0.12 From 1040e690c6c99979f8edf2a121de6d835be96dbe Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 14 Jan 2016 16:11:23 -0500 Subject: cmSystemTools: Teach RunSingleCommand to merge child pipes when possible Audit the code to make sure there are no callers that use OUTPUT_MERGE with separate capture strings. Then change RunSingleCommand to implement output merging by giving the child process a single pipe for both its stdout and stderr descriptors. This will more cleanly merge the content on atomic write boundaries in the child instead of on arbitrary buffering boundaries in the parent. --- Source/cmSystemTools.cxx | 44 +++++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx index 3a730b2..3ba7287 100644 --- a/Source/cmSystemTools.cxx +++ b/Source/cmSystemTools.cxx @@ -17,6 +17,7 @@ #include #include #include +#include #ifdef __QNX__ # include /* for malloc/free on QNX */ #endif @@ -673,7 +674,16 @@ bool cmSystemTools::RunSingleCommand(std::vectorconst& command, { cmsysProcess_SetPipeShared(cp, cmsysProcess_Pipe_STDOUT, 1); cmsysProcess_SetPipeShared(cp, cmsysProcess_Pipe_STDERR, 1); + captureStdOut = 0; + captureStdErr = 0; } + else if (outputflag == OUTPUT_MERGE || + (captureStdErr && captureStdErr == captureStdOut)) + { + cmsysProcess_SetOption(cp, cmsysProcess_Option_MergeOutput, 1); + captureStdErr = 0; + } + assert(!captureStdErr || captureStdErr != captureStdOut); cmsysProcess_SetTimeout(cp, timeout); cmsysProcess_Execute(cp); @@ -699,38 +709,26 @@ bool cmSystemTools::RunSingleCommand(std::vectorconst& command, } } - if(pipe == cmsysProcess_Pipe_STDOUT || - (pipe == cmsysProcess_Pipe_STDERR && - captureStdOut == captureStdErr)) + if (pipe == cmsysProcess_Pipe_STDOUT) { - if (captureStdOut) + if (outputflag != OUTPUT_NONE) { - tempStdOut.insert(tempStdOut.end(), data, data+length); + cmSystemTools::Stdout(data, length); } - } - else if(pipe == cmsysProcess_Pipe_STDERR) - { - if (captureStdErr) + if (captureStdOut) { - tempStdErr.insert(tempStdErr.end(), data, data+length); + tempStdOut.insert(tempStdOut.end(), data, data+length); } } - if(outputflag != OUTPUT_NONE) + else if (pipe == cmsysProcess_Pipe_STDERR) { - if(outputflag == OUTPUT_MERGE) + if (outputflag != OUTPUT_NONE) { - cmSystemTools::Stdout(data, length); + cmSystemTools::Stderr(data, length); } - else + if (captureStdErr) { - if(pipe == cmsysProcess_Pipe_STDERR) - { - cmSystemTools::Stderr(data, length); - } - else if(pipe == cmsysProcess_Pipe_STDOUT) - { - cmSystemTools::Stdout(data, length); - } + tempStdErr.insert(tempStdErr.end(), data, data+length); } } } @@ -741,7 +739,7 @@ bool cmSystemTools::RunSingleCommand(std::vectorconst& command, { captureStdOut->assign(tempStdOut.begin(), tempStdOut.end()); } - if (captureStdErr && captureStdErr != captureStdOut) + if (captureStdErr) { captureStdErr->assign(tempStdErr.begin(), tempStdErr.end()); } -- cgit v0.12