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

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

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