[PATCH] Making CFLAGS compilant with GNU Coding Standards

[PATCH] Making CFLAGS compilant with GNU Coding Standards

[Pavel Roskin]

Hello!

Quoting GNU Coding Standards ("info standards"):

"If there are C compiler options that _must_ be used for proper
compilation of certain files, do not include them in `CFLAGS'.  Users
expect to be able to specify `CFLAGS' freely themselves."

This patch renames COPTS to CFLAGS, because it's COPTS that was user
overridable.  Also, -Wall is moved there because it's optional.  What
was CFLAGS is now ALL_CFLAGS, which users should not override.

Defines are added to DEFINES.  Since ALL_CFLAGS is recursively expanded,
it uses the final value of DEFINES.

Implicit rules are made explicit since the implicit rules use CFLAGS
rather than ALL_CFLAGS.  I believe that serious projects should not rely
on implicit rules anyway.  Percent rules are used because they are used
already and because they don't need the .SUFFIXES target.

Signed-off-by: Pavel Roskin <proski@gnu.org>

diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -34,8 +34,8 @@
 
 GIT_VERSION=0.99.3
 
-COPTS?=-g -O2
-CFLAGS+=$(COPTS) -Wall $(DEFINES)
+CFLAGS ?= -g -O2 -Wall
+ALL_CFLAGS = $(CFLAGS) $(DEFINES)
 
 prefix=$(HOME)
 bindir=$(prefix)/bin
@@ -125,7 +125,7 @@ ifndef NO_OPENSSL
 	LIB_OBJS += epoch.o
 	OPENSSL_LIBSSL=-lssl
 else
-	CFLAGS += '-DNO_OPENSSL'
+	DEFINES += '-DNO_OPENSSL'
 	MOZILLA_SHA1=1
 	OPENSSL_LIBSSL=
 endif
@@ -146,8 +146,8 @@ endif
 endif
 endif
 
-CFLAGS += '-DSHA1_HEADER=$(SHA1_HEADER)'
-CFLAGS += '-DDEFAULT_GIT_TEMPLATE_ENVIRONMENT="$(etcgitdir)/templates"'
+DEFINES += '-DSHA1_HEADER=$(SHA1_HEADER)'
+DEFINES += '-DDEFAULT_GIT_TEMPLATE_ENVIRONMENT="$(etcgitdir)/templates"'
 
 
 
@@ -156,9 +156,15 @@ CFLAGS += '-DDEFAULT_GIT_TEMPLATE_ENVIRO
 all: $(PROG)
 
 
+%.o: %.c
+	$(CC) -c $(ALL_CFLAGS) $<
+
+%.o: %.S
+	$(CC) -c $(ALL_CFLAGS) $<
+
 .PRECIOUS: %.o
 git-%: %.o $(LIB_FILE)
-	$(CC) $(CFLAGS) -o $@ $(filter %.o,$^) $(LIBS)
+	$(CC) $(ALL_CFLAGS) -o $@ $(filter %.o,$^) $(LIBS)
 
 git-http-pull: pull.o
 git-local-pull: pull.o
@@ -185,13 +191,13 @@ test: all
 	$(MAKE) -C t/ all
 
 test-date: test-date.c date.o
-	$(CC) $(CFLAGS) -o $@ test-date.c date.o
+	$(CC) $(ALL_CFLAGS) -o $@ test-date.c date.o
 
 test-delta: test-delta.c diff-delta.o patch-delta.o
-	$(CC) $(CFLAGS) -o $@ $^
+	$(CC) $(ALL_CFLAGS) -o $@ $^
 
 check:
-	for i in *.c; do sparse $(CFLAGS) $(SPARSE_FLAGS) $$i; done
+	for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i; done
 
 
 

-- 
Regards,
Pavel Roskin

Re: [PATCH] Making CFLAGS compilant with GNU Coding Standards

[Junio C Hamano]

Pavel Roskin <proski@gnu.org> writes:

> Quoting GNU Coding Standards ("info standards"):
>
> "If there are C compiler options that _must_ be used for proper
> compilation of certain files, do not include them in `CFLAGS'.  Users
> expect to be able to specify `CFLAGS' freely themselves."

