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日 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:
Show 86 quoted lines
>
>
> -----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-GENIf 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
+endifMakefile 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.
