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

hebasto · 2019-01-23T09:34:17Z

This PR allows a user to run:

./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:

bitcoin/contrib/gitian-build.py

Lines 192 to 200 in 387eb5b

    
           # Signer and version shouldn't be empty 
        
           if args.signer == '': 
        
               print(script_name+': Missing signer.') 
        
               print('Try '+script_name+' --help for more information') 
        
               exit(1) 
        
           if args.version == '': 
        
               print(script_name+': Missing version.') 
        
               print('Try '+script_name+' --help for more information') 
        
               exit(1)

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.

contrib/gitian-build.py

laanwj · 2019-01-23T10:39:37Z

I think this breaks the functionality of using --setup together with other options?

hebasto · 2019-01-23T10:47:58Z

@laanwj

I think this breaks the functionality of using --setup together with other options?

Yes. This was made intended.

Currently, the script can stop regardless of other options ( --build, --sign etc):

bitcoin/contrib/gitian-build.py

Lines 41 to 44 in 03b8596

    
           if args.is_bionic and not args.kvm and not args.docker: 
        
               subprocess.check_call(['sudo', 'sed', '-i', 's/lxcbr0/br0/', '/etc/default/lxc-net']) 
        
               print('Reboot is required') 
        
               exit(0)

DrahtBot · 2019-01-23T11:59:19Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#13998 (Scripts and tools: gitian-build.py improvements and corrections 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.

practicalswift · 2019-01-23T12:09:58Z

contrib/gitian-build.py

@@ -184,6 +177,16 @@ def main():
        if not 'LXC_GUEST_IP' in os.environ.keys():
            os.environ['LXC_GUEST_IP'] = '10.0.3.5'

+    if args.setup:
+        setup()
+        exit(0)


Should be sys.exit(0) :-)

Surprised that flake8 didn't complain about that!

Fixed all occurrences in the script.

laanwj · 2019-01-23T12:34:27Z

Currently, the script will stop regardless of other options ( --build, --sign etc):

It should because a reboot is intended in those specific cases, not always!

hebasto · 2019-01-23T12:39:06Z

@laanwj

You are right. I mean "can stop", not "will stop".

It will stop while using LXC on Bionic.

hebasto · 2019-01-23T12:56:09Z

@laanwj

I think this breaks the functionality of using --setup together with other options?

Usually, --setup command is used only once at the first time.
With this PR a user still able to run:

./gitian-build.py --setup && ./gitian-build.py --build ${SIGNER} ${VERSION}

hebasto · 2019-01-23T18:11:39Z

Also, a user do not want to run --setup together with other commands because after --setup they usually want to copy MacOSX10.11.sdk.tar.gz to gitian-builder/inputs directory before --build.

laanwj · 2019-01-24T14:27:50Z

@ken2812221 you mostly wrote this code, can you take a look please?

ken2812221 · 2019-01-24T23:34:30Z

utACK 1950540

maflcko · 2019-01-25T00:05:13Z

utACk 1950540

hebasto · 2019-04-24T05:45:53Z

@practicalswift would you mind re-reviewing this PR?

hebasto · 2019-05-18T22:04:31Z

@laanwj

I think this breaks the functionality of using --setup together with other options?

After reconsidering your words, I believe you are right.
Fixed. No breaking changes now.
It is possible to run:

./gitian-build.py --setup --build ${SIGNER} ${VERSION}

or

./gitian-build.py --setup

hebasto · 2019-05-18T22:11:20Z

PR description updated.

A user can run 'gitian-build.py --setup' ignoring signer and version options. Get rid of warning about macOS build during setup for free.

hebasto · 2019-05-20T18:05:49Z

Rebased.

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

a20bf24 Make --setup command simpler (Hennadii Stepanov) Pull request description: After merging bitcoin/bitcoin#15236, the [`gitian-build.py`](https://github.com/bitcoin/bitcoin/blob/master/contrib/gitian-build.py) script allows to run `--setup` command ignoring `signer` and `version` options. Also my text editor is set to remove trailing spaces before saving. So, they are removed as well. Top commit has no ACKs. Tree-SHA512: 38d9418a977cac0ff8d322a030d5602aab4b16ef81ef388a7200a934c86300e2932e16cec536381d57102d3dd26454321206789c8644bcfbe33cb279f5cfc6db

