Re: infrastructure patch: validate ONLY_FOR_ARCHS/NOT_FOR_ARCHS

On Tue Jul 30, 2024 at 04:09:35PM GMT, Stuart Henderson wrote:
> On 2024/07/29 11:00, Marc Espie wrote:
> > Currently, there is nothing that prevents ONLY_FOR_ARCHS/NOT_FOR_ARCHS
> > from holding garbage.
> >
> > The following patch adds a check, and promptly fixes an issue, namely
> > that ALL_ARCHES should also reference powerpc.
> >
> > Index: arch-defines.mk
> > ===================================================================
> > RCS file: /build/data/openbsd/cvs/ports/infrastructure/mk/arch-defines.mk,v
> > diff -u -p -r1.107 arch-defines.mk
> > --- arch-defines.mk 25 Jun 2024 13:48:24 -0000 1.107
> > +++ arch-defines.mk 26 Jul 2024 10:34:49 -0000
> > @@ -12,7 +12,7 @@
> > ARCH ?!= uname -m
> >
> > ALL_ARCHS = aarch64 alpha amd64 arm arm64 armv7 hppa i386 landisk loongson \
> > - luna88k m88k macppc mips64 mips64el octeon powerpc64 riscv64 sgi \
> > + luna88k m88k macppc mips64 mips64el octeon powerpc powerpc64 riscv64 sgi \
> > sh sparc64
> > # normally only list MACHINE_ARCH (uname -p) names in these variables,
> > # but not all powerpc have apm(4), hence the use of macppc
>
> obviously right - committed.
>
> > Index: bsd.port.arch.mk
> > ===================================================================
> > RCS file: /build/data/openbsd/cvs/ports/infrastructure/mk/bsd.port.arch.mk,v
> > diff -u -p -r1.14 bsd.port.arch.mk
> > --- bsd.port.arch.mk 16 Apr 2020 19:33:29 -0000 1.14
> > +++ bsd.port.arch.mk 26 Jul 2024 10:35:24 -0000
>
> I think this check makes sense, what does anyone else think?

IMO helpful.

>
> > @@ -53,6 +53,11 @@ FLAVOR := ${FLAVOR:N$f}
> > # build the actual list of subpackages we want
> > BUILD_PACKAGES =
> >
> > +# compute pattern for identifying bad variables
> > +.for A in ${ALL_ARCHS}
> > +_arch_check := ${_arch_check}:N${A}
> > +.endfor
> > +
> > .for _s in ${MULTI_PACKAGES}
> >
> > # ONLY_FOR_ARCHS/NOT_FOR_ARCHS are special:
> > @@ -72,8 +77,13 @@ IGNORE${_s} += "Ignored as FLAVOR contai
> > . endif
> > . endfor
> >
> > -# compute _ARCH_OK for ignore
> > . if defined(ONLY_FOR_ARCHS${_s})
> > +# validate against full architecture list
> > +. for _m in ${_arch_check}
> > +. if !empty(ONLY_FOR_ARCHS${_m})
> > +ERRORS += "Fatal: unrecognized architecture ${ONLY_FOR_ARCHS${_m}} in ONLY_FOR_ARCHS${_s}"
> > +. endif
> > +. endfor
> > . for A B in ${MACHINE_ARCH} ${ARCH}
> > . if empty(ONLY_FOR_ARCHS${_s}:M$A) && empty(ONLY_FOR_ARCHS${_s}:M$B)
> > . if ${MACHINE_ARCH} == "${ARCH}"
> > @@ -85,6 +95,12 @@ IGNORE${_s} += "is only for ${ONLY_FOR_A
> > . endfor
> > . endif
> > . if defined(NOT_FOR_ARCHS${_s})
> > +# validate against full architecture list
> > +. for _m in ${_arch_check}
> > +. if !empty(NOT_FOR_ARCHS${_m})
> > +ERRORS += "Fatal: unrecognized architecture ${NOT_FOR_ARCHS${_m}} in NOT_FOR_ARCHS${_s}"
> > +. endif
> > +. endfor
> > . for A B in ${MACHINE_ARCH} ${ARCH}
> > . if !empty(NOT_FOR_ARCHS${_s}:M$A) || !empty(NOT_FOR_ARCHS${_s}:M$B)
> > IGNORE${_s} += "is not for ${NOT_FOR_ARCHS${_s}}"
> >
>