[PATCH] Beautify all & install Makefile targets

[PATCH] Beautify all & install Makefile targets

[Damien Lespiau]

Beautify make's output like kbuild or git. The install
target is especially much more readable.
make V=1 (install) to get back to the old behaviour.

Signed-off-by: Damien Lespiau <damien.lespiau@gmail.com>
---
 Makefile |   72 ++++++++++++++++++++++++++++++++++++++-----------------------
 1 files changed, 45 insertions(+), 27 deletions(-)

diff --git a/Makefile b/Makefile
index 77a6877..bacfba0 100644
--- a/Makefile
+++ b/Makefile
@@ -37,68 +37,83 @@ SLIB_FILE= libsparse.so
 
 LIBS=$(LIB_FILE)
 
+#
+# Pretty print
+#
+ifneq ($(findstring $(MAKEFLAGS),s),s)
+ifndef V
+	Q              = @
+	QUIET_CC       = @echo    '   ' CC'      '$@;
+	QUIET_AR       = @echo    '   ' AR'      '$@;
+	QUIET_GEN      = @echo    '   ' GEN'     '$@;
+	QUIET_LD       = @echo    '   ' LD'      '$@;
+	QUIET_INST_SH  = echo -n  '   ' INSTALL '';
+	QUIET_INST     = @echo -n '   ' INSTALL '';
+endif
+endif
+
 all: $(PROGRAMS) sparse.pc
 
 install: $(INST_PROGRAMS) $(LIBS) $(LIB_H) sparse.pc
-	install -d $(DESTDIR)$(BINDIR)
-	install -d $(DESTDIR)$(LIBDIR)
-	install -d $(DESTDIR)$(INCLUDEDIR)/sparse
-	install -d $(DESTDIR)$(PKGCONFIGDIR)
-	for f in $(INST_PROGRAMS); do \
-		install -v $$f $(DESTDIR)$(BINDIR)/$$f || exit 1; \
+	$(Q)install -d $(DESTDIR)$(BINDIR)
+	$(Q)install -d $(DESTDIR)$(LIBDIR)
+	$(Q)install -d $(DESTDIR)$(INCLUDEDIR)/sparse
+	$(Q)install -d $(DESTDIR)$(PKGCONFIGDIR)
+	$(Q)for f in $(INST_PROGRAMS); do \
+		$(QUIET_INST_SH)install -v $$f $(DESTDIR)$(BINDIR)/$$f || exit 1; \
 	done
-	for f in $(LIBS); do \
-		install -m 644 -v $$f $(DESTDIR)$(LIBDIR)/$$f || exit 1; \
+	$(Q)for f in $(LIBS); do \
+		$(QUIET_INST_SH)install -m 644 -v $$f $(DESTDIR)$(LIBDIR)/$$f || exit 1; \
 	done
-	for f in $(LIB_H); do \
-		install -m 644 -v $$f $(DESTDIR)$(INCLUDEDIR)/sparse/$$f || exit 1; \
+	$(Q)for f in $(LIB_H); do \
+		$(QUIET_INST_SH)install -m 644 -v $$f $(DESTDIR)$(INCLUDEDIR)/sparse/$$f || exit 1; \
 	done
-	install -m 644 -v sparse.pc $(DESTDIR)$(PKGCONFIGDIR)/sparse.pc
+	$(QUIET_INST)install -m 644 -v sparse.pc $(DESTDIR)$(PKGCONFIGDIR)/sparse.pc
 
 sparse.pc: sparse.pc.in
-	sed 's|@version@|$(VERSION)|g;s|@prefix@|$(PREFIX)|g;s|@libdir@|$(LIBDIR)|g;s|@includedir@|$(INCLUDEDIR)|g' sparse.pc.in > sparse.pc
+	$(QUIET_GEN)sed 's|@version@|$(VERSION)|g;s|@prefix@|$(PREFIX)|g;s|@libdir@|$(LIBDIR)|g;s|@includedir@|$(INCLUDEDIR)|g' sparse.pc.in > sparse.pc
 
 test-lexing: test-lexing.o $(LIBS)
-	$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
+	$(QUIET_LD)$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
 
 test-parsing: test-parsing.o $(LIBS)
-	$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
+	$(QUIET_LD)$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
 
 test-linearize: test-linearize.o $(LIBS)
-	$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
+	$(QUIET_LD)$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
 
 test-sort: test-sort.o $(LIBS)
-	$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
+	$(QUIET_LD)$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
 
 compile: compile.o compile-i386.o $(LIBS)
