Patchwork build: no need to build zstd in the bootstrap phase of a deb/rpm build

login
register
mail settings
Submitter via Mercurial-devel
Date Oct. 12, 2017, 7:09 p.m.
Message ID <f91472aa5015bd16b858.1507835366@cuben>
Download mbox | patch
Permalink /patch/24802/
State Superseded
Headers show

Comments

via Mercurial-devel - Oct. 12, 2017, 7:09 p.m.
# HG changeset patch
# User muxator <a.mux@inwind.it>
# Date 1507834828 -7200
#      Thu Oct 12 21:00:28 2017 +0200
# Node ID f91472aa5015bd16b8585e53ba34d64a7a05c8c7
# Parent  7259f0ddfc0f18138420e7c9c7e4145a25016d7b
build: no need to build zstd in the bootstrap phase of a deb/rpm build

When bootstrapping a deb/rpm build, packagelib.sh starts performing a local
build for the sole purpose of parsing the output of "hg version".
Then it "hg archive"s the source code, and builds everything again.

For that initial initial step, there is no purpose in compiling zstdlib.
This commit adds another target "make local-no-zstd", which is called by
packagelib.sh for the initial bootstrapping.

On my personal system, this cuts down 20 seconds for a package build (the
bootstrapping build goes from ~30 to ~10 seconds).
Augie Fackler - Oct. 13, 2017, 4:22 p.m.
On Thu, Oct 12, 2017 at 09:09:26PM +0200, muxator via Mercurial-devel wrote:
> # HG changeset patch
> # User muxator <a.mux@inwind.it>
> # Date 1507834828 -7200
> #      Thu Oct 12 21:00:28 2017 +0200
> # Node ID f91472aa5015bd16b8585e53ba34d64a7a05c8c7
> # Parent  7259f0ddfc0f18138420e7c9c7e4145a25016d7b
> build: no need to build zstd in the bootstrap phase of a deb/rpm build
>
> When bootstrapping a deb/rpm build, packagelib.sh starts performing a local
> build for the sole purpose of parsing the output of "hg version".
> Then it "hg archive"s the source code, and builds everything again.
>
> For that initial initial step, there is no purpose in compiling zstdlib.
> This commit adds another target "make local-no-zstd", which is called by
> packagelib.sh for the initial bootstrapping.
>
> On my personal system, this cuts down 20 seconds for a package build (the
> bootstrapping build goes from ~30 to ~10 seconds).
>
> diff --git a/Makefile b/Makefile
> --- a/Makefile
> +++ b/Makefile
> @@ -44,14 +44,20 @@
>
>  all: build doc
>
> -local:
> +local-inner:
>       $(PYTHON) setup.py $(PURE) \
>         build_py -c -d . \
> -       build_ext $(COMPILERFLAG) -i \
> +       build_ext $(COMPILERFLAG) $(ZSTD) -i \
>         build_hgexe $(COMPILERFLAG) -i \
>         build_mo
>       env HGRCPATH= $(PYTHON) hg version
>
> +local: ZSTD=--zstd
> +local: local-inner
> +
> +local-no-zstd: ZSTD=--no-zstd
> +local-no-zstd: local-inner

Could we skip the addition of the local-no-zstd thing, and make the
ZSTD var default to --zstd, and then in the packagelib script we can
jsut do `make local ZSTD=--no-zstd` instead?

