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

pull hebasto wants to merge 3 commits into bitcoin:master from hebasto:gitian-build-fix changing 1 files +41 −21
  1. hebasto commented at 10:04 pm on August 17, 2018: member

    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:

    0VMSW=KVM
1if [ -n "$USE_LXC" ]; then
2    VMSW=LXC
3elif [ -n "$USE_VBOX" ]; then
4    VMSW=VBOX
5elif [ -n "$USE_DOCKER" ]; then
6    VMSW=DOCKER
7fi
    1. 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: #14014.
  2. fanquake added the label Scripts and tools on Aug 18, 2018
  3. hebasto renamed this:
    Scripts and tools: fix error message if Docker installed already
    Scripts and tools: gitian-build.py improvements and corrections
    on Aug 19, 2018
  4. fanquake deleted a comment on Aug 19, 2018
  5. DrahtBot commented at 8:37 am on August 19, 2018: member

    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.

  6. hebasto force-pushed on Aug 19, 2018
  7. hebasto force-pushed on Aug 19, 2018
  8. hebasto force-pushed on Aug 19, 2018
  9. hebasto force-pushed on Aug 19, 2018
  10. laanwj requested review from ken2812221 on Sep 1, 2018
  11. laanwj requested review from MarcoFalke on Sep 1, 2018
  12. in contrib/gitian-build.py:170 in c45cf0ba56 outdated 
    174-    if args.docker:
175-        os.environ['USE_DOCKER'] = '1'
176-    elif not args.kvm:
177+    args.lxc = 'lxc' in args.virtualization
178+    args.kvm = 'kvm' in args.virtualization
179+    args.docker = 'docker' in args.virtualization
    ken2812221 commented at 4:47 pm on September 1, 2018:
    I think == would be better. To avoid lxckvm
  13. in contrib/gitian-build.py:15 in c45cf0ba56 outdated 
    13+        programs += ['apt-cacher-ng', 'lxc', 'debootstrap']
14     if args.kvm:
15-        programs += ['python-vm-builder', 'qemu-kvm', 'qemu-utils']
16-    elif args.docker:
17+        programs += ['apt-cacher-ng', 'python-vm-builder', 'qemu-kvm', 'qemu-utils']
18+    if args.docker and not os.path.isfile('/lib/systemd/system/docker.service'):
    ken2812221 commented at 4:50 pm on September 1, 2018:
    I think elif is better here.
  14. in contrib/gitian-build.py:35 in c45cf0ba56 outdated 
    31@@ -32,10 +32,10 @@ def setup():
32         subprocess.check_call(['git', 'clone', 'https://github.com/bitcoin/bitcoin.git'])
33     os.chdir('gitian-builder')
34     make_image_prog = ['bin/make-base-vm', '--suite', 'bionic', '--arch', 'amd64']
35+    if args.lxc:
36+        make_image_prog += ['--lxc']
37     if args.docker:
    ken2812221 commented at 10:55 am on September 5, 2018:
    elif
  15. ken2812221 approved
  16. ken2812221 commented at 11:01 am on September 5, 2018: contributor
    utACK c45cf0ba56d1405d8c6a87d899a8390c5a6e4b0b. Some nits and please squash.
  17. in contrib/gitian-build.py:147 in c45cf0ba56 outdated 
    142@@ -143,9 +143,8 @@ def main():
