Patchwork [1,of,2,STABLE] make: introduce a target to clean everything but packages

login
register
mail settings
Submitter Pierre-Yves David
Date July 28, 2016, 10:52 p.m.
Message ID <a52f8af0393b37ab6ddd.1469746368@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/16004/
State Accepted
Headers show

Comments

Pierre-Yves David - July 28, 2016, 10:52 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
# Date 1469745599 -7200
#      Fri Jul 29 00:39:59 2016 +0200
# Branch stable
# Node ID a52f8af0393b37ab6ddd8de4d91ee7d9cf2c36aa
# Parent  387bdd53c77e9f61bd7b0d491717440f7f57563a
make: introduce a target to clean everything but packages

Removing the 'packages' directory makes nightly builder life much harder.
Yuya Nishihara - July 29, 2016, 2:31 p.m.
On Fri, 29 Jul 2016 00:52:48 +0200, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> # Date 1469745599 -7200
> #      Fri Jul 29 00:39:59 2016 +0200
> # Branch stable
> # Node ID a52f8af0393b37ab6ddd8de4d91ee7d9cf2c36aa
> # Parent  387bdd53c77e9f61bd7b0d491717440f7f57563a
> make: introduce a target to clean everything but packages

Seems fine.

I'd rather add clean/distclean pair, but that might not be good for stable.

