void-packages icon indicating copy to clipboard operation
void-packages copied to clipboard

[RFC] [WIP] use bash arrays for some template fields

Open classabbyamp opened this issue 2 years ago • 13 comments

this will allow select variables to be defined as arrays or strings. if a string is specified, it will be converted to an array for internal usage. if it is an array, it passes through untouched. this should allow for a smooth transition to using more arrays inside xbps-src, and should solve issues with shell quoting in things like configure_args, make_check_args, etc.

to start, I've only done this with configure_args, and fixed some templates

would like to get some feedback before continuing, as this will be a fairly massive undertaking. what fields would be valuable as arrays? (I'm thinking mostly the *_args variables) should the internal string -> array conversion happen or should we just go all-in?

Testing the changes

  • I tested the changes in this PR: YES|briefly|NO

[ci skip]

classabbyamp avatar Mar 08 '23 09:03 classabbyamp

*depends variables also seem like a good pick for this.

I haven't checked if there's any convenient mechanism for concatenating arrays, which would be nice in python packages as an alternative to checkdepends="${depends} ...".

~~If possible, it would be nice if we could make this optional, i.e. still parse variables the old way, but if we detect an array, we skip the conversion step and just take it as-is.~~

EDIT: sorry I skimmed through the part where you already say it's optional by design.

tranzystorekk avatar Mar 08 '23 10:03 tranzystorekk

I haven't checked if there's any convenient mechanism for concatenating arrays

for the same array, just foo+=(new_element), for different:

$ foo=(foo bar baz)
$ bar=("${foo[@]}" idk)
$ declare -p foo
declare -a foo=([0]="foo" [1]="bar" [2]="baz")
$ declare -p bar
declare -a bar=([0]="foo" [1]="bar" [2]="baz" [3]="idk")

classabbyamp avatar Mar 08 '23 10:03 classabbyamp

I haven't checked if there's any convenient mechanism for concatenating arrays

for the same array, just foo+=(new_element), for different:

$ foo=(foo bar baz)
$ bar=("${foo[@]}" idk)
$ declare -p foo
declare -a foo=([0]="foo" [1]="bar" [2]="baz")
$ declare -p bar
declare -a bar=([0]="foo" [1]="bar" [2]="baz" [3]="idk")

Hm, for me the added explicitness of "${foo[@]}" is a plus, but probably not everyone would agree.

tranzystorekk avatar Mar 08 '23 10:03 tranzystorekk

this might be for another issue/PR, but we might also want to consider using associative arrays for build_options like this:

declare -A buildopts=([docs]=0 [wayland]=1 [xorg]=1)

if this was declared before buildopts are used (= usually before configure_args), we could parse the template only once instead of twice if we also used functions for accessing build options instead of variables like currently (because buildopts has to be combined with options from cli args and the config). The only disadvantage I can think of is that it would be ugly to have declare -A right at the top of the template.

paper42 avatar Mar 08 '23 18:03 paper42

I think that should be out of scope for this PR, but it may be useful to do

classabbyamp avatar Mar 08 '23 18:03 classabbyamp

We can do this by attrition, but relatively quickly, with a linter that fails if any of the should-be-arrays is a string that will split into an array with more than one element.

I like this, and I've done this in the array conversion function directly, instead of e.g. changing xlint. I feel that using xlint for that would just be a mess

classabbyamp avatar Mar 09 '23 05:03 classabbyamp

I'm not a big fan of these 0,1,2 parameter... could we use str ary or something?

leahneukirchen avatar Mar 09 '23 12:03 leahneukirchen

or 1 and n ;)

leahneukirchen avatar Mar 09 '23 12:03 leahneukirchen

i think licenses could be an array too, but we'd need to keep spdx expressions together for sanity, like licenses=(LGPL-2.1-or-later "BSD-3-Clause WITH BullshitException")

classabbyamp avatar Mar 09 '23 16:03 classabbyamp

We have nothing really to gain from making licenses an array, at the moment its free text that is not interpreted by anything, making it a shell array now might cause conflicts if we would want to actually use SPDX expressions where you can have nesting and AND/OR operators (MIT AND (LGPL-2.1-or-later OR BSD-3-Clause)).

https://spdx.github.io/spdx-spec/v2-draft/SPDX-license-expressions/

Duncaen avatar Mar 09 '23 16:03 Duncaen

I'm not a big fan of these 0,1,2 parameter... could we use str ary or something?

i've made it a bit more readable, but I suspect the code will change more once more things are arrays

classabbyamp avatar Mar 09 '23 16:03 classabbyamp

I think the best way to go about this will be:

  1. don't try to convert strings to arrrays
  2. don't try to assume strings are single-element arrays
  3. have some utility (maybe written in go with shfmt's library) that can do a best-effort conversion of templates

classabbyamp avatar Nov 02 '23 15:11 classabbyamp

Pull Requests become stale 90 days after last activity and are closed 14 days after that. If this pull request is still relevant bump it or assign it.

github-actions[bot] avatar Feb 01 '24 01:02 github-actions[bot]