[1/2] Buildsystem fix distclean error in pixman

Submitted by Wayne Xia on Nov. 16, 2012, 5:05 a.m.

Details

Message ID 1353042317-13688-2-git-send-email-xiawenc@linux.vnet.ibm.com
State New
Headers show

Commit Message

Wayne Xia Nov. 16, 2012, 5:05 a.m.
Currently if pixman have no config.log inside, make file still
try to clean it resulting error. This patch fix it.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 Makefile |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

Comments

Peter Maydell Nov. 16, 2012, 9:27 a.m.
On 16 November 2012 05:05, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>   Currently if pixman have no config.log inside, make file still
> try to clean it resulting error. This patch fix it.
>
> -       test -f pixman/config.log && make -C pixman distclean
> +       @if test -f pixman/config.log; \
> +       then \
> +               make -C pixman distclean;\
> +       fi

These two bits of shellscript both do the same thing, don't they?

-- PMM
Wayne Xia Nov. 16, 2012, 10:08 a.m.
于 2012-11-16 17:27, Peter Maydell 写道:
> On 16 November 2012 05:05, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>    Currently if pixman have no config.log inside, make file still
>> try to clean it resulting error. This patch fix it.
>>
>> -       test -f pixman/config.log && make -C pixman distclean
>> +       @if test -f pixman/config.log; \
>> +       then \
>> +               make -C pixman distclean;\
>> +       fi
>
> These two bits of shellscript both do the same thing, don't they?
>
> -- PMM
>
   Looks the same, but on my machine
make -C pixman distclean
got executed resulting error, I guess that is what original author
wants to avoid.
   I'll conclude this patch into my libqblock patch serials to make
it easier to be reviwed.
Peter Maydell Nov. 16, 2012, 10:16 a.m.
On 16 November 2012 10:08, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
> 于 2012-11-16 17:27, Peter Maydell 写道:
>> On 16 November 2012 05:05, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>> -       test -f pixman/config.log && make -C pixman distclean
>>> +       @if test -f pixman/config.log; \
>>> +       then \
>>> +               make -C pixman distclean;\
>>> +       fi

>> These two bits of shellscript both do the same thing, don't they?

>   Looks the same, but on my machine
> make -C pixman distclean
> got executed resulting error, I guess that is what original author
> wants to avoid.

If your shell executes Y in "X && Y" even if X fails, then your
shell is badly badly broken. More likely, there is some other
problem, and you need to find out what it is.
I don't think this patch should be applied, I'm afraid.

-- PMM
Paolo Bonzini Nov. 16, 2012, 10:23 a.m.
Il 16/11/2012 10:27, Peter Maydell ha scritto:
> On 16 November 2012 05:05, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>   Currently if pixman have no config.log inside, make file still
>> try to clean it resulting error. This patch fix it.
>>
>> -       test -f pixman/config.log && make -C pixman distclean
>> +       @if test -f pixman/config.log; \
>> +       then \
>> +               make -C pixman distclean;\
>> +       fi
> 
> These two bits of shellscript both do the same thing, don't they?

No, when "test" fails the && exits with a failure.  The "if" exits with
a success (not the most portable thing ever, but we assume a decent
shell elsewhere).

I think this patch is ok, but why the "@"?
Peter Maydell Nov. 16, 2012, 10:26 a.m.
On 16 November 2012 10:23, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 16/11/2012 10:27, Peter Maydell ha scritto:
>> On 16 November 2012 05:05, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>> -       test -f pixman/config.log && make -C pixman distclean
>>> +       @if test -f pixman/config.log; \
>>> +       then \
>>> +               make -C pixman distclean;\
>>> +       fi
>>
>> These two bits of shellscript both do the same thing, don't they?
>
> No, when "test" fails the && exits with a failure.  The "if" exits with
> a success (not the most portable thing ever, but we assume a decent
> shell elsewhere).

Aha. I love shell. (This detail could usefully have been
provided in the commit message...)

