build: Added double quotes #17074

pull mztriz wants to merge 1 commits into bitcoin:master from mztriz:master changing 1 files +2 −2

mztriz commented at 9:03 PM on October 7, 2019: contributor

Added double quotes to prevent globbing and word splitting.

Added double quotes

Added double quotes to prevent globbing and word splitting.

dd85166b6c

fanquake added the label Build system on Oct 7, 2019
laanwj commented at 8:49 AM on October 8, 2019: member

ACK dd85166b6c8200265ff222226305e6823149de19

in autogen.sh:8 in dd85166b6c

   4 | @@ -5,9 +5,9 @@
   5 |  
   6 |  export LC_ALL=C
   7 |  set -e
   8 | -srcdir="$(dirname $0)"
   9 | +srcdir="$(dirname "$0")"

laanwj commented at 8:53 AM on October 8, 2019:

TIL that nested quotes here works

practicalswift commented at 9:02 AM on October 8, 2019: contributor

Welcome as a contributor @mztriz! :)

ACK dd85166b6c8200265ff222226305e6823149de19 -- diff looks correct
laanwj renamed this:
~~Added double quotes~~
build: Added double quotes
on Oct 8, 2019
hebasto commented at 4:53 PM on October 8, 2019: member
ACK dd85166b6c8200265ff222226305e6823149de19, tested locally on Linux Mint 19.2.

The code on master failed if the absolute path /.../bitcoin/autogen.sh contains many-words directory name, and script runs from out-of-the-tree:
```
ls ~/git\ hub/bitcoin/autogen.sh 
'/home/hebasto/git hub/bitcoin/autogen.sh'

~/git\ hub/bitcoin/autogen.sh 
/home/hebasto/git hub/bitcoin/autogen.sh: 9: cd: can't cd to /home/hebasto
hub/bitcoin
```
With this PR it works as expected.

But our code base contains other the same cases (I mean special parameters $0), so I suggest to be consistent, and fix them all.
laanwj commented at 7:45 AM on October 9, 2019: member

The build system has never worked in paths with spaces in it, I hope at some point that can be solved, this is a step in the right direction at least.
laanwj referenced this in commit 1d9ac7fded on Oct 9, 2019
laanwj merged this on Oct 9, 2019
laanwj closed this on Oct 9, 2019
fanquake cross-referenced this on Oct 9, 2019 from issue Strip redacted ACKs from merge commits? by fanquake
sidhujag referenced this in commit 1511cffd41 on Oct 9, 2019
hebasto commented at 7:14 AM on November 3, 2019: member

@mztriz

Would it be feasible to enable SC2086 rule for ShellCheck linter globally?
MarkLTZ cross-referenced this on Apr 4, 2020 from issue Bitcoin PR tracking by MarkLTZ
bitcoin locked this on Dec 16, 2021

Contributors

Labels