Re: [PATCH 1/1] add support for debian build profiles

[vijai kumar <vijaikumar.kanagarajan@gmail.com>]

On Fri, Dec 31, 2021 at 4:53 PM Moessbauer, Felix
<felix.moessbauer@siemens.com> wrote:
>
> Hi Vijai,
>
> > -----Original Message-----
> > From: isar-users@googlegroups.com <isar-users@googlegroups.com> On
> > Behalf Of vijai kumar
> > Sent: Tuesday, December 21, 2021 9:55 PM
> > To: Moessbauer, Felix (T RDA IOT SES-DE) <felix.moessbauer@siemens.com>
> > Cc: isar-users <isar-users@googlegroups.com>; Kiszka, Jan (T RDA IOT)
> > <jan.kiszka@siemens.com>
> > Subject: Re: [PATCH 1/1] add support for debian build profiles
> >
> > On Wed, Dec 22, 2021 at 12:37 AM Felix Moessbauer
> > <felix.moessbauer@siemens.com> wrote:
> > >
> > > This patch adds the bitbake variables DEB_BUILD_PROFILES and
> > > DEB_BUILD_PROFILES_CROSS.
> > > The values of these variables are used to define the
> > > DEB_BUILD_PROFILES environment variable.
> > >
> > > When cross-compiling, the DEB_BUILD_PROFILES_CROSS variable is
> > > defaulted to "cross", to be consistent with upstream debian.
> > >
> > > Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
> > > ---
> > >  RECIPE-API-CHANGELOG.md        |  8 ++++++++
> > >  meta/classes/dpkg-base.bbclass | 12 ++++++++++++
> > >  meta/classes/dpkg.bbclass      |  5 ++---
> > >  3 files changed, 22 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/RECIPE-API-CHANGELOG.md b/RECIPE-API-CHANGELOG.md index
> > > 55836258..f95f7db5 100644
> > > --- a/RECIPE-API-CHANGELOG.md
> > > +++ b/RECIPE-API-CHANGELOG.md
> > > @@ -306,3 +306,11 @@ When using the plugins it is advised to name the
> > > partition "/boot" and to exclud
> > >
> > >  The variable is renamed to get closer to OE/Poky variables naming.
> > > The old naming  will still also work, but with deprecation warning shown.
> > > +
> > > +### Introduce debian build profiles
> > > +
> > > +All recipes that inherit from dpkg and dpkg-base can utilize the variables
> > DEB_BUILD_PROFILES and DEB_BUILD_PROFILES_CROSS.
> > > +These variables define the DEB_BUILD_PROFILES environment variable which
> > is available in do_install_builddeps and do_dpkg_build.
> > > +The DEB_BUILD_PROFILES_CROSS bitbake variable is set to "cross" when
> > cross compiling.
> >
> > Hi Felix,
> >
> > Thank you for the patch.
> >
> > We could modify the u-boot recipe to have it as an example.
>
> Good idea, I'll send a v2 out next week.
> >
> > Also, It might be useful if we can say something about custom profile names.
> > Debian has a standard[1] for those.
> > pkg.$sourcepackage.$anything
>
> That's true, but is it really the responsibility of ISAR to ensure that developers follow the Debian best practices?
> I doubt that. I would better only document ISAR specific stuff but avoid re-documenting Debian policies / best practices.

I was worried more about accidentally creating a custom profile name
that is predefined. for example nocheck and probably using it for a
different purpose thereby creating a possible conflict.
We could give a pointer to the predefined list if that helps.

I might need to do a PoC to confirm, but it seems very much possible.

>
> >
> > Better to recommend users to follow it, to make sure they don't accidentally
> > reuse one of the registered profile names when creating custom profiles.
> >
> > [1]
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwiki.debi
> > an.org%2FBuildProfileSpec&amp;data=04%7C01%7Cfelix.moessbauer%40sieme
> > ns.com%7C5bf5db77529f46681e4108d9c4c4297a%7C38ae3bcd95794fd4addab
> > 42e1495d55a%7C1%7C0%7C637757170088619029%7CUnknown%7CTWFpbGZs
> > b3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
> > %3D%7C3000&amp;sdata=aExYnPwm%2F2hvZ%2BkhJQA4Y%2BWfKRGfxet%2B
> > T1na%2FdpIgSQ%3D&amp;reserved=0
> >
> > > +
> > > +
> > > diff --git a/meta/classes/dpkg-base.bbclass
> > > b/meta/classes/dpkg-base.bbclass index 6704385b..c1588528 100644
> > > --- a/meta/classes/dpkg-base.bbclass
> > > +++ b/meta/classes/dpkg-base.bbclass
> > > @@ -13,6 +13,8 @@ inherit deb-dl-dir
> > >  DEPENDS ?= ""
> > >
> > >  DEPENDS_append_riscv64 = "${@' crossbuild-essential-riscv64' if
> > d.getVar('ISAR_CROSS_COMPILE', True) == '1' and d.getVar('PN') != 'crossbuild-
> > essential-riscv64' else ''}"
> > > +DEB_BUILD_PROFILES ?= ""
> > > +DEB_BUILD_PROFILES_CROSS ?= "cross"
> >
> > Do we really need two variables? Is the DEB_BUILD_PROFILES not sufficient?
>
> Well... I implemented this patch series while working on a layer that has to support both arm64 and x64.
> With just one variable the inline python functions became a pattern because (depending on the package), various profiles had to be activated only for arm64 (e.g. like "nocheck", "nodoc").
> That's why I prefer to have both variables.
> Is there any strong reason against it (apart from having two variables to maintain)?

