armbian / build

Armbian Linux build framework generates custom Debian or Ubuntu image for x86, aarch64, riscv64 & armhf
https://www.armbian.com
GNU General Public License v2.0
3.99k stars 2.25k forks source link

decide how we want `shopt` `nullglob`, set it only once, enforce it, and make consistent #4927

Open rpardini opened 1 year ago

rpardini commented 1 year ago

Which feature would you like to have?

old master had a nullglob setting mixed in with interactive configuration (!) main code possibly lost it / does not run interactive unless really needed - leading to undefined behaviour.

we should decide if enabled or disabled, set it only once, and enforce it -- if only for sanity.

See https://www.gnu.org/software/bash/manual/bash.html#The-Shopt-Builtin

Funding

github-actions[bot] commented 1 year ago

Jira ticket: AR-1594

ColorfulRhino commented 2 months ago

Trying to tackle this year-old to-do, I found this:

https://github.com/armbian/build/blob/02e0c14a8acae843bf7cc2ad23c6bfa6e5507ddd/lib/functions/compilation/patch/patching.sh#L49-L50

Researching the options lead to these sources:

nullglob: Filename globbing patterns that don't match any filenames are simply expanded to nothing rather than remaining unexpanded.

$ echo my*file
my*file
$ shopt -s nullglob
$ echo my*file

(no output from that last echo other than a blank line)

dotglob If set, Bash includes filenames beginning with a ‘.’ in the results of filename expansion. The filenames ‘.’ and ‘..’ must always be matched explicitly, even if dotglob is set.

It seems to me that these options are pretty sensible to set. Since they are included in the patching script, which is almost always (?) called, these options probably have been leaked to most of the build script already without negative consequences.

Maybe we just set shopt -s nullglob dotglob at the beginning of compile.sh and see how it goes? These options should then be active on the complete build script, until I misunderstood something.

rpardini commented 2 months ago

Since they are included in the patching script, which is almost always (?) called

That is the old patching script, in a function called advanced_patch() -- 90% of their usages has been replaced with the Python monster. I can only find 2 occurences of advanced_patch invocations: one for building atf and the other for crust. I don't think those happen too frequently, so assuming nullglob and dotglob are always in effect anyway is incorrect, I think

But I think when I wrote this Issue, I was mostly complaining about having to handle it here https://github.com/armbian/build/blob/main/lib/functions/compilation/uboot.sh#L260-L295 which is about the already-complex-enough UBOOT_TARGET_MAP, and yes, nullglob somehow affects that for some reason...