Migrate gitian-build.sh to python #13623

ken2812221 · 2018-07-10T14:32:58Z

Rename Mac OSX to MacOS, rename option from 'x' to 'm'

Fix a bug from

Lines 338 to 342 in b641f60

    
           # Signed Windows 
        
           echo "" 
        
           echo "Verifying v${VERSION} Signed Windows" 
        
           echo "" 
        
           ./bin/gverify -v -d ../gitian.sigs/ -r ${VERSION}-osx-signed ../bitcoin/contrib/gitian-descriptors/gitian-osx-signer.yml

laanwj · 2018-07-10T15:17:14Z

Thanks // Concept ACK, code looks good to me at first glance but ofcourse needs manual testing (as it's not tested by the test framework).

maflcko · 2018-07-10T15:45:07Z

Could remove the bash script in the same commit?

DrahtBot · 2018-07-10T16:27:58Z

Note to reviewers: This pull request conflicts with the following ones:

Switch to NSIS 3.03 to avoid DLL hijacking #13643 (Switch to NSIS 3.03 to avoid DLL hijacking by h4x3rotab)
Update gitian-build.sh for docker #13368 (Update gitian-build.sh for docker by achow101)
Change gitian-descriptors to use bionic instead #13171 (Change gitian-descriptors to use bionic instead by ken2812221)

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.

jtimon · 2018-07-10T20:18:40Z

Concept ACK

dongcarl · 2018-07-10T21:07:15Z

As gitian-build.sh is mostly calling other executables, I think bash/sh is still the most sensible/readable/least-dependencies choice. According to #13620 there are edge cases to fix, sure, but I'm not convinced that there's a need for a python rewrite.

Static analysis tools run against shell scripts in the check pipeline can eliminate/mitigate edge cases. I'd recommend: https://github.com/koalaman/shellcheck

laanwj · 2018-07-11T11:34:19Z

As gitian-build.sh is mostly calling other executables, I think bash/sh is still the most sensible/readable/least-dependencies choice.

The point is that most of the developers in this project have a much easier time reviewing and maintaining python code. Shell script has turned out to be a continuous struggle against shell differences, bad performance, bugs and other edge cases due to obscure quoting practices, and so on.

You might disagree personally--obviously everything is possible with shell script as well, but porting this to python is more of a social consensus.
(as for dependencies, I don't think that is a big thing here, Python 3 is already required to build the project and this requires no external modules)

maflcko · 2018-07-11T11:37:29Z

Also, the resulting python script is a lot shorter. Means less code weight to maintain in the future :)

mcdallas · 2018-07-11T16:34:24Z

contrib/gitian-build.py

You can add the parameter nargs=1 to make signer and version mandatory and skip lines 155-162. It will produce a list of one item. See https://docs.python.org/3/library/argparse.html#nargs

This isn't necessary since signer and version are positional arguments so if they aren't provided then an error will always be thrown. The checks later are still necessary since you can still pass in the empty string as an argument regardless of nargs.

Oh I missed the point of the checks. Still if we want to guard against empty string we can use the type argument with a custom callable (saves a few lines)

ken2812221 · 2018-07-12T02:45:12Z

Update: exit 1 if signer or version is empty.

jb55

I only tested some of the basic ergonomics since I don't have the gitian builder set up on my machine yet.

jb55 · 2018-07-13T06:05:14Z

contrib/gitian-build.py

is this option necessary? if there's a v you could just include it in the argument. I found this confusing when I ran the command, it was trying to checkout vmaster because I didn't have the --commit option.

The version without 'v' prefix is still used in gsign.

jb55 · 2018-07-13T06:06:16Z

contrib/gitian-build.py

this assumes you would be running the command from outside the bitcoin directory. is this correct? I found I had to cd .. and then run ./bitcoin/contrib/gitian-builder.py to make the command work?

From the instruction from https://github.com/bitcoin-core/docs/blob/master/gitian-building.md , you should copy the script to the same level as bitcoin folder

ah ok thanks

jb55 · 2018-07-13T06:11:17Z

contrib/gitian-build.py

perhaps we could default to build? or at least throw an error when all three are not selected?

fanquake · 2018-07-13T09:57:05Z

Using a67812d if I pass --setup and already have one of the required repos cloned I see:

Processing triggers for ureadahead (0.100.0-20) ...
Processing triggers for ufw (0.35-5) ...
fatal: destination path 'gitian.sigs' already exists and is not an empty directory.
Traceback (most recent call last):
  File "./gitian-build.py", line 188, in <module>
    main()
  File "./gitian-build.py", line 171, in main
    setup()
  File "./gitian-build.py", line 16, in setup
    subprocess.check_call(['git', 'clone', 'https://github.com/bitcoin-core/gitian.sigs.git'])
  File "/usr/lib/python3.6/subprocess.py", line 291, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'clone', 'https://github.com/bitcoin-core/gitian.sigs.git']' returned non-zero exit status 128.
ubuntu@ubuntu:~$ ls