> diff -r 387bdd53c77e -r a52f8af0393b Makefile
> --- a/Makefile	Mon Jul 25 12:00:55 2016 -0700
> +++ b/Makefile	Fri Jul 29 00:39:59 2016 +0200
> @@ -60,7 +60,7 @@ wheel:
>  doc:
>  	$(MAKE) -C doc
>  
> -clean:
> +cleanbutpackages:
>  	-$(PYTHON) setup.py clean --all # ignore errors from this command
>  	find contrib doc hgext hgext3rd i18n mercurial tests \
>  		\( -name '*.py[cdo]' -o -name '*.so' \) -exec rm -f '{}' ';'
> @@ -68,10 +68,13 @@ clean:
>  	rm -f MANIFEST MANIFEST.in hgext/__index__.py tests/*.err
>  	rm -f mercurial/__modulepolicy__.py
>  	if test -d .hg; then rm -f mercurial/__version__.py; fi
> -	rm -rf build packages mercurial/locale
> +	rm -rf build mercurial/locale
>  	$(MAKE) -C doc clean
>  	$(MAKE) -C contrib/chg distclean

Nit: cleanbutpackages should be included in .PHONY list. Perhaps it can be
fixed in flight.
Pierre-Yves David - July 30, 2016, 11:07 a.m.
On 07/29/2016 04:31 PM, Yuya Nishihara wrote:
> On Fri, 29 Jul 2016 00:52:48 +0200, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
>> # Date 1469745599 -7200
>> #      Fri Jul 29 00:39:59 2016 +0200
>> # Branch stable
>> # Node ID a52f8af0393b37ab6ddd8de4d91ee7d9cf2c36aa
>> # Parent  387bdd53c77e9f61bd7b0d491717440f7f57563a
>> make: introduce a target to clean everything but packages
> 
> Seems fine.
> 
> I'd rather add clean/distclean pair, but that might not be good for stable.
> 
>> diff -r 387bdd53c77e -r a52f8af0393b Makefile
>> --- a/Makefile	Mon Jul 25 12:00:55 2016 -0700
>> +++ b/Makefile	Fri Jul 29 00:39:59 2016 +0200
>> @@ -60,7 +60,7 @@ wheel:
>>  doc:
>>  	$(MAKE) -C doc
>>  
>> -clean:
>> +cleanbutpackages:
>>  	-$(PYTHON) setup.py clean --all # ignore errors from this command
>>  	find contrib doc hgext hgext3rd i18n mercurial tests \
>>  		\( -name '*.py[cdo]' -o -name '*.so' \) -exec rm -f '{}' ';'
>> @@ -68,10 +68,13 @@ clean:
>>  	rm -f MANIFEST MANIFEST.in hgext/__index__.py tests/*.err
>>  	rm -f mercurial/__modulepolicy__.py
>>  	if test -d .hg; then rm -f mercurial/__version__.py; fi
>> -	rm -rf build packages mercurial/locale
>> +	rm -rf build mercurial/locale
>>  	$(MAKE) -C doc clean
>>  	$(MAKE) -C contrib/chg distclean
> 
> Nit: cleanbutpackages should be included in .PHONY list. Perhaps it can be
> fixed in flight.

Do you want a V2 with this fix or do you want to fix this inflight ?
Yuya Nishihara - July 30, 2016, 12:12 p.m.
On Sat, 30 Jul 2016 13:07:56 +0200, Pierre-Yves David wrote:
> On 07/29/2016 04:31 PM, Yuya Nishihara wrote:
> > On Fri, 29 Jul 2016 00:52:48 +0200, Pierre-Yves David wrote:
> >> # HG changeset patch
> >> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> >> # Date 1469745599 -7200
> >> #      Fri Jul 29 00:39:59 2016 +0200
> >> # Branch stable
> >> # Node ID a52f8af0393b37ab6ddd8de4d91ee7d9cf2c36aa
> >> # Parent  387bdd53c77e9f61bd7b0d491717440f7f57563a
> >> make: introduce a target to clean everything but packages
> > 
> > Seems fine.
> > 
> > I'd rather add clean/distclean pair, but that might not be good for stable.

I was thinking about whether "clean" should remove packages or not. "dist"
directory isn't removed by "clean", but "packages" is, which seems inconsistent.
I slightly prefer standard clean/distclean than cleanbutpackages/clean.

> >> diff -r 387bdd53c77e -r a52f8af0393b Makefile
> >> --- a/Makefile	Mon Jul 25 12:00:55 2016 -0700
> >> +++ b/Makefile	Fri Jul 29 00:39:59 2016 +0200
> >> @@ -60,7 +60,7 @@ wheel:
> >>  doc:
> >>  	$(MAKE) -C doc
> >>  
> >> -clean:
> >> +cleanbutpackages:
> >>  	-$(PYTHON) setup.py clean --all # ignore errors from this command
> >>  	find contrib doc hgext hgext3rd i18n mercurial tests \
> >>  		\( -name '*.py[cdo]' -o -name '*.so' \) -exec rm -f '{}' ';'
> >> @@ -68,10 +68,13 @@ clean:
> >>  	rm -f MANIFEST MANIFEST.in hgext/__index__.py tests/*.err
> >>  	rm -f mercurial/__modulepolicy__.py
> >>  	if test -d .hg; then rm -f mercurial/__version__.py; fi
> >> -	rm -rf build packages mercurial/locale
> >> +	rm -rf build mercurial/locale
> >>  	$(MAKE) -C doc clean
> >>  	$(MAKE) -C contrib/chg distclean
> > 
> > Nit: cleanbutpackages should be included in .PHONY list. Perhaps it can be
> > fixed in flight.
> 
> Do you want a V2 with this fix or do you want to fix this inflight ?

I don't think V2 is necessary just for that.
Pierre-Yves David - July 30, 2016, 12:43 p.m.
On 07/30/2016 02:12 PM, Yuya Nishihara wrote:
> On Sat, 30 Jul 2016 13:07:56 +0200, Pierre-Yves David wrote:
>> On 07/29/2016 04:31 PM, Yuya Nishihara wrote:
>>> On Fri, 29 Jul 2016 00:52:48 +0200, Pierre-Yves David wrote:
>>>> # HG changeset patch
>>>> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
>>>> # Date 1469745599 -7200
>>>> #      Fri Jul 29 00:39:59 2016 +0200
>>>> # Branch stable
>>>> # Node ID a52f8af0393b37ab6ddd8de4d91ee7d9cf2c36aa
>>>> # Parent  387bdd53c77e9f61bd7b0d491717440f7f57563a
>>>> make: introduce a target to clean everything but packages
>>>
>>> Seems fine.
>>>
>>> I'd rather add clean/distclean pair, but that might not be good for stable.
> 
> I was thinking about whether "clean" should remove packages or not. "dist"
> directory isn't removed by "clean", but "packages" is, which seems inconsistent.
> I slightly prefer standard clean/distclean than cleanbutpackages/clean.

according to matt comment in issue5262 he wants clean to remove packages.

https://bz.mercurial-scm.org/show_bug.cgi?id=5262#c7

I would myself agree with you about a clean/distclean target.
Yuya Nishihara - July 31, 2016, 4:17 a.m.
On Sat, 30 Jul 2016 14:43:43 +0200, Pierre-Yves David wrote:
> On 07/30/2016 02:12 PM, Yuya Nishihara wrote:
> > On Sat, 30 Jul 2016 13:07:56 +0200, Pierre-Yves David wrote:
> >> On 07/29/2016 04:31 PM, Yuya Nishihara wrote:
> >>> On Fri, 29 Jul 2016 00:52:48 +0200, Pierre-Yves David wrote:
> >>>> # HG changeset patch
> >>>> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> >>>> # Date 1469745599 -7200
> >>>> #      Fri Jul 29 00:39:59 2016 +0200
> >>>> # Branch stable
> >>>> # Node ID a52f8af0393b37ab6ddd8de4d91ee7d9cf2c36aa
> >>>> # Parent  387bdd53c77e9f61bd7b0d491717440f7f57563a
> >>>> make: introduce a target to clean everything but packages
> >>>
> >>> Seems fine.
> >>>
> >>> I'd rather add clean/distclean pair, but that might not be good for stable.
> > 
> > I was thinking about whether "clean" should remove packages or not. "dist"
> > directory isn't removed by "clean", but "packages" is, which seems inconsistent.
> > I slightly prefer standard clean/distclean than cleanbutpackages/clean.
> 
> according to matt comment in issue5262 he wants clean to remove packages.
> 
> https://bz.mercurial-scm.org/show_bug.cgi?id=5262#c7

Okay, fixed .PHONY in-flight and queued the series. Thanks for pointing out
the discussion.

Patch

diff -r 387bdd53c77e -r a52f8af0393b Makefile
--- a/Makefile	Mon Jul 25 12:00:55 2016 -0700
+++ b/Makefile	Fri Jul 29 00:39:59 2016 +0200
@@ -60,7 +60,7 @@  wheel:
 doc:
 	$(MAKE) -C doc
 
-clean:
+cleanbutpackages:
 	-$(PYTHON) setup.py clean --all # ignore errors from this command
 	find contrib doc hgext hgext3rd i18n mercurial tests \
 		\( -name '*.py[cdo]' -o -name '*.so' \) -exec rm -f '{}' ';'
@@ -68,10 +68,13 @@  clean:
 	rm -f MANIFEST MANIFEST.in hgext/__index__.py tests/*.err
 	rm -f mercurial/__modulepolicy__.py
 	if test -d .hg; then rm -f mercurial/__version__.py; fi
-	rm -rf build packages mercurial/locale
+	rm -rf build mercurial/locale
 	$(MAKE) -C doc clean
 	$(MAKE) -C contrib/chg distclean
 
+clean: cleanbutpackages
+	rm -rf packages
+
 install: install-bin install-doc
 
 install-bin: build