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

[Jan Kiszka <jan.kiszka@siemens.com>]

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

-- 
Siemens AG, Technology
Competence Center Embedded Linux