Re: [PATCH 1/2] patch: special-case quilt in debian

[Henning Schild <henning.schild@siemens.com>]

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