jenkinsci / ansicolor-plugin

Jenkins ANSI Color Plugin
https://plugins.jenkins.io/ansicolor/
MIT License
253 stars 83 forks source link

Erase Line (EL) control sequence not handled correctly #151

Closed Pesa closed 4 years ago

Pesa commented 5 years ago

gcc -fdiagnostics-color emits Erase Line control sequences in addition to the SGR strings for colorization. The ANSI color plugin doesn't seem to handle these control sequences correctly, thus rendering the colorized output unreadable.

Test environment

Expected behavior

Readable warning/error messages from gcc in "Console output".

Actual behavior

The following mess in "Console output". Note the extraneous [K sequences and in some cases a missing character just before the sequence (e.g. it should be "In member function...", but the last "n" is missing).

../tools/dump/ndndump.c In member functiobool ndn::dump::NdnDump::printTcp(ndn::dump::OutputFormatter&, const uint8_t*, std::size_t) co’:
../tools/dump/ndndump.cpp:457:erroconst struct tcp’ has no member nameth_’; did you mead’?
   size_t tcpHdrLen = th->th_ * 4;
                          ^~~
                          d
../tools/dump/ndndump.c In member functiobool ndn::dump::NdnDump::printUdp(ndn::dump::OutputFormatter&, const uint8_t*, std::size_t) co’:
../tools/dump/ndndump.cpp:486:erroconst struct udp’ has no member nameuh_u’
   size_t udpLen = endian::big_to_native(uh->uh_u);
                                             ^~~~

Steps to reproduce the behavior

Just run gcc -fdiagnostics-color on anything that produces a compilation warning or error.

Pesa commented 5 years ago

And this is the same snippet of the log file viewed as "plain text". I'm assuming this is the raw output as printed by gcc.

../tools/dump/ndndump.cpp: In member function ‘bool ndn::dump::NdnDump::printTcp(ndn::dump::OutputFormatter&, const uint8_t*, std::size_t) const’:
../tools/dump/ndndump.cpp:457:26: error: ‘const struct tcphdr’ has no member named ‘th_off’; did you mean ‘doff’?
   size_t tcpHdrLen = th->th_off * 4;
                          ^~~~~~
                          doff
../tools/dump/ndndump.cpp: In member function ‘bool ndn::dump::NdnDump::printUdp(ndn::dump::OutputFormatter&, const uint8_t*, std::size_t) const’:
../tools/dump/ndndump.cpp:486:45: error: ‘const struct udphdr’ has no member named ‘uh_ulen’
   size_t udpLen = endian::big_to_native(uh->uh_ulen);
                                             ^~~~~~~
dwnusbaum commented 5 years ago

@Pesa Thanks for the report and the log snippet! Do you know if this is a regression from 0.5.x, or has it never worked?

Pesa commented 5 years ago

It's definitely a regression. Can't say exactly when it started happening, but it's been at least 2 or 3 months.

Pesa commented 5 years ago

Any updates on this?

dwnusbaum commented 5 years ago

I haven't had any time to investigate further.

My guess is that the problem is that as of 0.6.x, the implementation of the coloring for Pipeline jobs has switched to using a different set of Jenkins APIs (ConsoleAnnotator+MarkupText), where the plugin adds markup to the rendered log to handle ANSI escape sequences, whereas previously the plugin used the AnsiHtmlOutputStream to directly write the log handling ANSI escape sequences while writing. The old implementation filtered all ANSI escape sequences as it wrote the logs, but the APIs used by the new logs do not allow content to be deleted (you can only add markup), so it has to add markup to comment out the actual ANSI sequences themselves with <!-- --> when other markup is added. The new method relies on AnsiHtmlOutputStream calling AnsiAttributeElement.emitHtml(String) to signify that some markup should be emitted, but for invisible sequences like Erase Line, AnsiHtmlOutputStream never actually did anything in the past, it just filtered out the ANSI sequence, so AnsiAttributeElement.emitHtml(String) is not called and the new implementation is clueless that it needs to hide anything.

147 fixed a similar problem for text containing multiple Reset Graphics (SGR0) sequences in a row - the old implementation filtered the redundant sequences, but the new implementation only handled the sequences that actually resulted in emitting HTML to close open tags (usually the first one, but not always, and definitely not any of the subsequent ones).

If that is the problem, then my approach for a fix would be to make AnsiHtmlOutputStream override AnsiOutputStream.processEscapeLine and all of the other AnsiOutputStream.processX methods that are not already implemented in AnsiHtmlOutputStream. The implementations should call AnsiAttributeElement.emitRedundantReset() or a similar method (probably makes sense to rename the method to emitInvisibleAnsiSequence) so that ColorConsoleAnnotator runs through the logic to emit comments to hide AnsiSequences.

If someone has a lot of time to investigate, I would consider attempting to modify the current implementation to be a hybrid of the old and new approaches: Use a ConsoleLogFilter to hide the ANSI sequences and use ConsoleAnnotator to emit markup for the things that actual require markup. That might be impossible because ConsoleLogFilters run before ConsoleAnnotators, but I'm not sure without digging through code in Jenkins core.

dblock commented 5 years ago

@Pesa Can you try to write a failing test/spec for this maybe?

mathbunnyru commented 5 years ago

Unfortunately, this plugin is not compatible with C++ projects :(

This is the simplest way to reproduce the problem

pipeline {
    options {
        ansiColor('xterm')
    }
    stages {
        stage('Incorrect gcc build') {
            steps {
                sh "g++ -fdiagnostics-color=always"
            }
        }
    }
}

fatal error: should be red.

dwnusbaum commented 4 years ago

I just released 0.6.3, most notably including #168 which should fix this issue and maybe some other issues that were lacking enough info to reproduce, and #171, which should fix #136.