[PATCH] Pass -DDEFAULT_GIT_TEMPLATE_DIR only where actually used.

[PATCH] Pass -DDEFAULT_GIT_TEMPLATE_DIR only where actually used.

[Yakov Lerner]

Before this patch, -DDEFAULT_GIT_TEMPLATE_DIR was passed on compilation
command line to all and every %c compiled. In fact the macro
is used by only one .c file, and unused by all other .c files.
Remove -DDEFAULT_GIT_TEMPLATE_DIR where unused. Follow the examlpe of 
exec_cmd.o. Pass -DDEFAULT_GIT_TEMPLATE_DIR only where actually used. 

Signed-off-by: Yakov Lerner <iler.ml@gmail.com>
---
 Makefile |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index c602099..7af0937 100644
--- a/Makefile
+++ b/Makefile
@@ -472,7 +472,6 @@ PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON
 GIT_PYTHON_DIR_SQ = $(subst ','\'',$(GIT_PYTHON_DIR))
 
 ALL_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' $(COMPAT_CFLAGS)
-ALL_CFLAGS += -DDEFAULT_GIT_TEMPLATE_DIR='"$(template_dir_SQ)"'
 LIB_OBJS += $(COMPAT_OBJS)
 export prefix TAR INSTALL DESTDIR SHELL_PATH template_dir
 ### Build rules
@@ -548,6 +547,8 @@ git$X git.spec \
 
 exec_cmd.o: exec_cmd.c GIT-CFLAGS
 	$(CC) -o $*.o -c $(ALL_CFLAGS) '-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' $<
+builtin-init-db.o: builtin-init-db.c GIT-CFLAGS
+	$(CC) -o $*.o -c $(ALL_CFLAGS) -DDEFAULT_GIT_TEMPLATE_DIR='"$(template_dir_SQ)"' $<
 
 http.o: http.c GIT-CFLAGS
 	$(CC) -o $*.o -c $(ALL_CFLAGS) -DGIT_USER_AGENT='"git/$(GIT_VERSION)"' $<
-- 
1.4.0.ga40a

Re: [PATCH] Pass -DDEFAULT_GIT_TEMPLATE_DIR only where actually used.

[Junio C Hamano]

Yakov Lerner <iler.ml@gmail.com> writes:

> Before this patch, -DDEFAULT_GIT_TEMPLATE_DIR was passed on compilation
> command line to all and every %c compiled. In fact the macro
> is used by only one .c file, and unused by all other .c files.
> Remove -DDEFAULT_GIT_TEMPLATE_DIR where unused. Follow the examlpe of 
> exec_cmd.o. Pass -DDEFAULT_GIT_TEMPLATE_DIR only where actually used. 

Thanks.

By the way, I really started hating that we have $(GIT_VERSION)
in $(TRACK_CFLAGS).  Since the version string is tied to the
HEAD commit object name, having it in $(TRACK_CFLAGS) means that
every time I switch branches, make a new commit on top of the
current branch, or checkout-compile-and-then-make-local-change
sequence would force pretty much everything to be rebuilt.

Do you think of any downside if I remove it from the list of
symbols in TRACK_CFLAGS?

For that matter, I do not think tracking prefix_SQ makes much
sense since what matters are bindir, gitexecdir and template_dir
which are already covered, and prefix is merely a convenience to
set these three (four, counting GIT_PYTHON_DIR; we probably
should add it to TRACK_CFLAGS).

Thoughts?

Re: [PATCH] Pass -DDEFAULT_GIT_TEMPLATE_DIR only where actually used.

[Timo Hirvonen]

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

> Yakov Lerner <iler.ml@gmail.com> writes:
> 
> > Before this patch, -DDEFAULT_GIT_TEMPLATE_DIR was passed on compilation
> > command line to all and every %c compiled. In fact the macro
> > is used by only one .c file, and unused by all other .c files.
> > Remove -DDEFAULT_GIT_TEMPLATE_DIR where unused. Follow the examlpe of 
> > exec_cmd.o. Pass -DDEFAULT_GIT_TEMPLATE_DIR only where actually used. 
> 
> Thanks.
> 
> By the way, I really started hating that we have $(GIT_VERSION)
> in $(TRACK_CFLAGS).  Since the version string is tied to the
> HEAD commit object name, having it in $(TRACK_CFLAGS) means that
> every time I switch branches, make a new commit on top of the
> current branch, or checkout-compile-and-then-make-local-change
> sequence would force pretty much everything to be rebuilt.

I think this already makes sure git is compiled if version changes:

# These can record GIT_VERSION
git$X git.spec \
        $(patsubst %.sh,%,$(SCRIPT_SH)) \
        $(patsubst %.perl,%,$(SCRIPT_PERL)) \
        $(patsubst %.py,%,$(SCRIPT_PYTHON)) \
        : GIT-VERSION-FILE