…dependent

…dependent Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

…dependent

…dependent Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

…and docs Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> This adds a new streamlined Gitian setup guide tailored for Ubuntu/Debian users with Docker: doc/gitian-building/gitian-building-docker.md Included are some improvements to the build system itself: - A backport from Core simplifying `gitian-build.py --setup` calls such that the signer and version parameters are no longer required, as they are not used during setup. - Fixed the base VM automatically generated by `gitian-build.py` to Debian Buster. This seems to have been accidentally left as Ubuntu Bionic, which results in superfluous Bionic VMs being created. And some related documentation improvements: - Removed documentation about the old, currently unused build signing mechanism and updated it to point people to [Gitian signing](doc/gitian-signing.md) instead. - Added a link to the new Gitian on Docker setup document in the main [Gitian building](doc/gitian-building.md) guide. ## Ported PRs - bitcoin/bitcoin#15236 ## Test Plan - Run the Gitian builds by following the new guide (doc/gitian-building/gitian-building-docker.md), ideally on various different versions of Debian and Ubuntu. Make sure to use my fork/branch instead of the main repo in a couple places, though: 1. Instead of the instruction here: https://gitlab.com/andrew-128/bitcoin-cash-node/-/blob/gitian-docker-setup/doc/gitian-building/gitian-building-docker.md#L19, instead, do: `git clone https://gitlab.com/andrew-128/bitcoin-cash-node.git` 2. Instead of the instruction here: https://gitlab.com/andrew-128/bitcoin-cash-node/-/blob/gitian-docker-setup/doc/gitian-building/gitian-building-docker.md#L39, instead, do (replacing satoshi with your GitLab username): `./gitian-build.py --docker --detach-sign --no-commit --url https://gitlab.com/andrew-128/bitcoin-cash-node.git -b -c "satoshi" gitian-docker-setup` - Check that `gitian-build.py` still works as normal in other contexts. - `ninja check-lint`. - Review the updated documentation.

fanquake added the Scripts and tools label Jan 23, 2019

laanwj reviewed Jan 23, 2019

View reviewed changes

contrib/gitian-build.py Outdated Show resolved Hide resolved

practicalswift reviewed Jan 23, 2019

View reviewed changes

hebasto force-pushed the 20190123-gitian-build-setup branch from 42f4604 to 1950540 Compare January 23, 2019 12:26

hebasto changed the title ~~scripts and tools: Make --setup command independent~~ [WIP] scripts and tools: Make --setup command independent May 18, 2019

hebasto mentioned this pull request May 18, 2019

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

Merged

hebasto force-pushed the 20190123-gitian-build-setup branch from 1950540 to 416baac Compare May 18, 2019 21:53

hebasto changed the title ~~[WIP] scripts and tools: Make --setup command independent~~ scripts and tools: Make --setup command independent May 18, 2019

DrahtBot added the Needs rebase label May 20, 2019

Make --setup command independent

e0eae1b

A user can run 'gitian-build.py --setup' ignoring signer and version options. Get rid of warning about macOS build during setup for free.

hebasto force-pushed the 20190123-gitian-build-setup branch from 416baac to e0eae1b Compare May 20, 2019 18:05

DrahtBot removed the Needs rebase label May 20, 2019

maflcko merged commit e0eae1b into bitcoin:master May 20, 2019

hebasto deleted the 20190123-gitian-build-setup branch May 20, 2019 19:20

hebasto mentioned this pull request May 20, 2019

Make --setup command simpler bitcoin-core/docs#48

Merged

kwvg added a commit to kwvg/dash that referenced this pull request Jul 15, 2021

force-merge bitcoin#15236: scripts and tools: Make --setup command in…

09c063a

…dependent

kwvg added a commit to kwvg/dash that referenced this pull request Jul 15, 2021

force-merge bitcoin#15236: scripts and tools: Make --setup command in…

c79a6b5

…dependent

kwvg added a commit to kwvg/dash that referenced this pull request Jul 20, 2021

force-merge bitcoin#15236: scripts and tools: Make --setup command in…

e2451d3

…dependent

kwvg added a commit to kwvg/dash that referenced this pull request Jul 20, 2021

force-merge bitcoin#15236: scripts and tools: Make --setup command in…

