script: Lint Gitian descriptors with ShellCheck

hebasto commented at 9:10 pm on November 3, 2019: member

This PR extracts shell scripts from Gitian descriptors (contrib/gitian-descriptors/) and checks for ShellCheck warnings as any other one.

Some non-controversial warnings are fixed.

DrahtBot added the label Build system on Nov 3, 2019

DrahtBot added the label Scripts and tools on Nov 3, 2019

DrahtBot added the label Tests on Nov 3, 2019

in test/lint/print-gitian-script.py:1 in 9c23a46b24 outdated

0@@ -0,0 +1,22 @@
1+#!/usr/bin/env python3

promag commented at 10:23 pm on November 3, 2019:

Is this really necessary? Couldn’t use some YAML processor like yq?

hebasto commented at 5:58 am on November 4, 2019:

There is no straightforward way to install yq on Travis, unfortunately.

Or did you mean that yq?

hebasto commented at 6:42 am on November 4, 2019:

Couldn’t use some YAML processor like yq?

Done. TIL, thank you ;)

promag commented at 10:23 pm on November 3, 2019: member

Concept ACK.

hebasto force-pushed on Nov 4, 2019

practicalswift commented at 7:04 am on November 4, 2019: contributor

Concept ACK: good idea!

Could perhaps check for yq and skip the Gitian checking if it is not installed (with a warning message) in order to not break current setups?

hebasto force-pushed on Nov 4, 2019

hebasto commented at 7:22 am on November 4, 2019: member

@practicalswift

Could perhaps check for yq and skip the Gitian checking if it is not installed (with a warning message) in order to not break current setups?

Done. Thank you.

fanquake added the label Needs gitian build on Nov 4, 2019

fanquake requested review from dongcarl on Nov 4, 2019

in contrib/gitian-descriptors/gitian-win.yml:118 in 93faddf011 outdated