> +
>  build:
>       $(PYTHON) setup.py $(PURE) build $(COMPILERFLAG)
>
> @@ -303,7 +309,8 @@
>  linux-wheels-i686:
>       docker run -e "HGTEST_JOBS=$(shell nproc)" --rm -ti -v `pwd`:/src quay.io/pypa/manylinux1_i686 linux32 /src/contrib/build-linux-wheels.sh
>
> -.PHONY: help all local build doc cleanbutpackages clean install install-bin \
> +.PHONY: help all local local-inner local-no-zstd build doc \
> +	cleanbutpackages clean install install-bin \
>       install-doc install-home install-home-bin install-home-doc \
>       dist dist-notests check tests check-code update-pot \
>       osx deb ppa docker-debian-jessie docker-debian-stretch \
> diff --git a/contrib/packagelib.sh b/contrib/packagelib.sh
> --- a/contrib/packagelib.sh
> +++ b/contrib/packagelib.sh
> @@ -9,7 +9,7 @@
>  # node: the node|short hg was built from, or empty if built from a tag
>  gethgversion() {
>      make cleanbutpackages
> -    make local || make local PURE=--pure
> +    make local-no-zstd || make local-no-zstd PURE=--pure
>      HG="$PWD/hg"
>
>      $HG version > /dev/null || { echo 'abort: hg version failed!'; exit 1 ; }
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
via Mercurial-devel - Oct. 13, 2017, 8:53 p.m.
Way more elegant, thanks!

On a closer look, an even simpler fix is possible: a "make local 
PURE=--pure" in packagelib.sh would be sufficient: no need to compile 
any c modules at all (base85, bdiff, zstd).

I am going to send a simpler patch.


btw: is it better to send patches via email or Phabricator?



On 10/13/2017 06:22 PM, Augie Fackler wrote:
> On Thu, Oct 12, 2017 at 09:09:26PM +0200, muxator via Mercurial-devel wrote:
>> # HG changeset patch
>> # User muxator <a.mux@inwind.it>
>> # Date 1507834828 -7200
>> #      Thu Oct 12 21:00:28 2017 +0200
>> # Node ID f91472aa5015bd16b8585e53ba34d64a7a05c8c7
>> # Parent  7259f0ddfc0f18138420e7c9c7e4145a25016d7b
>> build: no need to build zstd in the bootstrap phase of a deb/rpm build
>>
>> When bootstrapping a deb/rpm build, packagelib.sh starts performing a local
>> build for the sole purpose of parsing the output of "hg version".
>> Then it "hg archive"s the source code, and builds everything again.
>>
>> For that initial initial step, there is no purpose in compiling zstdlib.
>> This commit adds another target "make local-no-zstd", which is called by
>> packagelib.sh for the initial bootstrapping.
>>
>> On my personal system, this cuts down 20 seconds for a package build (the
>> bootstrapping build goes from ~30 to ~10 seconds).
>>
>> diff --git a/Makefile b/Makefile
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -44,14 +44,20 @@
>>
>>   all: build doc
>>
>> -local:
>> +local-inner:
>>        $(PYTHON) setup.py $(PURE) \
>>          build_py -c -d . \
>> -       build_ext $(COMPILERFLAG) -i \
>> +       build_ext $(COMPILERFLAG) $(ZSTD) -i \
>>          build_hgexe $(COMPILERFLAG) -i \
>>          build_mo
>>        env HGRCPATH= $(PYTHON) hg version
>>
>> +local: ZSTD=--zstd
>> +local: local-inner
>> +
>> +local-no-zstd: ZSTD=--no-zstd
>> +local-no-zstd: local-inner
> Could we skip the addition of the local-no-zstd thing, and make the
> ZSTD var default to --zstd, and then in the packagelib script we can
> jsut do `make local ZSTD=--no-zstd` instead?
>
>> +
>>   build:
>>        $(PYTHON) setup.py $(PURE) build $(COMPILERFLAG)
>>
>> @@ -303,7 +309,8 @@
>>   linux-wheels-i686:
>>        docker run -e "HGTEST_JOBS=$(shell nproc)" --rm -ti -v `pwd`:/src quay.io/pypa/manylinux1_i686 linux32 /src/contrib/build-linux-wheels.sh
>>
>> -.PHONY: help all local build doc cleanbutpackages clean install install-bin \
>> +.PHONY: help all local local-inner local-no-zstd build doc \
>> +	cleanbutpackages clean install install-bin \
>>        install-doc install-home install-home-bin install-home-doc \
>>        dist dist-notests check tests check-code update-pot \
>>        osx deb ppa docker-debian-jessie docker-debian-stretch \
>> diff --git a/contrib/packagelib.sh b/contrib/packagelib.sh
>> --- a/contrib/packagelib.sh
>> +++ b/contrib/packagelib.sh
>> @@ -9,7 +9,7 @@
>>   # node: the node|short hg was built from, or empty if built from a tag
>>   gethgversion() {
>>       make cleanbutpackages
>> -    make local || make local PURE=--pure
>> +    make local-no-zstd || make local-no-zstd PURE=--pure
>>       HG="$PWD/hg"
>>
>>       $HG version > /dev/null || { echo 'abort: hg version failed!'; exit 1 ; }
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Augie Fackler - Oct. 13, 2017, 8:54 p.m.
> On Oct 13, 2017, at 16:53, muxator <a.mux@inwind.it> wrote:
> 
> Way more elegant, thanks!
> 
> On a closer look, an even simpler fix is possible: a "make local PURE=--pure" in packagelib.sh would be sufficient: no need to compile any c modules at all (base85, bdiff, zstd).
> 
> I am going to send a simpler patch.
> 
> 
> btw: is it better to send patches via email or Phabricator?

Neither is really preferential - I'm still happier to work on emailed patches, but if you're more comfortable with phabricator that's fine too. I do prioritize emailed patches though. :)

> 
> 
> 
> On 10/13/2017 06:22 PM, Augie Fackler wrote:
>> On Thu, Oct 12, 2017 at 09:09:26PM +0200, muxator via Mercurial-devel wrote:
>>> # HG changeset patch
>>> # User muxator <a.mux@inwind.it>
>>> # Date 1507834828 -7200
>>> #      Thu Oct 12 21:00:28 2017 +0200
>>> # Node ID f91472aa5015bd16b8585e53ba34d64a7a05c8c7
>>> # Parent  7259f0ddfc0f18138420e7c9c7e4145a25016d7b
>>> build: no need to build zstd in the bootstrap phase of a deb/rpm build
>>> 
>>> When bootstrapping a deb/rpm build, packagelib.sh starts performing a local
>>> build for the sole purpose of parsing the output of "hg version".
>>> Then it "hg archive"s the source code, and builds everything again.
>>> 
>>> For that initial initial step, there is no purpose in compiling zstdlib.
>>> This commit adds another target "make local-no-zstd", which is called by
>>> packagelib.sh for the initial bootstrapping.
>>> 
>>> On my personal system, this cuts down 20 seconds for a package build (the
>>> bootstrapping build goes from ~30 to ~10 seconds).
>>> 
>>> diff --git a/Makefile b/Makefile
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -44,14 +44,20 @@
>>> 
>>>  all: build doc
>>> 
>>> -local:
>>> +local-inner:
>>>       $(PYTHON) setup.py $(PURE) \
>>>         build_py -c -d . \
>>> -       build_ext $(COMPILERFLAG) -i \
>>> +       build_ext $(COMPILERFLAG) $(ZSTD) -i \
>>>         build_hgexe $(COMPILERFLAG) -i \
>>>         build_mo
>>>       env HGRCPATH= $(PYTHON) hg version
>>> 
>>> +local: ZSTD=--zstd
>>> +local: local-inner
>>> +
>>> +local-no-zstd: ZSTD=--no-zstd
>>> +local-no-zstd: local-inner
>> Could we skip the addition of the local-no-zstd thing, and make the
>> ZSTD var default to --zstd, and then in the packagelib script we can
>> jsut do `make local ZSTD=--no-zstd` instead?
>> 
>>> +
>>>  build:
>>>       $(PYTHON) setup.py $(PURE) build $(COMPILERFLAG)
>>> 
>>> @@ -303,7 +309,8 @@
>>>  linux-wheels-i686:
>>>       docker run -e "HGTEST_JOBS=$(shell nproc)" --rm -ti -v `pwd`:/src quay.io/pypa/manylinux1_i686 linux32 /src/contrib/build-linux-wheels.sh
>>> 
>>> -.PHONY: help all local build doc cleanbutpackages clean install install-bin \
>>> +.PHONY: help all local local-inner local-no-zstd build doc \
>>> +	cleanbutpackages clean install install-bin \
>>>       install-doc install-home install-home-bin install-home-doc \
>>>       dist dist-notests check tests check-code update-pot \
>>>       osx deb ppa docker-debian-jessie docker-debian-stretch \
>>> diff --git a/contrib/packagelib.sh b/contrib/packagelib.sh
>>> --- a/contrib/packagelib.sh
>>> +++ b/contrib/packagelib.sh
>>> @@ -9,7 +9,7 @@
>>>  # node: the node|short hg was built from, or empty if built from a tag
>>>  gethgversion() {
>>>      make cleanbutpackages
>>> -    make local || make local PURE=--pure
>>> +    make local-no-zstd || make local-no-zstd PURE=--pure
>>>      HG="$PWD/hg"
>>> 
>>>      $HG version > /dev/null || { echo 'abort: hg version failed!'; exit 1 ; }
>>> _______________________________________________
>>> Mercurial-devel mailing list
>>> Mercurial-devel@mercurial-scm.org
>>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>

Patch

diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -44,14 +44,20 @@ 
 
 all: build doc
 
-local:
+local-inner:
 	$(PYTHON) setup.py $(PURE) \
 	  build_py -c -d . \
-	  build_ext $(COMPILERFLAG) -i \
+	  build_ext $(COMPILERFLAG) $(ZSTD) -i \
 	  build_hgexe $(COMPILERFLAG) -i \
 	  build_mo
 	env HGRCPATH= $(PYTHON) hg version
 
+local: ZSTD=--zstd
+local: local-inner
+
+local-no-zstd: ZSTD=--no-zstd
+local-no-zstd: local-inner
+
 build:
 	$(PYTHON) setup.py $(PURE) build $(COMPILERFLAG)
 
@@ -303,7 +309,8 @@ 
 linux-wheels-i686:
 	docker run -e "HGTEST_JOBS=$(shell nproc)" --rm -ti -v `pwd`:/src quay.io/pypa/manylinux1_i686 linux32 /src/contrib/build-linux-wheels.sh
 
-.PHONY: help all local build doc cleanbutpackages clean install install-bin \
+.PHONY: help all local local-inner local-no-zstd build doc \
+	cleanbutpackages clean install install-bin \
 	install-doc install-home install-home-bin install-home-doc \
 	dist dist-notests check tests check-code update-pot \
 	osx deb ppa docker-debian-jessie docker-debian-stretch \
diff --git a/contrib/packagelib.sh b/contrib/packagelib.sh
--- a/contrib/packagelib.sh
+++ b/contrib/packagelib.sh
@@ -9,7 +9,7 @@ 
 # node: the node|short hg was built from, or empty if built from a tag
 gethgversion() {
     make cleanbutpackages
-    make local || make local PURE=--pure
+    make local-no-zstd || make local-no-zstd PURE=--pure
     HG="$PWD/hg"
 
     $HG version > /dev/null || { echo 'abort: hg version failed!'; exit 1 ; }