public inbox for isar-users@googlegroups.com
 help / color / mirror / Atom feed
* [PATCH v3] dpkg: sbuild allows extra arguments via DPKG_SBUILD_EXTRA_ARGS v3
@ 2023-01-25 16:42 roberto.foglietta
  2023-02-01  6:19 ` Uladzimir Bely
  2023-02-01 14:47 ` Jan Kiszka
  0 siblings, 2 replies; 9+ messages in thread
From: roberto.foglietta @ 2023-01-25 16:42 UTC (permalink / raw)
  To: isar-users; +Cc: roberto.foglietta

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] dpkg: sbuild allows extra arguments via DPKG_SBUILD_EXTRA_ARGS v3
  2023-01-25 16:42 [PATCH v3] dpkg: sbuild allows extra arguments via DPKG_SBUILD_EXTRA_ARGS v3 roberto.foglietta
@ 2023-02-01  6:19 ` Uladzimir Bely
  2023-02-01 14:46   ` Roberto A. Foglietta
  2023-02-01 14:47 ` Jan Kiszka
  1 sibling, 1 reply; 9+ messages in thread
From: Uladzimir Bely @ 2023-02-01  6:19 UTC (permalink / raw)
  To: isar-users

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.




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] dpkg: sbuild allows extra arguments via DPKG_SBUILD_EXTRA_ARGS v3
  2023-02-01  6:19 ` Uladzimir Bely
@ 2023-02-01 14:46   ` Roberto A. Foglietta
  0 siblings, 0 replies; 9+ messages in thread
From: Roberto A. Foglietta @ 2023-02-01 14:46 UTC (permalink / raw)
  To: Uladzimir Bely; +Cc: isar-users

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-

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] dpkg: sbuild allows extra arguments via DPKG_SBUILD_EXTRA_ARGS v3
  2023-01-25 16:42 [PATCH v3] dpkg: sbuild allows extra arguments via DPKG_SBUILD_EXTRA_ARGS v3 roberto.foglietta
  2023-02-01  6:19 ` Uladzimir Bely
@ 2023-02-01 14:47 ` Jan Kiszka
  2023-02-01 15:30   ` Roberto A. Foglietta
  1 sibling, 1 reply; 9+ messages in thread
From: Jan Kiszka @ 2023-02-01 14:47 UTC (permalink / raw)
  To: roberto.foglietta, isar-users, Uladzimir Bely

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

-- 
Siemens AG, Technology
Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] dpkg: sbuild allows extra arguments via DPKG_SBUILD_EXTRA_ARGS v3
  2023-02-01 14:47 ` Jan Kiszka
@ 2023-02-01 15:30   ` Roberto A. Foglietta
  2023-02-01 15:40     ` Jan Kiszka
  0 siblings, 1 reply; 9+ messages in thread
From: Roberto A. Foglietta @ 2023-02-01 15:30 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: roberto.foglietta, isar-users, Uladzimir Bely

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-

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] dpkg: sbuild allows extra arguments via DPKG_SBUILD_EXTRA_ARGS v3
  2023-02-01 15:30   ` Roberto A. Foglietta
@ 2023-02-01 15:40     ` Jan Kiszka
  2023-02-01 15:48       ` Roberto A. Foglietta
  0 siblings, 1 reply; 9+ messages in thread
From: Jan Kiszka @ 2023-02-01 15:40 UTC (permalink / raw)
  To: Roberto A. Foglietta; +Cc: roberto.foglietta, isar-users, Uladzimir Bely

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] dpkg: sbuild allows extra arguments via DPKG_SBUILD_EXTRA_ARGS v3
  2023-02-01 15:40     ` Jan Kiszka
@ 2023-02-01 15:48       ` Roberto A. Foglietta
  2023-02-01 19:02         ` Jan Kiszka
  0 siblings, 1 reply; 9+ messages in thread
From: Roberto A. Foglietta @ 2023-02-01 15:48 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: roberto.foglietta, isar-users, Uladzimir Bely

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] dpkg: sbuild allows extra arguments via DPKG_SBUILD_EXTRA_ARGS v3
  2023-02-01 15:48       ` Roberto A. Foglietta
@ 2023-02-01 19:02         ` Jan Kiszka
  2023-02-02  9:04           ` Uladzimir Bely
  0 siblings, 1 reply; 9+ messages in thread
From: Jan Kiszka @ 2023-02-01 19:02 UTC (permalink / raw)
  To: Roberto A. Foglietta, Uladzimir Bely; +Cc: roberto.foglietta, isar-users

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] dpkg: sbuild allows extra arguments via DPKG_SBUILD_EXTRA_ARGS v3
  2023-02-01 19:02         ` Jan Kiszka
@ 2023-02-02  9:04           ` Uladzimir Bely
  0 siblings, 0 replies; 9+ messages in thread
From: Uladzimir Bely @ 2023-02-02  9:04 UTC (permalink / raw)
  To: Jan Kiszka, isar-users

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. 




^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-02-02  9:05 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-25 16:42 [PATCH v3] dpkg: sbuild allows extra arguments via DPKG_SBUILD_EXTRA_ARGS v3 roberto.foglietta
2023-02-01  6:19 ` Uladzimir Bely
2023-02-01 14:46   ` Roberto A. Foglietta
2023-02-01 14:47 ` Jan Kiszka
2023-02-01 15:30   ` Roberto A. Foglietta
2023-02-01 15:40     ` Jan Kiszka
2023-02-01 15:48       ` Roberto A. Foglietta
2023-02-01 19:02         ` Jan Kiszka
2023-02-02  9:04           ` Uladzimir Bely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox