On Fri, 25 Jan 2019 at 16:05, Dave Reisner <d...@falconindy.com> wrote: > > On Fri, Jan 25, 2019 at 03:47:14PM +0000, Morgan Adamiec wrote: > > On Fri, 25 Jan 2019 at 00:52, Allan McRae <al...@archlinux.org> wrote: > > > > > > On 24/1/19 11:34 pm, Dave Reisner wrote: > > > > On Thu, Jan 24, 2019 at 09:09:59AM +0000, Morgan Adamiec wrote: > > > >> On Thu, 24 Jan 2019 at 03:01, Allan McRae <al...@archlinux.org> wrote: > > > >>> > > > >>> On 22/1/19 10:05 am, morganamilo wrote: > > > >>>> Variables such as 'pkgdesc_x86_64' are invalid, instead of ignoring > > > >>>> them > > > >>>> raise an error. > > > >>>> > > > >>>> This also disallows using 'any' as an architecture specific variable > > > >>>> > > > >>>> Signed-off-by: morganamilo <morganam...@gmail.com> > > > >>>> --- > > > >>>> > > > >>>> v5: > > > >>>> "libmakepkg: disallow using any as an architecture specific > > > >>>> variable" > > > >>>> was squashed into this commit. > > > >>>> > > > >>>> Move this lint to its own file. > > > >>> > > > >>> Moving this to its own file is fine in principle, but it has > > > >>> duplicated > > > >>> a few arrays of field values. After this patch there would be: > > > >>> > > > >>> scripts/makepkg.sh.in: > > > >>> splitpkg_overrides=(... > > > >>> > > > >>> scripts/libmakepkg/lint_pkgbuild/variable.sh.in: > > > >>> scripts/libmakepkg/lint_pkgbuild/arch_specific.sh.in: > > > >>> local array=(... > > > >>> local arch_array=(... > > > >>> local string=(... > > > >>> > > > >>> scripts/libmakepkg/lint_pkgbuild/package_function_variable.sh.in: > > > >>> local no_package=(... > > > >>> > > > >>> This will be annoying to update for any new fields or other changes. > > > >>> > > > >>> > > > >>> The properties of each field we are trying to capture are: > > > >>> 1) is an array/string > > > >>> 2) can be architecture specific > > > >>> 3) overridable in package function > > > >>> > > > >>> Can we store this in one file in a readily extendable fashion > > > >>> somewhere? > > > >>> > > > >>> A > > > >> > > > >> Good point. There was already a TODO on the fields, maybe it's finally > > > >> time to act on it. > > > >> I'm not too familiar with bash to really know a better way to > > > >> structure it. > > > >> Would just moving the arrays to a different file and sourcing be fine? > > > >> util/pkgbuild.sh perhaps? > > > > > > > > Yes, this can be moved to another file and just be declared as arrays. I > > > > would suggest making sure that the names are fairly unique (and > > > > readonly) to avoid accidentally clobbering. > > > > > > > > This feels like semantics that we're overlaying on top of shell, so my > > > > vote > > > > is for util/schema.sh. > > > > > > > > > > For names, I suggest: > > > > > > pkgbuild_array_variable > > > pkgbuild_string_variable > > > pkgbuild_arch_override_variable > > > pkgbuild_package_override_variable > > > > > > Kind of long, but won't get clobbered. > > > > > > Allan > > > > Ending a variable name with 'variable' seems weird to me. Also I > > prefer arrays to be plural. > > > > So I'd prefer 'pkgbuild_array_variables' or even better > > `pkgbuild_arrarys`. But then of course > > the shorter name is more risky. > > > > Still I'd put my vote on: > > > > pkgbuild_arrays > > pkgbuild_strings > > pkgbuild_arch_overrides > > pkgbuild_package_overrides > > > > Also seems as these are pretty much constants should they be in all > > caps? Or is that not > > done in bash? > > All caps vars are usually reserved for environment vars (e.g. SHELL, > PAGER, LESS). You might capitalize PKGBUILD (as the prefix), but > capitalizing the whole thing is unnecessary. > > Again, I'm going to harp on the fact that this feels like schema, so > paint it blue: > > pkgbuild_schema_arrays > pkgbuild_schema_strings > pkgbuild_schema_arch_overrides > pkgbuild_schema_package_overrides > > dR
I still like mine more, but I think `pkgbuild_schema_arrays` is fine. I prefer it to 'variable'.