* [PATCH] Makefile: don't include git version file on 'make clean' @ 2010-07-24 3:53 Lynn.Lin 2010-07-24 12:36 ` Ævar Arnfjörð Bjarmason 2010-07-25 18:49 ` Patch follow-up conventions (Re: [PATCH] Makefile: don't include git version file on 'make clean') Jonathan Nieder 0 siblings, 2 replies; 19+ messages in thread From: Lynn.Lin @ 2010-07-24 3:53 UTC (permalink / raw) To: git; +Cc: Lynn Lin From: Lynn Lin <Lynn.Lin@emc.com> --- Makefile | 4 +++- git-gui/Makefile | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index bc3c570..eb28b98 100644 --- a/Makefile +++ b/Makefile @@ -238,7 +238,9 @@ all:: GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN --include GIT-VERSION-FILE +ifneq "$(MAKECMDGOALS)" "clean" + -include GIT-VERSION-FILE +endif uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not') uname_M := $(shell sh -c 'uname -m 2>/dev/null || echo not') diff --git a/git-gui/Makefile b/git-gui/Makefile index 197b55e..91e1ea5 100644 --- a/git-gui/Makefile +++ b/git-gui/Makefile @@ -9,7 +9,9 @@ all:: GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN --include GIT-VERSION-FILE +ifneq "$(MAKECMDGOALS)" "clean" + -include GIT-VERSION-FILE +endif uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not') uname_O := $(shell sh -c 'uname -o 2>/dev/null || echo not') -- 1.7.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] Makefile: don't include git version file on 'make clean' 2010-07-24 3:53 [PATCH] Makefile: don't include git version file on 'make clean' Lynn.Lin @ 2010-07-24 12:36 ` Ævar Arnfjörð Bjarmason 2010-07-25 8:49 ` Kevin P. Fleming 2010-07-25 18:49 ` Patch follow-up conventions (Re: [PATCH] Makefile: don't include git version file on 'make clean') Jonathan Nieder 1 sibling, 1 reply; 19+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-24 12:36 UTC (permalink / raw) To: Lynn.Lin; +Cc: git On Sat, Jul 24, 2010 at 03:53, <Lynn.Lin@emc.com> wrote: > From: Lynn Lin <Lynn.Lin@emc.com> > > --- > Makefile | 4 +++- > git-gui/Makefile | 4 +++- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/Makefile b/Makefile > index bc3c570..eb28b98 100644 > --- a/Makefile > +++ b/Makefile > @@ -238,7 +238,9 @@ all:: > > GIT-VERSION-FILE: FORCE > @$(SHELL_PATH) ./GIT-VERSION-GEN > --include GIT-VERSION-FILE > +ifneq "$(MAKECMDGOALS)" "clean" > + -include GIT-VERSION-FILE > +endif > > uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not') > uname_M := $(shell sh -c 'uname -m 2>/dev/null || echo not') > diff --git a/git-gui/Makefile b/git-gui/Makefile > index 197b55e..91e1ea5 100644 > --- a/git-gui/Makefile > +++ b/git-gui/Makefile > @@ -9,7 +9,9 @@ all:: > > GIT-VERSION-FILE: FORCE > @$(SHELL_PATH) ./GIT-VERSION-GEN > --include GIT-VERSION-FILE > +ifneq "$(MAKECMDGOALS)" "clean" > + -include GIT-VERSION-FILE > +endif > > uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not') > uname_O := $(shell sh -c 'uname -o 2>/dev/null || echo not') > -- > 1.7.1 This patch needs a rationale, why was it needed? The "-include" directive will simply ignore files that don't exist (as opposed to "include"), so including GIT-VERSION-FILE during "make clean' shouldn't be an issue. Was it for you? And if so what version of make, what OS etc. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Makefile: don't include git version file on 'make clean' 2010-07-24 12:36 ` Ævar Arnfjörð Bjarmason @ 2010-07-25 8:49 ` Kevin P. Fleming 2010-07-25 11:28 ` lynn.lin 0 siblings, 1 reply; 19+ messages in thread From: Kevin P. Fleming @ 2010-07-25 8:49 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: Lynn.Lin, git On 07/24/2010 02:36 PM, Ævar Arnfjörð Bjarmason wrote: > On Sat, Jul 24, 2010 at 03:53, <Lynn.Lin@emc.com> wrote: >> From: Lynn Lin <Lynn.Lin@emc.com> >> >> --- >> Makefile | 4 +++- >> git-gui/Makefile | 4 +++- >> 2 files changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/Makefile b/Makefile >> index bc3c570..eb28b98 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -238,7 +238,9 @@ all:: >> >> GIT-VERSION-FILE: FORCE >> @$(SHELL_PATH) ./GIT-VERSION-GEN >> --include GIT-VERSION-FILE >> +ifneq "$(MAKECMDGOALS)" "clean" >> + -include GIT-VERSION-FILE >> +endif >> >> uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not') >> uname_M := $(shell sh -c 'uname -m 2>/dev/null || echo not') >> diff --git a/git-gui/Makefile b/git-gui/Makefile >> index 197b55e..91e1ea5 100644 >> --- a/git-gui/Makefile >> +++ b/git-gui/Makefile >> @@ -9,7 +9,9 @@ all:: >> >> GIT-VERSION-FILE: FORCE >> @$(SHELL_PATH) ./GIT-VERSION-GEN >> --include GIT-VERSION-FILE >> +ifneq "$(MAKECMDGOALS)" "clean" >> + -include GIT-VERSION-FILE >> +endif >> >> uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not') >> uname_O := $(shell sh -c 'uname -o 2>/dev/null || echo not') >> -- >> 1.7.1 > > This patch needs a rationale, why was it needed? The "-include" > directive will simply ignore files that don't exist (as opposed to > "include"), so including GIT-VERSION-FILE during "make clean' > shouldn't be an issue. Just guessing here, but since GIT-VERSION-FILE has a 'FORCE' prerequisite, that means that the operations to generate it will be run even for 'make clean', which is not useful for the cleaning operation. It's probably not harmful either... but maybe the OP has some more significant reason for this patch. -- Kevin P. Fleming Digium, Inc. | Director of Software Technologies 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA skype: kpfleming | jabber: kfleming@digium.com Check us out at www.digium.com & www.asterisk.org ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH] Makefile: don't include git version file on 'make clean' 2010-07-25 8:49 ` Kevin P. Fleming @ 2010-07-25 11:28 ` lynn.lin 2010-07-25 11:41 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 19+ messages in thread From: lynn.lin @ 2010-07-25 11:28 UTC (permalink / raw) To: kpfleming, avarab; +Cc: git -----Original Message----- From: Kevin P. Fleming [mailto:kpfleming@digium.com] Sent: 2010年7月25日 16:50 To: Ævar Arnfjörð Bjarmason Cc: Lin, Lynn; git@vger.kernel.org Subject: Re: [PATCH] Makefile: don't include git version file on 'make clean' On 07/24/2010 02:36 PM, Ævar Arnfjörð Bjarmason wrote: > On Sat, Jul 24, 2010 at 03:53, <Lynn.Lin@emc.com> wrote: >> From: Lynn Lin <Lynn.Lin@emc.com> >> >> --- >> Makefile | 4 +++- >> git-gui/Makefile | 4 +++- >> 2 files changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/Makefile b/Makefile >> index bc3c570..eb28b98 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -238,7 +238,9 @@ all:: >> >> GIT-VERSION-FILE: FORCE >> @$(SHELL_PATH) ./GIT-VERSION-GEN >> --include GIT-VERSION-FILE >> +ifneq "$(MAKECMDGOALS)" "clean" >> + -include GIT-VERSION-FILE >> +endif >> >> uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not') >> uname_M := $(shell sh -c 'uname -m 2>/dev/null || echo not') >> diff --git a/git-gui/Makefile b/git-gui/Makefile >> index 197b55e..91e1ea5 100644 >> --- a/git-gui/Makefile >> +++ b/git-gui/Makefile >> @@ -9,7 +9,9 @@ all:: >> >> GIT-VERSION-FILE: FORCE >> @$(SHELL_PATH) ./GIT-VERSION-GEN >> --include GIT-VERSION-FILE >> +ifneq "$(MAKECMDGOALS)" "clean" >> + -include GIT-VERSION-FILE >> +endif >> >> uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not') >> uname_O := $(shell sh -c 'uname -o 2>/dev/null || echo not') >> -- >> 1.7.1 > > This patch needs a rationale, why was it needed? The "-include" > directive will simply ignore files that don't exist (as opposed to > "include"), so including GIT-VERSION-FILE during "make clean' > shouldn't be an issue. Just guessing here, but since GIT-VERSION-FILE has a 'FORCE' prerequisite, that means that the operations to generate it will be run even for 'make clean', which is not useful for the cleaning operation. It's probably not harmful either... but maybe the OP has some more significant reason for this patch. Yes, when we run 'make clean' ,it also generate the git version file,then remove it .It's not necessary to trigger the operation when run 'make clean' command Lynn -- Kevin P. Fleming Digium, Inc. | Director of Software Technologies 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA skype: kpfleming | jabber: kfleming@digium.com Check us out at www.digium.com & www.asterisk.org ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Makefile: don't include git version file on 'make clean' 2010-07-25 11:28 ` lynn.lin @ 2010-07-25 11:41 ` Ævar Arnfjörð Bjarmason 2010-07-25 11:46 ` lynn.lin 0 siblings, 1 reply; 19+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-25 11:41 UTC (permalink / raw) To: lynn.lin; +Cc: kpfleming, git On Sun, Jul 25, 2010 at 11:28, <lynn.lin@emc.com> wrote: > > > -----Original Message----- > From: Kevin P. Fleming [mailto:kpfleming@digium.com] > Sent: 2010年7月25日 16:50 > To: Ævar Arnfjörð Bjarmason > Cc: Lin, Lynn; git@vger.kernel.org > Subject: Re: [PATCH] Makefile: don't include git version file on 'make clean' > > On 07/24/2010 02:36 PM, Ævar Arnfjörð Bjarmason wrote: >> On Sat, Jul 24, 2010 at 03:53, <Lynn.Lin@emc.com> wrote: >>> From: Lynn Lin <Lynn.Lin@emc.com> >>> >>> --- >>> Makefile | 4 +++- >>> git-gui/Makefile | 4 +++- >>> 2 files changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/Makefile b/Makefile >>> index bc3c570..eb28b98 100644 >>> --- a/Makefile >>> +++ b/Makefile >>> @@ -238,7 +238,9 @@ all:: >>> >>> GIT-VERSION-FILE: FORCE >>> @$(SHELL_PATH) ./GIT-VERSION-GEN >>> --include GIT-VERSION-FILE >>> +ifneq "$(MAKECMDGOALS)" "clean" >>> + -include GIT-VERSION-FILE >>> +endif >>> >>> uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not') >>> uname_M := $(shell sh -c 'uname -m 2>/dev/null || echo not') >>> diff --git a/git-gui/Makefile b/git-gui/Makefile >>> index 197b55e..91e1ea5 100644 >>> --- a/git-gui/Makefile >>> +++ b/git-gui/Makefile >>> @@ -9,7 +9,9 @@ all:: >>> >>> GIT-VERSION-FILE: FORCE >>> @$(SHELL_PATH) ./GIT-VERSION-GEN >>> --include GIT-VERSION-FILE >>> +ifneq "$(MAKECMDGOALS)" "clean" >>> + -include GIT-VERSION-FILE >>> +endif >>> >>> uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not') >>> uname_O := $(shell sh -c 'uname -o 2>/dev/null || echo not') >>> -- >>> 1.7.1 >> >> This patch needs a rationale, why was it needed? The "-include" >> directive will simply ignore files that don't exist (as opposed to >> "include"), so including GIT-VERSION-FILE during "make clean' >> shouldn't be an issue. > > Just guessing here, but since GIT-VERSION-FILE has a 'FORCE' > prerequisite, that means that the operations to generate it will be run > even for 'make clean', which is not useful for the cleaning operation. > It's probably not harmful either... but maybe the OP has some more > significant reason for this patch. > > > Yes, when we run 'make clean' ,it also generate the git version > file,then remove it .It's not necessary to trigger the operation > when run 'make clean' command Sure, it's not needed. But it's OK to have a bit of redundancy for simplicity, unless that redundancy is breaking something. Which is why I asked whether it was actually causing a problem in any case. With this patch we still call ./GIT-VERSION-GEN to make the ./GIT-VERSION-FILE, we just aren't including it anymore, and it would still be included on "make distclean" since you're just looking at $(MAKECMDGOALS). ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH] Makefile: don't include git version file on 'make clean' 2010-07-25 11:41 ` Ævar Arnfjörð Bjarmason @ 2010-07-25 11:46 ` lynn.lin 2010-07-25 11:55 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 19+ messages in thread From: lynn.lin @ 2010-07-25 11:46 UTC (permalink / raw) To: avarab; +Cc: kpfleming, git -----Original Message----- From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On Behalf Of ?var Arnfj?re Bjarmason Sent: 2010年7月25日 19:42 To: Lin, Lynn Cc: kpfleming@digium.com; git@vger.kernel.org Subject: Re: [PATCH] Makefile: don't include git version file on 'make clean' On Sun, Jul 25, 2010 at 11:28, <lynn.lin@emc.com> wrote: > > > -----Original Message----- > From: Kevin P. Fleming [mailto:kpfleming@digium.com] > Sent: 2010年7月25日 16:50 > To: Ævar Arnfjörð Bjarmason > Cc: Lin, Lynn; git@vger.kernel.org > Subject: Re: [PATCH] Makefile: don't include git version file on 'make clean' > > On 07/24/2010 02:36 PM, Ævar Arnfjörð Bjarmason wrote: >> On Sat, Jul 24, 2010 at 03:53, <Lynn.Lin@emc.com> wrote: >>> From: Lynn Lin <Lynn.Lin@emc.com> >>> >>> --- >>> Makefile | 4 +++- >>> git-gui/Makefile | 4 +++- >>> 2 files changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/Makefile b/Makefile >>> index bc3c570..eb28b98 100644 >>> --- a/Makefile >>> +++ b/Makefile >>> @@ -238,7 +238,9 @@ all:: >>> >>> GIT-VERSION-FILE: FORCE >>> @$(SHELL_PATH) ./GIT-VERSION-GEN >>> --include GIT-VERSION-FILE >>> +ifneq "$(MAKECMDGOALS)" "clean" >>> + -include GIT-VERSION-FILE >>> +endif >>> >>> uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not') >>> uname_M := $(shell sh -c 'uname -m 2>/dev/null || echo not') >>> diff --git a/git-gui/Makefile b/git-gui/Makefile >>> index 197b55e..91e1ea5 100644 >>> --- a/git-gui/Makefile >>> +++ b/git-gui/Makefile >>> @@ -9,7 +9,9 @@ all:: >>> >>> GIT-VERSION-FILE: FORCE >>> @$(SHELL_PATH) ./GIT-VERSION-GEN >>> --include GIT-VERSION-FILE >>> +ifneq "$(MAKECMDGOALS)" "clean" >>> + -include GIT-VERSION-FILE >>> +endif >>> >>> uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not') >>> uname_O := $(shell sh -c 'uname -o 2>/dev/null || echo not') >>> -- >>> 1.7.1 >> >> This patch needs a rationale, why was it needed? The "-include" >> directive will simply ignore files that don't exist (as opposed to >> "include"), so including GIT-VERSION-FILE during "make clean' >> shouldn't be an issue. > > Just guessing here, but since GIT-VERSION-FILE has a 'FORCE' > prerequisite, that means that the operations to generate it will be run > even for 'make clean', which is not useful for the cleaning operation. > It's probably not harmful either... but maybe the OP has some more > significant reason for this patch. > > > Yes, when we run 'make clean' ,it also generate the git version > file,then remove it .It's not necessary to trigger the operation > when run 'make clean' command Sure, it's not needed. But it's OK to have a bit of redundancy for simplicity, unless that redundancy is breaking something. Which is why I asked whether it was actually causing a problem in any case. With this patch we still call ./GIT-VERSION-GEN to make the ./GIT-VERSION-FILE, we just aren't including it anymore, and it would still be included on "make distclean" since you're just looking at $(MAKECMDGOALS). No,it won't call ./GIT-VERSION-GEN as it doesn't include GET-VERSION-FILE any more.so It won't trigger the GIT-VERSION-FILE target We can also handle distclean target -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Makefile: don't include git version file on 'make clean' 2010-07-25 11:46 ` lynn.lin @ 2010-07-25 11:55 ` Ævar Arnfjörð Bjarmason 2010-07-25 12:02 ` lynn.lin 2010-07-25 12:05 ` Andreas Schwab 0 siblings, 2 replies; 19+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-25 11:55 UTC (permalink / raw) To: lynn.lin; +Cc: kpfleming, git On Sun, Jul 25, 2010 at 11:46, <lynn.lin@emc.com> wrote: > > > -----Original Message----- > From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On Behalf Of ?var Arnfj?re Bjarmason > Sent: 2010年7月25日 19:42 > To: Lin, Lynn > Cc: kpfleming@digium.com; git@vger.kernel.org > Subject: Re: [PATCH] Makefile: don't include git version file on 'make clean' > > On Sun, Jul 25, 2010 at 11:28, <lynn.lin@emc.com> wrote: >> >> >> -----Original Message----- >> From: Kevin P. Fleming [mailto:kpfleming@digium.com] >> Sent: 2010年7月25日 16:50 >> To: Ævar Arnfjörð Bjarmason >> Cc: Lin, Lynn; git@vger.kernel.org >> Subject: Re: [PATCH] Makefile: don't include git version file on 'make clean' >> >> On 07/24/2010 02:36 PM, Ævar Arnfjörð Bjarmason wrote: >>> On Sat, Jul 24, 2010 at 03:53, <Lynn.Lin@emc.com> wrote: >>>> From: Lynn Lin <Lynn.Lin@emc.com> >>>> >>>> --- >>>> Makefile | 4 +++- >>>> git-gui/Makefile | 4 +++- >>>> 2 files changed, 6 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/Makefile b/Makefile >>>> index bc3c570..eb28b98 100644 >>>> --- a/Makefile >>>> +++ b/Makefile >>>> @@ -238,7 +238,9 @@ all:: >>>> >>>> GIT-VERSION-FILE: FORCE >>>> @$(SHELL_PATH) ./GIT-VERSION-GEN >>>> --include GIT-VERSION-FILE >>>> +ifneq "$(MAKECMDGOALS)" "clean" >>>> + -include GIT-VERSION-FILE >>>> +endif >>>> >>>> uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not') >>>> uname_M := $(shell sh -c 'uname -m 2>/dev/null || echo not') >>>> diff --git a/git-gui/Makefile b/git-gui/Makefile >>>> index 197b55e..91e1ea5 100644 >>>> --- a/git-gui/Makefile >>>> +++ b/git-gui/Makefile >>>> @@ -9,7 +9,9 @@ all:: >>>> >>>> GIT-VERSION-FILE: FORCE >>>> @$(SHELL_PATH) ./GIT-VERSION-GEN >>>> --include GIT-VERSION-FILE >>>> +ifneq "$(MAKECMDGOALS)" "clean" >>>> + -include GIT-VERSION-FILE >>>> +endif >>>> >>>> uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not') >>>> uname_O := $(shell sh -c 'uname -o 2>/dev/null || echo not') >>>> -- >>>> 1.7.1 >>> >>> This patch needs a rationale, why was it needed? The "-include" >>> directive will simply ignore files that don't exist (as opposed to >>> "include"), so including GIT-VERSION-FILE during "make clean' >>> shouldn't be an issue. >> >> Just guessing here, but since GIT-VERSION-FILE has a 'FORCE' >> prerequisite, that means that the operations to generate it will be run >> even for 'make clean', which is not useful for the cleaning operation. >> It's probably not harmful either... but maybe the OP has some more >> significant reason for this patch. >> >> > >> Yes, when we run 'make clean' ,it also generate the git version >> file,then remove it .It's not necessary to trigger the operation >> when run 'make clean' command > > Sure, it's not needed. But it's OK to have a bit of redundancy for > simplicity, unless that redundancy is breaking something. Which is why > I asked whether it was actually causing a problem in any case. > > With this patch we still call ./GIT-VERSION-GEN to make the > ./GIT-VERSION-FILE, we just aren't including it anymore, and it would > still be included on "make distclean" since you're just looking at > $(MAKECMDGOALS). > No,it won't call ./GIT-VERSION-GEN as it doesn't include > GET-VERSION-FILE any more.so It won't trigger the GIT-VERSION-FILE > target Yes it will. The version file is generated by this part: GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN But you've only wrapped the inclusion *after* the file is generated in an ifneq: +ifneq "$(MAKECMDGOALS)" "clean" + -include GIT-VERSION-FILE +endif Makefile targets aren't triggered by the include directive. > We can also handle distclean target Sure, it can be made to work. But can you tell my *why* this is needed (asking for the third time now). I'm more interested in the motivation than getting this particular patch working. If generating files like this during clean is breaking something it would be good to know, as we're probably doing it somewhere else too. If it's just OCD about not doing redundant work that's fine too. But it would be good to *know*. Thanks. ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH] Makefile: don't include git version file on 'make clean' 2010-07-25 11:55 ` Ævar Arnfjörð Bjarmason @ 2010-07-25 12:02 ` lynn.lin 2010-07-25 12:10 ` Ævar Arnfjörð Bjarmason 2010-07-25 12:05 ` Andreas Schwab 1 sibling, 1 reply; 19+ messages in thread From: lynn.lin @ 2010-07-25 12:02 UTC (permalink / raw) To: avarab; +Cc: kpfleming, git -----Original Message----- From: Ævar Arnfjörð Bjarmason [mailto:avarab@gmail.com] Sent: 2010年7月25日 19:56 To: Lin, Lynn Cc: kpfleming@digium.com; git@vger.kernel.org Subject: Re: [PATCH] Makefile: don't include git version file on 'make clean' On Sun, Jul 25, 2010 at 11:46, <lynn.lin@emc.com> wrote: > > > -----Original Message----- > From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On Behalf Of ?var Arnfj?re Bjarmason > Sent: 2010年7月25日 19:42 > To: Lin, Lynn > Cc: kpfleming@digium.com; git@vger.kernel.org > Subject: Re: [PATCH] Makefile: don't include git version file on 'make clean' > > On Sun, Jul 25, 2010 at 11:28, <lynn.lin@emc.com> wrote: >> >> >> -----Original Message----- >> From: Kevin P. Fleming [mailto:kpfleming@digium.com] >> Sent: 2010年7月25日 16:50 >> To: Ævar Arnfjörð Bjarmason >> Cc: Lin, Lynn; git@vger.kernel.org >> Subject: Re: [PATCH] Makefile: don't include git version file on 'make clean' >> >> On 07/24/2010 02:36 PM, Ævar Arnfjörð Bjarmason wrote: >>> On Sat, Jul 24, 2010 at 03:53, <Lynn.Lin@emc.com> wrote: >>>> From: Lynn Lin <Lynn.Lin@emc.com> >>>> >>>> --- >>>> Makefile | 4 +++- >>>> git-gui/Makefile | 4 +++- >>>> 2 files changed, 6 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/Makefile b/Makefile >>>> index bc3c570..eb28b98 100644 >>>> --- a/Makefile >>>> +++ b/Makefile >>>> @@ -238,7 +238,9 @@ all:: >>>> >>>> GIT-VERSION-FILE: FORCE >>>> @$(SHELL_PATH) ./GIT-VERSION-GEN >>>> --include GIT-VERSION-FILE >>>> +ifneq "$(MAKECMDGOALS)" "clean" >>>> + -include GIT-VERSION-FILE >>>> +endif >>>> >>>> uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not') >>>> uname_M := $(shell sh -c 'uname -m 2>/dev/null || echo not') >>>> diff --git a/git-gui/Makefile b/git-gui/Makefile >>>> index 197b55e..91e1ea5 100644 >>>> --- a/git-gui/Makefile >>>> +++ b/git-gui/Makefile >>>> @@ -9,7 +9,9 @@ all:: >>>> >>>> GIT-VERSION-FILE: FORCE >>>> @$(SHELL_PATH) ./GIT-VERSION-GEN >>>> --include GIT-VERSION-FILE >>>> +ifneq "$(MAKECMDGOALS)" "clean" >>>> + -include GIT-VERSION-FILE >>>> +endif >>>> >>>> uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not') >>>> uname_O := $(shell sh -c 'uname -o 2>/dev/null || echo not') >>>> -- >>>> 1.7.1 >>> >>> This patch needs a rationale, why was it needed? The "-include" >>> directive will simply ignore files that don't exist (as opposed to >>> "include"), so including GIT-VERSION-FILE during "make clean' >>> shouldn't be an issue. >> >> Just guessing here, but since GIT-VERSION-FILE has a 'FORCE' >> prerequisite, that means that the operations to generate it will be run >> even for 'make clean', which is not useful for the cleaning operation. >> It's probably not harmful either... but maybe the OP has some more >> significant reason for this patch. >> >> > >> Yes, when we run 'make clean' ,it also generate the git version >> file,then remove it .It's not necessary to trigger the operation >> when run 'make clean' command > > Sure, it's not needed. But it's OK to have a bit of redundancy for > simplicity, unless that redundancy is breaking something. Which is why > I asked whether it was actually causing a problem in any case. > > With this patch we still call ./GIT-VERSION-GEN to make the > ./GIT-VERSION-FILE, we just aren't including it anymore, and it would > still be included on "make distclean" since you're just looking at > $(MAKECMDGOALS). > No,it won't call ./GIT-VERSION-GEN as it doesn't include > GET-VERSION-FILE any more.so It won't trigger the GIT-VERSION-FILE > target Yes it will. The version file is generated by this part: GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN If we don't trigger include ,it won't call GIT-VERSION-FILE target But you've only wrapped the inclusion *after* the file is generated in an ifneq: +ifneq "$(MAKECMDGOALS)" "clean" + -include GIT-VERSION-FILE +endif Makefile targets aren't triggered by the include directive. > We can also handle distclean target Sure, it can be made to work. But can you tell my *why* this is needed (asking for the third time now).I'm more interested in the motivation than getting this particular patch working. If generating files like this during clean is breaking something it would be good to know, as we're probably doing it somewhere else too. Sorry. It doesn't break something. The motivation is that it's redundant code If it's just OCD about not doing redundant work that's fine too. But it would be good to *know*. Thanks. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Makefile: don't include git version file on 'make clean' 2010-07-25 12:02 ` lynn.lin @ 2010-07-25 12:10 ` Ævar Arnfjörð Bjarmason 0 siblings, 0 replies; 19+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-25 12:10 UTC (permalink / raw) To: lynn.lin; +Cc: kpfleming, git On Sun, Jul 25, 2010 at 12:02, <lynn.lin@emc.com> wrote: > Sorry. It doesn't break something. The motivation is that it's redundant code Ah, good to know. That was what I mainly wanted to know. Thanks. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Makefile: don't include git version file on 'make clean' 2010-07-25 11:55 ` Ævar Arnfjörð Bjarmason 2010-07-25 12:02 ` lynn.lin @ 2010-07-25 12:05 ` Andreas Schwab 2010-07-25 12:15 ` Ævar Arnfjörð Bjarmason 1 sibling, 1 reply; 19+ messages in thread From: Andreas Schwab @ 2010-07-25 12:05 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: lynn.lin, kpfleming, git Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Makefile targets aren't triggered by the include directive. Umm, yes they are, see (make) Remaking Makefiles. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Makefile: don't include git version file on 'make clean' 2010-07-25 12:05 ` Andreas Schwab @ 2010-07-25 12:15 ` Ævar Arnfjörð Bjarmason 2010-07-25 12:19 ` lynn.lin 0 siblings, 1 reply; 19+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-25 12:15 UTC (permalink / raw) To: Andreas Schwab; +Cc: lynn.lin, kpfleming, git On Sun, Jul 25, 2010 at 12:05, Andreas Schwab <schwab@linux-m68k.org> wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> Makefile targets aren't triggered by the include directive. > > Umm, yes they are, see (make) Remaking Makefiles. Ah, yes. But it was being included in more places than just that -include directive, so I didn't spot the difference: Without that directive, still generated on make clean: $ git diff -U0 | cat diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index e88f50c..f29406b 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -2,0 +3,2 @@ +echo MOO > /tmp/moo + diff --git a/Makefile b/Makefile index b6975aa..5edfeca 100644 --- a/Makefile +++ b/Makefile @@ -241 +240,0 @@ GIT-VERSION-FILE: FORCE --include GIT-VERSION-FILE $ rm -v /tmp/moo; make clean > /dev/null; cat /tmp/moo removed `/tmp/moo' GIT_VERSION = 1.7.2.6.g65a0d3.dirty GITGUI_VERSION = 0.12.0.64.g89d61-dirty MOO Deleted the rule, not generated, but other things are still calling the rule: $ git diff -U0 | cat diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index e88f50c..f29406b 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -2,0 +3,2 @@ +echo MOO > /tmp/moo + diff --git a/Makefile b/Makefile index b6975aa..1a189da 100644 --- a/Makefile +++ b/Makefile @@ -239,4 +238,0 @@ all:: -GIT-VERSION-FILE: FORCE - @$(SHELL_PATH) ./GIT-VERSION-GEN --include GIT-VERSION-FILE - $ rm -v /tmp/moo; make clean > /dev/null; cat /tmp/moo removed `/tmp/moo' make[2]: *** No rule to make target `GIT-VERSION-FILE'. Stop. make[2]: *** No rule to make target `GIT-VERSION-FILE'. Stop. make[2]: *** No rule to make target `GIT-VERSION-FILE'. Stop. GITGUI_VERSION = 0.12.0.64.g89d61-dirty cat: /tmp/moo: No such file or directory ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH] Makefile: don't include git version file on 'make clean' 2010-07-25 12:15 ` Ævar Arnfjörð Bjarmason @ 2010-07-25 12:19 ` lynn.lin 2010-07-25 12:21 ` lynn.lin 0 siblings, 1 reply; 19+ messages in thread From: lynn.lin @ 2010-07-25 12:19 UTC (permalink / raw) To: avarab, schwab; +Cc: kpfleming, git -----Original Message----- From: Ævar Arnfjörð Bjarmason [mailto:avarab@gmail.com] Sent: 2010年7月25日 20:16 To: Andreas Schwab Cc: Lin, Lynn; kpfleming@digium.com; git@vger.kernel.org Subject: Re: [PATCH] Makefile: don't include git version file on 'make clean' On Sun, Jul 25, 2010 at 12:05, Andreas Schwab <schwab@linux-m68k.org> wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> Makefile targets aren't triggered by the include directive. > > Umm, yes they are, see (make) Remaking Makefiles. Ah, yes. But it was being included in more places than just that -include directive, so I didn't spot the difference: Without that directive, still generated on make clean: $ git diff -U0 | cat diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index e88f50c..f29406b 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -2,0 +3,2 @@ +echo MOO > /tmp/moo + diff --git a/Makefile b/Makefile index b6975aa..5edfeca 100644 --- a/Makefile +++ b/Makefile @@ -241 +240,0 @@ GIT-VERSION-FILE: FORCE --include GIT-VERSION-FILE $ rm -v /tmp/moo; make clean > /dev/null; cat /tmp/moo removed `/tmp/moo' GIT_VERSION = 1.7.2.6.g65a0d3.dirty GITGUI_VERSION = 0.12.0.64.g89d61-dirty MOO Deleted the rule, not generated, but other things are still calling the rule: Why not delete the rule? We only handle this on 'make clean' command ('make distclean') target $ git diff -U0 | cat diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index e88f50c..f29406b 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -2,0 +3,2 @@ +echo MOO > /tmp/moo + diff --git a/Makefile b/Makefile index b6975aa..1a189da 100644 --- a/Makefile +++ b/Makefile @@ -239,4 +238,0 @@ all:: -GIT-VERSION-FILE: FORCE - @$(SHELL_PATH) ./GIT-VERSION-GEN --include GIT-VERSION-FILE - $ rm -v /tmp/moo; make clean > /dev/null; cat /tmp/moo removed `/tmp/moo' make[2]: *** No rule to make target `GIT-VERSION-FILE'. Stop. make[2]: *** No rule to make target `GIT-VERSION-FILE'. Stop. make[2]: *** No rule to make target `GIT-VERSION-FILE'. Stop. GITGUI_VERSION = 0.12.0.64.g89d61-dirty cat: /tmp/moo: No such file or directory ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH] Makefile: don't include git version file on 'make clean' 2010-07-25 12:19 ` lynn.lin @ 2010-07-25 12:21 ` lynn.lin 2010-07-25 12:29 ` lynn.lin 0 siblings, 1 reply; 19+ messages in thread From: lynn.lin @ 2010-07-25 12:21 UTC (permalink / raw) To: lynn.lin, avarab, schwab; +Cc: kpfleming, git -----Original Message----- From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On Behalf Of lynn.lin@emc.com Sent: 2010年7月25日 20:19 To: avarab@gmail.com; schwab@linux-m68k.org Cc: kpfleming@digium.com; git@vger.kernel.org Subject: RE: [PATCH] Makefile: don't include git version file on 'make clean' -----Original Message----- From: Ævar Arnfjörð Bjarmason [mailto:avarab@gmail.com] Sent: 2010年7月25日 20:16 To: Andreas Schwab Cc: Lin, Lynn; kpfleming@digium.com; git@vger.kernel.org Subject: Re: [PATCH] Makefile: don't include git version file on 'make clean' On Sun, Jul 25, 2010 at 12:05, Andreas Schwab <schwab@linux-m68k.org> wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> Makefile targets aren't triggered by the include directive. > > Umm, yes they are, see (make) Remaking Makefiles. Ah, yes. But it was being included in more places than just that -include directive, so I didn't spot the difference: Without that directive, still generated on make clean: $ git diff -U0 | cat diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index e88f50c..f29406b 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -2,0 +3,2 @@ +echo MOO > /tmp/moo + diff --git a/Makefile b/Makefile index b6975aa..5edfeca 100644 --- a/Makefile +++ b/Makefile @@ -241 +240,0 @@ GIT-VERSION-FILE: FORCE --include GIT-VERSION-FILE $ rm -v /tmp/moo; make clean > /dev/null; cat /tmp/moo removed `/tmp/moo' GIT_VERSION = 1.7.2.6.g65a0d3.dirty GITGUI_VERSION = 0.12.0.64.g89d61-dirty MOO Deleted the rule, not generated, but other things are still calling the rule: Why not delete the rule? We only handle this on 'make clean' command ('make distclean') target Sorry.it's typo .Why delete the rule $ git diff -U0 | cat diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index e88f50c..f29406b 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -2,0 +3,2 @@ +echo MOO > /tmp/moo + diff --git a/Makefile b/Makefile index b6975aa..1a189da 100644 --- a/Makefile +++ b/Makefile @@ -239,4 +238,0 @@ all:: -GIT-VERSION-FILE: FORCE - @$(SHELL_PATH) ./GIT-VERSION-GEN --include GIT-VERSION-FILE - $ rm -v /tmp/moo; make clean > /dev/null; cat /tmp/moo removed `/tmp/moo' make[2]: *** No rule to make target `GIT-VERSION-FILE'. Stop. make[2]: *** No rule to make target `GIT-VERSION-FILE'. Stop. make[2]: *** No rule to make target `GIT-VERSION-FILE'. Stop. GITGUI_VERSION = 0.12.0.64.g89d61-dirty cat: /tmp/moo: No such file or directory NryزXvؖ){nljض\x17}zj:v zZzf~zwڢ)^[ ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH] Makefile: don't include git version file on 'make clean' 2010-07-25 12:21 ` lynn.lin @ 2010-07-25 12:29 ` lynn.lin 2010-07-25 12:34 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 19+ messages in thread From: lynn.lin @ 2010-07-25 12:29 UTC (permalink / raw) To: lynn.lin, avarab, schwab; +Cc: kpfleming, git -----Original Message----- From: Lin, Lynn Sent: 2010年7月25日 20:22 To: Lin, Lynn; avarab@gmail.com; schwab@linux-m68k.org Cc: kpfleming@digium.com; git@vger.kernel.org Subject: RE: [PATCH] Makefile: don't include git version file on 'make clean' -----Original Message----- From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On Behalf Of lynn.lin@emc.com Sent: 2010年7月25日 20:19 To: avarab@gmail.com; schwab@linux-m68k.org Cc: kpfleming@digium.com; git@vger.kernel.org Subject: RE: [PATCH] Makefile: don't include git version file on 'make clean' -----Original Message----- From: Ævar Arnfjörð Bjarmason [mailto:avarab@gmail.com] Sent: 2010年7月25日 20:16 To: Andreas Schwab Cc: Lin, Lynn; kpfleming@digium.com; git@vger.kernel.org Subject: Re: [PATCH] Makefile: don't include git version file on 'make clean' On Sun, Jul 25, 2010 at 12:05, Andreas Schwab <schwab@linux-m68k.org> wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> Makefile targets aren't triggered by the include directive. > > Umm, yes they are, see (make) Remaking Makefiles. Ah, yes. But it was being included in more places than just that -include directive, so I didn't spot the difference: Without that directive, still generated on make clean: $ git diff -U0 | cat diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index e88f50c..f29406b 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -2,0 +3,2 @@ +echo MOO > /tmp/moo + diff --git a/Makefile b/Makefile index b6975aa..5edfeca 100644 --- a/Makefile +++ b/Makefile @@ -241 +240,0 @@ GIT-VERSION-FILE: FORCE --include GIT-VERSION-FILE $ rm -v /tmp/moo; make clean > /dev/null; cat /tmp/moo removed `/tmp/moo' GIT_VERSION = 1.7.2.6.g65a0d3.dirty GITGUI_VERSION = 0.12.0.64.g89d61-dirty MOO Deleted the rule, not generated, but other things are still calling the rule: Why not delete the rule? We only handle this on 'make clean' command ('make distclean') target Sorry.it's typo .Why delete the rule $ git diff -U0 | cat diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index e88f50c..f29406b 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -2,0 +3,2 @@ +echo MOO > /tmp/moo + diff --git a/Makefile b/Makefile index b6975aa..1a189da 100644 --- a/Makefile +++ b/Makefile @@ -239,4 +238,0 @@ all:: -GIT-VERSION-FILE: FORCE - @$(SHELL_PATH) ./GIT-VERSION-GEN --include GIT-VERSION-FILE - $ rm -v /tmp/moo; make clean > /dev/null; cat /tmp/moo removed `/tmp/moo' make[2]: *** No rule to make target `GIT-VERSION-FILE'. Stop. make[2]: *** No rule to make target `GIT-VERSION-FILE'. Stop. make[2]: *** No rule to make target `GIT-VERSION-FILE'. Stop. GITGUI_VERSION = 0.12.0.64.g89d61-dirty cat: /tmp/moo: No such file or directory NryزXvؖ){nljض\x17}zj:v zZzf~zwڢ)^[ We have two place to call GIT-VERSION-FILE target in top Makefile git.o git.spec \ $(patsubst %.sh,%,$(SCRIPT_SH)) \ $(patsubst %.perl,%,$(SCRIPT_PERL)) \ : GIT-VERSION-FILE clean: $(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-GUI-VARS GIT-BUILD-OPTIONS My patch is to don't call GIT-VERSION-FILE target when you run 'make clean' Thanks Lynn ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Makefile: don't include git version file on 'make clean' 2010-07-25 12:29 ` lynn.lin @ 2010-07-25 12:34 ` Ævar Arnfjörð Bjarmason 2010-07-25 12:37 ` lynn.lin 0 siblings, 1 reply; 19+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-25 12:34 UTC (permalink / raw) To: lynn.lin; +Cc: schwab, kpfleming, git On Sun, Jul 25, 2010 at 12:29, <lynn.lin@emc.com> wrote: > My patch is to don't call GIT-VERSION-FILE target when you run 'make clean' Yes, but as I demonstrated it gets called anyway. Presumably because of the $(MAKE) -C ... clean rules. But I haven't looked into it. ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH] Makefile: don't include git version file on 'make clean' 2010-07-25 12:34 ` Ævar Arnfjörð Bjarmason @ 2010-07-25 12:37 ` lynn.lin 2010-07-25 13:08 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 19+ messages in thread From: lynn.lin @ 2010-07-25 12:37 UTC (permalink / raw) To: avarab; +Cc: schwab, kpfleming, git -----Original Message----- From: Ævar Arnfjörð Bjarmason [mailto:avarab@gmail.com] Sent: 2010年7月25日 20:34 To: Lin, Lynn Cc: schwab@linux-m68k.org; kpfleming@digium.com; git@vger.kernel.org Subject: Re: [PATCH] Makefile: don't include git version file on 'make clean' On Sun, Jul 25, 2010 at 12:29, <lynn.lin@emc.com> wrote: > My patch is to don't call GIT-VERSION-FILE target when you run 'make clean' Yes, but as I demonstrated it gets called anyway. Presumably because of the $(MAKE) -C ... clean rules. But I haven't looked into it. If we don't specify special goals, when we run any target ,it will call GIT-VERSIONF-FILE target as it include this target Example from GNU make manual: http://www.gnu.org/software/autoconf/manual/make/Goals.html An example of appropriate use is to avoid including .d files during clean rules (see Automatic Prerequisites), so make won't create them only to immediately remove them again: sources = foo.c bar.c ifneq ($(MAKECMDGOALS),clean) include $(sources:.c=.d) endif Thanks Lynn ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Makefile: don't include git version file on 'make clean' 2010-07-25 12:37 ` lynn.lin @ 2010-07-25 13:08 ` Ævar Arnfjörð Bjarmason 2010-07-25 13:21 ` lynn.lin 0 siblings, 1 reply; 19+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-25 13:08 UTC (permalink / raw) To: lynn.lin; +Cc: schwab, kpfleming, git On Sun, Jul 25, 2010 at 12:37, <lynn.lin@emc.com> wrote: > > > -----Original Message----- > From: Ævar Arnfjörð Bjarmason [mailto:avarab@gmail.com] > Sent: 2010年7月25日 20:34 > To: Lin, Lynn > Cc: schwab@linux-m68k.org; kpfleming@digium.com; git@vger.kernel.org > Subject: Re: [PATCH] Makefile: don't include git version file on 'make clean' > > On Sun, Jul 25, 2010 at 12:29, <lynn.lin@emc.com> wrote: > >> My patch is to don't call GIT-VERSION-FILE target when you run 'make clean' > > Yes, but as I demonstrated it gets called anyway. Presumably because > of the $(MAKE) -C ... clean rules. But I haven't looked into it. > > > If we don't specify special goals, when we run any target ,it will call GIT-VERSIONF-FILE target as it include this target > > Example from GNU make manual: > http://www.gnu.org/software/autoconf/manual/make/Goals.html > > > An example of appropriate use is to avoid including .d files during clean rules (see Automatic Prerequisites), so make won't create them only to immediately remove them again: > > sources = foo.c bar.c > > ifneq ($(MAKECMDGOALS),clean) > include $(sources:.c=.d) > endif Yes, I know (now) how include directives work. What I'm saying is that your patch doesn't work because the main Makefile clean directive calls *other* makefiles, which in turn include the version file: $ rm GIT-VERSION-FILE ; make -C gitweb clean; cat GIT-VERSION-FILE make: Entering directory `/home/avar/g/git/gitweb' make[1]: Entering directory `/home/avar/g/git' GIT_VERSION = 1.7.2.6.g65a0d3 make[1]: Leaving directory `/home/avar/g/git' make[1]: Entering directory `/home/avar/g/git' make[1]: `GIT-VERSION-FILE' is up to date. make[1]: Leaving directory `/home/avar/g/git' make: Leaving directory `/home/avar/g/git/gitweb' make: Entering directory `/home/avar/g/git/gitweb' make[1]: Entering directory `/home/avar/g/git' make[1]: `GIT-VERSION-FILE' is up to date. make[1]: Leaving directory `/home/avar/g/git' rm -f gitweb.cgi static/gitweb.min.js static/gitweb.min.css GITWEB-BUILD-OPTIONS make: Leaving directory `/home/avar/g/git/gitweb' GIT_VERSION = 1.7.2.6.g65a0d3 So just removing the inclusion in the main Makefile doesn't do anything at all. To get it to work you need to patch the */Makefile files too, and patch other clean targets like distclean. But personally I think this whole thing is a bit silly, but others may disagree. I've said my bit. Thanks for contributing to Git anyway, your help is appreciated. ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH] Makefile: don't include git version file on 'make clean' 2010-07-25 13:08 ` Ævar Arnfjörð Bjarmason @ 2010-07-25 13:21 ` lynn.lin 0 siblings, 0 replies; 19+ messages in thread From: lynn.lin @ 2010-07-25 13:21 UTC (permalink / raw) To: avarab; +Cc: schwab, kpfleming, git -----Original Message----- From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On Behalf Of ?var Arnfj?re Bjarmason Sent: 2010年7月25日 21:08 To: Lin, Lynn Cc: schwab@linux-m68k.org; kpfleming@digium.com; git@vger.kernel.org Subject: Re: [PATCH] Makefile: don't include git version file on 'make clean' On Sun, Jul 25, 2010 at 12:37, <lynn.lin@emc.com> wrote: > > > -----Original Message----- > From: Ævar Arnfjörð Bjarmason [mailto:avarab@gmail.com] > Sent: 2010年7月25日 20:34 > To: Lin, Lynn > Cc: schwab@linux-m68k.org; kpfleming@digium.com; git@vger.kernel.org > Subject: Re: [PATCH] Makefile: don't include git version file on 'make clean' > > On Sun, Jul 25, 2010 at 12:29, <lynn.lin@emc.com> wrote: > >> My patch is to don't call GIT-VERSION-FILE target when you run 'make clean' > > Yes, but as I demonstrated it gets called anyway. Presumably because > of the $(MAKE) -C ... clean rules. But I haven't looked into it. > > > If we don't specify special goals, when we run any target ,it will call GIT-VERSIONF-FILE target as it include this target > > Example from GNU make manual: > http://www.gnu.org/software/autoconf/manual/make/Goals.html > > > An example of appropriate use is to avoid including .d files during clean rules (see Automatic Prerequisites), so make won't create them only to immediately remove them again: > > sources = foo.c bar.c > > ifneq ($(MAKECMDGOALS),clean) > include $(sources:.c=.d) > endif Yes, I know (now) how include directives work. What I'm saying is that your patch doesn't work because the main Makefile clean directive calls *other* makefiles, which in turn include the version file: $ rm GIT-VERSION-FILE ; make -C gitweb clean; cat GIT-VERSION-FILE make: Entering directory `/home/avar/g/git/gitweb' make[1]: Entering directory `/home/avar/g/git' GIT_VERSION = 1.7.2.6.g65a0d3 make[1]: Leaving directory `/home/avar/g/git' make[1]: Entering directory `/home/avar/g/git' make[1]: `GIT-VERSION-FILE' is up to date. make[1]: Leaving directory `/home/avar/g/git' make: Leaving directory `/home/avar/g/git/gitweb' make: Entering directory `/home/avar/g/git/gitweb' make[1]: Entering directory `/home/avar/g/git' make[1]: `GIT-VERSION-FILE' is up to date. make[1]: Leaving directory `/home/avar/g/git' rm -f gitweb.cgi static/gitweb.min.js static/gitweb.min.css GITWEB-BUILD-OPTIONS make: Leaving directory `/home/avar/g/git/gitweb' GIT_VERSION = 1.7.2.6.g65a0d3 So just removing the inclusion in the main Makefile doesn't do anything at all. To get it to work you need to patch the */Makefile files too, and patch other clean targets like distclean. There are Document,gitweb and git-gui module have the same "issues" But personally I think this whole thing is a bit silly, but others may disagree. I've said my bit. I think we can do better when we find redundant code, correct? Thanks for contributing to Git anyway, your help is appreciated. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Patch follow-up conventions (Re: [PATCH] Makefile: don't include git version file on 'make clean') 2010-07-24 3:53 [PATCH] Makefile: don't include git version file on 'make clean' Lynn.Lin 2010-07-24 12:36 ` Ævar Arnfjörð Bjarmason @ 2010-07-25 18:49 ` Jonathan Nieder 1 sibling, 0 replies; 19+ messages in thread From: Jonathan Nieder @ 2010-07-25 18:49 UTC (permalink / raw) To: Lynn.Lin; +Cc: git Hi Lynn, Lynn Lin wrote: > -----Original Message----- > From: Jonathan Nieder [mailto:jrnieder@gmail.com] > Sent: 2010年7月24日 19:52 > To: Lin, Lynn > Subject: Re: [PATCH] Makefile: don't include git version file on 'make clean' [...] > Thanks much ,Jonathan. > It's my first time to try submit patch to git project:) > > Do I need to re-submit patch to add more message in commit message ? No problem; it is always good to see people noticing things that can be improved and fixing them. :) In general the best thing to do (though hard) is to imagine what would be most convenient at the receiving end and support that. This means: - do not resend a whole patch when a small fixup would be easier; - if there has been a long discussion, once a patch is ready send a copy with [PATCH v2] in the subject, with a summary of the discussion after the "---" line and cc-ing Junio to let him know it is ready for application. Another piece of advice: please convince your mailer setup to present replies in a more useful form. That means snipping out any irrelevant text and somehow visually distinguishing the text you are quoting from your reply, like I have done with "> " above. Hope that helps, Jonathan ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2010-07-25 18:51 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-07-24 3:53 [PATCH] Makefile: don't include git version file on 'make clean' Lynn.Lin 2010-07-24 12:36 ` Ævar Arnfjörð Bjarmason 2010-07-25 8:49 ` Kevin P. Fleming 2010-07-25 11:28 ` lynn.lin 2010-07-25 11:41 ` Ævar Arnfjörð Bjarmason 2010-07-25 11:46 ` lynn.lin 2010-07-25 11:55 ` Ævar Arnfjörð Bjarmason 2010-07-25 12:02 ` lynn.lin 2010-07-25 12:10 ` Ævar Arnfjörð Bjarmason 2010-07-25 12:05 ` Andreas Schwab 2010-07-25 12:15 ` Ævar Arnfjörð Bjarmason 2010-07-25 12:19 ` lynn.lin 2010-07-25 12:21 ` lynn.lin 2010-07-25 12:29 ` lynn.lin 2010-07-25 12:34 ` Ævar Arnfjörð Bjarmason 2010-07-25 12:37 ` lynn.lin 2010-07-25 13:08 ` Ævar Arnfjörð Bjarmason 2010-07-25 13:21 ` lynn.lin 2010-07-25 18:49 ` Patch follow-up conventions (Re: [PATCH] Makefile: don't include git version file on 'make clean') Jonathan Nieder
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).