Bug 186851 - [patch] Adding colour support to devel/ccache
Summary: [patch] Adding colour support to devel/ccache
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: Bryan Drewery
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-18 00:40 UTC by Olivier - interfaSys sàrl
Modified: 2014-03-02 16:30 UTC (History)
0 users

See Also:


Attachments
file.txt (3.56 KB, text/plain)
2014-02-18 00:40 UTC, Olivier - interfaSys sàrl
no flags Details
patch-colours-ccache.c.txt (3.77 KB, patch)
2014-02-18 17:12 UTC, info
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Olivier - interfaSys sàrl 2014-02-18 00:40:00 UTC
Clang and a patch gcc48 or a standard gcc49 support colour diagnostics.
The problem is that the current version of ccache doesn't and so, without this patch, we have to choose between having colours or faster compilation times.

Fix: The patch works

Patch attached with submission follows:
How-To-Repeat: Install lang/gcc49
Make sure devel/ccache is not installed
Compile archivers/unzip

Notice the colours in the warning messages

Now install devel/ccache
Compile archivers/unzip

The colours should be gone
Comment 1 Edwin Groothuis freebsd_committer freebsd_triage 2014-02-18 00:40:12 UTC
Responsible Changed
From-To: freebsd-ports-bugs->bdrewery

Over to maintainer (via the GNATS Auto Assign Tool)
Comment 2 info 2014-02-18 17:12:25 UTC
Updated patch which works with -fdiagnostics-color=auto
Comment 3 Bryan Drewery freebsd_committer freebsd_triage 2014-03-02 06:13:01 UTC
FYI your patch does not build. It's against ccache master (unreleased
3.2), not 3.1. I have adapted it to 3.1 and added support for 'cc' though.

It's good to test patches when submitting and also give proper
attribution to author :)

-- 
Regards,
Bryan Drewery
Comment 4 Bryan Drewery freebsd_committer freebsd_triage 2014-03-02 06:23:15 UTC
State Changed
From-To: open->closed

COLORS added
Comment 5 info 2014-03-02 10:57:53 UTC
I apologise about that, I must have submitted the wrong one :S. I forked
the project on Github in case they wanted to do a pull request. Thanks
for making the changes

And I can't believe I forgot to link to the original patch as I did on
Github
https://github.com/interfasys/ccache/tree/maint

If you can, make sure to mention that the original patch was provided by
Lubos Lunak.

Regards,

Olivier
interfaSys sàrl

On 02/03/2014 07:13, Bryan Drewery wrote:
> FYI your patch does not build. It's against ccache master (unreleased
> 3.2), not 3.1. I have adapted it to 3.1 and added support for 'cc' though.
> 
> It's good to test patches when submitting and also give proper
> attribution to author :)
>
Comment 6 dfilter service freebsd_committer freebsd_triage 2014-03-02 11:31:29 UTC
Author: bdrewery
Date: Sun Mar  2 06:23:02 2014
New Revision: 346739
URL: http://svnweb.freebsd.org/changeset/ports/346739
QAT: https://qat.redports.org/buildarchive/r346739/

Log:
  - Add COLORS option to enable contributed colors patch that uses colors
    when possible
  
  PR:		ports/186851
  Submitted by:	Olivier <software-freebsd@interfasys.ch>
  Obtained from:	https://groups.google.com/forum/#!topic/ccache/-Zehp1Zs3Lg [*]
  [*] Backported to 3.1.9 by me and CC_IS_CLANG and CC_IS_GCC support added.

Added:
  head/devel/ccache/files/extra-patch-colors   (contents, props changed)
Modified:
  head/devel/ccache/Makefile

Modified: head/devel/ccache/Makefile
==============================================================================
--- head/devel/ccache/Makefile	Sun Mar  2 05:47:53 2014	(r346738)
+++ head/devel/ccache/Makefile	Sun Mar  2 06:23:02 2014	(r346739)
@@ -3,7 +3,7 @@
 
 PORTNAME=	ccache
 PORTVERSION=	3.1.9
