Re: [PATCH v3] dpkg: sbuild allows extra arguments via DPKG_SBUILD_EXTRA_ARGS v3

["Roberto A. Foglietta" <roberto.foglietta@gmail.com>]

On Wed, 1 Feb 2023 at 16:40, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> On 01.02.23 16:30, Roberto A. Foglietta wrote:
> > On Wed, 1 Feb 2023 at 15:47, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> >>
> >> On 25.01.23 17:42, roberto.foglietta@linuxteam.org wrote:
> >>> From: "Roberto A. Foglietta" <roberto.foglietta@gmail.com>
> >>>
> >>> Sometimes it is necessary to add some extra commands or arguments for
> >>> the sbuild process which produces customs packages but creating a class
> >>> into an upper layer just for this will create difficulties in managing
> >>> the updates from the upstream project.
> >>>
> >>> So, this patch allows setting extra parameters via this variable:
> >>>
> >>>         DPKG_SBUILD_EXTRA_ARGS
> >>>
> >>> v.2: just a single variable and not anymore two of them
> >>>
> >>> v.3: the variable is set in the middle, just in case order matters, it
> >>>      is the last of 'setup chroot' and the first of 'final build' commands
> >>>
> >>> Signed-off-by: Roberto A. Foglietta <roberto.foglietta@gmail.com>
> >>> ---
> >>> v.2: just a single variable and not anymore two of them
> >>>
> >>> v.3: the variable is set in the middle, just in case order matters, it
> >>>      is the last of 'setup chroot' and the first of 'final build' commands
> >>>
> >>>  meta/classes/dpkg.bbclass | 3 +++
> >>>  1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
> >>> index 7822b14d..8785237c 100644
> >>> --- a/meta/classes/dpkg.bbclass
> >>> +++ b/meta/classes/dpkg.bbclass
> >>> @@ -23,6 +23,8 @@ do_prepare_build_append() {
> >>>      env > ${DPKG_PREBUILD_ENV_FILE}
> >>>  }
> >>>
> >>> +DPKG_SBUILD_EXTRA_ARGS ?= ""
> >>> +
> >>>  # Build package from sources using build script
> >>>  dpkg_runbuild[vardepsexclude] += "${SBUILD_PASSTHROUGH_ADDITIONS}"
> >>>  dpkg_runbuild() {
> >>> @@ -109,6 +111,7 @@ dpkg_runbuild() {
> >>>          --chroot-setup-commands="echo \"APT::Get::allow-downgrades 1;\" > /etc/apt/apt.conf.d/50isar-apt" \
> >>>          --chroot-setup-commands="rm -f /var/log/dpkg.log" \
> >>>          --chroot-setup-commands="cp -n --no-preserve=owner ${ext_deb_dir}/*.deb -t ${deb_dir}/ || :" \
> >>> +        ${DPKG_SBUILD_EXTRA_ARGS} \
> >>>          --finished-build-commands="rm -f ${deb_dir}/sbuild-build-depends-main-dummy_*.deb" \
> >>>          --finished-build-commands="cp -n --no-preserve=owner ${deb_dir}/*.deb -t ${ext_deb_dir}/ || :" \
> >>>          --finished-build-commands="cp /var/log/dpkg.log ${ext_root}/dpkg_partial.log" \
> >>
> >> I'm seeing this in next, but it seems everyone missed that this should
> >> not go in like this:
> >>
> >> Missing elaborated reasoning. No in-tree use case or at least some
> >> explanation why we should open such a low-level interface to recipes.
> >
> > At least one Siemens project uses it, unless it has been changed after
> > I left. In general there is no reason to exclude that building a
> > custom .deb package does not require to use this variable. If not
> > used, it does not hurt. If used, avoid duplicating the dpkg class in
> > the top layer and go out of the upstream. Moreover, ISAR has plenty of
> > variables that modify the low-level interface or its behaviour. After
> > all, flexibility is what makes ISAR valuable.
>
> I'm not categorically arguing against it, but in the absence of any use
> case, it is hard to assess if there are reasonable ones. We already had
> fun recently with "EXTRA_ARGS" [1], and this goes even more to the core.
>

It has been done once, it could be done twice. However, it is not my
problem anymore if a project in Siemens will require a change to get
upstream with ISAR or continuously be in maintenance or be kept
downstream. I preferably focus on another revert [1] which actually
allows a system which failed to grow its last partition to ignore it
and go into production without any complaints - which can make a huge
difference while adding a variable that is just a matter of adding
flexibility and this should be the standard for all the subsystems at
any level.

[1] https://github.com/ilbers/isar/commit/22a014087ac8fde2e45e90c5cc2827b7f9e78863

Best regards,
-R