Re: [PATCH 29/36] build: use git-clean

[Josh Triplett <josh@joshtriplett.org>]

On Sun, Nov 05, 2017 at 09:16:31PM +0100, Luc Van Oostenryck wrote:
> On Sun, Nov 05, 2017 at 09:41:02AM -0800, Josh Triplett wrote:
> > On Sun, Nov 05, 2017 at 05:25:37PM +0100, Luc Van Oostenryck wrote:
> > > Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> > > ---
> > >  Makefile | 12 ++----------
> > >  1 file changed, 2 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/Makefile b/Makefile
> > > index afc1efe98..f0f7a5b69 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -233,16 +233,8 @@ check: all
> > >  	$(Q)cd validation && ./test-suite
> > >  
> > >  
> > > -clean: clean-check
> > > -	@rm -f *.[oa] .*.d $(PROGRAMS) version.h
> > > -clean-check:
> > > +clean: FORCE
> > >  	@echo '     CLEAN'
> > > -	@find validation/ \( -name "*.c.output.expected" \
> > > -	                 -o -name "*.c.output.got" \
> > > -	                 -o -name "*.c.output.diff" \
> > > -	                 -o -name "*.c.error.expected" \
> > > -	                 -o -name "*.c.error.got" \
> > > -	                 -o -name "*.c.error.diff" \
> > > -	                 \) -exec rm {} \;
> > > +	@git clean -q -d -x -e .sparse.mk -e '*.[ch]'
> > 
> > No, you can't use git-clean here, for two reasons.
> > 
> > First, it has to be possible to build, and clean, sparse from a
> > tarball release, not just a git repository.
> > 
> > And second, even if not for that, this should *not* use -x or -d, *only*
> > -X. Otherwise, you might remove, say, a pile of carefully written
> > patches and cover letters sitting in the user's Sparse source directory.
> 
> Hehe, I thought this would be somehow controversial ;)
> 
> TBH, I'm far from conviced for the first point.
> There hasn't been any tarbal release since 2014 and, unless I'm
> mistaken, people can't anymore get a tarball from kernel.org's cgit.

As far as I can tell,
https://git.kernel.org/pub/scm/devel/sparse/sparse.git/snapshot/sparse-0.5.1.tar.gz
still works, for instance.

> The real question would be: "is there anybody, doing some kind of
> development on sparse and who won't use git?"

Sure: anyone doing packaging. If someone wants to package Sparse for a
distribution, they'll likely do so in a way that does not include .git
in the source package. And if that package wants to "make clean" for any
reason, or if someone grabs the source from that distribution for
development and happens to run "make clean", that would be *bad*.

Worse yet: I keep my home directory in git, and would very much not like
"git clean" run in *that* if .git doesn't exist in Sparse. I'd be
immediately filing a "critical" bug report about data loss against the
package, and trying to restore what data I can from backups.

> For the second point, well ... I purposely added '-e *.[ch]' to
> avoid some problems and if I call 'make clean' I expect that things
> like patches and cover letter are removed, but I agree that my
> expectations here most probably doesn't match other's.

I most assuredly would not expect "make clean" to do this. I regularly
"make clean" in repositories while having valuable files in the
directory: patches, cover-letters, lists of email addresess to CC
patches to, benchmark results, etc.

> I think I'll drop this patch and continue to directly use
> 'git clean -x' when I needed it.

Thank you.