Removed the repos and re-ran with --setup:

|               E |
+----[SHA256]-----+
sudo: debootstrap: command not found
Traceback (most recent call last):
  File "./gitian-build.py", line 188, in <module>
    main()
  File "./gitian-build.py", line 171, in main
    setup()
  File "./gitian-build.py", line 23, in setup
    subprocess.check_call(make_image_prog)
  File "/usr/lib/python3.6/subprocess.py", line 291, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['bin/make-base-vm', '--suite', 'trusty', '--arch', 'amd64', '--lxc']' returned non-zero exit status 1.

--setup --kvm

invoke-rc.d: policy-rc.d denied execution of restart.

Configuration file '/etc/sudoers'
 ==> Modified (by you or by a script) since installation.
 ==> Package distributor has shipped an updated version.
   What would you like to do about it ?  Your options are:
    Y or I  : install the package maintainer's version
    N or O  : keep your currently-installed version
      D     : show the differences between the versions
      Z     : start a shell to examine the situation
 The default action is to keep your current version.
*** sudoers (Y/I/N/O/D/Z) [default=N] ? dpkg: error processing package sudo (--configure):
 EOF on stdin at conffile prompt
invoke-rc.d: policy-rc.d denied execution of restart.
Errors were encountered while processing:
 sudo
E: Sub-process /usr/bin/dpkg returned an error code (1)

Traceback (most recent call last):
  File "./gitian-build.py", line 188, in <module>
    main()
  File "./gitian-build.py", line 171, in main
    setup()
  File "./gitian-build.py", line 23, in setup
    subprocess.check_call(make_image_prog)
  File "/usr/lib/python3.6/subprocess.py", line 291, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['bin/make-base-vm', '--suite', 'trusty', '--arch', 'amd64']' returned non-zero exit status 1.

ken2812221 · 2018-07-13T11:48:03Z

I don't know how to deal with KVM errors since I haven't used KVM ever, I just follow the same command with the original script. Can anyone give a hint?

fanquake · 2018-07-15T23:44:25Z

@ken2812221 Now that #13368 has been merged, can you update the new Python script to add Docker support. I will retest the changes here shortly.

DrahtBot · 2018-07-16T11:16:25Z

Needs rebase

Sjors · 2018-07-17T16:17:39Z

This is great, especially not having to worry about the exact sequence of arguments (the bash script had some quirks).

I'm trying to sign master on a Bionic VM (haven't tried a Debian 9 host).

# git clone bitcoin, gitian.sigs, etc
cp bitcoin/contrib/gitian-build.py .
# add MacOS stuff 
python3 gitian-build.py --setup sjors -c master
python3 gitian-build.py --detach-sign --no-commit -b sjors -c master

This throws an error for me, see #13623. The original bash script also throws that error, so probably not related. Just means I can't test the rest of the process on this VM.

When running --setup --docker it throws an error:

ERRO[0000] failed to dial gRPC: cannot connect to the Docker daemon. Is 'docker daemon' running on this host?: dial unix /var/run/docker.sock: connect: permission denied

docker ps also throws a permission error; sudo docker ps works. sudo usermod -aG docker $USER solves the problem. It creates an image base-bionic-amd64.

Trying to build with Docker complains:

Compiling master Linux
--- Building for bionic amd64 ---
Stopping target if it is up
Making a new image copy
Error: No such container: gitian-target

The bash script throws the same complaint, so that's not related.

When running gitian-build.py without arguments, it should probably just return --help (or otherwise point out that this exists).

Is there a corresponding PR to update the instructions? (mostly just renaming .sh to .py)

Can you add make to the dependencies installed during --setup?

For another PR: having a config flag to get a Trusty base image can be useful when building backports.

ken2812221 · 2018-07-17T17:37:06Z

Compiling master Linux
--- Building for bionic amd64 ---
Stopping target if it is up
Making a new image copy
Error: No such container: gitian-target

I believe the docker problem is already exist on gitian-build.sh because I saw the same error trying to use gitian-build.sh to create docker image.

Can you add make to the dependencies installed during --setup?

Why?

Sjors · 2018-07-17T17:53:37Z

Correct, it happens both with bash and python.

Without "make" setup throws an error on a fresh Bionic machine (unless you install all dependencies recommended in the docs, which is overkill).

maflcko · 2018-07-17T18:02:33Z

utACK 78f06e4

78f06e4 Migrate gitian-build.sh to python (Chun Kuan Lee) Pull request description: Fixes #13620 - Rename Mac OSX to MacOS, rename option from 'x' to 'm' - Fix a bug from https://github.com/bitcoin/bitcoin/blob/b641f60425674d737d77abd8c49929d953ea4154/contrib/gitian-build.sh#L338-L342 Tree-SHA512: ff943055d5feca345bd17b64311374db3937d14d2f21493116fd7ab7b4cb7042480abd6a3d1d7640073b00bc0badb300e8dd32618bf73ba182df417c0633397a

