RE: [PATCH v5 07/12] sbuild: support of shell exports from dpkg_runbuild_prepend

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

> -----Original Message-----
> From: isar-users@googlegroups.com <isar-users@googlegroups.com> On
> Behalf Of Jan Kiszka
> Sent: Wednesday, February 2, 2022 7:57 AM
> To: Uladzimir Bely <ubely@ilbers.de>; isar-users@googlegroups.com
> Subject: Re: [PATCH v5 07/12] sbuild: support of shell exports from
> dpkg_runbuild_prepend
> 
> On 01.02.22 19:41, Uladzimir Bely wrote:
> > In the email from Tuesday, 1 February 2022 21:09:07 +03 user Jan Kiszka
> wrote:
> >> On 01.02.22 18:00, Uladzimir Bely wrote:
> >>> Many of recipes often use shell exports done in dpkg_run_prepend, so
> >>> that this changed environment is used during build.
> >>>
> >>> While sbuild is performed in isolated environment, we need a way to
> >>> pass these variables to it. This is done by storing environment
> >>> before dpkg_runbuild (after prepare_build) and finding just before
> >>> the actual build what was changed or added.
> >>>
> >>> Signed-off-by: Uladzimir Bely <ubely@ilbers.de>
> >>> ---
> >>>
> >>>  meta/classes/dpkg.bbclass | 18 ++++++++++++++++++
> >>>  1 file changed, 18 insertions(+)
> >>>
> >>> diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
> >>> index 66db7ec5..c252e9b3 100644
> >>> --- a/meta/classes/dpkg.bbclass
> >>> +++ b/meta/classes/dpkg.bbclass
> >>> @@ -29,12 +29,30 @@ do_install_builddeps[lockfiles] +=
> >>> "${REPO_ISAR_DIR}/isar.lock">  addtask devshell after
> >>> do_install_builddeps
> >>>
> >>> +DPKG_PREBUILD_ENV_FILE="${WORKDIR}/dpkg_prebuild.env"
> >>> +
> >>> +do_prepare_build_append() {
> >>> +    env > ${DPKG_PREBUILD_ENV_FILE} }
> >>> +
> >>>
> >>>  # Build package from sources using build script
> >>>  dpkg_runbuild() {
> >>>
> >>>      E="${@ isar_export_proxies(d)}"
> >>>      E="${@ isar_export_ccache(d)}"
> >>>      export PARALLEL_MAKE="${PARALLEL_MAKE}"
> >>>
> >>> +    env | while read -r line; do
> >>> +        # Filter the same lines
> >>> +        grep -q "^${line}" ${DPKG_PREBUILD_ENV_FILE} && continue
> >>> +        # Filter some standard variables
> >>> +        echo ${line} | grep -q "^HOME=" && continue
> >>> +        echo ${line} | grep -q "^PWD=" && continue
> >>> +
> >>> +        var=$(echo "${line}" | cut -d '=' -f1)
> >>> +        value=$(echo "${line}" | cut -d '=' -f2-)
> >>> +        sbuild_export $var "$value"
> >>> +    done
> >>> +
> >>>
> >>>      profiles=$(grep "DEB_BUILD_PROFILES" ${SBUILD_CONFIG} | tail -n1 |
> >>>      cut -d "'" -f 4) if [ ${ISAR_CROSS_COMPILE} -eq 1 ]; then
> >>>
> >>>          profiles="${profiles} cross nocheck"
> >>
> >> So, this basically decouples "Avoid using shell environment during
> >> the build" and similar conversions downstream from this series,
> >> right? It's indeed good to have a compat path now.
> >>
> >> However, what are the patterns we want to push? Avoiding exports?
> >> Then we should probably warn here that this compat path should be
> >> avoided and might be removed in the future.
> >>
> >> If we want to keep both, we can probably leave several recipes alone
> >> in that other series.
> >>
> >> Jan
> >
> > You might be right. Current solution doesn't force users to avoid
> > using exports while they are silently supported.
> >
> > The first internal solution was keeping dpkg_build_export function but
> > also passing custom shell exports (from dpkg_runbuild_prepend) to
> > sbuild environment. And a warning about migration was shown.
> >
> > Later I removed this to make new patchset less invasive for downstreams.
> > Anyway, if we agree that "dpkg_build_export" (like API function) is
> > more preferable, I can get back this solution with warnings.
> >
> 
> This is not necessarily what I'm suggesting. Your conversion series works
> without that API, primarily using template. We also have build profiles. But we
> first of all need to define what we actually want.
> 
> The benefit of pushing things into the build env, rather than feeding it from the
> recipe, is likely that the resulting deb-src of such generated packages can
> actually be used for rebuilding independently of Isar. So there is likely value in
> deprecating classic export.

Using environment variables to control the build in Debian/rules is generally discouraged (except for DEB_BUILD_PROFILES and DEB_BUILD_OPTIONS).
But even for these variables, semantics are documented [1] and we / an ISAR user should strictly follow them when packaging.

There are also reasons why you want to build a package in a clean environment (e.g. to avoid accidentally setting CC or CXX to icc or clang).
Hence, I highly vote for not forwarding the environment into the sbuild build environment.

Felix

[1] https://wiki.debian.org/BuildProfileSpec#Registered_profile_names

> 
> Jan
> 
> --
> Siemens AG, Technology
> Competence Center Embedded Linux
> 
> --
> 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%2F9f8b4874-62b5-546f-5dc9-
> 8d9c0bd5b1cf%2540siemens.com&amp;data=04%7C01%7Cfelix.moessbauer%4
> 0siemens.com%7C9ed643fe46b945b7d2a008d9e619471f%7C38ae3bcd95794fd
> 4addab42e1495d55a%7C1%7C0%7C637793819407336907%7CUnknown%7CTW
> FpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI
> 6Mn0%3D%7C3000&amp;sdata=Wfb%2F4NfV1M9LjodozJ1V5p3riY8HHobGS31
> %2B6gJ7y6c%3D&amp;reserved=0.