threads / patch / 4608

patchPass -DDEFAULT_GIT_TEMPLATE_DIR only where actually used.

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

## tl;dr

8 messages between Jun 22, 2006 and Jun 22, 2006. Diffs are folded; open one to read it.

replies: 7people: 4as markdown or json

Yakov Lerner· Jun 22, 2006, 01:47 UTC · lore

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(-)
Show changes to Makefile +2 −1
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
Junio C Hamano· Jun 22, 2006, 07:19 UTC · re: Yakov Lerner · lore

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

Yakov Lerner <iler.ml@gmail.com> writes:
Show 5 quoted lines
> 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?
Timo Hirvonen· Jun 22, 2006, 07:49 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <junkio@cox.net> wrote:
Show 16 quoted lines
> 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.
Show 5 quoted lines
> 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/
Junio C Hamano· Jun 22, 2006, 09:04 UTC · re: Timo Hirvonen · lore

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

Timo Hirvonen <tihirvon@gmail.com> writes:
Show 17 quoted lines
> 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
---
Show changes to Makefile +1 −1
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
Petr Baudis· Jun 22, 2006, 13:12 UTC · re: Junio C Hamano · lore

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

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...

Show 5 quoted lines
> 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.
Junio C Hamano· Jun 22, 2006, 17:11 UTC · re: Petr Baudis · lore

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

Petr Baudis <pasky@suse.cz> writes:
Show 9 quoted lines
> 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.

Petr Baudis· Jun 22, 2006, 17:58 UTC · re: Junio C Hamano · lore

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

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...

Show 14 quoted lines
> 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.
Junio C Hamano· Jun 22, 2006, 18:58 UTC · re: Petr Baudis · lore

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

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.

← back to recent threads