Quoting Documentation/CodingStyle:

    This is a short document describing the preferred coding style for the
    linux kernel.  Coding style is very personal, and I won't _force_ my
    views on anybody, but this is what goes for anything that I have to be
    able to maintain, and I'd prefer it for most other things too.  Please
    at least consider the points made here.

    First off, I'd suggest printing out a copy of the GNU coding
    standards, and NOT read it.  Burn them, it's a great symbolic
    gesture.

This was only a tongue-in-cheek comment ;-).

Seriously, I do not intend to discarded this patch, but I am
currently not taking Makefile patches unless they fix real
breakage.

I do want to revisit Makefile issues after 0.99.4, along with
the changes Pasky sent several days ago.  Please remind me about
them after Wednesday.

Re: [PATCH] Making CFLAGS compilant with GNU Coding Standards

[Ryan Anderson]

On Mon, Aug 08, 2005 at 01:53:24PM -0700, Junio C Hamano wrote:
> I do want to revisit Makefile issues after 0.99.4, along with
> the changes Pasky sent several days ago.  Please remind me about
> them after Wednesday.

We should discuss (or just do) a reorganization of how we lay out the
source tree.

There's probably a few too many files in the top directory, from all the
porcelain we've built on the plumbing. (heh)

I haven't really given a lot of thought to this yet, but I was thinking
of something along these lines:

Rename tools/ to mail-tools/, push git-send-email-script into that.
Create a "porcelain" directory for things like git-bisect-script
Create a "core" directory for things like "git-commit-script"
Create a "src" (?) directory for *.[ch]
Maybe move documentation into the same directory as the scripts they
belong to.

Then we can argue over where the leftover things fall.

If we can get the directory structure to make sense and start putting
new things into the right spots, the exact location of where *current*
things end up is probably not so important.

I consider this post 1.0 material, personally.

-- 

Ryan Anderson
  sometimes Pug Majere

Re: [PATCH] Making CFLAGS compilant with GNU Coding Standards

[Johannes Schindelin]

Hi,

On Mon, 8 Aug 2005, Ryan Anderson wrote:

> [...]
>
> We should discuss (or just do) a reorganization of how we lay out the
> source tree.
>
> [...]

I, for one, like the layout. There are not yet enough files to merit a 
hierarchy, and what's more: I can add the source-directory to my PATH and 
be done by compiling every time I pull.

But you're right in any case, it is post 1.0 discussion material.

Ciao,
Dscho

Re: [PATCH] Making CFLAGS compilant with GNU Coding Standards

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> I, for one, like the layout. There are not yet enough files to merit a 
> hierarchy, and what's more: I can add the source-directory to my PATH and 
> be done by compiling every time I pull.
>
> But you're right in any case, it is post 1.0 discussion material.

For quite some time, I've been wanting to install some of the
stuff under /usr/lib{,exec}/git-core/, but that is a post 1.0
material.  I do not care much about the way we organize the
sources at this stage.

Johannes, could you sanity check the commit change I have in the
proposed updates branch when things percolate through kernel.org
mirror network?  I ended up pushing it out before I found you
already did a patch.

Sanity check of git-commit patch, was Re: [PATCH] Making CFLAGS compilant with GNU Coding Standards

[Johannes Schindelin]

Hi,

On Mon, 8 Aug 2005, Junio C Hamano wrote:

> Johannes, could you sanity check the commit change I have in the
> proposed updates branch when things percolate through kernel.org
> mirror network?  I ended up pushing it out before I found you
> already did a patch.

I just checked. Three nitpicks:

- I don't like the GNU way to abbreviate long options too much. Either you 
  know exactly what you're doing, and you use the short form, or you want 
  to make sure that you have spelt it correctly. Besides, it makes the 
  code less readable and more prone to errors.

- Multiple -m options was actually a feature of my version of the patch.

- The "case .. in x) .. ;; esac;" construct is sometimes more confusing 
  than a simple "if", or even a "[ .. = x ] && ..".

As I said, just nitpicks. I didn't test, though. I'll do that when the 
commit is in the master branch :-) Maybe I'll even write my first git test 
case, who knows?

Ciao,
Dscho

Re: Sanity check of git-commit patch, was Re: [PATCH] Making CFLAGS compilant with GNU Coding Standards

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> I just checked. Three nitpicks:
>
> - I don't like the GNU way to abbreviate long options too much...

True.  My bad old habit.

> - Multiple -m options was actually a feature of my version of the patch.

Ah, OK.

