You can pass -fcolor-diganostics to gcc 4.9 and higher in order to get colorized warnings and errors. This makes errors especially much easier to spot in a sea of build output.

I think we should just make this the default if it works. (Maybe conditional on MOZ_AUTOMATION or something like that.)

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #2) > I think we should just make this the default if it works. (Maybe conditional > on MOZ_AUTOMATION or something like that.) Yeah, probably so. We can probably just check if stdout is a terminal. We'd also need to make sure the option is supported by $CXX somehow.

There are a few things to know: - while clang defaults to coloring its output *when the output is a terminal", gcc doesn't. - if you build with make+clang, you'll see the colors, but not when building with mach, because it intercepts the output. - clang has a flag to force-enable the colors even when using mach. That flag is -fcolor-diagnostics - gcc's flag is -fdiagnostics-color. Seriously, they couldn't use the same flag?! - note that gcc -fdiagnostics-color=auto has the same behavior as clang's default. Now the tricky thing is that while it should be the default, it also makes redirecting the output to a file more painful, because the resulting file then has the ansi escape sequences, making the file harder to read. Output redirection is why flags like -fdiagnostics-color=auto exist. One can use less -R to read such logs, though, but not everyone will know to do that. That being said, that's where mach could be improved to make things better. First, it should log the last build by default. How many times have you had to ask someone to re-run the build to provide a fuller log? We shouldn't have to do this. Then, we can add a mach command to display that log and make /that/ use less -R. /Then/, we can make coloring the default.

Comment on attachment 8615480 [details] [diff] [review] Add --enable-color-diagnostics to enable colorized errors/warnings Reassigning to glandium since he already brain dumped.

Comment on attachment 8615480 [details] [diff] [review] Add --enable-color-diagnostics to enable colorized errors/warnings Review of attachment 8615480 [details] [diff] [review]: ----------------------------------------------------------------- ::: configure.in @@ +1389,5 @@ > + CXXFLAGS="$CXXFLAGS -fdiagnostics-color" > + CPPFLAGS="$CPPFLAGS -fdiagnostics-color" > + LDFLAGS="$LDFLAGS -fdiagnostics-color",) > + > +dnl ======================================================== It should be the default, with a flag to disable it. You should also check it's supported by the compiler, and use the right -f option for gcc or clang since they decided to use a different option. Note that CPPFLAGS is passed on the compiler command line alongside CFLAGS and CXXFLAGS. Note that CFLAGS and CXXFLAGS are passed on the linker command line alongside LDFLAGS. So it's probably better to only add it to CPPFLAGS and LDFLAGS. You should also add it to CMMFLAGS and CMFLAGS, because those are not sent alongside CPPFLAGS. And then, you need to pass it along in js/src/configure.in, and other subconfigures... at which point it's probably just easier to add it to CC and CXX.

It looks like clang supports -fdiagnostics-color as well, or at least it does for the version I have installed on this Mac (Apple LLVM 6.0).

nspr and friends do their own cross-compiler detection and overwrite CC, so that approach isn't gonna work.

This turns turns on colorized stuff for gecko and js, nspr would needs its own thing still (or use this macro)

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #8) > nspr and friends do their own cross-compiler detection and overwrite CC, so > that approach isn't gonna work. nspr and friends, in the vast majority of cases, should be reusing CC from top-level configure.

... except we reset CC with _SUBDIR_CC for some weird reasons... man I hate our configure.

Comment on attachment 8616925 [details] [diff] [review] Automatically use colorized warnings and errors if available Review of attachment 8616925 [details] [diff] [review]: ----------------------------------------------------------------- ::: build/autoconf/compiler-opts.m4 @@ +357,5 @@ > fi > > +MOZ_COLOR_DIAGNOSTICS=1 > +MOZ_ARG_DISABLE_BOOL(color-diagnostics, > +[ --disable-color-diagnostics Disable colorized warnings and errors (default=yes)], Remove that "(default=yes)", it's confusing.

This was totally wrong before, as it turns out. This one works on Try and locally.

Backed out for bustage. https://treeherder.mozilla.org/logviewer.html#?job_id=11215675&repo=mozilla-inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/0eeda0627da5

Can we try this again?

I haven't had time to mess with it. I just set the env vars in my mozconfig as such: export CFLAGS="-fdiagnostics-color" export CPPFLAGS="-fdiagnostics-color" export CXXFLAGS="-fdiagnostics-color" export LDFLAGS="-fdiagnostics-color"

Patch to be posted shortly...

Comment on attachment 8809188 [details] Bug 1171610 - Automatically enable color in compiler output; https://reviewboard.mozilla.org/r/91806/#review91832 > The only issue I see with this is that Clang doesn't "unset" its color > sequences when printing a newline. As a result, mach's time line > prefixing can sometimes inherit "bold" or other stylings. AFAICT this is > only a minor cosmetic issue. GCC does not exhibit this issue. We could, in a followup, add a sgr0 sequence at the beginning or the end of lines when printing them. ::: config/config.mk:344 (Diff revision 1) > > HOST_CFLAGS += $(HOST_DEFINES) $(MOZBUILD_HOST_CFLAGS) > HOST_CXXFLAGS += $(HOST_DEFINES) $(MOZBUILD_HOST_CXXFLAGS) > > +define colorize_flags > +ifeq (,$(findstring $(COLOR_CFLAGS),$$(1))) the findstring here doesn't really help the case where someone explicitly adds the flag to disable, which is of the -fno-... form.

Comment on attachment 8809188 [details] Bug 1171610 - Automatically enable color in compiler output; https://reviewboard.mozilla.org/r/91806/#review92162 ::: build/moz.configure/toolchain.configure:854 (Diff revision 2) > + # using exact string matching on the returned value. So if the return > + # value changes to e.g. "<x>=always", exact string match may fail and > + # multiple color flags could be added. So examine downstream consumers > + # before adding flags to return values. > + if info.type == 'gcc' and info.version >= '4.9.0': > + return '-fdiagnostics-color', '-fno-diagnostics-color' If you return a namespace() instead, you can use delayed_getattr in set_config and remove the intermediate functions below. I don't think the @checking output is really interesting anyways. ::: config/config.mk:347 (Diff revision 2) > +ifeq (,$(filter $(NO_COLOR_CFLAGS),$$(1))) > +ifeq (,$(findstring $(COLOR_CFLAGS),$$(1))) You could make that a one-liner ifeq(,$(filter)$(findstring)) Alternatively, you could go back to what you had in the first iteration and just derive NO_COLOR_CFLAGS from COLOR_CFLAGS on the make side: $(COLOR_CFLAGS:-f%=-fno-%), which is overall, simpler.

Comment on attachment 8809188 [details] Bug 1171610 - Automatically enable color in compiler output; https://reviewboard.mozilla.org/r/91806/#review92970

Pushed by gszorc@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1caba990bddf Automatically enable color in compiler output; r=glandium

(add-hook 'compilation-filter-hook (function (lambda () (ansi-color-apply-on-region compilation-filter-start (point))))) for emacs.