-PORTREVISION=	5
+PORTREVISION=	6
 CATEGORIES=	devel
 MASTER_SITES=	http://www.samba.org/ftp/ccache/ \
 		CRITICAL
@@ -21,17 +21,30 @@ SUB_FILES=	${HOWTO} world-ccache pkg-mes
 
 PORTDOCS=	ccache-howto-freebsd.txt MANUAL.html
 
-OPTIONS_DEFINE=	CLANGLINK LLVMLINK STATIC DOCS TINDERBOX
+OPTIONS_DEFINE=	CLANGLINK LLVMLINK STATIC DOCS TINDERBOX COLORS
 
+COLORS_DESC=	Support compiler colors
 CLANGLINK_DESC=	Create clang compiler links if clang is installed
 LLVMLINK_DESC=	Create llvm compiler links if llvm is installed
 TINDERBOX_DESC=	Create tarball for tinderbox usage
 
+COLORS_EXTRA_PATCHES=	${FILESDIR}/extra-patch-colors
+COLORS_USES=		compiler
+
 OPTIONS_SUB=	yes
 
 STATIC_LDFLAGS=	-static
 
 .include <bsd.port.options.mk>
+.include <bsd.port.pre.mk>
+
+.if ${PORT_OPTIONS:MCOLORS}
+.  if ${COMPILER_TYPE} == clang
+CPPFLAGS+=	-DCC_IS_CLANG
+.  elif ${COMPILER_TYPE} = gcc
+CPPFLAGS+=	-DCC_IS_GCC
+.  endif
+.endif
 
 PLIST_SUB+=	CCLINKDIR="${CCLINKDIR}"
 

