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

[Jan Kiszka <jan.kiszka@siemens.com>]

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