2b6eba4

…dependent

kwvg added a commit to kwvg/dash that referenced this pull request Jul 20, 2021

force-merge bitcoin#15236: scripts and tools: Make --setup command in…

94eb5d3

…dependent

kwvg added a commit to kwvg/dash that referenced this pull request Jul 20, 2021

force-merge bitcoin#15236: scripts and tools: Make --setup command in…

c388468

…dependent

kwvg added a commit to kwvg/dash that referenced this pull request Aug 1, 2021

force-merge bitcoin#15236: scripts and tools: Make --setup command in…

3fd62e3

…dependent

kwvg added a commit to kwvg/dash that referenced this pull request Aug 24, 2021

force-merge bitcoin#15236: scripts and tools: Make --setup command in…

87c5325

…dependent

kwvg added a commit to kwvg/dash that referenced this pull request Aug 24, 2021

force-merge bitcoin#15236: scripts and tools: Make --setup command in…

59b4ffb

…dependent

kwvg added a commit to kwvg/dash that referenced this pull request Aug 25, 2021

force-merge bitcoin#15236: scripts and tools: Make --setup command in…

a98bf45

…dependent

kwvg added a commit to kwvg/dash that referenced this pull request Aug 26, 2021

force-merge bitcoin#15236: scripts and tools: Make --setup command in…

94bc4d5

…dependent

kwvg added a commit to kwvg/dash that referenced this pull request Aug 26, 2021

force-merge bitcoin#15236: scripts and tools: Make --setup command in…

21fcb1a

…dependent

kwvg added a commit to kwvg/dash that referenced this pull request Aug 26, 2021

force-merge bitcoin#15236: scripts and tools: Make --setup command in…

d62ac3f

…dependent Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

kwvg added a commit to kwvg/dash that referenced this pull request Aug 27, 2021

force-merge bitcoin#15236: scripts and tools: Make --setup command in…

3c514ea

…dependent Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

kwvg added a commit to kwvg/dash that referenced this pull request Aug 27, 2021

force-merge bitcoin#15236: scripts and tools: Make --setup command in…

5a90948

…dependent

kwvg added a commit to kwvg/dash that referenced this pull request Aug 30, 2021

force-merge bitcoin#15236: scripts and tools: Make --setup command in…

3567740

…dependent Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

kwvg added a commit to kwvg/dash that referenced this pull request Aug 31, 2021

force-merge bitcoin#15236: scripts and tools: Make --setup command in…

1e288e2

…dependent Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

kwvg added a commit to kwvg/dash that referenced this pull request Sep 1, 2021

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

a8e8a3d

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

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

	# Signer and version shouldn't be empty
	if args.signer == '':
	print(script_name+': Missing signer.')
	print('Try '+script_name+' --help for more information')
	exit(1)
	if args.version == '':
	print(script_name+': Missing version.')
	print('Try '+script_name+' --help for more information')
	exit(1)

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

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

Uh oh!

Conversation

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

Uh oh!

Uh oh!

Uh oh!

laanwj commented Jan 23, 2019

Uh oh!

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

Uh oh!

Uh oh!

DrahtBot commented Jan 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

practicalswift Jan 23, 2019

Choose a reason for hiding this comment

Uh oh!

hebasto Jan 23, 2019

Choose a reason for hiding this comment

Uh oh!

practicalswift Jan 23, 2019

Choose a reason for hiding this comment

Uh oh!

laanwj commented Jan 23, 2019

Uh oh!

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

Uh oh!

Uh oh!

hebasto commented Jan 23, 2019

Uh oh!

hebasto commented Jan 23, 2019

Uh oh!

laanwj commented Jan 24, 2019

Uh oh!

ken2812221 commented Jan 24, 2019

Uh oh!

maflcko commented Jan 25, 2019

Uh oh!

hebasto commented Apr 24, 2019

Uh oh!

hebasto commented May 18, 2019

Uh oh!

hebasto commented May 18, 2019

Uh oh!

hebasto commented May 20, 2019

Uh oh!

Uh oh!

hebasto commented Jan 23, 2019 •

edited

Loading

hebasto commented Jan 23, 2019 •

edited

Loading

DrahtBot commented Jan 23, 2019 •

edited

Loading

hebasto commented Jan 23, 2019 •

edited

Loading