On 26/1/19 9:40 am, Morgan Adamiec wrote: > 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'. >
Those names are good. BTW, I used 'variable' in my suggested naming because it is an array of variable names. So not referring to the array, but what is in the array. Thanks, Allan