Minor fixes to pass shellcheck

[miervasi-edalab]

These are just two minor patches to the following two scripts that comes with the base-files package to solve errors given by the ShellCheck utility:

• /bin/board_detect
• /sbin/firstboot

The first script contained parsing of the ls command which is not recommended:

In shell scripting, the use of command expansion - for looping over a set of files - is not
recommended. Command expansion, in fact, may cause word splitting. This issue refers to the
process by which the shell splits a string into separate words based on whitespace or other
delimiters. Word splitting can cause problems for certain filenames.

While the second script contained potential re-splitting:

In shell scripting, "splitting" refers to the process by which the shell splits a string into
separate words based on whitespace or other delimiters. This can happen when a variable is
expanded. For example, if an array's string item contains the whitespaces, each resulting
substring will be considered as a different array's element. This can cause several runtime
issues and bugs.

[rany2]

While I agree with this change, is there any reason why someone would want to do ls <some glob>? Probably this is done because ls guarantees that they are sorted which makes sense in this context.

[rany2]

P.S. it does seem like echo /etc/board.d/* is sorted, but the ls man page explicitly mentions that the output would be sorted by default. I'm not sure if we can rely on it for glob.

[ehem]

The commit message for when this was introduced didn't shed any light on this. I think @rany2 almost certainly has the right of it though. $(ls /etc/board.d/*) is used since it gives the files in sorted order. A favorite approach is to name the scripts 0-first-script.sh, 1-second-script.sh, 2-third-script.sh, etc in order to ensure ordering of the scripts. Same idea as patch filenames starting with 3 digits. As a result this first line is almost certainly wrong.

[ehem]

I think @rany2 almost certainly has the situation correct, which means a fix is needed.

[ehem]

First commit looks simple and reasonable. The subject line (first line used by shortlog) should be prefixed with "base-files:".

[ehem]

Even though it is unlikely for files in /etc/board.d/ to have spaces in their filenames, this is quite appropriate.

[ehem]

I would be awfully tempted to substitute find /etc/board.d -maxdepth 1 -print | sort | while read a; do as this avoids having the shell scan the directory.

[stklcode]

Which would not yield equivalent results though, as it returns /etc/board.d at depth 0 as well, so it should probably be -mindepth 1 -maxdepth 1.

While generally preferring find for finding operations, the line gets pretty unhandy here and would also depend on different flavors of find and sort (BusyBox, findutils, coreutils) installed on the target. (though they all should behave the same in this case). I'd therefore slightly prefer the shorter glob version here.

[ehem]

The commit message for when this was introduced didn't shed any light on this. I think @rany2 almost certainly has the right of it though. $(ls /etc/board.d/*) is used since it gives the files in sorted order. A favorite approach is to name the scripts 0-first-script.sh, 1-second-script.sh, 2-third-script.sh, etc in order to ensure ordering of the scripts. Same idea as patch filenames starting with 3 digits. As a result this first line is almost certainly wrong.

[stklcode]

Fix shellcheck issue in firstboot script
Fix board_detect script to pass shellcheck

The actual "issue" is not "not passing shellcheck". The tool just detects potential issues like unintended behavior. And the solution is not # shellcheck disable=... here. So I'd rather point to the actual changes in the commit message than just blaming the tool.

[miervasi-edalab]

Fix shellcheck issue in firstboot script
Fix board_detect script to pass shellcheck

The actual "issue" is not "not passing shellcheck". The tool just detects potential issues like unintended behavior. And the solution is not # shellcheck disable=... here. So I'd rather point to the actual changes in the commit message than just blaming the tool.

Ok I'll rephrase the commits then. What about the file loop? I've understood that we want the files to be sorted while the glob in fact doesn't guarantee that, but the alternative then is to let the line unchanged or to use find.