Added: head/devel/ccache/files/extra-patch-colors
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ head/devel/ccache/files/extra-patch-colors	Sun Mar  2 06:23:02 2014	(r346739)
@@ -0,0 +1,199 @@
+CC_IS_CLANG and CC_IS_GCC added by me. Adapated from 3.2 patch. - bdrewery
+
+Original author:
+
+From cacb14929748ae93eacefcfa194aa93689d217eb Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Lubo=C5=A1=20Lu=C5=88=C3=A1k?= <l.lunak@centrum.cz>
+Date: Fri, 29 Nov 2013 12:14:03 +0100
+Subject: [PATCH] support compiler color diagnostics if possible
+
+Clang and GCC (starting with 4.9) support color output if possible,
+but since ccache redirects stderr to a file, they detect the output
+is not a terminal and do not enable colors. Try to detect if colors
+would be used and force colors explicitly.
+
+Caveats:
+- Compiles with and without colors are considered different from each
+  other (so they are "duplicated").
+- GCC decided to roll its own name for the option, so it's necessary
+  to guess which compiler is actually used.
+
+--- ccache.c.orig	2014-03-02 00:16:33.335546870 -0600
++++ ccache.c	2014-03-02 00:16:35.888546332 -0600
+@@ -849,14 +849,42 @@ hash_compiler(struct mdfour *hash, struc
+ 		if (!hash_multicommand_output(hash, compilercheck, orig_args->argv[0])) {
+ 			fatal("Failure running compiler check command: %s", compilercheck);
+ 		}
+ 	}
+ }
+ 
+ /*
++ * Note that these compiler checks are unreliable, so nothing should hard-depend on them.
++ */
++
++static bool compiler_is_clang()
++{
++	const char* name = strrchr( orig_args->argv[ 0 ], '/' );
++	name = name ? name + 1 : orig_args->argv[ 0 ];
++#ifdef CC_IS_CLANG
++	if (strcmp(name, "cc") == 0 || strcmp(name, "CC") == 0 ||
++	    strcmp(name, "c++") == 0)
++	        return true;
++#endif
++	return strstr( name, "clang" ) != NULL;
++}
++
++static bool compiler_is_gcc()
++{
++	const char* name = strrchr(orig_args->argv[ 0 ], '/' );
++#ifdef CC_IS_GCC
++	if (strcmp(name, "cc") == 0 || strcmp(name, "CC") == 0 ||
++	    strcmp(name, "c++") == 0)
++	        return true;
++#endif
++	name = name ? name + 1 : orig_args->argv[ 0 ];
++	return strstr(name, "gcc") != NULL || strstr(name, "g++") != NULL;
++}
++
++/*
+  * Update a hash sum with information common for the direct and preprocessor
+  * modes.
+  */
+ static void
+ calculate_common_hash(struct args *args, struct mdfour *hash)
+ {
+ 	struct stat st;
+@@ -913,14 +941,23 @@ calculate_common_hash(struct args *args,
+ 				stats_update(STATS_BADEXTRAFILE);
+ 				failed();
+ 			}
+ 			q = NULL;
+ 		}
+ 		free(p);
+ 	}
++
++	/* Possibly hash GCC_COLORS (for color diagnostics). */
++	if (compiler_is_gcc()) {
++		const char* gcc_colors = getenv("GCC_COLORS");
++		if (gcc_colors != NULL) {
++			hash_delimiter(hash,"gcccolors");
++			hash_string(hash, gcc_colors);
++		}
++	}
+ }
+ 
+ /*
+  * Update a hash sum with information specific to the direct and preprocessor
+  * modes and calculate the object hash. Returns the object hash on success,
+  * otherwise NULL. Caller frees.
+  */
+@@ -1273,14 +1310,21 @@ find_compiler(int argc, char **argv)
+ 
+ bool
+ is_precompiled_header(const char *path)
+ {
+ 	return str_eq(get_extension(path), ".gch");
+ }
+ 
++static bool color_output_possible()
++{
++	const char* term_env = getenv("TERM");
++
++	return isatty(STDERR_FILENO) && term_env && strcasecmp(term_env, "DUMB") != 0;
++}
++
+ /*
+  * Process the compiler options into options suitable for passing to the
+  * preprocessor and the real compiler. The preprocessor options don't include
+  * -E; this is added later. Returns true on success, otherwise false.
+  */
+ bool
+ cc_process_args(struct args *orig_args, struct args **preprocessor_args,
+@@ -1301,14 +1345,15 @@ cc_process_args(struct args *orig_args, 
+ 	bool dependency_filename_specified = false;
+ 	/* is the dependency makefile target name specified with -MT or -MQ? */
+ 	bool dependency_target_specified = false;
+ 	struct args *stripped_args = NULL, *dep_args = NULL;
+ 	int argc = orig_args->argc;
+ 	char **argv = orig_args->argv;
+ 	bool result = true;
++	bool found_color_diagnostics = false;
+ 
+ 	stripped_args = args_init(0, NULL);
+ 	dep_args = args_init(0, NULL);
+ 
+ 	args_add(stripped_args, argv[0]);
+ 
+ 	for (i = 1; i < argc; i++) {
+@@ -1551,14 +1596,34 @@ cc_process_args(struct args *orig_args, 
+ 
+ 		/* Input charset needs to be handled specially. */
+ 		if (str_startswith(argv[i], "-finput-charset=")) {
+ 			input_charset = argv[i];
+ 			continue;
+ 		}
+ 
++		if (str_eq(argv[i], "-fcolor-diagnostics")
++		    || str_eq(argv[i], "-fno-color-diagnostics")
++		    || str_eq(argv[i], "-fdiagnostics-color")
++		    || str_eq(argv[i], "-fdiagnostics-color=always")
++		    || str_eq(argv[i], "-fno-diagnostics-color")
++		    || str_eq(argv[i], "-fdiagnostics-color=never")) {
++			args_add(stripped_args, argv[i]);
++			found_color_diagnostics = true;
++			continue;
++		}
++		if (str_eq(argv[i], "-fdiagnostics-color=auto")) {
++			if (color_output_possible()) {
++				/* Output is redirected, so color output must be forced. */
++				args_add(stripped_args, "-fdiagnostics-color=always");
++				cc_log("Automatically forcing colors");
++			}
++			found_color_diagnostics = true;
++			continue;
++		}
++
+ 		/*
+ 		 * Options taking an argument that that we may want to rewrite
+ 		 * to relative paths to get better hit rate. A secondary effect
+ 		 * is that paths in the standard error output produced by the
+ 		 * compiler will be normalized.
+ 		 */
+ 		if (compopt_takes_path(argv[i])) {
+@@ -1765,14 +1830,36 @@ cc_process_args(struct args *orig_args, 
+ 		cc_log("Not a regular file: %s", output_obj);
+ 		stats_update(STATS_DEVICE);
+ 		result = false;
+ 		goto out;
+ 	}
+ 
+ 	/*
++	 * Since output is redirected, compilers will not color their output by default,
++	 * so force it explicitly if it would be otherwise done.
++	 */
++	if (!found_color_diagnostics && color_output_possible()) {
++		if (compiler_is_clang()) {
++			args_add(stripped_args, "-fcolor-diagnostics");
++			cc_log("Automatically enabling colors");
++		} else if (compiler_is_gcc()) {
++			/*
++			 * GCC has it since 4.9, but that'd require detecting what GCC
++			 * version is used for the actual compile. However it requires
++			 * also GCC_COLORS to be set (and not empty), so use that
++			 * for detecting if GCC would use colors.
++			 */
++			if (getenv("GCC_COLORS") != NULL && getenv("GCC_COLORS")[ 0 ] != '\0') {
++				args_add(stripped_args, "-fdiagnostics-color");
++				cc_log("Automatically enabling colors");
++			}
++		}
++	}
++
++	/*
+ 	 * Some options shouldn't be passed to the real compiler when it compiles
+ 	 * preprocessed code:
+ 	 *
+ 	 * -finput-charset=XXX (otherwise conversion happens twice)
+ 	 * -x XXX (otherwise the wrong language is selected)
+ 	 */
+ 	*preprocessor_args = args_copy(stripped_args);
_______________________________________________
svn-ports-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-ports-all
To unsubscribe, send any mail to "svn-ports-all-unsubscribe@freebsd.org"
Comment 7 Bryan Drewery freebsd_committer freebsd_triage 2014-03-02 15:43:32 UTC
On 3/2/2014 4:57 AM, interfaSys sàrl wrote:
> I apologise about that, I must have submitted the wrong one :S. I forked
> the project on Github in case they wanted to do a pull request. Thanks
> for making the changes
> 
> And I can't believe I forgot to link to the original patch as I did on
> Github
> https://github.com/interfasys/ccache/tree/maint
> 
> If you can, make sure to mention that the original patch was provided by
> Lubos Lunak.
> 
> Regards,
> 
> Olivier
> interfaSys sàrl
> 


Ah. It's too bad that this still does not work with bmake -j though. Ie,
you must have DISABLE_MAKE_JOBS to see colors in port building.


-- 
Regards,
Bryan Drewery
Comment 8 info 2014-03-02 16:03:13 UTC
It does work for me if I modify the color_output_possible() function,
but I'm using GCC.
I've learned from this thread that isatty() is not reliable within ccache:
https://bugzilla.mozilla.org/show_bug.cgi?id=777952#c3

So I've modified
return isatty(STDERR_FILENO) && term_env && strcasecmp(term_env, "DUMB")
!= 0;
to
return term_env && strcasecmp(term_env, "DUMB") != 0;

and I get colours when compiling ports with parallel jobs and
customising them with GCC_COLOURS does work.

I have not tested with Clang though, so I'm not sure if it makes a
difference there.

--
Olivier
interfaSys sàrl


On 02/03/2014 16:43, Bryan Drewery wrote:
> On 3/2/2014 4:57 AM, interfaSys sàrl wrote:
>> I apologise about that, I must have submitted the wrong one :S. I forked
>> the project on Github in case they wanted to do a pull request. Thanks
>> for making the changes
>>
>> And I can't believe I forgot to link to the original patch as I did on
>> Github
>> https://github.com/interfasys/ccache/tree/maint
>>
>> If you can, make sure to mention that the original patch was provided by
>> Lubos Lunak.
>>
>> Regards,
>>
>> Olivier
>> interfaSys sàrl
>>
> 
> Ah. It's too bad that this still does not work with bmake -j though. Ie,
> you must have DISABLE_MAKE_JOBS to see colors in port building.
> 
>
Comment 9 Bryan Drewery freebsd_committer freebsd_triage 2014-03-02 16:08:12 UTC
On 3/2/2014 10:03 AM, interfaSys sàrl wrote:
> It does work for me if I modify the color_output_possible() function,
> but I'm using GCC.
> I've learned from this thread that isatty() is not reliable within ccache:
> https://bugzilla.mozilla.org/show_bug.cgi?id=777952#c3
> 
> So I've modified
> return isatty(STDERR_FILENO) && term_env && strcasecmp(term_env, "DUMB")
> != 0;
> to
> return term_env && strcasecmp(term_env, "DUMB") != 0;
> 
> and I get colours when compiling ports with parallel jobs and
> customising them with GCC_COLOURS does work.
> 
> I have not tested with Clang though, so I'm not sure if it makes a
> difference there.
> 


Hmm, the check seems to make sense though. Perhaps bmake/fmake are not
hooking up stderr correctly. gmake does work as I expect here.

You can see with building ccache itself:

make -C /usr/ports/devel/ccache clean all MAKE_CMD=bmake
make -C /usr/ports/devel/ccache clean all MAKE_CMD=fmake
make -C /usr/ports/devel/ccache clean all MAKE_CMD=gmake


-- 
Regards,
Bryan Drewery
Comment 10 info 2014-03-02 16:27:38 UTC
You're right only bmake and fmake don't show the colours...
I never played with the different "makes" and so could not see the
colours when using something like portmaster and the standard make
configuration.

I have no idea how to fix this though...

--
Olivier
interfaSys sàrl

On 02/03/2014 17:08, Bryan Drewery wrote:
> On 3/2/2014 10:03 AM, interfaSys sàrl wrote:
>> It does work for me if I modify the color_output_possible() function,
>> but I'm using GCC.
>> I've learned from this thread that isatty() is not reliable within ccache:
>> https://bugzilla.mozilla.org/show_bug.cgi?id=777952#c3
>>
>> So I've modified
>> return isatty(STDERR_FILENO) && term_env && strcasecmp(term_env, "DUMB")
>> != 0;
>> to
>> return term_env && strcasecmp(term_env, "DUMB") != 0;
>>
>> and I get colours when compiling ports with parallel jobs and
>> customising them with GCC_COLOURS does work.
>>
>> I have not tested with Clang though, so I'm not sure if it makes a
>> difference there.
>>
> 
> Hmm, the check seems to make sense though. Perhaps bmake/fmake are not
> hooking up stderr correctly. gmake does work as I expect here.
> 
> You can see with building ccache itself:
> 
> make -C /usr/ports/devel/ccache clean all MAKE_CMD=bmake
> make -C /usr/ports/devel/ccache clean all MAKE_CMD=fmake
> make -C /usr/ports/devel/ccache clean all MAKE_CMD=gmake
> 
>
Comment 11 Bryan Drewery freebsd_committer freebsd_triage 2014-03-02 16:28:31 UTC
On 3/2/2014 10:27 AM, interfaSys sàrl wrote:
> You're right only bmake and fmake don't show the colours...
> I never played with the different "makes" and so could not see the
> colours when using something like portmaster and the standard make
> configuration.
> 
> I have no idea how to fix this though...
> 


I am looking at bmake source to see if this is fixable there.

-- 
Regards,
Bryan Drewery