diff mbox series

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

Message ID 20230125164227.1448218-1-roberto.foglietta@linuxteam.org
State Accepted, archived
Headers show
Series [v3] dpkg: sbuild allows extra arguments via DPKG_SBUILD_EXTRA_ARGS v3 | expand

Commit Message

roberto.foglietta@linuxteam.org Jan. 25, 2023, 4:42 p.m. UTC 
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(+)

Comments

Uladzimir Bely Feb. 1, 2023, 6:19 a.m. UTC | #1 
In mail from Wednesday, 25 January 2023 19:42:27 +03 user 
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(+)
> 
Applied to next (with cleaned version history in commit message), thanks.
Roberto A. Foglietta Feb. 1, 2023, 2:46 p.m. UTC | #2 
On Wed, 1 Feb 2023 at 07:19, Uladzimir Bely <ubely@ilbers.de> wrote:
>
> In mail from Wednesday, 25 January 2023 19:42:27 +03 user
> 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(+)
> >
> Applied to next (with cleaned version history in commit message), thanks.

I will make a change to my git-isar-email-send script in such a way it
will remove any "^v.*NN: '' strings. So, we will finally reach a
polite conclusion about versioning. LOL

 Best regards, R-
Jan Kiszka Feb. 1, 2023, 2:47 p.m. UTC | #3 
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.

Please clarify or revert.

Thanks,
Jan
Roberto A. Foglietta Feb. 1, 2023, 3:30 p.m. UTC | #4 
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.

Best regards, R-
Jan Kiszka Feb. 1, 2023, 3:40 p.m. UTC | #5 
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
Roberto A. Foglietta Feb. 1, 2023, 3:48 p.m. UTC | #6 
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
Jan Kiszka Feb. 1, 2023, 7:02 p.m. UTC | #7 
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
Uladzimir Bely Feb. 2, 2023, 9:04 a.m. UTC | #8 
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.
diff mbox series

Patch

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" \