MantisBT - CMake
View Issue Details
0016070CMake(No Category)public2016-04-17 17:522016-06-10 14:31
Nils Gladitz 
Kitware Robot 
normalmajoralways
closedmoved 
Windows
CMake 3.6 
 
0016070: Malformed newline sequences from externally executed processes
When running ctest in script mode verbosely e.g. the executed build command produces "\r\n" newlines. When ctest forwards that output to stdout/stderr these "\r\n" newlines turn into "\r\r\n".

The same happens when using execute_process() without the OUTPUT_/ERROR_ options.

When using the OUTPUT_/ERROR_ VARIABLE/FILE options "\r" newlines are explicitly stripped resulting in correct/expected output.

While the windows console and e.g. notepad interpret these "\r\r\n" newline sequences as a single newline other tools (e.g. buildbot) interpret such a sequence as two newlines.
No tags attached.
Issue History
2016-04-17 17:52Nils GladitzNew Issue
2016-04-18 08:25Brad KingNote Added: 0040878
2016-04-18 08:38Nils GladitzNote Added: 0040881
2016-04-19 02:32Nils GladitzNote Added: 0040884
2016-04-19 11:21Brad KingNote Added: 0040886
2016-04-19 11:36Nils GladitzNote Added: 0040887
2016-04-19 11:45Brad KingNote Added: 0040888
2016-04-19 11:57Nils GladitzNote Added: 0040889
2016-04-19 12:42Brad KingNote Added: 0040891
2016-04-20 10:04Ben BoeckelNote Added: 0040899
2016-04-20 10:19Brad KingNote Added: 0040900
2016-04-20 10:34Nils GladitzNote Added: 0040901
2016-06-10 14:29Kitware RobotNote Added: 0042985
2016-06-10 14:29Kitware RobotStatusnew => resolved
2016-06-10 14:29Kitware RobotResolutionopen => moved
2016-06-10 14:29Kitware RobotAssigned To => Kitware Robot
2016-06-10 14:31Kitware RobotStatusresolved => closed

Notes
(0040878)
Brad King   
2016-04-18 08:25   
> Product Version: CMake 3.6

Is this new since 3.5?
(0040881)
Nils Gladitz   
2016-04-18 08:38   
Probably not. I just didn't look at older sources.
I think it might actually always have been there ... I've seen odd extra newlines on CDash in the past as well but I haven't looked if the issue is still present / related.
(0040884)
Nils Gladitz   
2016-04-19 02:32   
Would it make sense to perform the "\r\n" -> "\n" conversion in kwsys directly? Perhaps right after ReadFile?

Could it be done unconditionally or are there contexts where newlines have to be preserved?
(0040886)
Brad King   
2016-04-19 11:21   
Re 0016070:0040884: I'd like to preserve test output and such, so the input may not be the proper place to convert. Another approach is to make sure all the output is done in binary mode rather than text mode. Either way it would help to have an understanding of the various paths that text is taking to end up this way.
(0040887)
Nils Gladitz   
2016-04-19 11:36   
I think in context of ctest tests "\r" characters are also already handled:
  https://cmake.org/gitweb?p=cmake.git;a=blob;f=Source/CTest/cmProcess.cxx;h=6441886486d4bc4c01202162598634673de2e571;hb=refs/heads/master#l82 [^]

I presume that at least within CMake all places where handling is missing are erroneous and that the places where newlines are explicitly handled should be consolidated.

I know nothing of use cases outside of CMake though.
(0040888)
Brad King   
2016-04-19 11:45   
I think KWSys.Process is too low level to do this filtering, especially because that affects non-CMake projects too. It doesn't have any notion of line-wise output, and cannot even assume that the data coming from the child is text.
(0040889)
Nils Gladitz   
2016-04-19 11:57   
Maybe one of:
  - make it opt-in through kwsysProcess_SetOption
  - a wrapper for kwsysProcess_WaitForData e.g. kwsysProcess_WaitForText
  - an utility function in cmake (outside of kwys) to be called after kwsysProcess_WaitForData

I think the last one might be the least invasive but might also require additional external state for the process (e.g. in case the last read data ends on \r (in the middle of a newline sequence)?).
(0040891)
Brad King   
2016-04-19 12:42   
Re 0016070:0040889: I'd prefer not to add more infrastructure to KWSys.Process because in the long run it should be replaced with libuv or another separate library. Having custom functionality will make that harder.
(0040899)
Ben Boeckel   
2016-04-20 10:04   
This was fixed in CTest by hooking up pipes in binary mode. Previously, it had read in binary and written in text which caused the \n to get expanded to \r\n even if it already had a \r in front of it. I don't see anything obvious in the execute_process implementation from a glance though.
(0040900)
Brad King   
2016-04-20 10:19   
For reference, the fix mentioned in 0016070:0040899 is:

ctest --launch: write to cout and cerr in binary
https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=29b0c8c3 [^]
(0040901)
Nils Gladitz   
2016-04-20 10:34   
As-is there seem to be plenty of individual workarounds and fixes in some but not all cases where kwsys's process code is used.

E.g. that ctest fix looks like it applies to ctest launchers but not e.g. the main ctest process running in dashboard mode with verbose output (where I see these malformed newline sequences currently).

For now I locally worked around it by piping ctest's output though a second process that filters out these sequences.
(0042985)
Kitware Robot   
2016-06-10 14:29   
Resolving issue as `moved`.

This issue tracker is no longer used. Further discussion of this issue may take place in the current CMake Issues page linked in the banner at the top of this page.