Re: [oe] [meta-oe][PATCH v2] vk-gl-cts: allow the user to specify the target

["Trevor Woerner" <twoerner@gmail.com>]

On Thu 2021-09-23 @ 07:24:07 PM, Peter Kjellerstedt wrote:
> > -----Original Message-----
> > From: openembedded-devel@lists.openembedded.org <openembedded-
> > devel@lists.openembedded.org> On Behalf Of Trevor Woerner
> > Sent: den 23 september 2021 20:55
> > To: openembedded-devel@lists.openembedded.org
> > Subject: [oe] [meta-oe][PATCH v2] vk-gl-cts: allow the user to specify the
> > target
> > 
> > When building opengl-es-cts (for example), instead of hard-coding a
> > specific target to build (surfaceless), leave the choice empty by default
> > which instructs the package's build system to search the sysroot and try
> > to
> > determine which target to use (i.e. the "Default" target).
> > 
> > Provide overrides (in the form of PACAKGECONFIGs) to allow the user to
> > specify a specific target to build (if they don't want the target selected
> > automatically).
> > 
> > Signed-off-by: Trevor Woerner <twoerner@gmail.com>
> > ---
> > changes in v2:
> > - add logic to try to guess which dependencies (x11/wayland) will be
> >   required in the case where the user doesn't set a target explicitly
> > - add comments to explain the code, and explain how the newly added
> >   PACKAGECONFIGs work
> > ---
> >  .../vk-gl-cts/khronos-cts.inc                 | 33 +++++++++++++++++--
> >  1 file changed, 30 insertions(+), 3 deletions(-)
> > 
> > diff --git a/meta-oe/recipes-graphics/vk-gl-cts/khronos-cts.inc b/meta-
> > oe/recipes-graphics/vk-gl-cts/khronos-cts.inc
> > index f61921473..903f006db 100644
> > --- a/meta-oe/recipes-graphics/vk-gl-cts/khronos-cts.inc
> > +++ b/meta-oe/recipes-graphics/vk-gl-cts/khronos-cts.inc
> > @@ -18,9 +18,9 @@ S = "${WORKDIR}/git"
> > 
> >  inherit pkgconfig cmake features_check
> > 
> > -REQUIRED_DISTRO_FEATURES += "opengl"
> > +ANY_OF_DISTRO_FEATURES += "opengl wayland"
> > 
> > -DEPENDS += "libpng zlib virtual/libgles2 virtual/egl"
> > +DEPENDS += "libpng zlib virtual/libgles2"
> > 
> >  SRC_URI += "file://0001-Workaround-for-GCC-11-uninit-variable-warnings-946.patch;patchdir=external/amber/src \
> >              file://0001-Include-limits-header-for-numeric_limits.patch;patchdir=external/vulkancts \
> > @@ -36,7 +36,34 @@ SRC_URI:append:toolchain-clang = "\
> >  	file://fix-clang-private-operator.patch \
> >  "
> > 
> > -EXTRA_OECMAKE:append = " -DDEQP_TARGET=surfaceless"
> > +# The best thing for the user to do is to not specify any of the following
> > +# PACKAGECONFIGs (i.e. leave it blank) which tells the project to do its own
> > +# probing and build what it thinks is appropriate.
> > +# However, if you want, you can specify one of the following PACKAGECONFIGs
> > +# to override this behaviour.
> > +PACKAGECONFIG ??= ""
> > +PACKAGECONFIG[surfaceless] = "-DDEQP_TARGET=surfaceless,,"
> > +PACKAGECONFIG[wayland] = "-DDEQP_TARGET=wayland,,wayland"
> > +PACKAGECONFIG[x11_egl] = "-DDEQP_TARGET=x11_egl,,virtual/libx11 virtual/egl"
> > +PACKAGECONFIG[x11_glx] = "-DDEQP_TARGET=x11_glx,,virtual/libx11"
> > +PACKAGECONFIG[x11_egl_glx] = "-DDEQP_TARGET=x11_glx,,virtual/libx11 virtual/egl"
> > +
> > +python __anonymous() {
> > +    packageconfig = (d.getVar("PACKAGECONFIG") or "").split()
> > +    if len(packageconfig) > 1:
> > +        bb.fatal("only one PACKAGECONFIG of 'surfaceless wayland x11_egl x11_glx x11_egl_glx' can be specified at a time")
> 
> If they are mutually exclusive, it is better to use the sixth argument for 

There's a *sixth* argument to PACKAGECONFIG??! What kind of sorcery is this??!

> the PACKAGECONFIG varflags, which allows you to specify other incompatible 
> varflags. I.e, the above would then become:
> 
> PACKAGECONFIG[surfaceless] = "-DDEQP_TARGET=surfaceless,,,,, wayland x11_egl x11_glx x11_egl_glx"
> PACKAGECONFIG[wayland] = "-DDEQP_TARGET=wayland,, wayland,,, surfaceless x11_egl x11_glx x11_egl_glx"
> PACKAGECONFIG[x11_egl] = "-DDEQP_TARGET=x11_egl,, virtual/libx11 virtual/egl,,, surfaceless wayland x11_glx x11_egl_glx"
> PACKAGECONFIG[x11_glx] = "-DDEQP_TARGET=x11_glx,, virtual/libx11,,, surfaceless wayland x11_egl x11_egl_glx"
> PACKAGECONFIG[x11_egl_glx] = "-DDEQP_TARGET=x11_glx,, virtual/libx11 virtual/egl,,, surfaceless wayland x11_egl x11_glx"
> 
> This allows other PACKAGECONFIGs to be added without breaking the logic.

I was going to leave that as a problem for whoever came along after me and
wanted to add something else ;-)

(just kidding)

> 
> > +
> > +    # if the user doesn't specify any PACKAGECONFIG then the cts build system
> > +    # is going to probe the sysroot to try to figure out what to build
> > +    # in this case we try to guess whether the user is building for wayland
> > +    # or x11 and add the required dependencies automatically
> > +    distrofeatures = (d.getVar("DISTRO_FEATURES") or "")
> > +    if len(packageconfig) == 0:
> 
> A better test is:
> 
>     if not bb.utils.contains_any("PACKAGECONFIG", [ "surfaceless", "wayland", "x11_egl", "x11_glx", "x11_egl_glx" ], True, False, d):
> 
> > +        if "wayland" in distrofeatures:
> > +            d.appendVar("DEPENDS", " wayland ")
> > +        if "x11" in distrofeatures:
> > +            d.appendVar("DEPENDS", " virtual/libx11 virtual/egl ")
> > +}
> > 
> >  CTSDIR = "/usr/lib/${BPN}"
> > 
> > --
> > 2.30.0.rc0
>