[PATCH] patch.py: Use git format-patch with --no-signature --no-numbered params

[PATCH] patch.py: Use git format-patch with --no-signature --no-numbered params

[Martin Jansa]

* --no-signature saves unnecessary .patch modifications when executed on
  host with different git version
* --no-numbered saves unnecessary .patch modifications when number of the
  applied patches is changed (the number is still in the filename so the
  order how they should be applied is still preserved)
* both options exist for very long time, I've tested them with git 1.9.1
  from Ubuntu 14.04 and I'm quite sure they were available even in much
  older releases, so there shouldn't be any issue on relatively new sanity
  tested distros

Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
---
 meta/lib/oe/patch.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/lib/oe/patch.py b/meta/lib/oe/patch.py
index bfa7d21879..af7aa52351 100644
--- a/meta/lib/oe/patch.py
+++ b/meta/lib/oe/patch.py
@@ -448,7 +448,7 @@ class GitApplyTree(PatchTree):
         import re
         tempdir = tempfile.mkdtemp(prefix='oepatch')
         try:
-            shellcmd = ["git", "format-patch", startcommit, "-o", tempdir]
+            shellcmd = ["git", "format-patch", "--no-signature", "--no-numbered", startcommit, "-o", tempdir]
             if paths:
                 shellcmd.append('--')
                 shellcmd.extend(paths)
-- 
2.15.1

Re: [PATCH] patch.py: Use git format-patch with --no-signature --no-numbered params

[Alexander Kanavin]

On 03/16/2018 12:50 AM, Martin Jansa wrote:
> * --no-signature saves unnecessary .patch modifications when executed on
>    host with different git version
> * --no-numbered saves unnecessary .patch modifications when number of the
>    applied patches is changed (the number is still in the filename so the
>    order how they should be applied is still preserved)
> * both options exist for very long time, I've tested them with git 1.9.1
>    from Ubuntu 14.04 and I'm quite sure they were available even in much
>    older releases, so there shouldn't be any issue on relatively new sanity
>    tested distros

Thanks, this should reduce the churn. By the way, when reviewing a 
change to a patch, it is *far* easier to look at it with a side by side 
diff tool than the usual +- format. I use vscode with gitlens extension 
(can't believe I am recommending a Microsoft product, but here we are).

Alex

Re: [PATCH] patch.py: Use git format-patch with --no-signature --no-numbered params

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 6127 bytes --]

What a blasphemy to recommend it, but so many people did, that I'll
probably try it as well soon.

BTW: I wanted to improve the warning message, because if you grep for
^WARNING: in fairly large build, warnings like:
WARNING: nettle-2.7.1-r0 do_patch:
doesn't say much (e.g. you cannot immediately grep for the patch filename
to discover which layer introduced it).

I've checked where this warning is added in meta/lib/oe/patch.py:runcmd(args,
dir = None) and my naive 1st attempt was to just include the args
(expecting to see the .patch file as well), but for default quilt it's
quite useless:
WARNING: nettle-2.7.1-r0 do_patch: Patch cannot be applied cleanly with:
['quilt', '--quiltrc', '/OE/build/oe-core/tmp-glibc/
work/core2-64-oe-linux/nettle/2.7.1-r0/recipe-sysroot-native/etc/quiltrc',
'push']

So I've tried to use
PATCHTOOL = "patch"
and it fails completely:

ERROR: nettle-2.7.1-r0 do_patch: Error executing a python function in
exec_python_func() autogenerated:

The stack trace of python calls that resulted in this exception/failure was:
File: 'exec_python_func() autogenerated', lineno: 2, function: <module>
     0001:
 *** 0002:patch_do_patch(d)
     0003:
File: '/OE/build/oe-core/openembedded-core/meta/classes/patch.bbclass',
lineno: 134, function: patch_do_patch
     0130:        if not patchdir in classes:
     0131:            patchset = cls(patchdir, d)
     0132:            resolver = rcls(patchset, oe_terminal)
     0133:            classes[patchdir] = (patchset, resolver)
 *** 0134:            patchset.Clean()
     0135:        else:
     0136:            patchset, resolver = classes[patchdir]
     0137:
     0138:        bb.note("Applying patch '%s' (%s)" % (parm['patchname'],
oe.path.format_display(local, d)))
File: '/OE/build/oe-core/openembedded-core/meta/lib/oe/patch.py', lineno:
293, function: Clean
     0289:            self._current = self._current - 1
     0290:
     0291:    def Clean(self):
     0292:        """"""
 *** 0293:        self.Pop(all=True)
     0294:
     0295:class GitApplyTree(PatchTree):
     0296:    patch_line_prefix = '%% original patch'
     0297:    ignore_commit_prefix = '%% ignore'
