* [PATCH 2/2] meta-isar: make sure to quilt patch a debian package
2020-07-28 20:31 [PATCH 1/2] patch: special-case quilt in debian Henning Schild
@ 2020-07-28 20:31 ` Henning Schild
2020-07-28 20:40 ` Henning Schild
2020-07-28 20:33 ` [PATCH 1/2] patch: special-case quilt in debian Henning Schild
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Henning Schild @ 2020-07-28 20:31 UTC (permalink / raw)
To: isar-users; +Cc: Henning Schild
From: Henning Schild <henning.schild@siemens.com>
We have special debian quilt support, test it somewhere.
Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
meta-isar/recipes-app/cowsay/cowsay_git.bb | 1 +
meta-isar/recipes-app/cowsay/files/isar.patch | 12 ++++++++++++
2 files changed, 13 insertions(+)
create mode 100644 meta-isar/recipes-app/cowsay/files/isar.patch
diff --git a/meta-isar/recipes-app/cowsay/cowsay_git.bb b/meta-isar/recipes-app/cowsay/cowsay_git.bb
index 7aca1b4560..c3a1f4942a 100644
--- a/meta-isar/recipes-app/cowsay/cowsay_git.bb
+++ b/meta-isar/recipes-app/cowsay/cowsay_git.bb
@@ -6,4 +6,5 @@
inherit dpkg-gbp
SRC_URI = "git://salsa.debian.org/debian/cowsay.git;protocol=https"
+SRC_URI += "file://isar.patch"
SRCREV = "756f0c41fbf582093c0c1dff9ff77734716cb26f"
diff --git a/meta-isar/recipes-app/cowsay/files/isar.patch b/meta-isar/recipes-app/cowsay/files/isar.patch
new file mode 100644
index 0000000000..00dae536ac
--- /dev/null
+++ b/meta-isar/recipes-app/cowsay/files/isar.patch
@@ -0,0 +1,12 @@
+Index: git/cows/elephant.cow
+===================================================================
+--- git.orig/cows/elephant.cow
++++ git/cows/elephant.cow
+@@ -1,6 +1,7 @@
+ ##
+ ## An elephant out and about
+ ##
++##
+ $the_cow = <<EOC;
+ $thoughts /\\ ___ /\\
+ $thoughts // \\/ \\/ \\\\
--
2.26.2
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 2/2] meta-isar: make sure to quilt patch a debian package
2020-07-28 20:31 ` [PATCH 2/2] meta-isar: make sure to quilt patch a debian package Henning Schild
@ 2020-07-28 20:40 ` Henning Schild
0 siblings, 0 replies; 13+ messages in thread
From: Henning Schild @ 2020-07-28 20:40 UTC (permalink / raw)
To: isar-users
On Tue, 28 Jul 2020 22:31:53 +0200
Henning Schild <henning.schild@siemens.com> wrote:
> From: Henning Schild <henning.schild@siemens.com>
>
> We have special debian quilt support, test it somewhere.
>
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
> meta-isar/recipes-app/cowsay/cowsay_git.bb | 1 +
> meta-isar/recipes-app/cowsay/files/isar.patch | 12 ++++++++++++
> 2 files changed, 13 insertions(+)
> create mode 100644 meta-isar/recipes-app/cowsay/files/isar.patch
>
> diff --git a/meta-isar/recipes-app/cowsay/cowsay_git.bb
> b/meta-isar/recipes-app/cowsay/cowsay_git.bb index
> 7aca1b4560..c3a1f4942a 100644 ---
> a/meta-isar/recipes-app/cowsay/cowsay_git.bb +++
> b/meta-isar/recipes-app/cowsay/cowsay_git.bb @@ -6,4 +6,5 @@
> inherit dpkg-gbp
>
> SRC_URI = "git://salsa.debian.org/debian/cowsay.git;protocol=https"
> +SRC_URI += "file://isar.patch"
> SRCREV = "756f0c41fbf582093c0c1dff9ff77734716cb26f"
Patching this actually means we need to change CHANGELOG_V, i will do
that. But for now let us focus on p1.
Henning
> diff --git a/meta-isar/recipes-app/cowsay/files/isar.patch
> b/meta-isar/recipes-app/cowsay/files/isar.patch new file mode 100644
> index 0000000000..00dae536ac
> --- /dev/null
> +++ b/meta-isar/recipes-app/cowsay/files/isar.patch
> @@ -0,0 +1,12 @@
> +Index: git/cows/elephant.cow
> +===================================================================
> +--- git.orig/cows/elephant.cow
> ++++ git/cows/elephant.cow
> +@@ -1,6 +1,7 @@
> + ##
> + ## An elephant out and about
> + ##
> ++##
> + $the_cow = <<EOC;
> + $thoughts /\\ ___ /\\
> + $thoughts // \\/ \\/ \\\\
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] patch: special-case quilt in debian
2020-07-28 20:31 [PATCH 1/2] patch: special-case quilt in debian Henning Schild
2020-07-28 20:31 ` [PATCH 2/2] meta-isar: make sure to quilt patch a debian package Henning Schild
@ 2020-07-28 20:33 ` Henning Schild
2020-07-29 14:06 ` Jan Kiszka
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Henning Schild @ 2020-07-28 20:33 UTC (permalink / raw)
To: isar-users
Patching has always been a huge pain, please test this and let me know
if it works for your layers.
Henning
On Tue, 28 Jul 2020 22:31:52 +0200
Henning Schild <henning.schild@siemens.com> wrote:
> From: Henning Schild <henning.schild@siemens.com>
>
> The OE patch lib uses quilt and so do many debian packages as well.
> Those two do not work well together, it is really hard to create a
> patch that will apply and not break what debian does later. debian is
> very pedantic about unexpected changes so even if patching works,
> building might not.
>
> Introduce a special-case where we detect quilt usage of a debian
> package and hook in there. Also make sure we are on top of debian so
> we do not risk breaking patches we inherit from there.
>
> If anyone ever managed to create a patch that works well in the face
> of two quilts, that might break with this change. You can set
> PATCHTOOL to "quilt" in your recipe to disable the magic.
>
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
> meta/classes/patch.bbclass | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
> index 3060755a5c..06f32a2197 100644
> --- a/meta/classes/patch.bbclass
> +++ b/meta/classes/patch.bbclass
> @@ -91,6 +91,28 @@ def should_apply(parm, d):
>
> should_apply[vardepsexclude] = "DATE SRCDATE"
>
> +def patch_do_debian_quilt(patchdir, d):
> + import oe.patch
> + class DummyPatchSet(oe.patch.PatchSet):
> + def Clean(self):
> + True
> +
> + def Import(self, patch, force):
> + os.putenv('QUILT_PATCHES', 'debian/patches')
> + # push all so we are on top of debian
> + pushed = False
> + if os.path.exists(os.path.join(self.dir,
> 'debian/patches/series')):
> + oe.patch.runcmd(["quilt", "push", "-a"], self.dir)
> + pushed = True
> + oe.patch.runcmd(["quilt", "import", "-f",
> os.path.join(d.getVar('WORKDIR'), os.path.basename(patch['file']))],
> self.dir)
> + if pushed:
> + oe.patch.runcmd(["quilt", "pop", "-a"], self.dir)
> +
> + def Push(self, force = False):
> + True
> +
> + return DummyPatchSet(patchdir, d)
> +
> python patch_do_patch() {
> import sys
>
> @@ -118,6 +140,12 @@ python patch_do_patch() {
>
> s = d.getVar('S')
>
> + debianformat = os.path.join(s, 'debian/source/format')
> + if os.path.exists(debianformat) and d.getVar('PATCHTOOL') !=
> 'quilt':
> + with open(debianformat, 'r+') as f:
> + if f.readline() == '3.0 (quilt)\n':
> + cls = patch_do_debian_quilt
> +
> os.putenv('PATH', d.getVar('PATH'))
>
> # We must use one TMPDIR per process so that the "patch"
> processes
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/2] patch: special-case quilt in debian
2020-07-28 20:31 [PATCH 1/2] patch: special-case quilt in debian Henning Schild
2020-07-28 20:31 ` [PATCH 2/2] meta-isar: make sure to quilt patch a debian package Henning Schild
2020-07-28 20:33 ` [PATCH 1/2] patch: special-case quilt in debian Henning Schild
@ 2020-07-29 14:06 ` Jan Kiszka
2020-07-29 14:36 ` Henning Schild
2020-07-29 14:39 ` Henning Schild
2020-07-29 15:08 ` Henning Schild
4 siblings, 1 reply; 13+ messages in thread
From: Jan Kiszka @ 2020-07-29 14:06 UTC (permalink / raw)
To: [ext] Henning Schild, isar-users
On 28.07.20 22:31, [ext] Henning Schild wrote:
> From: Henning Schild <henning.schild@siemens.com>
>
> The OE patch lib uses quilt and so do many debian packages as well.
> Those two do not work well together, it is really hard to create a patch
> that will apply and not break what debian does later. debian is very
> pedantic about unexpected changes so even if patching works, building
> might not.
>
> Introduce a special-case where we detect quilt usage of a debian package
> and hook in there. Also make sure we are on top of debian so we do not
> risk breaking patches we inherit from there.
>
> If anyone ever managed to create a patch that works well in the face of
> two quilts, that might break with this change. You can set PATCHTOOL to
> "quilt" in your recipe to disable the magic.
>
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
> meta/classes/patch.bbclass | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
> index 3060755a5c..06f32a2197 100644
> --- a/meta/classes/patch.bbclass
> +++ b/meta/classes/patch.bbclass
> @@ -91,6 +91,28 @@ def should_apply(parm, d):
>
> should_apply[vardepsexclude] = "DATE SRCDATE"
>
> +def patch_do_debian_quilt(patchdir, d):
> + import oe.patch
> + class DummyPatchSet(oe.patch.PatchSet):
> + def Clean(self):
> + True
> +
> + def Import(self, patch, force):
> + os.putenv('QUILT_PATCHES', 'debian/patches')
> + # push all so we are on top of debian
> + pushed = False
> + if os.path.exists(os.path.join(self.dir, 'debian/patches/series')):
> + oe.patch.runcmd(["quilt", "push", "-a"], self.dir)
> + pushed = True
> + oe.patch.runcmd(["quilt", "import", "-f", os.path.join(d.getVar('WORKDIR'), os.path.basename(patch['file']))], self.dir)
> + if pushed:
> + oe.patch.runcmd(["quilt", "pop", "-a"], self.dir)
> +
> + def Push(self, force = False):
> + True
> +
> + return DummyPatchSet(patchdir, d)
> +
> python patch_do_patch() {
> import sys
>
> @@ -118,6 +140,12 @@ python patch_do_patch() {
>
> s = d.getVar('S')
>
> + debianformat = os.path.join(s, 'debian/source/format')
> + if os.path.exists(debianformat) and d.getVar('PATCHTOOL') != 'quilt':
> + with open(debianformat, 'r+') as f:
> + if f.readline() == '3.0 (quilt)\n':
> + cls = patch_do_debian_quilt
> +
> os.putenv('PATH', d.getVar('PATH'))
>
> # We must use one TMPDIR per process so that the "patch" processes
>
I'm not yet sure I understand the use case of this. Does it obsolete
something like
https://github.com/siemens/meta-iot2050/blob/master/recipes-security/openssl/openssl_1.1.1d.bb#L22
ie. the quilt import and push? Or where does it help?
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/2] patch: special-case quilt in debian
2020-07-29 14:06 ` Jan Kiszka
@ 2020-07-29 14:36 ` Henning Schild
2020-07-29 15:06 ` Jan Kiszka
0 siblings, 1 reply; 13+ messages in thread
From: Henning Schild @ 2020-07-29 14:36 UTC (permalink / raw)
To: Jan Kiszka; +Cc: isar-users
On Wed, 29 Jul 2020 16:06:03 +0200
Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 28.07.20 22:31, [ext] Henning Schild wrote:
> > From: Henning Schild <henning.schild@siemens.com>
> >
> > The OE patch lib uses quilt and so do many debian packages as well.
> > Those two do not work well together, it is really hard to create a
> > patch that will apply and not break what debian does later. debian
> > is very pedantic about unexpected changes so even if patching
> > works, building might not.
> >
> > Introduce a special-case where we detect quilt usage of a debian
> > package and hook in there. Also make sure we are on top of debian
> > so we do not risk breaking patches we inherit from there.
> >
> > If anyone ever managed to create a patch that works well in the
> > face of two quilts, that might break with this change. You can set
> > PATCHTOOL to "quilt" in your recipe to disable the magic.
> >
> > Signed-off-by: Henning Schild <henning.schild@siemens.com>
> > ---
> > meta/classes/patch.bbclass | 28 ++++++++++++++++++++++++++++
> > 1 file changed, 28 insertions(+)
> >
> > diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
> > index 3060755a5c..06f32a2197 100644
> > --- a/meta/classes/patch.bbclass
> > +++ b/meta/classes/patch.bbclass
> > @@ -91,6 +91,28 @@ def should_apply(parm, d):
> >
> > should_apply[vardepsexclude] = "DATE SRCDATE"
> >
> > +def patch_do_debian_quilt(patchdir, d):
> > + import oe.patch
> > + class DummyPatchSet(oe.patch.PatchSet):
> > + def Clean(self):
> > + True
> > +
> > + def Import(self, patch, force):
> > + os.putenv('QUILT_PATCHES', 'debian/patches')
> > + # push all so we are on top of debian
> > + pushed = False
> > + if os.path.exists(os.path.join(self.dir,
> > 'debian/patches/series')):
> > + oe.patch.runcmd(["quilt", "push", "-a"], self.dir)
> > + pushed = True
> > + oe.patch.runcmd(["quilt", "import", "-f",
> > os.path.join(d.getVar('WORKDIR'),
> > os.path.basename(patch['file']))], self.dir)
> > + if pushed:
> > + oe.patch.runcmd(["quilt", "pop", "-a"], self.dir)
> > +
> > + def Push(self, force = False):
> > + True
> > +
> > + return DummyPatchSet(patchdir, d)
> > +
> > python patch_do_patch() {
> > import sys
> >
> > @@ -118,6 +140,12 @@ python patch_do_patch() {
> >
> > s = d.getVar('S')
> >
> > + debianformat = os.path.join(s, 'debian/source/format')
> > + if os.path.exists(debianformat) and d.getVar('PATCHTOOL') !=
> > 'quilt':
> > + with open(debianformat, 'r+') as f:
> > + if f.readline() == '3.0 (quilt)\n':
> > + cls = patch_do_debian_quilt
> > +
> > os.putenv('PATH', d.getVar('PATH'))
> >
> > # We must use one TMPDIR per process so that the "patch"
> > processes
>
> I'm not yet sure I understand the use case of this. Does it obsolete
> something like
>
> https://github.com/siemens/meta-iot2050/blob/master/recipes-security/openssl/openssl_1.1.1d.bb#L22
>
> ie. the quilt import and push? Or where does it help?
With the proposed patch your example could drop all but
deb_add_changelog together with the apply=no.
Henning
> Jan
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/2] patch: special-case quilt in debian
2020-07-29 14:36 ` Henning Schild
@ 2020-07-29 15:06 ` Jan Kiszka
0 siblings, 0 replies; 13+ messages in thread
From: Jan Kiszka @ 2020-07-29 15:06 UTC (permalink / raw)
To: Henning Schild; +Cc: isar-users
On 29.07.20 16:36, Henning Schild wrote:
> On Wed, 29 Jul 2020 16:06:03 +0200
> Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
>> On 28.07.20 22:31, [ext] Henning Schild wrote:
>>> From: Henning Schild <henning.schild@siemens.com>
>>>
>>> The OE patch lib uses quilt and so do many debian packages as well.
>>> Those two do not work well together, it is really hard to create a
>>> patch that will apply and not break what debian does later. debian
>>> is very pedantic about unexpected changes so even if patching
>>> works, building might not.
>>>
>>> Introduce a special-case where we detect quilt usage of a debian
>>> package and hook in there. Also make sure we are on top of debian
>>> so we do not risk breaking patches we inherit from there.
>>>
>>> If anyone ever managed to create a patch that works well in the
>>> face of two quilts, that might break with this change. You can set
>>> PATCHTOOL to "quilt" in your recipe to disable the magic.
>>>
>>> Signed-off-by: Henning Schild <henning.schild@siemens.com>
>>> ---
>>> meta/classes/patch.bbclass | 28 ++++++++++++++++++++++++++++
>>> 1 file changed, 28 insertions(+)
>>>
>>> diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
>>> index 3060755a5c..06f32a2197 100644
>>> --- a/meta/classes/patch.bbclass
>>> +++ b/meta/classes/patch.bbclass
>>> @@ -91,6 +91,28 @@ def should_apply(parm, d):
>>>
>>> should_apply[vardepsexclude] = "DATE SRCDATE"
>>>
>>> +def patch_do_debian_quilt(patchdir, d):
>>> + import oe.patch
>>> + class DummyPatchSet(oe.patch.PatchSet):
>>> + def Clean(self):
>>> + True
>>> +
>>> + def Import(self, patch, force):
>>> + os.putenv('QUILT_PATCHES', 'debian/patches')
>>> + # push all so we are on top of debian
>>> + pushed = False
>>> + if os.path.exists(os.path.join(self.dir,
>>> 'debian/patches/series')):
>>> + oe.patch.runcmd(["quilt", "push", "-a"], self.dir)
>>> + pushed = True
>>> + oe.patch.runcmd(["quilt", "import", "-f",
>>> os.path.join(d.getVar('WORKDIR'),
>>> os.path.basename(patch['file']))], self.dir)
>>> + if pushed:
>>> + oe.patch.runcmd(["quilt", "pop", "-a"], self.dir)
>>> +
>>> + def Push(self, force = False):
>>> + True
>>> +
>>> + return DummyPatchSet(patchdir, d)
>>> +
>>> python patch_do_patch() {
>>> import sys
>>>
>>> @@ -118,6 +140,12 @@ python patch_do_patch() {
>>>
>>> s = d.getVar('S')
>>>
>>> + debianformat = os.path.join(s, 'debian/source/format')
>>> + if os.path.exists(debianformat) and d.getVar('PATCHTOOL') !=
>>> 'quilt':
>>> + with open(debianformat, 'r+') as f:
>>> + if f.readline() == '3.0 (quilt)\n':
>>> + cls = patch_do_debian_quilt
>>> +
>>> os.putenv('PATH', d.getVar('PATH'))
>>>
>>> # We must use one TMPDIR per process so that the "patch"
>>> processes
>>
>> I'm not yet sure I understand the use case of this. Does it obsolete
>> something like
>>
>> https://github.com/siemens/meta-iot2050/blob/master/recipes-security/openssl/openssl_1.1.1d.bb#L22
>>
>> ie. the quilt import and push? Or where does it help?
>
> With the proposed patch your example could drop all but
> deb_add_changelog together with the apply=no.
>
That would be nice.
But it seems for your other reply you need another round to address
apt:// as well. Now you know where to find a test case ;).
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] patch: special-case quilt in debian
2020-07-28 20:31 [PATCH 1/2] patch: special-case quilt in debian Henning Schild
` (2 preceding siblings ...)
2020-07-29 14:06 ` Jan Kiszka
@ 2020-07-29 14:39 ` Henning Schild
2020-07-29 15:08 ` Henning Schild
4 siblings, 0 replies; 13+ messages in thread
From: Henning Schild @ 2020-07-29 14:39 UTC (permalink / raw)
To: isar-users; +Cc: Schmidt, Adriaan (CT RDA IOT SES-DE)
On Tue, 28 Jul 2020 22:31:52 +0200
Henning Schild <henning.schild@siemens.com> wrote:
> From: Henning Schild <henning.schild@siemens.com>
>
> The OE patch lib uses quilt and so do many debian packages as well.
> Those two do not work well together, it is really hard to create a
> patch that will apply and not break what debian does later. debian is
> very pedantic about unexpected changes so even if patching works,
> building might not.
>
> Introduce a special-case where we detect quilt usage of a debian
> package and hook in there. Also make sure we are on top of debian so
> we do not risk breaking patches we inherit from there.
>
> If anyone ever managed to create a patch that works well in the face
> of two quilts, that might break with this change. You can set
> PATCHTOOL to "quilt" in your recipe to disable the magic.
>
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
> meta/classes/patch.bbclass | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
> index 3060755a5c..06f32a2197 100644
> --- a/meta/classes/patch.bbclass
> +++ b/meta/classes/patch.bbclass
> @@ -91,6 +91,28 @@ def should_apply(parm, d):
>
> should_apply[vardepsexclude] = "DATE SRCDATE"
>
> +def patch_do_debian_quilt(patchdir, d):
> + import oe.patch
> + class DummyPatchSet(oe.patch.PatchSet):
> + def Clean(self):
> + True
> +
> + def Import(self, patch, force):
> + os.putenv('QUILT_PATCHES', 'debian/patches')
> + # push all so we are on top of debian
> + pushed = False
> + if os.path.exists(os.path.join(self.dir,
> 'debian/patches/series')):
> + oe.patch.runcmd(["quilt", "push", "-a"], self.dir)
We might actually find ourselves in a situation where the debian quilt
series is already pushed. That happens when using an "apt://" SRC_URI.
While i already have a mitigation patch i think the whole thing needs
more discussion and i will send another mail.
Henning
> + pushed = True
> + oe.patch.runcmd(["quilt", "import", "-f",
> os.path.join(d.getVar('WORKDIR'), os.path.basename(patch['file']))],
> self.dir)
> + if pushed:
> + oe.patch.runcmd(["quilt", "pop", "-a"], self.dir)
> +
> + def Push(self, force = False):
> + True
> +
> + return DummyPatchSet(patchdir, d)
> +
> python patch_do_patch() {
> import sys
>
> @@ -118,6 +140,12 @@ python patch_do_patch() {
>
> s = d.getVar('S')
>
> + debianformat = os.path.join(s, 'debian/source/format')
> + if os.path.exists(debianformat) and d.getVar('PATCHTOOL') !=
> 'quilt':
> + with open(debianformat, 'r+') as f:
> + if f.readline() == '3.0 (quilt)\n':
> + cls = patch_do_debian_quilt
> +
> os.putenv('PATH', d.getVar('PATH'))
>
> # We must use one TMPDIR per process so that the "patch"
> processes
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/2] patch: special-case quilt in debian
2020-07-28 20:31 [PATCH 1/2] patch: special-case quilt in debian Henning Schild
` (3 preceding siblings ...)
2020-07-29 14:39 ` Henning Schild
@ 2020-07-29 15:08 ` Henning Schild
2020-07-29 15:31 ` Jan Kiszka
4 siblings, 1 reply; 13+ messages in thread
From: Henning Schild @ 2020-07-29 15:08 UTC (permalink / raw)
To: isar-users
Cc: Schmidt, Adriaan (CT RDA IOT SES-DE), Kiszka, Jan (CT RDA IOT SES-DE)
Hi,
working on that and getting some feedback from Adriaan the whole issue
is harder so solve than i initially thought.
Having Isar and Debian use quilt is hard if not impossible to get
right. Here i propose to detect Debian-quilt usage and hook into it.
While that is a nice idea and actually does work, it has one major
problem. We will not be able to patch "debian/" with it, which some
patches might want to do.
Now why cant we use two quilts (in some cases):
1. in addition to the "patches" dir quilt creates a dir with its state,
and it finds that directory on more levels than it finds "patches".
That actually means that the first call to quilt will potentially break
the other caller, because it will find the wrong state dir and work on
the wrong patches. We do not even know which instance comes first, if
we fetch with "apt://" debian comes first, otherwise isar comes first.
2. isar never should come first because patching before debian applies
its patches means potentially breaking the debian-series, isar needs to
patch last ... maybe unless it patches "debian/" ...
So i suggest to stick to what i propose with that patch and add a
special patching method just for "debian/" and just for the
"debian-uses-quilt" case. The user would have to mark the patches
somehow, maybe with a naming convention or a SRC_URI paramter. Now we
need a special patch method to apply those outside of the debian-quilt
universe.
SRC_URI+="foo.patch;apply-with-isar=1" What do you think?
The two quilts getting into each others ways can potentially also be
avoided if we prepare a QUILTRC for the Isar side. In that case we can
maybe still use two quilts and give users an option to hook individual
patches into debian instead of isar. Because isar should not actually
patch before debian does it ... In fact if debian uses quilt any change
that debian did not apply itself will trigger build errors, only
patches to "debian/" will be ok.
Henning
On Tue, 28 Jul 2020 22:31:52 +0200
Henning Schild <henning.schild@siemens.com> wrote:
> From: Henning Schild <henning.schild@siemens.com>
>
> The OE patch lib uses quilt and so do many debian packages as well.
> Those two do not work well together, it is really hard to create a
> patch that will apply and not break what debian does later. debian is
> very pedantic about unexpected changes so even if patching works,
> building might not.
>
> Introduce a special-case where we detect quilt usage of a debian
> package and hook in there. Also make sure we are on top of debian so
> we do not risk breaking patches we inherit from there.
>
> If anyone ever managed to create a patch that works well in the face
> of two quilts, that might break with this change. You can set
> PATCHTOOL to "quilt" in your recipe to disable the magic.
>
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
> meta/classes/patch.bbclass | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
> index 3060755a5c..06f32a2197 100644
> --- a/meta/classes/patch.bbclass
> +++ b/meta/classes/patch.bbclass
> @@ -91,6 +91,28 @@ def should_apply(parm, d):
>
> should_apply[vardepsexclude] = "DATE SRCDATE"
>
> +def patch_do_debian_quilt(patchdir, d):
> + import oe.patch
> + class DummyPatchSet(oe.patch.PatchSet):
> + def Clean(self):
> + True
> +
> + def Import(self, patch, force):
> + os.putenv('QUILT_PATCHES', 'debian/patches')
> + # push all so we are on top of debian
> + pushed = False
> + if os.path.exists(os.path.join(self.dir,
> 'debian/patches/series')):
> + oe.patch.runcmd(["quilt", "push", "-a"], self.dir)
> + pushed = True
> + oe.patch.runcmd(["quilt", "import", "-f",
> os.path.join(d.getVar('WORKDIR'), os.path.basename(patch['file']))],
> self.dir)
> + if pushed:
> + oe.patch.runcmd(["quilt", "pop", "-a"], self.dir)
> +
> + def Push(self, force = False):
> + True
> +
> + return DummyPatchSet(patchdir, d)
> +
> python patch_do_patch() {
> import sys
>
> @@ -118,6 +140,12 @@ python patch_do_patch() {
>
> s = d.getVar('S')
>
> + debianformat = os.path.join(s, 'debian/source/format')
> + if os.path.exists(debianformat) and d.getVar('PATCHTOOL') !=
> 'quilt':
> + with open(debianformat, 'r+') as f:
> + if f.readline() == '3.0 (quilt)\n':
> + cls = patch_do_debian_quilt
> +
> os.putenv('PATH', d.getVar('PATH'))
>
> # We must use one TMPDIR per process so that the "patch"
> processes
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/2] patch: special-case quilt in debian
2020-07-29 15:08 ` Henning Schild
@ 2020-07-29 15:31 ` Jan Kiszka
2020-07-29 19:48 ` Henning Schild
0 siblings, 1 reply; 13+ messages in thread
From: Jan Kiszka @ 2020-07-29 15:31 UTC (permalink / raw)
To: Henning Schild, isar-users; +Cc: Schmidt, Adriaan (CT RDA IOT SES-DE)
On 29.07.20 17:08, Henning Schild wrote:
> Hi,
>
> working on that and getting some feedback from Adriaan the whole issue
> is harder so solve than i initially thought.
>
> Having Isar and Debian use quilt is hard if not impossible to get
> right. Here i propose to detect Debian-quilt usage and hook into it.
> While that is a nice idea and actually does work, it has one major
> problem. We will not be able to patch "debian/" with it, which some
> patches might want to do.
>
> Now why cant we use two quilts (in some cases):
> 1. in addition to the "patches" dir quilt creates a dir with its state,
> and it finds that directory on more levels than it finds "patches".
> That actually means that the first call to quilt will potentially break
> the other caller, because it will find the wrong state dir and work on
> the wrong patches. We do not even know which instance comes first, if
> we fetch with "apt://" debian comes first, otherwise isar comes first.
> 2. isar never should come first because patching before debian applies
> its patches means potentially breaking the debian-series, isar needs to
> patch last ... maybe unless it patches "debian/" ...
>
> So i suggest to stick to what i propose with that patch and add a
> special patching method just for "debian/" and just for the
> "debian-uses-quilt" case. The user would have to mark the patches
> somehow, maybe with a naming convention or a SRC_URI paramter. Now we
> need a special patch method to apply those outside of the debian-quilt
> universe.
> SRC_URI+="foo.patch;apply-with-isar=1" What do you think?
If we are to add a special flag, I would say "debian-patch". Semantic
would be "inject into the debian package patch queue (and, implicitly,
"apply=no" for Isar).
>
> The two quilts getting into each others ways can potentially also be
> avoided if we prepare a QUILTRC for the Isar side. In that case we can
> maybe still use two quilts and give users an option to hook individual
> patches into debian instead of isar. Because isar should not actually
> patch before debian does it ... In fact if debian uses quilt any change
> that debian did not apply itself will trigger build errors, only
> patches to "debian/" will be ok.
Indeed. We likely need to split patches into those that target the
source and those that address debian/. The former should be hooked into
debian/, the latter could be applied the classic way.
Jan
>
> Henning
>
> On Tue, 28 Jul 2020 22:31:52 +0200
> Henning Schild <henning.schild@siemens.com> wrote:
>
>> From: Henning Schild <henning.schild@siemens.com>
>>
>> The OE patch lib uses quilt and so do many debian packages as well.
>> Those two do not work well together, it is really hard to create a
>> patch that will apply and not break what debian does later. debian is
>> very pedantic about unexpected changes so even if patching works,
>> building might not.
>>
>> Introduce a special-case where we detect quilt usage of a debian
>> package and hook in there. Also make sure we are on top of debian so
>> we do not risk breaking patches we inherit from there.
>>
>> If anyone ever managed to create a patch that works well in the face
>> of two quilts, that might break with this change. You can set
>> PATCHTOOL to "quilt" in your recipe to disable the magic.
>>
>> Signed-off-by: Henning Schild <henning.schild@siemens.com>
>> ---
>> meta/classes/patch.bbclass | 28 ++++++++++++++++++++++++++++
>> 1 file changed, 28 insertions(+)
>>
>> diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
>> index 3060755a5c..06f32a2197 100644
>> --- a/meta/classes/patch.bbclass
>> +++ b/meta/classes/patch.bbclass
>> @@ -91,6 +91,28 @@ def should_apply(parm, d):
>>
>> should_apply[vardepsexclude] = "DATE SRCDATE"
>>
>> +def patch_do_debian_quilt(patchdir, d):
>> + import oe.patch
>> + class DummyPatchSet(oe.patch.PatchSet):
>> + def Clean(self):
>> + True
>> +
>> + def Import(self, patch, force):
>> + os.putenv('QUILT_PATCHES', 'debian/patches')
>> + # push all so we are on top of debian
>> + pushed = False
>> + if os.path.exists(os.path.join(self.dir,
>> 'debian/patches/series')):
>> + oe.patch.runcmd(["quilt", "push", "-a"], self.dir)
>> + pushed = True
>> + oe.patch.runcmd(["quilt", "import", "-f",
>> os.path.join(d.getVar('WORKDIR'), os.path.basename(patch['file']))],
>> self.dir)
>> + if pushed:
>> + oe.patch.runcmd(["quilt", "pop", "-a"], self.dir)
>> +
>> + def Push(self, force = False):
>> + True
>> +
>> + return DummyPatchSet(patchdir, d)
>> +
>> python patch_do_patch() {
>> import sys
>>
>> @@ -118,6 +140,12 @@ python patch_do_patch() {
>>
>> s = d.getVar('S')
>>
>> + debianformat = os.path.join(s, 'debian/source/format')
>> + if os.path.exists(debianformat) and d.getVar('PATCHTOOL') !=
>> 'quilt':
>> + with open(debianformat, 'r+') as f:
>> + if f.readline() == '3.0 (quilt)\n':
>> + cls = patch_do_debian_quilt
>> +
>> os.putenv('PATH', d.getVar('PATH'))
>>
>> # We must use one TMPDIR per process so that the "patch"
>> processes
>
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/2] patch: special-case quilt in debian
2020-07-29 15:31 ` Jan Kiszka
@ 2020-07-29 19:48 ` Henning Schild
2021-02-08 15:50 ` Anton Mikanovich
0 siblings, 1 reply; 13+ messages in thread
From: Henning Schild @ 2020-07-29 19:48 UTC (permalink / raw)
To: Jan Kiszka; +Cc: isar-users, Schmidt, Adriaan (CT RDA IOT SES-DE)
On Wed, 29 Jul 2020 17:31:34 +0200
Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 29.07.20 17:08, Henning Schild wrote:
> > Hi,
> >
> > working on that and getting some feedback from Adriaan the whole
> > issue is harder so solve than i initially thought.
> >
> > Having Isar and Debian use quilt is hard if not impossible to get
> > right. Here i propose to detect Debian-quilt usage and hook into it.
> > While that is a nice idea and actually does work, it has one major
> > problem. We will not be able to patch "debian/" with it, which some
> > patches might want to do.
> >
> > Now why cant we use two quilts (in some cases):
> > 1. in addition to the "patches" dir quilt creates a dir with its
> > state, and it finds that directory on more levels than it finds
> > "patches". That actually means that the first call to quilt will
> > potentially break the other caller, because it will find the wrong
> > state dir and work on the wrong patches. We do not even know which
> > instance comes first, if we fetch with "apt://" debian comes first,
> > otherwise isar comes first. 2. isar never should come first because
> > patching before debian applies its patches means potentially
> > breaking the debian-series, isar needs to patch last ... maybe
> > unless it patches "debian/" ...
> >
> > So i suggest to stick to what i propose with that patch and add a
> > special patching method just for "debian/" and just for the
> > "debian-uses-quilt" case. The user would have to mark the patches
> > somehow, maybe with a naming convention or a SRC_URI paramter. Now
> > we need a special patch method to apply those outside of the
> > debian-quilt universe.
> > SRC_URI+="foo.patch;apply-with-isar=1" What do you think?
>
> If we are to add a special flag, I would say "debian-patch". Semantic
> would be "inject into the debian package patch queue (and,
> implicitly, "apply=no" for Isar).
I am not sure about the user interface, when patching the source we
should magically hook into debians quilt by default, because we need to
be after debian. That breaks legacy recipes that patch "debian/" ...
And "debian-patch" could also be misunderstood as patching "debian/".
Let me come up with an implementation and discuss those details later.
> >
> > The two quilts getting into each others ways can potentially also be
> > avoided if we prepare a QUILTRC for the Isar side. In that case we
> > can maybe still use two quilts and give users an option to hook
> > individual patches into debian instead of isar. Because isar should
> > not actually patch before debian does it ... In fact if debian uses
> > quilt any change that debian did not apply itself will trigger
> > build errors, only patches to "debian/" will be ok.
>
> Indeed. We likely need to split patches into those that target the
> source and those that address debian/. The former should be hooked
> into debian/, the latter could be applied the classic way.
I am afraid the "classic way" wont work, unless we manage to convince
quilt to work on S/patches and do not fall for S/debian/patches in case
of "apt://". I think it is not only "apt://" but might also affect
partial rebuilds where somehow debians quilt already ran before isars
quilt enters the stage. Let us see.
Henning
> Jan
>
> >
> > Henning
> >
> > On Tue, 28 Jul 2020 22:31:52 +0200
> > Henning Schild <henning.schild@siemens.com> wrote:
> >
> >> From: Henning Schild <henning.schild@siemens.com>
> >>
> >> The OE patch lib uses quilt and so do many debian packages as well.
> >> Those two do not work well together, it is really hard to create a
> >> patch that will apply and not break what debian does later. debian
> >> is very pedantic about unexpected changes so even if patching
> >> works, building might not.
> >>
> >> Introduce a special-case where we detect quilt usage of a debian
> >> package and hook in there. Also make sure we are on top of debian
> >> so we do not risk breaking patches we inherit from there.
> >>
> >> If anyone ever managed to create a patch that works well in the
> >> face of two quilts, that might break with this change. You can set
> >> PATCHTOOL to "quilt" in your recipe to disable the magic.
> >>
> >> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> >> ---
> >> meta/classes/patch.bbclass | 28 ++++++++++++++++++++++++++++
> >> 1 file changed, 28 insertions(+)
> >>
> >> diff --git a/meta/classes/patch.bbclass
> >> b/meta/classes/patch.bbclass index 3060755a5c..06f32a2197 100644
> >> --- a/meta/classes/patch.bbclass
> >> +++ b/meta/classes/patch.bbclass
> >> @@ -91,6 +91,28 @@ def should_apply(parm, d):
> >>
> >> should_apply[vardepsexclude] = "DATE SRCDATE"
> >>
> >> +def patch_do_debian_quilt(patchdir, d):
> >> + import oe.patch
> >> + class DummyPatchSet(oe.patch.PatchSet):
> >> + def Clean(self):
> >> + True
> >> +
> >> + def Import(self, patch, force):
> >> + os.putenv('QUILT_PATCHES', 'debian/patches')
> >> + # push all so we are on top of debian
> >> + pushed = False
> >> + if os.path.exists(os.path.join(self.dir,
> >> 'debian/patches/series')):
> >> + oe.patch.runcmd(["quilt", "push", "-a"], self.dir)
> >> + pushed = True
> >> + oe.patch.runcmd(["quilt", "import", "-f",
> >> os.path.join(d.getVar('WORKDIR'),
> >> os.path.basename(patch['file']))], self.dir)
> >> + if pushed:
> >> + oe.patch.runcmd(["quilt", "pop", "-a"], self.dir)
> >> +
> >> + def Push(self, force = False):
> >> + True
> >> +
> >> + return DummyPatchSet(patchdir, d)
> >> +
> >> python patch_do_patch() {
> >> import sys
> >>
> >> @@ -118,6 +140,12 @@ python patch_do_patch() {
> >>
> >> s = d.getVar('S')
> >>
> >> + debianformat = os.path.join(s, 'debian/source/format')
> >> + if os.path.exists(debianformat) and d.getVar('PATCHTOOL') !=
> >> 'quilt':
> >> + with open(debianformat, 'r+') as f:
> >> + if f.readline() == '3.0 (quilt)\n':
> >> + cls = patch_do_debian_quilt
> >> +
> >> os.putenv('PATH', d.getVar('PATH'))
> >>
> >> # We must use one TMPDIR per process so that the "patch"
> >> processes
> >
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/2] patch: special-case quilt in debian
2020-07-29 19:48 ` Henning Schild
@ 2021-02-08 15:50 ` Anton Mikanovich
2021-02-08 16:18 ` Henning Schild
0 siblings, 1 reply; 13+ messages in thread
From: Anton Mikanovich @ 2021-02-08 15:50 UTC (permalink / raw)
To: Henning Schild, Jan Kiszka
Cc: isar-users, Schmidt, Adriaan (CT RDA IOT SES-DE)
Hello, that is the status of patch rebuild?
29.07.2020 22:48, Henning Schild wrote:
> On Wed, 29 Jul 2020 17:31:34 +0200
> Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
>> On 29.07.20 17:08, Henning Schild wrote:
>>> Hi,
>>>
>>> working on that and getting some feedback from Adriaan the whole
>>> issue is harder so solve than i initially thought.
>>>
>>> Having Isar and Debian use quilt is hard if not impossible to get
>>> right. Here i propose to detect Debian-quilt usage and hook into it.
>>> While that is a nice idea and actually does work, it has one major
>>> problem. We will not be able to patch "debian/" with it, which some
>>> patches might want to do.
>>>
>>> Now why cant we use two quilts (in some cases):
>>> 1. in addition to the "patches" dir quilt creates a dir with its
>>> state, and it finds that directory on more levels than it finds
>>> "patches". That actually means that the first call to quilt will
>>> potentially break the other caller, because it will find the wrong
>>> state dir and work on the wrong patches. We do not even know which
>>> instance comes first, if we fetch with "apt://" debian comes first,
>>> otherwise isar comes first. 2. isar never should come first because
>>> patching before debian applies its patches means potentially
>>> breaking the debian-series, isar needs to patch last ... maybe
>>> unless it patches "debian/" ...
>>>
>>> So i suggest to stick to what i propose with that patch and add a
>>> special patching method just for "debian/" and just for the
>>> "debian-uses-quilt" case. The user would have to mark the patches
>>> somehow, maybe with a naming convention or a SRC_URI paramter. Now
>>> we need a special patch method to apply those outside of the
>>> debian-quilt universe.
>>> SRC_URI+="foo.patch;apply-with-isar=1" What do you think?
>> If we are to add a special flag, I would say "debian-patch". Semantic
>> would be "inject into the debian package patch queue (and,
>> implicitly, "apply=no" for Isar).
> I am not sure about the user interface, when patching the source we
> should magically hook into debians quilt by default, because we need to
> be after debian. That breaks legacy recipes that patch "debian/" ...
>
> And "debian-patch" could also be misunderstood as patching "debian/".
> Let me come up with an implementation and discuss those details later.
>
>>> The two quilts getting into each others ways can potentially also be
>>> avoided if we prepare a QUILTRC for the Isar side. In that case we
>>> can maybe still use two quilts and give users an option to hook
>>> individual patches into debian instead of isar. Because isar should
>>> not actually patch before debian does it ... In fact if debian uses
>>> quilt any change that debian did not apply itself will trigger
>>> build errors, only patches to "debian/" will be ok.
>> Indeed. We likely need to split patches into those that target the
>> source and those that address debian/. The former should be hooked
>> into debian/, the latter could be applied the classic way.
> I am afraid the "classic way" wont work, unless we manage to convince
> quilt to work on S/patches and do not fall for S/debian/patches in case
> of "apt://". I think it is not only "apt://" but might also affect
> partial rebuilds where somehow debians quilt already ran before isars
> quilt enters the stage. Let us see.
>
> Henning
>
>> Jan
>>
>>> Henning
>>>
>>> On Tue, 28 Jul 2020 22:31:52 +0200
>>> Henning Schild <henning.schild@siemens.com> wrote:
>>>
>>>> From: Henning Schild <henning.schild@siemens.com>
>>>>
>>>> The OE patch lib uses quilt and so do many debian packages as well.
>>>> Those two do not work well together, it is really hard to create a
>>>> patch that will apply and not break what debian does later. debian
>>>> is very pedantic about unexpected changes so even if patching
>>>> works, building might not.
>>>>
>>>> Introduce a special-case where we detect quilt usage of a debian
>>>> package and hook in there. Also make sure we are on top of debian
>>>> so we do not risk breaking patches we inherit from there.
>>>>
>>>> If anyone ever managed to create a patch that works well in the
>>>> face of two quilts, that might break with this change. You can set
>>>> PATCHTOOL to "quilt" in your recipe to disable the magic.
>>>>
>>>> Signed-off-by: Henning Schild <henning.schild@siemens.com>
>>>> ---
>>>> meta/classes/patch.bbclass | 28 ++++++++++++++++++++++++++++
>>>> 1 file changed, 28 insertions(+)
>>>>
>>>> diff --git a/meta/classes/patch.bbclass
>>>> b/meta/classes/patch.bbclass index 3060755a5c..06f32a2197 100644
>>>> --- a/meta/classes/patch.bbclass
>>>> +++ b/meta/classes/patch.bbclass
>>>> @@ -91,6 +91,28 @@ def should_apply(parm, d):
>>>>
>>>> should_apply[vardepsexclude] = "DATE SRCDATE"
>>>>
>>>> +def patch_do_debian_quilt(patchdir, d):
>>>> + import oe.patch
>>>> + class DummyPatchSet(oe.patch.PatchSet):
>>>> + def Clean(self):
>>>> + True
>>>> +
>>>> + def Import(self, patch, force):
>>>> + os.putenv('QUILT_PATCHES', 'debian/patches')
>>>> + # push all so we are on top of debian
>>>> + pushed = False
>>>> + if os.path.exists(os.path.join(self.dir,
>>>> 'debian/patches/series')):
>>>> + oe.patch.runcmd(["quilt", "push", "-a"], self.dir)
>>>> + pushed = True
>>>> + oe.patch.runcmd(["quilt", "import", "-f",
>>>> os.path.join(d.getVar('WORKDIR'),
>>>> os.path.basename(patch['file']))], self.dir)
>>>> + if pushed:
>>>> + oe.patch.runcmd(["quilt", "pop", "-a"], self.dir)
>>>> +
>>>> + def Push(self, force = False):
>>>> + True
>>>> +
>>>> + return DummyPatchSet(patchdir, d)
>>>> +
>>>> python patch_do_patch() {
>>>> import sys
>>>>
>>>> @@ -118,6 +140,12 @@ python patch_do_patch() {
>>>>
>>>> s = d.getVar('S')
>>>>
>>>> + debianformat = os.path.join(s, 'debian/source/format')
>>>> + if os.path.exists(debianformat) and d.getVar('PATCHTOOL') !=
>>>> 'quilt':
>>>> + with open(debianformat, 'r+') as f:
>>>> + if f.readline() == '3.0 (quilt)\n':
>>>> + cls = patch_do_debian_quilt
>>>> +
>>>> os.putenv('PATH', d.getVar('PATH'))
>>>>
>>>> # We must use one TMPDIR per process so that the "patch"
>>>> processes
>>>
--
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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/2] patch: special-case quilt in debian
2021-02-08 15:50 ` Anton Mikanovich
@ 2021-02-08 16:18 ` Henning Schild
0 siblings, 0 replies; 13+ messages in thread
From: Henning Schild @ 2021-02-08 16:18 UTC (permalink / raw)
To: Anton Mikanovich
Cc: Jan Kiszka, isar-users, Schmidt, Adriaan (CT RDA IOT SES-DE)
Am Mon, 8 Feb 2021 18:50:00 +0300
schrieb Anton Mikanovich <amikan@ilbers.de>:
> Hello, that is the status of patch rebuild?
This one stays open. The other one - the git one - is ready to go.
Henning
>
> 29.07.2020 22:48, Henning Schild wrote:
> > On Wed, 29 Jul 2020 17:31:34 +0200
> > Jan Kiszka <jan.kiszka@siemens.com> wrote:
> >
> >> On 29.07.20 17:08, Henning Schild wrote:
> >>> Hi,
> >>>
> >>> working on that and getting some feedback from Adriaan the whole
> >>> issue is harder so solve than i initially thought.
> >>>
> >>> Having Isar and Debian use quilt is hard if not impossible to get
> >>> right. Here i propose to detect Debian-quilt usage and hook into
> >>> it. While that is a nice idea and actually does work, it has one
> >>> major problem. We will not be able to patch "debian/" with it,
> >>> which some patches might want to do.
> >>>
> >>> Now why cant we use two quilts (in some cases):
> >>> 1. in addition to the "patches" dir quilt creates a dir with its
> >>> state, and it finds that directory on more levels than it finds
> >>> "patches". That actually means that the first call to quilt will
> >>> potentially break the other caller, because it will find the wrong
> >>> state dir and work on the wrong patches. We do not even know which
> >>> instance comes first, if we fetch with "apt://" debian comes
> >>> first, otherwise isar comes first. 2. isar never should come
> >>> first because patching before debian applies its patches means
> >>> potentially breaking the debian-series, isar needs to patch last
> >>> ... maybe unless it patches "debian/" ...
> >>>
> >>> So i suggest to stick to what i propose with that patch and add a
> >>> special patching method just for "debian/" and just for the
> >>> "debian-uses-quilt" case. The user would have to mark the patches
> >>> somehow, maybe with a naming convention or a SRC_URI paramter. Now
> >>> we need a special patch method to apply those outside of the
> >>> debian-quilt universe.
> >>> SRC_URI+="foo.patch;apply-with-isar=1" What do you think?
> >> If we are to add a special flag, I would say "debian-patch".
> >> Semantic would be "inject into the debian package patch queue (and,
> >> implicitly, "apply=no" for Isar).
> > I am not sure about the user interface, when patching the source we
> > should magically hook into debians quilt by default, because we
> > need to be after debian. That breaks legacy recipes that patch
> > "debian/" ...
> >
> > And "debian-patch" could also be misunderstood as patching
> > "debian/". Let me come up with an implementation and discuss those
> > details later.
> >>> The two quilts getting into each others ways can potentially also
> >>> be avoided if we prepare a QUILTRC for the Isar side. In that
> >>> case we can maybe still use two quilts and give users an option
> >>> to hook individual patches into debian instead of isar. Because
> >>> isar should not actually patch before debian does it ... In fact
> >>> if debian uses quilt any change that debian did not apply itself
> >>> will trigger build errors, only patches to "debian/" will be ok.
> >> Indeed. We likely need to split patches into those that target the
> >> source and those that address debian/. The former should be hooked
> >> into debian/, the latter could be applied the classic way.
> > I am afraid the "classic way" wont work, unless we manage to
> > convince quilt to work on S/patches and do not fall for
> > S/debian/patches in case of "apt://". I think it is not only
> > "apt://" but might also affect partial rebuilds where somehow
> > debians quilt already ran before isars quilt enters the stage. Let
> > us see.
> >
> > Henning
> >
> >> Jan
> >>
> >>> Henning
> >>>
> >>> On Tue, 28 Jul 2020 22:31:52 +0200
> >>> Henning Schild <henning.schild@siemens.com> wrote:
> >>>
> >>>> From: Henning Schild <henning.schild@siemens.com>
> >>>>
> >>>> The OE patch lib uses quilt and so do many debian packages as
> >>>> well. Those two do not work well together, it is really hard to
> >>>> create a patch that will apply and not break what debian does
> >>>> later. debian is very pedantic about unexpected changes so even
> >>>> if patching works, building might not.
> >>>>
> >>>> Introduce a special-case where we detect quilt usage of a debian
> >>>> package and hook in there. Also make sure we are on top of debian
> >>>> so we do not risk breaking patches we inherit from there.
> >>>>
> >>>> If anyone ever managed to create a patch that works well in the
> >>>> face of two quilts, that might break with this change. You can
> >>>> set PATCHTOOL to "quilt" in your recipe to disable the magic.
> >>>>
> >>>> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> >>>> ---
> >>>> meta/classes/patch.bbclass | 28 ++++++++++++++++++++++++++++
> >>>> 1 file changed, 28 insertions(+)
> >>>>
> >>>> diff --git a/meta/classes/patch.bbclass
> >>>> b/meta/classes/patch.bbclass index 3060755a5c..06f32a2197 100644
> >>>> --- a/meta/classes/patch.bbclass
> >>>> +++ b/meta/classes/patch.bbclass
> >>>> @@ -91,6 +91,28 @@ def should_apply(parm, d):
> >>>>
> >>>> should_apply[vardepsexclude] = "DATE SRCDATE"
> >>>>
> >>>> +def patch_do_debian_quilt(patchdir, d):
> >>>> + import oe.patch
> >>>> + class DummyPatchSet(oe.patch.PatchSet):
> >>>> + def Clean(self):
> >>>> + True
> >>>> +
> >>>> + def Import(self, patch, force):
> >>>> + os.putenv('QUILT_PATCHES', 'debian/patches')
> >>>> + # push all so we are on top of debian
> >>>> + pushed = False
> >>>> + if os.path.exists(os.path.join(self.dir,
> >>>> 'debian/patches/series')):
> >>>> + oe.patch.runcmd(["quilt", "push", "-a"],
> >>>> self.dir)
> >>>> + pushed = True
> >>>> + oe.patch.runcmd(["quilt", "import", "-f",
> >>>> os.path.join(d.getVar('WORKDIR'),
> >>>> os.path.basename(patch['file']))], self.dir)
> >>>> + if pushed:
> >>>> + oe.patch.runcmd(["quilt", "pop", "-a"],
> >>>> self.dir) +
> >>>> + def Push(self, force = False):
> >>>> + True
> >>>> +
> >>>> + return DummyPatchSet(patchdir, d)
> >>>> +
> >>>> python patch_do_patch() {
> >>>> import sys
> >>>>
> >>>> @@ -118,6 +140,12 @@ python patch_do_patch() {
> >>>>
> >>>> s = d.getVar('S')
> >>>>
> >>>> + debianformat = os.path.join(s, 'debian/source/format')
> >>>> + if os.path.exists(debianformat) and d.getVar('PATCHTOOL') !=
> >>>> 'quilt':
> >>>> + with open(debianformat, 'r+') as f:
> >>>> + if f.readline() == '3.0 (quilt)\n':
> >>>> + cls = patch_do_debian_quilt
> >>>> +
> >>>> os.putenv('PATH', d.getVar('PATH'))
> >>>>
> >>>> # We must use one TMPDIR per process so that the "patch"
> >>>> processes
> >>>
>
^ permalink raw reply [flat|nested] 13+ messages in thread