> - The "case .. in x) .. ;; esac;" construct is sometimes more confusing 
>   than a simple "if", or even a "[ .. = x ] && ..".

Yes sometimes.

Re: [PATCH] Making CFLAGS compilant with GNU Coding Standards

[Junio C Hamano]

Ryan Anderson <ryan@michonline.com> writes:

> I haven't really given a lot of thought to this yet, but I was thinking
> of something along these lines:
>
> Rename tools/ to mail-tools/, push git-send-email-script into that.
> Create a "porcelain" directory for things like git-bisect-script
> Create a "core" directory for things like "git-commit-script"
> Create a "src" (?) directory for *.[ch]
> Maybe move documentation into the same directory as the scripts they
> belong to.

A good place to start may be to reorganize the categorized list
of commands in Documentation/git.txt.

Re: Sanity check of git-commit patch, was Re: [PATCH] Making CFLAGS compilant with GNU Coding Standards

[Johannes Schindelin]

Hi,

On Tue, 9 Aug 2005, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > - I don't like the GNU way to abbreviate long options too much...
> 
> True.  My bad old habit.

An elegant method to do that:

case --some-long-option in "$1"*) ..; esac

Ciao,
Dscho

Re: Sanity check of git-commit patch, was Re: [PATCH] Making CFLAGS compilant with GNU Coding Standards

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> True.  My bad old habit.
>
> An elegant method to do that:
>
> case --some-long-option in "$1"*) ..; esac

You are almost correct, but you need to realize that I generate
that long "case -s|--s|--so|--som|..." chain using a script that
takes all potential option names as its arguments, and makes
case arms that contain only unambiguous ones, so that I can
handle --some-long-option and --some-other-long-option sensibly.

Also you forgot to grok --some-option-with-args=* in your
version ;-).

Re: Sanity check of git-commit patch, was Re: [PATCH] Making CFLAGS compilant with GNU Coding Standards

[Horst von Brand]

Junio C Hamano <junkio@cox.net> wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> True.  My bad old habit.
> >
> > An elegant method to do that:
> >
> > case --some-long-option in "$1"*) ..; esac
> 
> You are almost correct, but you need to realize that I generate
> that long "case -s|--s|--so|--som|..." chain using a script that
> takes all potential option names as its arguments, and makes
> case arms that contain only unambiguous ones, so that I can
> handle --some-long-option and --some-other-long-option sensibly.

My head spins....

Isn't it easier to just use getopt(1)?

> Also you forgot to grok --some-option-with-args=* in your
> version ;-).

Please stop! I'm dizzy already!
-- 
Dr. Horst H. von Brand                   User #22616 counter.li.org
Departamento de Informatica                     Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria              +56 32 654239
Casilla 110-V, Valparaiso, Chile                Fax:  +56 32 797513

Re: Sanity check of git-commit patch, was Re: [PATCH] Making CFLAGS compilant with GNU Coding Standards

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

>> - Multiple -m options was actually a feature of my version of the patch.
>
> Ah, OK.

I said "OK" and thought about it again.  While thinking about
what is the right inter-message-piece separator for multiple -m
parameters (you use "\n"), I got a headache.  I decided to keep
what is already there after all ;-).

I'll push things out tonight and expect to tag things tomorrow
late afternoon to declare 0.99.4

Re: Sanity check of git-commit patch, was Re: [PATCH] Making CFLAGS compilant with GNU Coding Standards

[Johannes Schindelin]

Hi,

On Tue, 9 Aug 2005, Horst von Brand wrote:

> Isn't it easier to just use getopt(1)?

Only if GNU getopt is available. On Mac OS X, only the BSD version is 
installed by default, which does not handle long options at all.

> Please stop! I'm dizzy already!

:-)

Ciao,
Dscho

Re: [PATCH] Making CFLAGS compilant with GNU Coding Standards

[Pavel Roskin]

Hi, Junio!

On Mon, 2005-08-08 at 13:53 -0700, Junio C Hamano wrote:
> Seriously, I do not intend to discarded this patch, but I am
> currently not taking Makefile patches unless they fix real
> breakage.

Try "make CFLAGS=-O3"

> I do want to revisit Makefile issues after 0.99.4, along with
> the changes Pasky sent several days ago.  Please remind me about
> them after Wednesday.

It's "after Wednesday" and 0.99.4 is out (many thanks).

-- 
Regards,
Pavel Roskin