lint-shell: Enable SC2046 and SC2086, prevent unintentional word-splitting #20879

1. 
   dongcarl commented at 8:20 pm on January 7, 2021: member

   We currently disable the shellcheck rules SC2046 and SC2086 in our lint-shell.sh script, however, I think it is important that we enable them and make our scripts conformant and therefore more robust.

   I have encountered many inscrutable build failures where the ultimate culprit was missing quoting, and from personal experience, I think word-splitting is the exception rather than the norm, and it would make our scripts easier to read if instances of word-splitting are pointed out by the shellcheck directives.

2. 
   laanwj commented at 8:26 pm on January 7, 2021: member

   Concept ACK, was just thinking about this in response to https://github.com/bitcoin/bitcoin/pull/19683/commits/80331107416b8a6cb487ee1c89a39c6a8bced27b We’ve historically have a lot of word splitting issues, resulting in problems when building in a directory with a space in the name, for example, so this sounds like a useful lint.
3. laanwj added the label Tests on Jan 7, 2021
4. 
   hebasto commented at 8:37 pm on January 7, 2021: member

   Concept ACK.
5. 
   hebasto commented at 9:40 am on January 9, 2021: member

   FWIW,

   1. SC2046 cases

   0--- a/test/lint/lint-shell.sh
   1+++ b/test/lint/lint-shell.sh
   2@@ -10,7 +10,6 @@ export LC_ALL=C
   3 
   4 # Disabled warnings:
   5 disabled=(
   6-    SC2046 # Quote this to prevent word splitting.
   7     SC2086 # Double quote to prevent globbing and word splitting.
   8     SC2162 # read without -r will mangle backslashes.
   9 )
   

   0$ test/lint/lint-shell.sh | grep SC2046 | wc -l
   117
   

   2. SC2086 cases

   0--- a/test/lint/lint-shell.sh
   1+++ b/test/lint/lint-shell.sh
   2@@ -11,7 +11,6 @@ export LC_ALL=C
   3 # Disabled warnings:
   4 disabled=(
   5     SC2046 # Quote this to prevent word splitting.
   6-    SC2086 # Double quote to prevent globbing and word splitting.
   7     SC2162 # read without -r will mangle backslashes.
   8 )
   9 disabled_gitian=(
   

   0$ test/lint/lint-shell.sh | grep SC2086 | wc -l
   1371
   

6. 
   practicalswift commented at 5:48 pm on January 9, 2021: contributor

   Strong concept ACK

   Shell scripting without the helping hand of shellcheck is a nightmare: more is more :)

7. fanquake added the label good first issue on Aug 12, 2021
8. laanwj closed this on Nov 15, 2021

   ---

9. DrahtBot locked this on Nov 15, 2022

Contributors
dongcarl laanwj hebasto practicalswift

Labels
Tests good first issue