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

["Moessbauer, Felix" <felix.moessbauer@siemens.com>]

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.

> 
> 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)?

> 
> >
> >  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.

> 
> > +    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.