File: '/OE/build/oe-core/openembedded-core/meta/lib/oe/patch.py', lineno:
280, function: Pop
     0276:            return ret
     0277:
     0278:    def Pop(self, force = None, all = None):
     0279:        if all:
 *** 0280:            self._removePatchFile(True)
     0281:            self._current = None
     0282:        else:
     0283:            self._removePatchFile(False)
     0284:
File: '/OE/build/oe-core/openembedded-core/meta/lib/oe/patch.py', lineno:
211, function: _removePatchFile
     0207:        with open(self.seriespath, 'r+') as f:
     0208:            patches = f.readlines()
     0209:        if all:
     0210:            for p in reversed(patches):
 *** 0211:                self._removePatch(os.path.join(self.patchdir,
p.strip()))
     0212:            patches = []
     0213:        else:
     0214:            self._removePatch(os.path.join(self.patchdir,
patches[-1].strip()))
     0215:            patches.pop()
File: '/OE/build/oe-core/openembedded-core/meta/lib/oe/patch.py', lineno:
201, function: _removePatch
     0197:
     0198:    def _removePatch(self, p):
     0199:        patch = {}
     0200:        patch['file'] = p.split(",")[0]
 *** 0201:        patch['strippath'] = p.split(",")[1]
     0202:        self._applypatch(patch, False, True)
     0203:
     0204:    def _removePatchFile(self, all = False):
     0205:        if not os.path.exists(self.seriespath):
Exception: IndexError: list index out of range

ERROR: nettle-2.7.1-r0 do_patch: Function failed: patch_do_patch
ERROR: Logfile of failure stored in: /OE/build/oe-core/tmp-glibc/
work/core2-64-oe-linux/nettle/2.7.1-r0/temp/log.do_patch.2168
ERROR: Task (/OE/build/oe-core/meta-gplv2/recipes-support/nettle/nettle_
2.7.1.bb:do_patch) failed with exit code '1'

PATCHTOOL = "git" fails the same.

The questions are:

Is patch and git as a PATCHTOOL broken for everybody?
Is it worth parsing the fuzz in the patch output somewhere outside runcmd
and try _harder_ to show the patch name on the first line of the warning?
Maybe using the last line from quilt output (hopefully the "Now at patch
Add-target-to-only-build-tests-not-run-them.patch" line) for warning and
leaving the long explanation only as a bb.note? Not sure how this is meant
to work with various PATCHTOOLs.

FYI: I had one .patch which was always being applied with small fuzz, it
was caused by some strange character (might be some Korean letter) in
comment above the modified lines, so the quilt was somehow counting the
lines incorrectly and always showed:
Hunk #1 succeeded at 69 with fuzz 2.
I've tried to refresh the patch with devtool, then manually and it was
always failing like this, until I've tried to remove the strange characters
in the same patch and now it applies cleanly.

Regards,



On Fri, Mar 16, 2018 at 11:37 AM, Alexander Kanavin <
alexander.kanavin@linux.intel.com> wrote:

> On 03/16/2018 12:50 AM, Martin Jansa wrote:
>
>> * --no-signature saves unnecessary .patch modifications when executed on
>>    host with different git version
>> * --no-numbered saves unnecessary .patch modifications when number of the
>>    applied patches is changed (the number is still in the filename so the
>>    order how they should be applied is still preserved)
>> * both options exist for very long time, I've tested them with git 1.9.1
>>    from Ubuntu 14.04 and I'm quite sure they were available even in much
>>    older releases, so there shouldn't be any issue on relatively new
>> sanity
>>    tested distros
>>
>
> Thanks, this should reduce the churn. By the way, when reviewing a change
> to a patch, it is *far* easier to look at it with a side by side diff tool
> than the usual +- format. I use vscode with gitlens extension (can't
> believe I am recommending a Microsoft product, but here we are).
>
> Alex
>

[-- Attachment #2: Type: text/html, Size: 8597 bytes --]