Can you give me an example? Does DEB_BUILD_PROFILES not give an option
to activate profile based on arch?

>
> >
> > >
> > >  python do_adjust_git() {
> > >      import subprocess
> > > @@ -201,7 +203,16 @@ dpkg_runbuild() {
> > >      die "This should never be called, overwrite it in your derived class"
> > >  }
> > >
> > > +def isar_export_build_profiles(d):
> > > +    import os
> > > +    deb_build_profiles = os.environ['DEB_BUILD_PROFILES'] if
> > 'DEB_BUILD_PROFILES' in os.environ else ''
> >
> > Ideally the user should set DEB_BUILD_PROFILES in the recipe and the below line
> > would be sufficient.
>
> That's for backward compatibility. We already have a ton of recipes where the profiles are manually activated by exporting the DEB_BUILD_PROFILES in the do_install_builddeps and do_dpkg_build step.
> IMO it's not good to "magically" overwrite environment variables, but your points are also valid.

Ok. In case you have a lot of recipes in such a way, and it needs time
to migrate. But we might need to have a deprecation notice(warning).

Thanks,
Vijai Kumar K

>
> >
> > > +    deb_build_profiles += ' ' + d.getVar('DEB_BUILD_PROFILES', True)
> > > +    if d.getVar("ISAR_CROSS_COMPILE") == "1":
> > > +        deb_build_profiles += ' ' +
> > > + d.getVar('DEB_BUILD_PROFILES_CROSS', True)
> >
> > Probably we can directly add "cross" and drop DEB_BUILD_PROFILES_CROSS
>
> See my comment above.
>
> >
> > > +    os.environ['DEB_BUILD_PROFILES'] = deb_build_profiles.strip()
> > > +
> > >  python do_dpkg_build() {
> > > +    isar_export_build_profiles(d)
> > >      lock = bb.utils.lockfile(d.getVar("REPO_ISAR_DIR") + "/isar.lock",
> > >                               shared=True)
> > >      bb.build.exec_func("dpkg_do_mounts", d) @@ -278,6 +289,7 @@
> > > python do_devshell() {
> > >      bb.build.exec_func('dpkg_do_mounts', d)
> > >
> > >      isar_export_proxies(d)
> > > +    isar_export_build_profiles(d)
> > >
> > >      buildchroot = d.getVar('BUILDCHROOT_DIR')
> > >      pp_pps = os.path.join(d.getVar('PP'), d.getVar('PPS')) diff --git
> > > a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass index
> > > 7da73341..8e5626e8 100644
> > > --- a/meta/classes/dpkg.bbclass
> > > +++ b/meta/classes/dpkg.bbclass
> > > @@ -9,10 +9,9 @@ PACKAGE_ARCH ?= "${DISTRO_ARCH}"
> > >  do_install_builddeps() {
> > >      dpkg_do_mounts
> > >      E="${@ isar_export_proxies(d)}"
> > > +    E="${@ isar_export_build_profiles(d)}"
> > >      distro="${DISTRO}"
> > > -    if [ ${ISAR_CROSS_COMPILE} -eq 1 ]; then
> > > -       distro="${HOST_DISTRO}"
> > > -    fi
> >
> > Probably a mistake dropping the above lines?
>
> Good catch! Yes, that's a mistake. Will be fixed in v2.
>
> Felix!
>
> >
> > Thanks,
> > Vijai Kumar K
> >
> > > +
> > >      deb_dl_dir_import "${BUILDCHROOT_DIR}" "${distro}"
> > >      sudo -E chroot ${BUILDCHROOT_DIR} /isar/deps.sh \
> > >          ${PP}/${PPS} ${PACKAGE_ARCH} --download-only
> > > --
> > > 2.30.2
> > >
> > > --
> > > You received this message because you are subscribed to the Google Groups
> > "isar-users" group.
> > > To unsubscribe from this group and stop receiving emails from it, send an
> > email to isar-users+unsubscribe@googlegroups.com.
> > > To view this discussion on the web visit
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.g
> > oogle.com%2Fd%2Fmsgid%2Fisar-users%2F20211221190653.1142246-1-
> > felix.moessbauer%2540siemens.com&amp;data=04%7C01%7Cfelix.moessbauer
> > %40siemens.com%7C5bf5db77529f46681e4108d9c4c4297a%7C38ae3bcd95794
> > fd4addab42e1495d55a%7C1%7C0%7C637757170088619029%7CUnknown%7CT
> > WFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXV
> > CI6Mn0%3D%7C3000&amp;sdata=eCOTmdSafQe7QH5a%2BG6CmdvqmUXyse9
> > K06QPIZHVXMM%3D&amp;reserved=0.
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "isar-users" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email
> > to isar-users+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.g
> > oogle.com%2Fd%2Fmsgid%2Fisar-
> > users%2FCALLGG_J19GiSTcmyOgOeUftatj2kkbZMaVZReJYRon_YAmJBmA%2540
> > mail.gmail.com&amp;data=04%7C01%7Cfelix.moessbauer%40siemens.com%7C
> > 5bf5db77529f46681e4108d9c4c4297a%7C38ae3bcd95794fd4addab42e1495d55
> > a%7C1%7C0%7C637757170088619029%7CUnknown%7CTWFpbGZsb3d8eyJWIj
> > oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C300
> > 0&amp;sdata=KXiRwZVA0MUjQCThhD5SZ5r4DmvkqiAj5692jiNAIfw%3D&amp;re
> > served=0.
>