116+  CONFIG_SITE=${BASEPREFIX}/$(echo "${HOSTS}" | awk '{print $1;}')/share/config.site ./configure --prefix=/
117   make dist
118-  SOURCEDIST=`echo bitcoin-*.tar.gz`
119-  DISTNAME=`echo ${SOURCEDIST} | sed 's/.tar.*//'`
120+  SOURCEDIST=$(echo bitcoin-*.tar.gz)
121+  DISTNAME=${SOURCEDIST//.tar.*/}

dongcarl commented at 4:50 pm on November 4, 2019:

0  DISTNAME=${SOURCEDIST/%.tar.gz}

hebasto commented at 5:32 pm on November 4, 2019:

Done.

in contrib/gitian-descriptors/gitian-win-signer.yml:40 in 93faddf011 outdated

35@@ -36,7 +36,7 @@ script: |
36   ./configure --without-gsf --without-curl --disable-dependency-tracking
37   make
38   find ${UNSIGNED_DIR} -name "*-unsigned.exe" | while read i; do
39-    INFILE="`basename "${i}"`"
40-    OUTFILE="`echo "${INFILE}" | sed s/-unsigned//`"
41+    INFILE="$(basename "${i}")"
42+    OUTFILE="${INFILE//-unsigned/}"

dongcarl commented at 4:51 pm on November 4, 2019:

0    OUTFILE="${INFILE/%-unsigned}"

hebasto commented at 5:33 pm on November 4, 2019:

Done.

in contrib/gitian-descriptors/gitian-osx.yml:111 in 93faddf011 outdated

109+  CONFIG_SITE=${BASEPREFIX}/$(echo "${HOSTS}" | awk '{print $1;}')/share/config.site ./configure --prefix=/
110   make dist
111-  SOURCEDIST=`echo bitcoin-*.tar.gz`
112-  DISTNAME=`echo ${SOURCEDIST} | sed 's/.tar.*//'`
113+  SOURCEDIST=$(echo bitcoin-*.tar.gz)
114+  DISTNAME=${SOURCEDIST//.tar.*/}

dongcarl commented at 4:52 pm on November 4, 2019:

Same here.

hebasto commented at 5:33 pm on November 4, 2019:

Done.

in contrib/gitian-descriptors/gitian-linux.yml:148 in 93faddf011 outdated

146+  CONFIG_SITE=${BASEPREFIX}/$(echo "${HOSTS}" | awk '{print $1;}')/share/config.site ./configure --prefix=/
147   make dist
148-  SOURCEDIST=`echo bitcoin-*.tar.gz`
149-  DISTNAME=`echo ${SOURCEDIST} | sed 's/.tar.*//'`
150+  SOURCEDIST=$(echo bitcoin-*.tar.gz)
151+  DISTNAME=${SOURCEDIST//.tar.*/}

dongcarl commented at 4:52 pm on November 4, 2019:

Same here.

hebasto commented at 5:33 pm on November 4, 2019:

Done.

dongcarl changes_requested

fanquake added the label Waiting for author on Nov 4, 2019

hebasto force-pushed on Nov 4, 2019

hebasto commented at 5:31 pm on November 4, 2019: member

@dongcarl Thank you for your review. All your comments have been addressed.

in test/lint/lint-shell.sh:54 in a953da7804 outdated

51+
52+for descriptor in $(git ls-files -- 'contrib/gitian-descriptors/*.yml')
53+do
54+    echo
55+    echo "$descriptor"
56+    SCRIPT=$'#!/bin/bash\n'$(yq -r .script "$descriptor")

dongcarl commented at 5:41 pm on November 4, 2019:

0    SCRIPT=$'#!/usr/bin/env bash\n'$(yq -r .script "$descriptor")

hebasto commented at 6:03 pm on November 4, 2019:

It just mimics the way gitian-builder/bin/gbuild makes shell script:

0  File.open("var/build-script", "w") do |script|
1    script.puts "#!/bin/bash"

dongcarl commented at 6:07 pm on November 4, 2019:

Ah! Makes sense to do it this way, could you add a comment?

hebasto commented at 6:11 pm on November 4, 2019:

Sure!

hebasto commented at 10:52 pm on November 4, 2019:

Done.

dongcarl changes_requested

dongcarl commented at 5:43 pm on November 4, 2019: member

For all the places where you had to split export DIR=$(pwd), just use export DIR="$PWD", which doesn’t require the splitting

hebasto force-pushed on Nov 4, 2019

hebasto commented at 10:53 pm on November 4, 2019: member

For all the places where you had to split export DIR=$(pwd), just use export DIR="$PWD", which doesn’t require the splitting

Done.

DrahtBot commented at 10:06 am on November 5, 2019: member

Gitian builds

File	commit fba574c908bb61eff1a0e83c935f3526ba9035f2(master)	commit 03cba5165b32831d5b4c925c508bd45858136321(master and this pull)
bitcoin-0.19.99-aarch64-linux-gnu-debug.tar.gz	`de237fcbed0428e9...`	`30695e643ab9f5c8...`
bitcoin-0.19.99-aarch64-linux-gnu.tar.gz	`37414ac687fca757...`	`bf632b6eb372cc1e...`
bitcoin-0.19.99-arm-linux-gnueabihf-debug.tar.gz	`15ec11ef799983be...`	`8eafdee2275e4747...`
bitcoin-0.19.99-arm-linux-gnueabihf.tar.gz	`d2a4b5c5eecf4867...`	`9c083fd1ca78126e...`
bitcoin-0.19.99-i686-pc-linux-gnu-debug.tar.gz	`93c7a296c2b1892a...`	`5e764f885367d3f0...`
bitcoin-0.19.99-i686-pc-linux-gnu.tar.gz	`6ba72605de448718...`	`35bd351eb4b6e563...`
bitcoin-0.19.99-osx-unsigned.dmg	`9f5e8bd0f7b5d596...`	`037dc66f64c011a8...`
bitcoin-0.19.99-osx64.tar.gz	`e0c9bae97735661b...`	`5ce605bc2e7eecad...`
bitcoin-0.19.99-riscv64-linux-gnu-debug.tar.gz	`00eb708d00240949...`	`3bb47e7fb3adfd13...`
bitcoin-0.19.99-riscv64-linux-gnu.tar.gz	`01c2cac7151b4b97...`	`d2c2da2bcdc3caed...`
bitcoin-0.19.99-win64-debug.zip	`39909ee795800bfb...`
bitcoin-0.19.99-win64-setup-unsigned.exe	`c2a4f6aa56e0ae3b...`
bitcoin-0.19.99-win64.zip	`55d8e692451261b6...`
bitcoin-0.19.99-x86_64-linux-gnu-debug.tar.gz	`38343844b0191747...`	`2a7ebe8ad5c2bd4d...`
bitcoin-0.19.99-x86_64-linux-gnu.tar.gz	`9b7e7f889b59604c...`	`183f728c9b074532...`
bitcoin-0.19.99.tar.gz	`468e3d9979c27430...`	`224f38363c8c26c7...`
bitcoin-core-linux-0.20-res.yml	`1147a1c215d2cfbd...`	`b0a281cb13e2d3ae...`
bitcoin-core-osx-0.20-res.yml	`48b30111fb80c772...`	`a1142403eb634cdc...`
bitcoin-core-win-0.20-res.yml	`84c74d55aeea95e2...`
linux-build.log	`6898cbc40626a205...`	`61792b23cc02b876...`
osx-build.log	`91702333e74cd1f8...`	`b1d5202ba081635f...`
win-build.log	`fefebb4a795de7f7...`
bitcoin-core-linux-0.20-res.yml.diff		`ab3952092e2ea658...`
bitcoin-core-osx-0.20-res.yml.diff		`44759f80b2f42f19...`
linux-build.log.diff		`203d923c4540f2aa...`
osx-build.log.diff		`2c8087914b8df914...`

DrahtBot removed the label Needs gitian build on Nov 5, 2019

MarcoFalke added the label Needs gitian build on Nov 5, 2019

DrahtBot commented at 4:36 am on November 6, 2019: member

Gitian builds

File	commit bdda137878904e9401a84e308ac74c93c2ef87c1(master)	commit 52a158a088072a8dc312ec7053c76ea114ef44bd(master and this pull)
bitcoin-0.19.99-aarch64-linux-gnu-debug.tar.gz	`338c2453e92576d6...`	`d00f76215eb7cad7...`
bitcoin-0.19.99-aarch64-linux-gnu.tar.gz	`ebfe02ebb327a5fd...`	`829fca33b72e3e49...`
bitcoin-0.19.99-arm-linux-gnueabihf-debug.tar.gz	`10685a8364a42850...`	`809e882222ecdb8b...`
bitcoin-0.19.99-arm-linux-gnueabihf.tar.gz	`5debad8ecf03f614...`	`bd7f0819847ee09a...`
bitcoin-0.19.99-i686-pc-linux-gnu-debug.tar.gz	`c94274278265de7d...`	`568b25450ace1558...`
bitcoin-0.19.99-i686-pc-linux-gnu.tar.gz	`9199c2a5e431eb9a...`	`a0d9500045d0ff3b...`
bitcoin-0.19.99-osx-unsigned.dmg	`7da5a7f569a4318d...`	`9e25d1d09f726e61...`
bitcoin-0.19.99-osx64.tar.gz	`a459a9173951314d...`	`255e95cfc99e80e3...`
bitcoin-0.19.99-riscv64-linux-gnu-debug.tar.gz	`660347859b4073ee...`	`09871d8a73d30994...`
bitcoin-0.19.99-riscv64-linux-gnu.tar.gz	`feb185a593b038d9...`	`e0064e2658430257...`
bitcoin-0.19.99-win64-debug.zip	`a58bd2fba33f4e15...`	`548fc80baf980816...`
bitcoin-0.19.99-win64-setup-unsigned.exe	`501af3958d794c3d...`	`52437c0e5bd068d8...`
bitcoin-0.19.99-win64.zip	`fb49f8e495bd1bdc...`	`81b3b6ffa93ffb2b...`
bitcoin-0.19.99-x86_64-linux-gnu-debug.tar.gz	`8b1f655dfa7aab99...`	`7c4742ffc5d1bcff...`
bitcoin-0.19.99-x86_64-linux-gnu.tar.gz	`21b36fd6521700d8...`	`f5bf832c0c5bf244...`
bitcoin-0.19.99.tar.gz	`96c42231845ab16d...`	`93fd881e2c8eacc2...`
bitcoin-core-linux-0.20-res.yml	`5fa050a02e3cb95c...`	`e72dce94ec5b9090...`
bitcoin-core-osx-0.20-res.yml	`d0d8fb2e491f47a0...`	`2fe9ef65e4f22aa9...`
bitcoin-core-win-0.20-res.yml	`6f9be4b4073760a5...`	`7035f5f17d238e3d...`
linux-build.log	`b31ba24d0a233210...`	`91def7219b8c52c1...`
osx-build.log	`eaeccfa77df8d98a...`	`8cea9c3430cca6ab...`
win-build.log	`b9e547f8f55648b3...`	`20d51977604eee0f...`
bitcoin-core-linux-0.20-res.yml.diff		`391052be640490b5...`
bitcoin-core-osx-0.20-res.yml.diff		`96a8469c9b7db345...`
bitcoin-core-win-0.20-res.yml.diff		`9a5fcddeec8edff2...`
linux-build.log.diff		`7f52c250016bb7f7...`
osx-build.log.diff		`85d75ca12c4b66ee...`
win-build.log.diff		`c3fb2c988d5d37bb...`

DrahtBot removed the label Needs gitian build on Nov 6, 2019

laanwj commented at 10:46 am on November 6, 2019: member

Concept ACK

promag commented at 10:50 am on November 6, 2019: member

Please update https://github.com/bitcoin/bitcoin/blob/master/test/README.md#dependencies-1

hebasto commented at 11:00 am on November 6, 2019: member

@promag

Please update https://github.com/bitcoin/bitcoin/blob/master/test/README.md#dependencies-1

It already lacks ShellCheck. Maybe just add yq dependency to #17353?

promag commented at 11:08 am on November 6, 2019: member

@hebasto let’s see if that one is merged first so you can update the doc here.

script: Lint Gitian descriptors with ShellCheck 14aded46df

script: Enable SC2006 rule for Gitian scripts 577682d9e8

hebasto force-pushed on Nov 6, 2019

hebasto commented at 1:16 pm on November 6, 2019: member

Latest push:

rebased on top of merged #17353 and @promag’s #17361#pullrequestreview-312342839 has been addressed
@laanwj’s #17353 (review) have been addresed

fanquake removed the label Waiting for author on Nov 11, 2019

hebasto commented at 7:58 pm on November 25, 2019: member

@dongcarl @promag

Thank you for your reviews. All your comments have been addressed. Would you mind re-reviewing this PR?

MarcoFalke added the label Needs gitian build on Nov 25, 2019

dongcarl commented at 9:00 pm on November 25, 2019: member

ACK from me if Gitian build passes.

practicalswift commented at 9:11 pm on November 25, 2019: contributor

ACK 93b18e5f8de0669272618e8098b7c3fdc435906b – diff looks correct

in contrib/gitian-descriptors/gitian-win.yml:99 in 93b18e5f8d outdated

 95@@ -96,7 +96,7 @@ script: |
 96   export PATH=${WRAP_DIR}:${PATH}
 97 
 98   cd bitcoin
 99-  BASEPREFIX=`pwd`/depends
100+  BASEPREFIX=$(pwd)/depends

laanwj commented at 12:02 pm on November 26, 2019:

~~Did you mean ${PWD}?~~ never mind, different kind of brackets

script: Enable SC2155 rule for Gitian scripts

Also pwd command is replaced with $PWD variable everywhere for
consistency.

61bb21b418

script: Enable SC2001 rule for Gitian scripts 17f81e9648

in contrib/gitian-descriptors/gitian-linux.yml:125 in 93b18e5f8d outdated

121@@ -122,7 +122,7 @@ script: |
122   done
123 
124   cd bitcoin
125-  BASEPREFIX=`pwd`/depends
126+  BASEPREFIX=$(pwd)/depends

dongcarl commented at 5:30 pm on November 26, 2019:

Let’s be consistent and just use $PWD everywhere.

0  BASEPREFIX="${PWD}/depends"

hebasto commented at 8:58 am on November 27, 2019:

Let’s be consistent and just use $PWD everywhere.

Agree.

Ref:

 0$ echo $0
 1bash
 2$ type pwd
 3pwd is a shell builtin
 4$ pwd --help
 5pwd: pwd [-LP]
 6    Print the name of the current working directory.
 7    
 8    Options:
 9      -L	print the value of $PWD if it names the current working
10    		directory
11      -P	print the physical directory, without any symbolic links
12    
13    By default, `pwd' behaves as if `-L' were specified.
14    
15    Exit Status:
16    Returns 0 unless an invalid option is given or the current directory
17    cannot be read.

Going to fix tonight.

hebasto commented at 5:31 pm on November 27, 2019:

Done.

hebasto force-pushed on Nov 27, 2019

hebasto commented at 5:32 pm on November 27, 2019: member

All comments has been addressed.

practicalswift commented at 6:42 pm on November 27, 2019: contributor

ACK 17f81e96486780df5d464487975ecb11b278ec8d – diff looks correct

laanwj referenced this in commit 1f59885d27 on Nov 28, 2019

laanwj merged this on Nov 28, 2019

laanwj closed this on Nov 28, 2019

DrahtBot commented at 9:20 am on November 28, 2019: member

Gitian builds

File	commit 2c1c43754b7a29e2fc852c2ad6430ba27aa2d7db(master)	commit e1e8ea9316781e39a6b71bd929e1a37c3fa6cae2(master and this pull)
bitcoin-0.19.99-aarch64-linux-gnu-debug.tar.gz	`38cbfb45b6a7086d...`	`7ba0f549f2ed91ef...`
bitcoin-0.19.99-aarch64-linux-gnu.tar.gz	`da5cf67a007aed08...`	`70382c7e6b493022...`
bitcoin-0.19.99-arm-linux-gnueabihf-debug.tar.gz	`184e8ce64969a801...`	`3fb8c94836da0f09...`
bitcoin-0.19.99-arm-linux-gnueabihf.tar.gz	`281f743a1af9f788...`	`abd409698155674c...`
bitcoin-0.19.99-i686-pc-linux-gnu-debug.tar.gz	`094e8232c0b5fc1e...`	`d90e78dea4446ebd...`
bitcoin-0.19.99-i686-pc-linux-gnu.tar.gz	`ab4deb19e22c2e9c...`	`a3c44fd884261792...`
bitcoin-0.19.99-osx-unsigned.dmg	`bd9ae85f63baebb0...`	`fe603d3aefd61c2d...`
bitcoin-0.19.99-osx64.tar.gz	`61305776c2b32f02...`	`a0d5faeba2594264...`
bitcoin-0.19.99-riscv64-linux-gnu-debug.tar.gz	`9c9b077e4c62ca11...`	`d2e9a90664cca51e...`
bitcoin-0.19.99-riscv64-linux-gnu.tar.gz	`b33148372a530635...`	`1a4cf4782a2c95dd...`
bitcoin-0.19.99-win64-debug.zip	`20a7fd94fde51027...`	`58c4e1e68643ccd4...`
bitcoin-0.19.99-win64-setup-unsigned.exe	`94be28f0f2a7d072...`	`767c0ef26f2ac293...`
bitcoin-0.19.99-win64.zip	`77303c3b0e4d270b...`	`02d0b6ed8278e7d5...`
bitcoin-0.19.99-x86_64-linux-gnu-debug.tar.gz	`41dec10c82bf5058...`	`20781c0c3d9e3436...`
bitcoin-0.19.99-x86_64-linux-gnu.tar.gz	`ee561a8a74b45caa...`	`b64567787c7290ec...`
bitcoin-0.19.99.tar.gz	`a2c202a483d1f9e1...`	`b61484289eb50989...`
bitcoin-core-linux-0.20-res.yml	`5d71778589f1dbbe...`	`03feefb1d5fda8de...`
bitcoin-core-osx-0.20-res.yml	`fefe01def2f61fe3...`	`62a1f8a6a060cc46...`
bitcoin-core-win-0.20-res.yml	`c6d1798223f3d9aa...`	`70f85831b67fb9bb...`
linux-build.log	`7babaa31c8e7e008...`	`2600460a3d5f2bd1...`
osx-build.log	`44cec8cec24a7204...`	`4eed66aeb79e91a6...`
win-build.log	`11636a6fb5df52aa...`	`098298a3780dee45...`
bitcoin-core-linux-0.20-res.yml.diff		`8e548761a8c4b3a8...`
bitcoin-core-osx-0.20-res.yml.diff		`e82d90cea801de1c...`
bitcoin-core-win-0.20-res.yml.diff		`56b482b1ddbd66d2...`
linux-build.log.diff		`e7c1c5d6dfdc8b6d...`
osx-build.log.diff		`fc2e24b02d3a348b...`
win-build.log.diff		`a587f844b67ecdaa...`

DrahtBot removed the label Needs gitian build on Nov 28, 2019

hebasto deleted the branch on Nov 28, 2019

sidhujag referenced this in commit a27f4e1ed7 on Nov 28, 2019

laanwj referenced this in commit ac09180128 on Dec 9, 2019

MarkLTZ referenced this in commit c864dba23e on Apr 10, 2020

sidhujag referenced this in commit f6ee5e6fbe on Nov 10, 2020

kittywhiskers referenced this in commit 6f88cb0004 on Jul 15, 2021

kittywhiskers referenced this in commit 236dbaff97 on Jul 15, 2021

kittywhiskers referenced this in commit f783ad0572 on Jul 20, 2021

kittywhiskers referenced this in commit 02d8447e22 on Jul 20, 2021

kittywhiskers referenced this in commit 0c41b0577e on Jul 20, 2021

kittywhiskers referenced this in commit 93c40757a1 on Jul 20, 2021

kittywhiskers referenced this in commit 6a0190304a on Aug 1, 2021

kittywhiskers referenced this in commit 2f2344df5a on Aug 24, 2021

kittywhiskers referenced this in commit 3f89162873 on Aug 24, 2021

kittywhiskers referenced this in commit f7ec6bc006 on Aug 25, 2021

kittywhiskers referenced this in commit bd75f8e73f on Aug 25, 2021

kittywhiskers referenced this in commit b6a324ba04 on Aug 26, 2021

kittywhiskers referenced this in commit 53a977783f on Aug 26, 2021

kittywhiskers referenced this in commit f05a2c701d on Aug 26, 2021

kittywhiskers referenced this in commit 5fc409f6b8 on Aug 27, 2021

kittywhiskers referenced this in commit 011dc6e177 on Aug 30, 2021

kittywhiskers referenced this in commit 434a6377d0 on Sep 1, 2021

kittywhiskers referenced this in commit f8a9057391 on Sep 1, 2021

kittywhiskers referenced this in commit fb490aaea2 on Sep 1, 2021

kittywhiskers referenced this in commit b78a12ff29 on Sep 1, 2021

kittywhiskers referenced this in commit b726339303 on Sep 2, 2021

kittywhiskers referenced this in commit 3fe50a2d0c on Sep 3, 2021

kittywhiskers referenced this in commit 482d8f6e75 on Sep 3, 2021

kittywhiskers referenced this in commit 056590a1da on Sep 3, 2021

PastaPastaPasta referenced this in commit f12ca55d84 on Sep 17, 2021

thelazier referenced this in commit b95eab40da on Sep 25, 2021

MarcoFalke locked this on Dec 16, 2021

script: Lint Gitian descriptors with ShellCheck #17361

Gitian builds

Gitian builds

Gitian builds