Re: [docs] [PATCH 3/3] common-tasks: Add an example of using bbappends to add a file

["Tom Rini" <trini@konsulko.com>]

On Fri, Aug 13, 2021 at 11:15:13AM +0200, Michael Opdenacker wrote:
> Tom, thanks for the patch.
> Quentin, thanks for the review.
> 
> On 8/13/21 10:27 AM, Quentin Schulz wrote:
> > Hi Tom,
> >
> > On Thu, Aug 12, 2021 at 12:10:27PM -0400, Tom Rini wrote:
> >> Use the xserver-xf86-config_%.bbappend from meta-raspberrypi to provide
> >> an example of having a bbappend file add files to an existing recipe.
> >>
> >> Signed-off-by: Tom Rini <trini@konsulko.com>
> >> ---
> >>  documentation/dev-manual/common-tasks.rst | 61 +++++++++++++++++++++++
> >>  1 file changed, 61 insertions(+)
> >>
> >> diff --git a/documentation/dev-manual/common-tasks.rst b/documentation/dev-manual/common-tasks.rst
> >> index f15e72887c04..87abef9ad52f 100644
> >> --- a/documentation/dev-manual/common-tasks.rst
> >> +++ b/documentation/dev-manual/common-tasks.rst
> >> @@ -554,6 +554,67 @@ The end result of this ``.bbappend`` file is that on a Raspberry Pi, where
> >>  used during ``do_fetch`` and the test for a non-zero file size in
> >>  ``do_install`` will return true, and the file will be installed.
> >>  
> >> +Installing Additional Files Using Your Layer
> >> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> +
> >> +As another example, consider the main xserver-xf86-config recipe and a corresponding
> > s/xserver-xf86-config/``xserver-xf86-config``/
> >
> >> +xserver-xf86-config append file both from the :term:`Source Directory`.
> > s/xserver-xf86-config/``xserver-xf86-config``/
> >
> >> +Here is the main
> >> +xserver-xf86-config recipe, which is named ``xserver-xf86-config_0.1.bb`` and located in
> > s/xserver-xf86-config/``xserver-xf86-config``/

Whoops.

> >> +the "meta" layer at ``meta/recipes-graphics/xorg-xserver``::
> >> +
> >> +   SUMMARY = "X.Org X server configuration file"
> >> +   HOMEPAGE = "https://urldefense.proofpoint.com/v2/url?u=http-3A__www.x.org&d=DwIBAg&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=XXgYJ9tKXtc_aBqPPa-9XgJe1nXbWkl8dkudu8oFs_Y&s=0zjLRjQF3x_Fv-B-gOPuwnCc_TfOzMJ50SOySYZMbys&e= "
> >> +   SECTION = "x11/base"
> >> +   LICENSE = "MIT-X"
> >> +   LIC_FILES_CHKSUM = "file://${COREBASE}/meta/COPYING.MIT;md5=3da9cfbcb788c80a0384361b4de20420"
> >> +   PR = "r33"
> >> +
> >> +   SRC_URI = "file://xorg.conf"
> >> +
> >> +   S = "${WORKDIR}"
> >> +
> >> +   CONFFILES:${PN} = "${sysconfdir}/X11/xorg.conf"
> >> +
> >> +   PACKAGE_ARCH = "${MACHINE_ARCH}"
> >> +   ALLOW_EMPTY:${PN} = "1"
> >> +
> >> +   do_install () {
> >> +	if test -s ${WORKDIR}/xorg.conf; then
> >> +		install -d ${D}/${sysconfdir}/X11
> >> +		install -m 0644 ${WORKDIR}/xorg.conf ${D}/${sysconfdir}/X11/
> >> +	fi
> >> +   }
> >> +
> >> +Following is the append file, which is named ``xserver-xf86-config_%.bbappend``
> >> +and is from the Raspberry Pi BSP Layer named ``meta-raspberrypi``. The
> >> +file is in the layer at ``recipes-graphics/xorg-xserver``::
> >> +
> >> +   FILESEXTRAPATHS:prepend := "${THISDIR}/${PN}:"
> >> +
> >> +   SRC_URI:append:rpi = " \
> >> +       file://xorg.conf.d/98-pitft.conf \
> >> +       file://xorg.conf.d/99-calibration.conf \
> >> +   "
> >> +   do_install:append:rpi () {
> >> +       PITFT="${@bb.utils.contains("MACHINE_FEATURES", "pitft", "1", "0", d)}"
> >> +       if [ "${PITFT}" = "1" ]; then
> >> +           install -d ${D}/${sysconfdir}/X11/xorg.conf.d/
> >> +           install -m 0644 ${WORKDIR}/xorg.conf.d/98-pitft.conf ${D}/${sysconfdir}/X11/xorg.conf.d/
> >> +           install -m 0644 ${WORKDIR}/xorg.conf.d/99-calibration.conf ${D}/${sysconfdir}/X11/xorg.conf.d/
> >> +       fi
> >> +   }
> >> +
> >> +   FILES:${PN}:rpi += "${sysconfdir}/X11/xorg.conf ${sysconfdir}/X11/xorg.conf.d/*"
> >> +
> > Highly suspect this is wrong. It should be FILES:${PN}:append:rpi
> > instead, otherwise it's completely overriding FILES:${PN} for rpi (even
> > if the operator that is following is +=).

Yeah, it does.

> > I guess you took this from the bbappend within meta-raspberrypi so it
> > would probably  need fixing there.

Yup, copy/paste/indent of the current upstream recipe.

> >> +Building off of the previous example, we once again are setting the
> >> +:term:`FILESEXTRAPATHS` variable.  In this case we are also usine the
> > s/usine the/using/
> >
> >> +:term:`SRC_URI` to list additional source files to use when ``rpi`` is found in
> >> +the list of :term:`OVERRIDES`.  The ``do_install`` task will then perform a
> > s/``do_install``/:ref:`ref-tasks-install`/
> >
> >> +check for an additional :term:`MACHINE_FEATURES` that if set will cause these
> > "The do_install task will be extended, for ``rpi`` only, to perform..." instead?
> >
> >> +additional files to be installed.  These additional files are listed in
> >> +:term:`FILES` so that they will be packaged.
> >> +
> > Same, it's rpi-specific here.

What I was trying to convey here is that in the meta-raspberrypi layer,
all of the machines add "rpi" to the OVERRIDE list, for a common one.
At the high level, I was trying to find a simple example of using an
append to add files, but also didn't want to add another example layer
and it seems like meta-raspberrypi is what's used everywhere else.  I'll
see about re-wording things a bit more to be clearer / more consistent.

> Tom, it's probably better if you prepare a V2 by yourself.
> If you can prepare it against "master-next", that will even be easier
> for me, as the first two patches were merged there.

Agreed.  First up, I'm going to need to go and get meta-raspberrypi
fixed for this example, or switch to the u-boot example there, which I
avoided since it's doing two things.

Thanks for the review and feedback.

-- 
Tom