Scripts and tools: gitian-build.py improvements and corrections #13998

hebasto · 2018-08-17T22:04:35Z

The Docker does not depend on apt-cacher-ng package. Ref: Scripts and tools: fixed Docker dependencies #14002.
Do not try to install the Docker if docker.service is detected on the system (e.g., the Docker was installed manually). Fix Migrate gitian-build.sh to python #13623 (comment) by Sjors.
Prevent the setting of more than one environment variable for the gitian-builder (an alternative to Scripts and tools: Dedicated --virtualization option #13999). E.g., USE_LXC being set shadows USE_DOCKER; for details see gitian-builder/libexec/make-clean-vm:

VMSW=KVM
if [ -n "$USE_LXC" ]; then
    VMSW=LXC
elif [ -n "$USE_VBOX" ]; then
    VMSW=VBOX
elif [ -n "$USE_DOCKER" ]; then
    VMSW=DOCKER
fi

The gitian-builder/bin/gverify script returns the exit code 1 if a signature verification ends with 'BAD SIGNATURE' or 'MISMATCH' by design. This PR allows to see the verification results for all signatures without a premature fail of the gitian-build.py script. Ref: Scripts and tools: fix gitian-build.py --verify option #14014.

DrahtBot · 2018-08-19T08:37:46Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#15236 (scripts and tools: Make --setup command independent by hebasto)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

ken2812221

utACK c45cf0ba56d1405d8c6a87d899a8390c5a6e4b0b. Some nits and please squash.

ken2812221 · 2018-09-01T16:47:17Z

contrib/gitian-build.py

-    elif not args.kvm:
+    args.lxc = 'lxc' in args.virtualization
+    args.kvm = 'kvm' in args.virtualization
+    args.docker = 'docker' in args.virtualization


I think == would be better. To avoid lxckvm

ken2812221 · 2018-09-01T16:50:43Z

contrib/gitian-build.py

-        programs += ['python-vm-builder', 'qemu-kvm', 'qemu-utils']
-    elif args.docker:
+        programs += ['apt-cacher-ng', 'python-vm-builder', 'qemu-kvm', 'qemu-utils']
+    if args.docker and not os.path.isfile('/lib/systemd/system/docker.service'):


I think elif is better here.

ken2812221 · 2018-09-05T10:55:12Z

contrib/gitian-build.py

@@ -32,10 +32,10 @@ def setup():
        subprocess.check_call(['git', 'clone', 'https://github.com/bitcoin/bitcoin.git'])
    os.chdir('gitian-builder')
    make_image_prog = ['bin/make-base-vm', '--suite', 'bionic', '--arch', 'amd64']
+    if args.lxc:
+        make_image_prog += ['--lxc']
    if args.docker:


maflcko · 2018-09-05T12:28:44Z

contrib/gitian-build.py

-    parser.add_argument('-k', '--kvm', action='store_true', dest='kvm', help='Use KVM instead of LXC')
-    parser.add_argument('-d', '--docker', action='store_true', dest='docker', help='Use Docker instead of LXC')
-    parser.add_argument('-S', '--setup', action='store_true', dest='setup', help='Set up the Gitian building environment. Uses LXC. If you want to use KVM, use the --kvm option. Only works on Debian-based systems (Ubuntu, Debian)')
+    parser.add_argument('-V', '--virtualization', dest='virtualization', default='lxc', help='Specify virtualization technology to use: lxc for LXC, kvm for KVM, docker for Docker. Default is %(default)s')


Just noting that external documentation would have to be updated to reflect this change.

All related documentation which describes this script options is located in the https://github.com/bitcoin-core/docs repository. Therefore, IMO, there is nothing to add to this PR regarding docs.
Or did I miss something?

I'll submit an appropriate PR to the https://github.com/bitcoin-core/docs repository in a timely manner.

This can be achieved without a breaking change via

args.lxc = not args.kvm and not args.docker

hebasto · 2018-09-05T17:23:22Z

@ken2812221 Thank you for your review.

Some nits and please squash.

All nits are fixed and commits are squashed.

hebasto · 2018-09-06T06:02:14Z

Rebased.

ken2812221 · 2018-09-17T00:10:52Z

re-utACK be88ede

hebasto · 2018-10-20T18:11:49Z

@fanquake #14014 has been combined into this PR. Would you mind reviewing it?

hebasto · 2018-11-02T19:03:26Z

@fanquake May I ask you to restart travis against this PR?

ken2812221 · 2018-11-02T20:09:46Z

utACK dbd217be69ab6453f0adeb52cbacdace78406a45
travis restarted

maflcko · 2018-11-02T21:56:40Z

contrib/gitian-build.py

    print('\nVerifying v'+args.version+' Signed MacOS\n')
-    subprocess.check_call(['bin/gverify', '-v', '-d', '../gitian.sigs/', '-r', args.version+'-osx-signed', '../bitcoin/contrib/gitian-descriptors/gitian-osx-signer.yml'])
+    subprocess.call(['bin/gverify', '-v', '-d', '../gitian.sigs/', '-r', args.version+'-osx-signed', '../bitcoin/contrib/gitian-descriptors/gitian-osx-signer.yml'])


Why should this not fail if there is a mismatch?

The gitian-builder/bin/gverify script returns the exit code 1 if a signature verification ends with 'BAD SIGNATURE' or 'MISMATCH' by design.

From the Python docs:

subprocess.check_call():

If the return code was zero then return, otherwise raise CalledProcessError.

subprocess.call():

Wait for command to complete, then return the returncode attribute.

I understand that by this change the return code is ignored, but I'd like to understand why.

If the signatures are bad or the hashes mismatch, the verification should fail.

I mean script fails. You cannot see the result of verifying of the signatures that remain.
With this commit script will not fail and you will see the results for all signatures.

Fine, if you want to print the result for all signatures, but please don't change the behaviour (fail on non-zero exit code) of the script.

hebasto · 2018-11-22T23:43:31Z

@MarcoFalke Thank you for your review.

Fine, if you want to print the result for all signatures, but please don't change the behaviour (fail on non-zero exit code) of the script.

Fixed. Would you mind re-reviewing?

The Docker does not depend on apt-cacher-ng package. Do not try to install the Docker if docker.service is detected on the system (e.g., the Docker was installed manually). Also small style corrections were applied.

This prevents the setting of more than one environment variable for the gitian-builder (e.g., USE_LXC being set shadows USE_DOCKER; for details see gitian-builder/libexec/make-clean-vm).

The gitian-builder/bin/gverify script returns the exit code 1 if a signature verification ends with 'BAD SIGNATURE' or 'MISMATCH' by design. This commit allows to see the verification results for all signatures without a premature fail of the gitian-build.py script.

hebasto · 2019-05-18T16:52:57Z

Rebased.

@MarcoFalke

This can be achieved without a breaking change via
args.lxc = not args.kvm and not args.docker

Dropped new --virtualization option. So, no breaking changes now.

Also "do not warn about macOS build on setup" feature dropped in favor of #15236.

Commit messages have been brushed.
PR description updated.

hebasto · 2019-05-18T17:15:16Z

@Sjors
Would you mind reviewing this PR regarding your comment?

…rections 0f22a0c Fix gitian-build.py --verify option (Hennadii Stepanov) 4c56a79 Set/unset USE_LXC, USE_VBOX, USE_DOCKER explicitly (Hennadii Stepanov) cbbd988 Fix Docker related issues for gitian-build.py (Hennadii Stepanov) Pull request description: 1. The Docker does not depend on `apt-cacher-ng` package. Ref: #14002. 2. Do not try to install the Docker if `docker.service` is detected on the system (e.g., the Docker was installed manually). Fix #13623 (comment) by **Sjors**. 3. Prevent the setting of more than one environment variable for the `gitian-builder` (an alternative to #13999). E.g., USE_LXC being set shadows USE_DOCKER; for details see [`gitian-builder/libexec/make-clean-vm`](https://github.com/devrandom/gitian-builder/blob/93a62c7d7d018c66c02a19bac3d751144043cfec/libexec/make-clean-vm#L7): ```sh VMSW=KVM if [ -n "$USE_LXC" ]; then VMSW=LXC elif [ -n "$USE_VBOX" ]; then VMSW=VBOX elif [ -n "$USE_DOCKER" ]; then VMSW=DOCKER fi ``` 4. The [`gitian-builder/bin/gverify`](https://github.com/devrandom/gitian-builder/blob/master/bin/gverify) script returns the exit code 1 if a signature verification ends with 'BAD SIGNATURE' or 'MISMATCH' by design. This PR allows to see the verification results for all signatures without a premature fail of the `gitian-build.py` script. Ref: #14014. ACKs for commit 0f22a0: Tree-SHA512: 55f8a5cffa20d0c745f51a687f3199cea015fa616e56a0aee4c25b5ca0985036c61e8cf1922515338d8c6a85f873674ebe7a9a56a5069d65a187e383150f1a83

…and corrections 0f22a0c Fix gitian-build.py --verify option (Hennadii Stepanov) 4c56a79 Set/unset USE_LXC, USE_VBOX, USE_DOCKER explicitly (Hennadii Stepanov) cbbd988 Fix Docker related issues for gitian-build.py (Hennadii Stepanov) Pull request description: 1. The Docker does not depend on `apt-cacher-ng` package. Ref: bitcoin#14002. 2. Do not try to install the Docker if `docker.service` is detected on the system (e.g., the Docker was installed manually). Fix bitcoin#13623 (comment) by **Sjors**. 3. Prevent the setting of more than one environment variable for the `gitian-builder` (an alternative to bitcoin#13999). E.g., USE_LXC being set shadows USE_DOCKER; for details see [`gitian-builder/libexec/make-clean-vm`](https://github.com/devrandom/gitian-builder/blob/93a62c7d7d018c66c02a19bac3d751144043cfec/libexec/make-clean-vm#L7): ```sh VMSW=KVM if [ -n "$USE_LXC" ]; then VMSW=LXC elif [ -n "$USE_VBOX" ]; then VMSW=VBOX elif [ -n "$USE_DOCKER" ]; then VMSW=DOCKER fi ``` 4. The [`gitian-builder/bin/gverify`](https://github.com/devrandom/gitian-builder/blob/master/bin/gverify) script returns the exit code 1 if a signature verification ends with 'BAD SIGNATURE' or 'MISMATCH' by design. This PR allows to see the verification results for all signatures without a premature fail of the `gitian-build.py` script. Ref: bitcoin#14014. ACKs for commit 0f22a0: Tree-SHA512: 55f8a5cffa20d0c745f51a687f3199cea015fa616e56a0aee4c25b5ca0985036c61e8cf1922515338d8c6a85f873674ebe7a9a56a5069d65a187e383150f1a83

e0eae1b Make --setup command independent (Hennadii Stepanov) Pull request description: This PR allows a user to run: ```sh ./gitian-build.py --setup ``` without unused `signer` and `version` options. In master the `signer` and `version` options are mandatory. This implies the following code is dead: https://github.com/bitcoin/bitcoin/blob/387eb5b34307448f16d3769ac0245c4e3d996a38/contrib/gitian-build.py#L192-L200 This PR fixes those lines of code. Also this PR has a nice side effect: there is no more warnings about macOS build during processing `--setup` command. Ref: #13998 (comment) Note: https://github.com/bitcoin-core/docs/blob/master/gitian-building.md will be updated when this PR is merged. ACKs for commit e0eae1: Tree-SHA512: df851fe461e402229c57b410f30f1d8bc816e8a2600ece4249aa39c763566de5b661e7aa0af171d484727eb463a6d0e10cfcf459aa60ae1a5d4e12974a8615c6

…and corrections 0f22a0c Fix gitian-build.py --verify option (Hennadii Stepanov) 4c56a79 Set/unset USE_LXC, USE_VBOX, USE_DOCKER explicitly (Hennadii Stepanov) cbbd988 Fix Docker related issues for gitian-build.py (Hennadii Stepanov) Pull request description: 1. The Docker does not depend on `apt-cacher-ng` package. Ref: bitcoin#14002. 2. Do not try to install the Docker if `docker.service` is detected on the system (e.g., the Docker was installed manually). Fix bitcoin#13623 (comment) by **Sjors**. 3. Prevent the setting of more than one environment variable for the `gitian-builder` (an alternative to bitcoin#13999). E.g., USE_LXC being set shadows USE_DOCKER; for details see [`gitian-builder/libexec/make-clean-vm`](https://github.com/devrandom/gitian-builder/blob/93a62c7d7d018c66c02a19bac3d751144043cfec/libexec/make-clean-vm#L7): ```sh VMSW=KVM if [ -n "$USE_LXC" ]; then VMSW=LXC elif [ -n "$USE_VBOX" ]; then VMSW=VBOX elif [ -n "$USE_DOCKER" ]; then VMSW=DOCKER fi ``` 4. The [`gitian-builder/bin/gverify`](https://github.com/devrandom/gitian-builder/blob/master/bin/gverify) script returns the exit code 1 if a signature verification ends with 'BAD SIGNATURE' or 'MISMATCH' by design. This PR allows to see the verification results for all signatures without a premature fail of the `gitian-build.py` script. Ref: bitcoin#14014. ACKs for commit 0f22a0: Tree-SHA512: 55f8a5cffa20d0c745f51a687f3199cea015fa616e56a0aee4c25b5ca0985036c61e8cf1922515338d8c6a85f873674ebe7a9a56a5069d65a187e383150f1a83

fanquake added the Scripts and tools label Aug 18, 2018

fanquake mentioned this pull request Aug 19, 2018

Scripts and tools: fixed Docker dependencies #14002

Closed

hebasto mentioned this pull request Aug 19, 2018

Scripts and tools: Dedicated --virtualization option #13999

Closed

hebasto changed the title ~~Scripts and tools: fix error message if Docker installed already~~ Scripts and tools: gitian-build.py improvements and corrections Aug 19, 2018

bitcoin deleted a comment from DrahtBot Aug 19, 2018

DrahtBot mentioned this pull request Aug 19, 2018

Warn (don't fail!) on spelling errors. Fix typos reported by codespell. #13954

Merged

hebasto force-pushed the gitian-build-fix branch 4 times, most recently from e212ccb to 0131194 Compare August 19, 2018 22:03

laanwj requested review from ken2812221 and maflcko September 1, 2018 09:50

ken2812221 approved these changes Sep 5, 2018

View reviewed changes

maflcko reviewed Sep 5, 2018

View reviewed changes

hebasto force-pushed the gitian-build-fix branch from c45cf0b to a2b1d10 Compare September 5, 2018 17:12

hebasto force-pushed the gitian-build-fix branch from a2b1d10 to bc4ca46 Compare September 5, 2018 17:52

DrahtBot added the Needs rebase label Sep 5, 2018

hebasto force-pushed the gitian-build-fix branch from bc4ca46 to be88ede Compare September 6, 2018 05:44

DrahtBot removed the Needs rebase label Sep 6, 2018

DrahtBot mentioned this pull request Sep 9, 2018

Scripts and tools: fix gitian build related to Debian/MacOSX-SDK/curl #14176

Closed

fanquake mentioned this pull request Oct 20, 2018

Scripts and tools: fix gitian-build.py --verify option #14014

Closed

DrahtBot mentioned this pull request Oct 21, 2018

Enable flake8 rule E225 which checks for missing whitespace around op… #14534

Closed

maflcko reviewed Nov 2, 2018

View reviewed changes

hebasto force-pushed the gitian-build-fix branch from dbd217b to 710a636 Compare November 22, 2018 23:39

hebasto added 3 commits May 18, 2019 19:34

Fix Docker related issues for gitian-build.py

cbbd988

The Docker does not depend on apt-cacher-ng package. Do not try to install the Docker if docker.service is detected on the system (e.g., the Docker was installed manually). Also small style corrections were applied.

Set/unset USE_LXC, USE_VBOX, USE_DOCKER explicitly

4c56a79

This prevents the setting of more than one environment variable for the gitian-builder (e.g., USE_LXC being set shadows USE_DOCKER; for details see gitian-builder/libexec/make-clean-vm).

hebasto force-pushed the gitian-build-fix branch from 710a636 to 0f22a0c Compare May 18, 2019 16:39

hebasto mentioned this pull request May 19, 2019

scripts and tools: Make --setup command independent #15236

Merged

maflcko merged commit 0f22a0c into bitcoin:master May 20, 2019

hebasto deleted the gitian-build-fix branch May 20, 2019 17:09

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

Scripts and tools: gitian-build.py improvements and corrections #13998

Scripts and tools: gitian-build.py improvements and corrections #13998

Uh oh!

Conversation

hebasto commented Aug 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Aug 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

ken2812221 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hebasto commented Sep 5, 2018

Uh oh!

hebasto commented Sep 6, 2018

Uh oh!

ken2812221 commented Sep 17, 2018

Uh oh!

hebasto commented Oct 20, 2018

Uh oh!

hebasto commented Nov 2, 2018

Uh oh!

ken2812221 commented Nov 2, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hebasto commented Nov 22, 2018

Uh oh!

hebasto commented May 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented May 18, 2019

Uh oh!

Uh oh!

hebasto commented Aug 17, 2018 •

edited

Loading

DrahtBot commented Aug 19, 2018 •

edited

Loading

ken2812221 left a comment •

edited

Loading

hebasto commented May 18, 2019 •

edited

Loading