public inbox for isar-users@googlegroups.com
 help / color / mirror / Atom feed
* [PATCH] template: Make templates passthrough
@ 2021-11-15 15:09 Anton Mikanovich
  2021-11-15 15:57 ` Henning Schild
  0 siblings, 1 reply; 5+ messages in thread
From: Anton Mikanovich @ 2021-11-15 15:09 UTC (permalink / raw)
  To: isar-users; +Cc: Anton Mikanovich

There is no need to store original template files after the conversion
in real scenarios. To make working folder little cleaner we can remove
them. This can be disabled with TEMPLATE_FILES_KEEP variable.
Moreover output file should have exactly the same flags as input one,
which is usefull for the cases like debian/rules or other executables.
So we should copy this part of metadata after the conversion.

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
 meta/classes/template.bbclass              | 7 ++++++-
 meta/recipes-kernel/linux/linux-custom.inc | 3 ---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/meta/classes/template.bbclass b/meta/classes/template.bbclass
index fb9d1186..f5760e15 100644
--- a/meta/classes/template.bbclass
+++ b/meta/classes/template.bbclass
@@ -4,11 +4,12 @@
 # SPDX-License-Identifier: MIT
 
 TEMPLATE_FILES ?= ""
+TEMPLATE_FILES_KEEP ?= "0"
 TEMPLATE_VARS ?= "PN PV DESCRIPTION HOMEPAGE MAINTAINER DISTRO_ARCH"
 
 do_transform_template[vardeps] = "TEMPLATE_FILES ${TEMPLATE_VARS}"
 python do_transform_template() {
-    import subprocess, contextlib
+    import subprocess, contextlib, shutil
 
     workdir = os.path.normpath(d.getVar('WORKDIR', True))
 
@@ -56,5 +57,9 @@ python do_transform_template() {
                                           stdout=output, env=env))
             if process.wait() != 0:
                 bb.fatal("processing of template failed")
+
+        shutil.copymode(template_file, output_file)
+        if d.getVar('TEMPLATE_FILES_KEEP', True) != '1':
+            os.remove(template_file)
 }
 addtask do_transform_template after do_unpack
diff --git a/meta/recipes-kernel/linux/linux-custom.inc b/meta/recipes-kernel/linux/linux-custom.inc
index ed89aa09..57740860 100644
--- a/meta/recipes-kernel/linux/linux-custom.inc
+++ b/meta/recipes-kernel/linux/linux-custom.inc
@@ -122,9 +122,6 @@ do_prepare_build_prepend() {
 	rm -rf ${S}/debian
 	cp -r ${WORKDIR}/debian ${S}/
 
-	# remove templates from the source tree
-	find ${S}/debian -name *.tmpl | xargs rm -f
-
 	# rename install/remove hooks to match user-specified name for our linux-image package
 	mv ${S}/debian/linux-image.postinst ${S}/debian/linux-image-${KERNEL_NAME_PROVIDED}.postinst
 	mv ${S}/debian/linux-image.postrm ${S}/debian/linux-image-${KERNEL_NAME_PROVIDED}.postrm
-- 
2.20.1


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

* Re: [PATCH] template: Make templates passthrough
  2021-11-15 15:09 [PATCH] template: Make templates passthrough Anton Mikanovich
@ 2021-11-15 15:57 ` Henning Schild
  2021-11-16 16:35   ` Anton Mikanovich
  2021-12-10 11:30   ` Baurzhan Ismagulov
  0 siblings, 2 replies; 5+ messages in thread
From: Henning Schild @ 2021-11-15 15:57 UTC (permalink / raw)
  To: Anton Mikanovich; +Cc: isar-users

Does that removal have any side-effects on ... say the fetch task?
If not .. i guess toggeling that KEEP from 0 to 1 would result in "no
such file"


Am Mon, 15 Nov 2021 18:09:35 +0300
schrieb Anton Mikanovich <amikan@ilbers.de>:

> There is no need to store original template files after the conversion
> in real scenarios. To make working folder little cleaner we can remove
> them. This can be disabled with TEMPLATE_FILES_KEEP variable.

i do not buy the argument and would keep it the way it is ... reject
that patch, but maybe i do not yet get the full picture.
Not the whole patch ... the bits in the custom linux could be valid ...
always keep as with all other templates, unless it makes debian unhappy.

