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

[Uladzimir Bely <ubely@ilbers.de>]

In mail from Wednesday, 1 February 2023 22:02:56 +03 user Jan Kiszka wrote:
> On 01.02.23 16:48, Roberto A. Foglietta wrote:
> > 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.
> 
> Looked around internally, and I can confirm that we do not have such a
> demand in our layers. So, in the absence of a reasonable use case for
> this interface, I vote for reverting it. Uladzimir, do you need a revert
> patch from me?
> 
> Jan

Yes, please, send a revert with an explanation why we don't want it.