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

You can always do "funny" things in downstream, and Isar can't prevent
that technically. Still, suggesting that this here is an official recipe
API is more than that.

Jan

[1]
https://groups.google.com/d/msgid/isar-users/20230105060757.2918-1-ubely%40ilbers.de

-- 
Siemens AG, Technology
Competence Center Embedded Linux