| Message ID | 1353042317-13688-2-git-send-email-xiawenc@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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
于 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.
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
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 "@"?
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
于 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.
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
于 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.
于 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.
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 \
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(-)