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

[Anton Mikanovich <amikan@ilbers.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