-	$(CC) $(LDFLAGS) -o $@ $< compile-i386.o $(LIBS)
+	$(QUIET_LD)$(CC) $(LDFLAGS) -o $@ $< compile-i386.o $(LIBS)
 
 obfuscate: obfuscate.o $(LIBS)
-	$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
+	$(QUIET_LD)$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
 
 sparse: sparse.o $(LIBS)
-	$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
+	$(QUIET_LD)$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
 
 graph: graph.o $(LIBS)
-	$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
+	$(QUIET_LD)$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
 
 example: example.o $(LIBS)
-	$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
+	$(QUIET_LD)$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
 
 test-unssa: test-unssa.o $(LIBS)
-	$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
+	$(QUIET_LD)$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
 
 test-dissect: test-dissect.o $(LIBS)
-	$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
+	$(QUIET_LD)$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
 
 ctags: ctags.o $(LIBS)
-	$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
+	$(QUIET_LD)$(CC) $(LDFLAGS) -o $@ $< $(LIBS)
 
 $(LIB_FILE): $(LIB_OBJS)
-	$(AR) rcs $@ $(LIB_OBJS)
+	$(QUIET_AR)$(AR) rcs $@ $(LIB_OBJS)
 
 $(SLIB_FILE): $(LIB_OBJS)
-	$(CC) -shared -o $@ $(LIB_OBJS)
+	$(QUIET_LD)$(CC) -shared -o $@ $(LIB_OBJS)
 
 evaluate.o: $(LIB_H)
 expression.o: $(LIB_H)
@@ -141,7 +156,10 @@ compat-mingw.o: $(LIB_H)
 compat-cygwin.o: $(LIB_H)
 
 pre-process.h:
-	echo "#define GCC_INTERNAL_INCLUDE \"`$(CC) -print-file-name=include`\"" > pre-process.h
+	$(QUIET_GEN)echo "#define GCC_INTERNAL_INCLUDE \"`$(CC) -print-file-name=include`\"" > pre-process.h
+
+%.o: %.c
+	$(QUIET_CC)$(CC) -o $*.o -c $(CFLAGS) $<
 
 clean:
 	rm -f *.[oasi] core core.[0-9]* $(PROGRAMS) $(SLIB_FILE) pre-process.h sparse.pc
-- 
1.5.2.rc3.87.g404fd

[PATCH] __DATE__ & __TIME expansion

[Damien Lespiau]

Makes __DATE__expand to a character string literal of
the form "Mmm dd yyyy" where the names of the months
are the same as those generated by the asctime function,
and the first character of dd is a space character if the
value is less than 10.
Makes __TIME__ expand to a a character string literal of
the form "hh:mm:ss" as in the time generated by the
asctime function.

So says C99.

Signed-off-by: Damien Lespiau <damien.lespiau@gmail.com>
---
 ident-list.h                        |    2 ++
 lib.c                               |    2 --
 pre-process.c                       |   11 +++++++++++
 validation/preprocessor-date-time.c |   22 ++++++++++++++++++++++
 4 files changed, 35 insertions(+), 2 deletions(-)
 create mode 100644 validation/preprocessor-date-time.c

diff --git a/ident-list.h b/ident-list.h
index 7633a2f..dad22ef 100644
--- a/ident-list.h
+++ b/ident-list.h
@@ -75,6 +75,8 @@ __IDENT(pragma_ident, "__pragma__", 0);
 __IDENT(__VA_ARGS___ident, "__VA_ARGS__", 0);
 __IDENT(__LINE___ident, "__LINE__", 0);
 __IDENT(__FILE___ident, "__FILE__", 0);
+__IDENT(__DATE___ident, "__DATE__", 0);
+__IDENT(__TIME___ident, "__TIME__", 0);
 __IDENT(__func___ident, "__func__", 0);
 __IDENT(__FUNCTION___ident, "__FUNCTION__", 0);
 __IDENT(__PRETTY_FUNCTION___ident, "__PRETTY_FUNCTION__", 0);
diff --git a/lib.c b/lib.c
index 2162007..b678990 100644
--- a/lib.c
+++ b/lib.c
@@ -602,8 +602,6 @@ void create_builtin_stream(void)
 
 	/* FIXME! We need to do these as special magic macros at expansion time! */
 	add_pre_buffer("#define __BASE_FILE__ \"base_file.c\"\n");
-	add_pre_buffer("#define __DATE__ \"??? ?? ????\"\n");
-	add_pre_buffer("#define __TIME__ \"??:??:??\"\n");
 
 	if (optimize)
 		add_pre_buffer("#define __OPTIMIZE__ 1\n");