143     parser.add_argument('-o', '--os', dest='os', default='lwm', help='Specify which Operating Systems the build is for. Default is %(default)s. l for Linux, w for Windows, m for MacOS')
144     parser.add_argument('-j', '--jobs', dest='jobs', default='2', help='Number of processes to use. Default %(default)s')
145     parser.add_argument('-m', '--memory', dest='memory', default='2000', help='Memory to allocate in MiB. Default %(default)s')
146-    parser.add_argument('-k', '--kvm', action='store_true', dest='kvm', help='Use KVM instead of LXC')
147-    parser.add_argument('-d', '--docker', action='store_true', dest='docker', help='Use Docker instead of LXC')
148-    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)')
149+    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')
    MarcoFalke commented at 12:28 pm on September 5, 2018:
    Just noting that external documentation would have to be updated to reflect this change.
    hebasto commented at 6:37 pm on September 5, 2018:

    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.

    MarcoFalke commented at 8:53 pm on May 17, 2019:

    This can be achieved without a breaking change via

    0args.lxc = not args.kvm and not args.docker
  18. hebasto force-pushed on Sep 5, 2018
  19. hebasto commented at 5:23 pm on September 5, 2018: member

    @ken2812221 Thank you for your review.

    Some nits and please squash.

    All nits are fixed and commits are squashed.

  20. hebasto force-pushed on Sep 5, 2018
  21. DrahtBot added the label Needs rebase on Sep 5, 2018
  22. hebasto force-pushed on Sep 6, 2018
  23. hebasto commented at 6:02 am on September 6, 2018: member
    Rebased.
  24. DrahtBot removed the label Needs rebase on Sep 6, 2018
  25. ken2812221 commented at 0:10 am on September 17, 2018: contributor
    re-utACK be88ede
  26. hebasto commented at 6:11 pm on October 20, 2018: member
    @fanquake #14014 has been combined into this PR. Would you mind reviewing it?
  27. hebasto commented at 7:03 pm on November 2, 2018: member
    @fanquake May I ask you to restart travis against this PR?
  28. ken2812221 commented at 8:09 pm on November 2, 2018: contributor
    utACK dbd217be69ab6453f0adeb52cbacdace78406a45 travis restarted
  29. in contrib/gitian-build.py:129 in dbd217be69 outdated 
    129     print('\nVerifying v'+args.version+' Signed Windows\n')
130-    subprocess.check_call(['bin/gverify', '-v', '-d', '../gitian.sigs/', '-r', args.version+'-win-signed', '../bitcoin/contrib/gitian-descriptors/gitian-win-signer.yml'])
131+    subprocess.call(['bin/gverify', '-v', '-d', '../gitian.sigs/', '-r', args.version+'-win-signed', '../bitcoin/contrib/gitian-descriptors/gitian-win-signer.yml'])
132     print('\nVerifying v'+args.version+' Signed MacOS\n')
133-    subprocess.check_call(['bin/gverify', '-v', '-d', '../gitian.sigs/', '-r', args.version+'-osx-signed', '../bitcoin/contrib/gitian-descriptors/gitian-osx-signer.yml'])
134+    subprocess.call(['bin/gverify', '-v', '-d', '../gitian.sigs/', '-r', args.version+'-osx-signed', '../bitcoin/contrib/gitian-descriptors/gitian-osx-signer.yml'])
    MarcoFalke commented at 9:56 pm on November 2, 2018:
    Why should this not fail if there is a mismatch?
    hebasto commented at 10:09 pm on November 2, 2018:

    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:

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

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

    MarcoFalke commented at 10:17 pm on November 2, 2018:

    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.

    hebasto commented at 10:24 pm on November 2, 2018:
    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.
    MarcoFalke commented at 5:22 pm on November 15, 2018:
    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.
  30. hebasto force-pushed on Nov 22, 2018
  31. hebasto commented at 11:43 pm on November 22, 2018: member

    @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?

  32. Fix Docker related issues for gitian-build.py 
    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.
    cbbd98863b
  33. Set/unset USE_LXC, USE_VBOX, USE_DOCKER explicitly 
    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).
    4c56a798c0
  34. Fix gitian-build.py --verify option 
    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.
    0f22a0cf2f
  35. hebasto force-pushed on May 18, 2019
  36. hebasto commented at 4:52 pm on May 18, 2019: member

    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.

  37. hebasto commented at 5:15 pm on May 18, 2019: member
    @Sjors Would you mind reviewing this PR regarding your comment?
  38. MarcoFalke merged this on May 20, 2019
  39. MarcoFalke closed this on May 20, 2019
  40. MarcoFalke referenced this in commit f49b8d4783 on May 20, 2019
  41. hebasto deleted the branch on May 20, 2019
  42. sidhujag referenced this in commit 19c473999e on May 20, 2019
  43. MarcoFalke referenced this in commit 2d1583ee6a on May 20, 2019
  44. UdjinM6 referenced this in commit 84b3c0b1ee on Sep 1, 2021
  45. UdjinM6 referenced this in commit 5baf92492b on Sep 2, 2021
  46. UdjinM6 referenced this in commit 10c56c07b9 on Sep 2, 2021
  47. UdjinM6 referenced this in commit 0f8ae5e6e6 on Sep 2, 2021
  48. UdjinM6 referenced this in commit 678e2af0f3 on Sep 2, 2021
  49. UdjinM6 referenced this in commit f512e704d5 on Sep 2, 2021
  50. MarcoFalke locked this on Dec 16, 2021


hebasto DrahtBot ken2812221 MarcoFalke

Labels
Scripts and tools

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-07-08 22:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me