[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Makefile: Don't find and delete when $(DSOSUF)
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH] Makefile: Don't find and delete when $(DSOSUF) is empty in "make clean" |
Date: |
Mon, 17 Mar 2014 09:34:41 +0800 |
User-agent: |
Mutt/1.5.22 (2013-10-16) |
On Fri, 03/14 18:49, Stefan Weil wrote:
> Am 14.03.2014 09:38, schrieb Fam Zheng:
> > DANGEROUS: don't try it before you read to the end.
> >
> > A first "make distclean" will unset $(DSOSUF), a following "make
> > distclean" or "make clean" will find all the files and delete it.
> >
> > Including all the files in the .git directory!
>
> If you only use out-of-tree build, you are safe here. Maybe we should no
> longer support in-tree builds. Personally, I nearly never use them.
>
> > Fix it by only do it when $(DSOSUF) is not empty.
>
> s/do/doing/
>
> > Signed-off-by: Fam Zheng <address@hidden>
> > ---
> > Makefile | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index bd9cd4f..0666d6e 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -267,7 +267,7 @@ clean:
> > rm -f qemu-options.def
> > find . -name '*.[oda]' -type f -exec rm -f {} +
> > find . -name '*.l[oa]' -type f -exec rm -f {} +
> > - find . -name '*$(DSOSUF)' -type f -exec rm -f {} +
> > + if test -n "$(DSOSUF)"; then find . -name '*$(DSOSUF)' -type f -exec rm
> > -f {} +; fi
> > find . -name '*.mo' -type f -exec rm -f {} +
> > rm -f $(filter-out %.tlb,$(TOOLS)) $(HELPERS-y) qemu-ga TAGS cscope.*
> > *.pod *~ */*~
> > rm -f fsdev/*.pod
>
> No, I think it is still too dangerous to use a macro here. There are
> only two valid possibilities, so it's easy to name them explicitly:
>
> find -name "*.dll" -o -name "*.so"
>
> Is there a good reason why rm is called with option -f? Normally, -f
> should not be needed when cleaning generated files because those files
> are not write protected. The only other reason for -f would be
> suppressing an error message if rm tries to remove a non existing files,
> but that does not apply here.
>
> I'd also combine all find statements in a single statement. It is not
> necessary to parse the directory tree several times. That can be done in
> a separate patch.
>
Sounds good to me. I'll respin.
Thanks,
Fam