$ git grep GIT_VERSION *.sh *.perl
git-send-email.perl:	my $gitversion = '@@GIT_VERSION@@';
git-send-email.perl:	if ($gitversion =~ m/..GIT_VERSION../) {

Only git-send-email and git needs to depend on GIT_VERSION.

> For that matter, I do not think tracking prefix_SQ makes much
> sense since what matters are bindir, gitexecdir and template_dir
> which are already covered, and prefix is merely a convenience to
> set these three (four, counting GIT_PYTHON_DIR; we probably
> should add it to TRACK_CFLAGS).

Yes, only ALL_CFLAGS, bindir, gitexecdir, template_dir and
GIT_PYTHON_DIR should be in TRACK_CFLAGS.

-- 
http://onion.dynserv.net/~timo/

Re: [PATCH] Pass -DDEFAULT_GIT_TEMPLATE_DIR only where actually used.

[Junio C Hamano]

Timo Hirvonen <tihirvon@gmail.com> writes:

> Junio C Hamano <junkio@cox.net> wrote:
>
>> By the way, I really started hating that we have $(GIT_VERSION)
>> in $(TRACK_CFLAGS).  Since the version string is tied to the
>> HEAD commit object name, having it in $(TRACK_CFLAGS) means that
>> every time I switch branches, make a new commit on top of the
>> current branch, or checkout-compile-and-then-make-local-change
>> sequence would force pretty much everything to be rebuilt.
>...
>> For that matter, I do not think tracking prefix_SQ makes much
>> sense since what matters are bindir, gitexecdir and template_dir
>> which are already covered, and prefix is merely a convenience to
>> set these three (four, counting GIT_PYTHON_DIR; we probably
>> should add it to TRACK_CFLAGS).
>
> Yes, only ALL_CFLAGS, bindir, gitexecdir, template_dir and
> GIT_PYTHON_DIR should be in TRACK_CFLAGS.

Exactly my thinking.  Let's do it like this, then.

-- >8 --
Makefile: do not force unneeded recompilation upon GIT_VERSION changes

---
diff --git a/Makefile b/Makefile
index 55a84d4..d45f3dc 100644
--- a/Makefile
+++ b/Makefile
@@ -612,7 +612,7 @@ tags:
 	find . -name '*.[hcS]' -print | xargs ctags -a
 
 ### Detect prefix changes
-TRACK_CFLAGS = $(subst ','\'',$(ALL_CFLAGS)):$(GIT_VERSION):\
+TRACK_CFLAGS = $(subst ','\'',$(ALL_CFLAGS)):$(GIT_PYTHON_DIR_SQ):\
              $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ)
 
 GIT-CFLAGS: .FORCE-GIT-CFLAGS

Re: [PATCH] Pass -DDEFAULT_GIT_TEMPLATE_DIR only where actually used.

[Petr Baudis]

Dear diary, on Thu, Jun 22, 2006 at 09:19:52AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> For that matter, I do not think tracking prefix_SQ makes much
> sense since what matters are bindir, gitexecdir and template_dir
> which are already covered, and prefix is merely a convenience to
> set these three (four, counting GIT_PYTHON_DIR; we probably
> should add it to TRACK_CFLAGS).

$(prefix) will be passed to perl/Makefile.PL.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
A person is just about as big as the things that make them angry.

Re: [PATCH] Pass -DDEFAULT_GIT_TEMPLATE_DIR only where actually used.

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> Dear diary, on Thu, Jun 22, 2006 at 09:19:52AM CEST, I got a letter
> where Junio C Hamano <junkio@cox.net> said that...
>> For that matter, I do not think tracking prefix_SQ makes much
>> sense since what matters are bindir, gitexecdir and template_dir
>> which are already covered, and prefix is merely a convenience to
>> set these three (four, counting GIT_PYTHON_DIR; we probably
>> should add it to TRACK_CFLAGS).
>
> $(prefix) will be passed to perl/Makefile.PL.

Then probably it shouldn't; instead we woulld probably want to
pass the moral equivalent of GIT_PYTHON_DIR.

Re: [PATCH] Pass -DDEFAULT_GIT_TEMPLATE_DIR only where actually used.

[Petr Baudis]

Dear diary, on Thu, Jun 22, 2006 at 07:11:49PM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Petr Baudis <pasky@suse.cz> writes:
> 
> > Dear diary, on Thu, Jun 22, 2006 at 09:19:52AM CEST, I got a letter
> > where Junio C Hamano <junkio@cox.net> said that...
> >> For that matter, I do not think tracking prefix_SQ makes much
> >> sense since what matters are bindir, gitexecdir and template_dir
> >> which are already covered, and prefix is merely a convenience to
> >> set these three (four, counting GIT_PYTHON_DIR; we probably
> >> should add it to TRACK_CFLAGS).
> >
> > $(prefix) will be passed to perl/Makefile.PL.
> 
> Then probably it shouldn't; instead we woulld probably want to
> pass the moral equivalent of GIT_PYTHON_DIR.

I'm not sure about what the rationale behind GIT_PYTHON_DIR was, but it
seems to be used only by a library supposedly internal to some Git
commands, while the Git.pm module should be available systemwide even
for non-Git applications, so it's really best to leave it to Perl where
to put it.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
A person is just about as big as the things that make them angry.

Re: [PATCH] Pass -DDEFAULT_GIT_TEMPLATE_DIR only where actually used.

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> ... while the Git.pm module should be available systemwide even
> for non-Git applications, so it's really best to leave it to Perl where
> to put it.

OK, if that is the intention passing prefix might be sensible.