thanks
-- PMM
Wayne Xia Nov. 16, 2012, 10:27 a.m.
于 2012-11-16 18:16, Peter Maydell 写道:
> On 16 November 2012 10:08, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>> 于 2012-11-16 17:27, Peter Maydell 写道:
>>> On 16 November 2012 05:05, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>>> -       test -f pixman/config.log && make -C pixman distclean
>>>> +       @if test -f pixman/config.log; \
>>>> +       then \
>>>> +               make -C pixman distclean;\
>>>> +       fi
>
>>> These two bits of shellscript both do the same thing, don't they?
>
>>    Looks the same, but on my machine
>> make -C pixman distclean
>> got executed resulting error, I guess that is what original author
>> wants to avoid.
>
> If your shell executes Y in "X && Y" even if X fails, then your
> shell is badly badly broken. More likely, there is some other
> problem, and you need to find out what it is.
> I don't think this patch should be applied, I'm afraid.
>
> -- PMM
>
   My shell is OK, I missed the error reporting source before,
in my test I think it is first part reporting error, that
is "test -f pixman/config.log" directly report error.
   This patch would eliminate this error.
Gerd Hoffmann Nov. 16, 2012, 10:38 a.m.
On 11/16/12 11:26, Peter Maydell wrote:
> On 16 November 2012 10:23, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 16/11/2012 10:27, Peter Maydell ha scritto:
>>> On 16 November 2012 05:05, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>>> -       test -f pixman/config.log && make -C pixman distclean
>>>> +       @if test -f pixman/config.log; \
>>>> +       then \
>>>> +               make -C pixman distclean;\
>>>> +       fi
>>>
>>> These two bits of shellscript both do the same thing, don't they?
>>
>> No, when "test" fails the && exits with a failure.  The "if" exits with
>> a success (not the most portable thing ever, but we assume a decent
>> shell elsewhere).
> 
> Aha. I love shell. (This detail could usefully have been
> provided in the commit message...)

Agree.  Can I get a version with an updated commit message for the
pixman patch queue?

thanks,
  Gerd
Wayne Xia Nov. 16, 2012, 10:51 a.m.
于 2012-11-16 18:23, Paolo Bonzini 写道:
> Il 16/11/2012 10:27, Peter Maydell ha scritto:
>> On 16 November 2012 05:05, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>>    Currently if pixman have no config.log inside, make file still
>>> try to clean it resulting error. This patch fix it.
>>>
>>> -       test -f pixman/config.log && make -C pixman distclean
>>> +       @if test -f pixman/config.log; \
>>> +       then \
>>> +               make -C pixman distclean;\
>>> +       fi
>>
>> These two bits of shellscript both do the same thing, don't they?
>
> No, when "test" fails the && exits with a failure.  The "if" exits with
> a success (not the most portable thing ever, but we assume a decent
> shell elsewhere).
>
> I think this patch is ok, but why the "@"?
>
   print out seems ugly :|, maybe I should cancel @ and adjust the
format.
Wayne Xia Nov. 16, 2012, 10:53 a.m.
于 2012-11-16 18:38, Gerd Hoffmann 写道:
> On 11/16/12 11:26, Peter Maydell wrote:
>> On 16 November 2012 10:23, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> Il 16/11/2012 10:27, Peter Maydell ha scritto:
>>>> On 16 November 2012 05:05, Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>>>> -       test -f pixman/config.log && make -C pixman distclean
>>>>> +       @if test -f pixman/config.log; \
>>>>> +       then \
>>>>> +               make -C pixman distclean;\
>>>>> +       fi
>>>>
>>>> These two bits of shellscript both do the same thing, don't they?
>>>
>>> No, when "test" fails the && exits with a failure.  The "if" exits with
>>> a success (not the most portable thing ever, but we assume a decent
>>> shell elsewhere).
>>
>> Aha. I love shell. (This detail could usefully have been
>> provided in the commit message...)
>
> Agree.  Can I get a version with an updated commit message for the
> pixman patch queue?
>
> thanks,
>    Gerd
>
   Sure,I'll resend according to more comments later.

Patch hide | download patch | download mbox

diff --git a/Makefile b/Makefile
index 81c660f..f40885b 100644
--- a/Makefile
+++ b/Makefile
@@ -278,7 +278,10 @@  distclean: clean
 	for d in $(TARGET_DIRS) $(QEMULIBS); do \
 	rm -rf $$d || exit 1 ; \
         done
-	test -f pixman/config.log && make -C pixman distclean
+	@if test -f pixman/config.log; \
+	then \
+		make -C pixman distclean;\
+	fi
 
 KEYMAPS=da     en-gb  et  fr     fr-ch  is  lt  modifiers  no  pt-br  sv \
 ar      de     en-us  fi  fr-be  hr     it  lv  nl         pl  ru     th \