diff --git a/pre-process.c b/pre-process.c
index afae77a..eb291f2 100644
--- a/pre-process.c
+++ b/pre-process.c
@@ -18,6 +18,7 @@
 #include <unistd.h>
 #include <fcntl.h>
 #include <limits.h>
+#include <time.h>
 
 #include "pre-process.h"
 #include "lib.h"
@@ -141,6 +142,8 @@ static int expand_one_symbol(struct token **list)
 {
 	struct token *token = *list;
 	struct symbol *sym;
+	static char buffer[12]; /* __DATE__: 3 + ' ' + 2 + ' ' + 4 + '\0' */
+	time_t t;
 
 	if (token->pos.noexpand)
 		return 1;
@@ -154,6 +157,14 @@ static int expand_one_symbol(struct token **list)
 		replace_with_integer(token, token->pos.line);
 	} else if (token->ident == &__FILE___ident) {
 		replace_with_string(token, stream_name(token->pos.stream));
+	} else if (token->ident == &__DATE___ident) {
+		time(&t);
+		strftime(buffer, 12, "%b %e %Y", localtime(&t));
+		replace_with_string(token, buffer);
+	} else if (token->ident == &__TIME___ident) {
+		time(&t);
+		strftime(buffer, 9, "%T", localtime(&t));
+		replace_with_string(token, buffer);
 	}
 	return 1;
 }
diff --git a/validation/preprocessor-date-time.c b/validation/preprocessor-date-time.c
new file mode 100644
index 0000000..18d8bdb
--- /dev/null
+++ b/validation/preprocessor-date-time.c
@@ -0,0 +1,22 @@
+/*
+ * __DATE__ & __TIME__ macros handling test.
+ */
+
+/*
+ * Testing it against GCC preprocessor in the same second gives:
+ *
+ * $ gcc -E validation/preprocessor-date-time.c 
+ * # 1 "validation/preprocessor-date-time.c"
+ * # 1 "<built-in>"
+ * # 1 "<command line>"
+ * # 1 "validation/preprocessor-date-time.c"
+ * # 11 "validation/preprocessor-date-time.c"
+ * "May 20 2007" "20:19:00"
+ * $ ./cgcc -E validation/preprocessor-date-time.c 
+ * 
+ * "May 20 2007" "20:19:00"
+ * $
+*/
+
+__DATE__ __TIME__
+
-- 
1.5.2.rc3.87.g404fd

Re: [PATCH] __DATE__ & __TIME expansion

[Neil Booth]

Damien Lespiau wrote:-

> Makes __DATE__expand to a character string literal of
> the form "Mmm dd yyyy" where the names of the months
> are the same as those generated by the asctime function,
> and the first character of dd is a space character if the
> value is less than 10.
> Makes __TIME__ expand to a a character string literal of
> the form "hh:mm:ss" as in the time generated by the
> asctime function.
> 
> So says C99.
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@gmail.com>

It seems your code could lead to these macros having different
values in a long compilation, which the standard doesn't permit.

Neil.

Re: [PATCH] __DATE__ & __TIME expansion

[Josh Triplett]

[-- Attachment #1: Type: text/plain, Size: 922 bytes --]

Damien Lespiau wrote:
> Makes __DATE__expand to a character string literal of
> the form "Mmm dd yyyy" where the names of the months
> are the same as those generated by the asctime function,
> and the first character of dd is a space character if the
> value is less than 10.
> Makes __TIME__ expand to a a character string literal of
> the form "hh:mm:ss" as in the time generated by the
> asctime function.

Thanks for this patch.  I think I'd prefer to avoid calling time more than
once per run of the program.  __DATE__ and __TIME__ seem sufficiently rare
(ideally at most once each per project anyway) that avoiding repeated calls to
strftime probably doesn't matter; however, avoiding repeated calls to time
seems useful, even if it only avoids the one extra call when you use __DATE__
and __TIME__ once each.

Could you make the time_t static and only call time once?

Thanks,
Josh Triplett


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

Re: [PATCH] __DATE__ & __TIME expansion

[Josh Triplett]

[-- Attachment #1: Type: text/plain, Size: 898 bytes --]

Neil Booth wrote:
> Damien Lespiau wrote:-
> 
>> Makes __DATE__expand to a character string literal of
>> the form "Mmm dd yyyy" where the names of the months
>> are the same as those generated by the asctime function,
>> and the first character of dd is a space character if the
>> value is less than 10.
>> Makes __TIME__ expand to a a character string literal of
>> the form "hh:mm:ss" as in the time generated by the
>> asctime function.
>>
>> So says C99.
>>
>> Signed-off-by: Damien Lespiau <damien.lespiau@gmail.com>
> 
> It seems your code could lead to these macros having different
> values in a long compilation, which the standard doesn't permit.

Ah, I didn't know that.  So calling time more than once doesn't just slow down
compilation; it also violates the standard.  I feel better about asking for a
fix to only call time once, now. :)

- Josh Triplett



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

Re: [PATCH] __DATE__ & __TIME expansion

[Damien Lespiau]

> Neil Booth wrote:
> > It seems your code could lead to these macros having different
> > values in a long compilation, which the standard doesn't permit.

Ouch, I didn't know that either. I learned to be humble before C. Still, I had
a quick look at the C99 standard and couldn't find it explicitly. No wonder
why I didn't know it in the first place.

On 5/21/07, Josh Triplett <josh@freedesktop.org> wrote:
> I feel better about asking for a fix to only call time once, now. :)