> Moreover output file should have exactly the same flags as input one,
> which is usefull for the cases like debian/rules or other executables.
> So we should copy this part of metadata after the conversion.
> 
> Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
> ---
>  meta/classes/template.bbclass              | 7 ++++++-
>  meta/recipes-kernel/linux/linux-custom.inc | 3 ---
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/meta/classes/template.bbclass
> b/meta/classes/template.bbclass index fb9d1186..f5760e15 100644
> --- a/meta/classes/template.bbclass
> +++ b/meta/classes/template.bbclass
> @@ -4,11 +4,12 @@
>  # SPDX-License-Identifier: MIT
>  
>  TEMPLATE_FILES ?= ""
> +TEMPLATE_FILES_KEEP ?= "0"

Why offer a way at all? And if we do not default to 1, do we need a
changelog entry?

All in all i think we can keep the files ... always. It is not like
they add a lot of overhead or confusion. And nobody would delete c
files in a Makefile after the .o s are created ...

regards,
Henning

>  TEMPLATE_VARS ?= "PN PV DESCRIPTION HOMEPAGE MAINTAINER DISTRO_ARCH"
>  
>  do_transform_template[vardeps] = "TEMPLATE_FILES ${TEMPLATE_VARS}"
>  python do_transform_template() {
> -    import subprocess, contextlib
> +    import subprocess, contextlib, shutil
>  
>      workdir = os.path.normpath(d.getVar('WORKDIR', True))
>  
> @@ -56,5 +57,9 @@ python do_transform_template() {
>                                            stdout=output, env=env))
>              if process.wait() != 0:
>                  bb.fatal("processing of template failed")
> +
> +        shutil.copymode(template_file, output_file)
> +        if d.getVar('TEMPLATE_FILES_KEEP', True) != '1':
> +            os.remove(template_file)
>  }
>  addtask do_transform_template after do_unpack
> diff --git a/meta/recipes-kernel/linux/linux-custom.inc
> b/meta/recipes-kernel/linux/linux-custom.inc index ed89aa09..57740860
> 100644 --- a/meta/recipes-kernel/linux/linux-custom.inc
> +++ b/meta/recipes-kernel/linux/linux-custom.inc
> @@ -122,9 +122,6 @@ do_prepare_build_prepend() {
>  	rm -rf ${S}/debian
>  	cp -r ${WORKDIR}/debian ${S}/
>  
> -	# remove templates from the source tree
> -	find ${S}/debian -name *.tmpl | xargs rm -f
> -
>  	# rename install/remove hooks to match user-specified name
> for our linux-image package mv ${S}/debian/linux-image.postinst
> ${S}/debian/linux-image-${KERNEL_NAME_PROVIDED}.postinst mv
> ${S}/debian/linux-image.postrm
> ${S}/debian/linux-image-${KERNEL_NAME_PROVIDED}.postrm


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

* Re: [PATCH] template: Make templates passthrough
  2021-11-15 15:57 ` Henning Schild
@ 2021-11-16 16:35   ` Anton Mikanovich
  2021-12-10 11:30   ` Baurzhan Ismagulov
  1 sibling, 0 replies; 5+ messages in thread
From: Anton Mikanovich @ 2021-11-16 16:35 UTC (permalink / raw)
  To: Henning Schild; +Cc: isar-users, Baurzhan Ismagulov

[-- Attachment #1: Type: text/plain, Size: 2612 bytes --]

15.11.2021 18:57, Henning Schild wrote:
> Does that removal have any side-effects on ... say the fetch task?
> If not .. i guess toggeling that KEEP from 0 to 1 would result in "no
> such file"

Task do_transform_template depends on do_fetch, rebuild after template 
change scenarios works.
Can you give an example of any case where "No such file" issue can happen?

>> There is no need to store original template files after the conversion
>> in real scenarios. To make working folder little cleaner we can remove
>> them. This can be disabled with TEMPLATE_FILES_KEEP variable.
> i do not buy the argument and would keep it the way it is ... reject
> that patch, but maybe i do not yet get the full picture.
> Not the whole patch ... the bits in the custom linux could be valid ...
> always keep as with all other templates, unless it makes debian unhappy.
>
Yes, looks like you don't get the picture.
If talking about templates removing in linux-custom, we just move it to 
bbclass to not mention rm in any recipe needed clean sources.

>
>> Moreover output file should have exactly the same flags as input one,
>> which is usefull for the cases like debian/rules or other executables.
>> So we should copy this part of metadata after the conversion.
>>
>> Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
>> ---
>>   meta/classes/template.bbclass              | 7 ++++++-
>>   meta/recipes-kernel/linux/linux-custom.inc | 3 ---
>>   2 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/meta/classes/template.bbclass
>> b/meta/classes/template.bbclass index fb9d1186..f5760e15 100644
>> --- a/meta/classes/template.bbclass
>> +++ b/meta/classes/template.bbclass
>> @@ -4,11 +4,12 @@
>>   # SPDX-License-Identifier: MIT
>>   
>>   TEMPLATE_FILES ?= ""
>> +TEMPLATE_FILES_KEEP ?= "0"
> Why offer a way at all? And if we do not default to 1, do we need a
> changelog entry?
>
> All in all i think we can keep the files ... always. It is not like
> they add a lot of overhead or confusion. And nobody would delete c
> files in a Makefile after the .o s are created ...
>
> regards,
> Henning

This will not change any recipe API.

I've kept TEMPLATE_FILES_KEEP mostly for debugging. Can't think of any 
real usage of keeping templates inside work dir after the build finish.
We do not delete them from the sources, so it can't be compared with 
removing .c after .o generation.

-- 
Anton Mikanovich
Promwad Ltd.
External service provider of ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn, Germany
+49 (89) 122 67 24-0
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov


[-- Attachment #2: Type: text/html, Size: 4147 bytes --]

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

* Re: [PATCH] template: Make templates passthrough
  2021-11-15 15:57 ` Henning Schild
  2021-11-16 16:35   ` Anton Mikanovich
@ 2021-12-10 11:30   ` Baurzhan Ismagulov
  2021-12-13 10:53     ` Jan Kiszka
  1 sibling, 1 reply; 5+ messages in thread
From: Baurzhan Ismagulov @ 2021-12-10 11:30 UTC (permalink / raw)
  To: isar-users

On Mon, Nov 15, 2021 at 04:57:45PM +0100, Henning Schild wrote:
> always keep as with all other templates, unless it makes debian unhappy.

It does. In short, this patch:

* Removes spurious source changes for proper source package and .changes
  generation

* Keep the permissions the same as for the template files

Since sbuild requires proper source packages (which is a Good Thing), this
patch is also a pre-requisite for sbuild. If there are no objections, we would
like to apply this next week.

With kind regards,
Baurzhan.

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

* Re: [PATCH] template: Make templates passthrough
  2021-12-10 11:30   ` Baurzhan Ismagulov
@ 2021-12-13 10:53     ` Jan Kiszka
  0 siblings, 0 replies; 5+ messages in thread
From: Jan Kiszka @ 2021-12-13 10:53 UTC (permalink / raw)
  To: isar-users

On 10.12.21 12:30, Baurzhan Ismagulov wrote:
> On Mon, Nov 15, 2021 at 04:57:45PM +0100, Henning Schild wrote:
>> always keep as with all other templates, unless it makes debian unhappy.
> 
> It does. In short, this patch:
> 
> * Removes spurious source changes for proper source package and .changes
>   generation
> 
> * Keep the permissions the same as for the template files
> 
> Since sbuild requires proper source packages (which is a Good Thing), this
> patch is also a pre-requisite for sbuild. If there are no objections, we would
> like to apply this next week.
> 

The patch is correct when it works on template files that have been
copied from the workdir to some place where they are unexpected. The
patch is incorrect in removing files from the workdir where the fetcher
put them.

And, as Henning pointed out, that option to keep the input files is
redundant if the problem is solved properly - we must keep anyway what
the fetcher placed into workdir.

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

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

end of thread, other threads:[~2021-12-13 10:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-15 15:09 [PATCH] template: Make templates passthrough Anton Mikanovich
2021-11-15 15:57 ` Henning Schild
2021-11-16 16:35   ` Anton Mikanovich
2021-12-10 11:30   ` Baurzhan Ismagulov
2021-12-13 10:53     ` Jan Kiszka

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