Sjors · 2018-07-17T18:35:59Z

On Debian 9, when trying to --setup with --docker, I'm getting Package 'docker.io' has no installation candidate.

So I installed Docker manually, but get the same error. Disabling programs += ['docker.io'] does the trick.

Fix bitcoin#13623 (comment)

* backport python version of gitian-build from bitcoin rename gitian-build script fix release notes typo 0.12.3-backports * change gitian host IP address * docker/etc fixes * use docker as default virtualization tech * add checksum to depends download stage * add SDK download checksum added * remove SDK check * fix verification

…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

* backport python version of gitian-build from bitcoin rename gitian-build script fix release notes typo 0.12.3-backports * change gitian host IP address * docker/etc fixes * use docker as default virtualization tech * add checksum to depends download stage * add SDK download checksum added * remove SDK check * fix verification

…hpay#2319) * backport python version of gitian-build from bitcoin rename gitian-build script fix release notes typo 0.12.3-backports * change gitian host IP address * docker/etc fixes * use docker as default virtualization tech * add checksum to depends download stage * add SDK download checksum added * remove SDK check * fix verification

Backport bitcoin bitcoin#13623 Migrate gitian-build.sh to python (dashpay#2319) * backport python version of gitian-build from bitcoin

…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

laanwj added the Build system label Jul 10, 2018

ken2812221 changed the title ~~[WIP] Migrate gitian-build.sh to python~~ Migrate gitian-build.sh to python Jul 10, 2018

laanwj mentioned this pull request Jul 11, 2018

gitian-build.sh cleanup #13628

Closed

mcdallas reviewed Jul 11, 2018

View reviewed changes

DrahtBot mentioned this pull request Jul 12, 2018

Switch to NSIS 3.03 to avoid DLL hijacking #13643

Closed

jb55 suggested changes Jul 13, 2018

View reviewed changes

fanquake mentioned this pull request Jul 15, 2018

Update gitian-build.sh for docker #13368

Merged

DrahtBot added the Needs rebase label Jul 16, 2018

Migrate gitian-build.sh to python

78f06e4

fanquake removed the Needs rebase label Jul 17, 2018

Sjors mentioned this pull request Jul 17, 2018

Gitian with Bionic host #13688

Closed

maflcko merged commit 78f06e4 into bitcoin:master Jul 17, 2018

ken2812221 deleted the python-gitian-build branch July 17, 2018 18:04

Bushstar mentioned this pull request Jul 19, 2018

commits from bitcoin/master FeatherCoin/Feathercoin#356

Merged

hebasto mentioned this pull request Aug 17, 2018

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

Merged

hebasto added a commit to hebasto/bitcoin that referenced this pull request Aug 19, 2018

Fix error message if Docker installed already

a2a316e

Fix bitcoin#13623 (comment)

CryptoCentric pushed a commit to absolute-community/absolute that referenced this pull request May 4, 2021

Migrate gitian-build.sh

b1d54c1

Backport bitcoin bitcoin#13623 Migrate gitian-build.sh to python (dashpay#2319) * backport python version of gitian-build from bitcoin

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

	# Signed Windows
	echo ""
	echo "Verifying v${VERSION} Signed Windows"
	echo ""
	./bin/gverify -v -d ../gitian.sigs/ -r ${VERSION}-osx-signed ../bitcoin/contrib/gitian-descriptors/gitian-osx-signer.yml

Migrate gitian-build.sh to python #13623

Migrate gitian-build.sh to python #13623

Uh oh!

Conversation

ken2812221 commented Jul 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj commented Jul 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko commented Jul 10, 2018

Uh oh!

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

Uh oh!

Uh oh!

jtimon commented Jul 10, 2018

Uh oh!

dongcarl commented Jul 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj commented Jul 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko commented Jul 11, 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!

ken2812221 commented Jul 12, 2018

Uh oh!

jb55 left a comment

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!

ken2812221 Jul 13, 2018 • 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!

fanquake commented Jul 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ken2812221 commented Jul 13, 2018

Uh oh!

fanquake commented Jul 15, 2018

Uh oh!

DrahtBot commented Jul 16, 2018

Uh oh!

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

Uh oh!

Uh oh!

ken2812221 commented Jul 17, 2018

Uh oh!

Sjors commented Jul 17, 2018

Uh oh!

maflcko commented Jul 17, 2018

Uh oh!

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

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

ken2812221 commented Jul 10, 2018 •

edited

Loading

laanwj commented Jul 10, 2018 •

edited

Loading

DrahtBot commented Jul 10, 2018 •

edited

Loading

dongcarl commented Jul 10, 2018 •

edited

Loading

laanwj commented Jul 11, 2018 •

edited

Loading

ken2812221 Jul 13, 2018 •

edited

Loading

fanquake commented Jul 13, 2018 •

edited

Loading

Sjors commented Jul 17, 2018 •

edited

Loading

Sjors commented Jul 17, 2018 •

edited

Loading