Don't be afraid to hurt my feelings, I still have a lot to learn and I know it
well :p

-- 
Damien Lespiau

Re: [PATCH] __DATE__ & __TIME expansion

[Derek M Jones]

Damien,

>> > It seems your code could lead to these macros having different
>> > values in a long compilation, which the standard doesn't permit.
> 
> Ouch, I didn't know that either. I learned to be humble before C. Still, 
> I had
> a quick look at the C99 standard and couldn't find it explicitly. No wonder
> why I didn't know it in the first place.

Sentence 2004: http://c0x.coding-guidelines.com/6.10.8.html

-- 
Derek M. Jones                              tel: +44 (0) 1252 520 667
Knowledge Software Ltd                      mailto:derek@knosof.co.uk
Applications Standards Conformance Testing    http://www.knosof.co.uk

Re: [PATCH] Beautify all & install Makefile targets

[Josh Triplett]

[-- Attachment #1: Type: text/plain, Size: 1836 bytes --]

Damien Lespiau wrote:
> Beautify make's output like kbuild or git. The install
> target is especially much more readable.
> make V=1 (install) to get back to the old behaviour.

The Makefile currently doesn't require anything particularly special from
make, and in theory should work with any reasonable make, not just GNU make.
I hestitate to change that for purely aesthetic purposes.  The % rules seem
easy enough to replace with a suffix rule .c.o.  You can use substitution to
check V, like this:

josh@josh-mobile:/tmp$ cat Makefile 
V=@
Q=$(V:1=)
QUIET_SOMETHING=$(Q:@=@echo SOMETHING;)
#...

test:
        @echo V=$(V)
        @echo Q=$(Q)
        @echo QUIET_SOMETHING=$(QUIET_SOMETHING)
josh@josh-mobile:/tmp$ make
V=@
Q=@
QUIET_SOMETHING=@echo SOMETHING
josh@josh-mobile:/tmp$ make V=1
V=1
Q=
QUIET_SOMETHING=

You can write more rules of the form QUIET_SOMETHING=$(Q:@=value if quiet)

I'd also like to hear from other people on whether or not they prefer this
style of output.  It doesn't matter much to me either way.

> +	QUIET_LD       = @echo    '   ' LD'      '$@;

This always seems misleading to me, given that the corresponding rules run CC,
not LD.

> +	QUIET_INST_SH  = echo -n  '   ' INSTALL '';
> +	QUIET_INST     = @echo -n '   ' INSTALL '';

This doesn't appear to output the installed file; it took a minute to figure
out that install -v provides the output.  Could you add a comment about that?
Also, why not write these strings as (for example):

echo -n '    INSTALL '

? That seems cleaner than breaking the lines up into quoted whitespace and
unquoted text.

> +%.o: %.c
> +	$(QUIET_CC)$(CC) -o $*.o -c $(CFLAGS) $<

I think you can use $@ here rather than $*.o .  Also, as mentioned above, this
should become a suffix rule.

- Josh Triplett


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

Re: [PATCH] Beautify all & install Makefile targets

[Damien Lespiau]

> I'd also like to hear from other people on whether or not they prefer this
> style of output.  It doesn't matter much to me either way.

That won't prevent me from resending a more decent patch :p

> > +     QUIET_LD       = @echo    '   ' LD'      '$@;
>
> This always seems misleading to me, given that the corresponding rules run CC,
> not LD.

Well, calling the linker through gcc front-end does not change the fact
that we are linking. To me CC or LD are just saying we are compiling
or linking.

I'll resend a patch, trying to take care of your comments.

-- 
Damien