search

AOSC-Archive / autobuild3

AOSC OS package maintenance toolkit (version 3)
https://aosc.io
GNU General Public License v2.0
24 stars 17 forks source link

Bug: `arch_loadvar()` does not handle array correctly #140

Closed CamberLoid closed 2 years ago

CamberLoid commented 2 years ago

In file https://github.com/AOSC-Dev/autobuild3/blob/e058475b3e3bbd3b7c17b6eb7c6cba8dbdeedb1f/lib/arch.sh#L39-L42

When it comes to array, the _commonvar="${_archvar}" only replace the first element of $_commonvar with $_archvar, leading to an unexpected behavior.

The expected behavior is that arch_loadvar() replaces the whole array with another. Additionally, we need to handle a situation that when archvar is array and commonvar is not, or the opposite situation.

See also: File https://github.com/AOSC-Dev/aosc-os-abbs/blob/ab7ba3e45a59845337491702f74675e7ee024c18/extra-database/mariadb/autobuild/defines in commit https://github.com/AOSC-Dev/aosc-os-abbs/commit/ab7ba3e45a59845337491702f74675e7ee024c18 , where the CMAKE_AFTER_RISCV64 does not correctly replace the CMAKE_AFTER.

cthbleachbit commented 2 years ago

Suggested fix: change line 41 to _commonvar=("${_archvar[@]}").

The only side effect is that _commonvar (i.e. CMAKE_AFTER) is now always an array after substitution - but logic-wise it should be identical. For regular variables, "${VAR[@]}" and "${VAR}" is equivalent.

cthbleachbit commented 2 years ago

Actually no. if _archvar is an array then this would copy the entire array over to _commonvar verbatim. If _archvar is a plain string w/ spaces, we'll need to split the string at whitespace boundary and should do _commonvar=(${_archvar[@]}) instead (note there are no quotes to allow splitting at white space).

Unfortunately abisarray doesn't work on variables declared -n. Call abisarray ${1}__${ABHOST^^} to figure out whether _archvar is an array.