git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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 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: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 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).