[PATCH] lib/oe/patch.py: Prefer "git am" over "git apply" when applying git patches

[PATCH] lib/oe/patch.py: Prefer "git am" over "git apply" when applying git patches

[Laszlo Papp]

It is better to use "git am" when possible to preserve the commit messages and
the mail format in general for patches when those are present. A typical use
case is when developers would like to keep the changes on top of the latest
upstream, and they may occasionally need to rebase. This is not possible with
"git diff" and "diff" generated patches.

Since this is not always the case, the fallback would be the "git apply"
operation which is currently available.

Signed-off-by: Laszlo Papp <lpapp@kde.org>
---
 meta/lib/oe/patch.py | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/meta/lib/oe/patch.py b/meta/lib/oe/patch.py
index 59abd0a..b085c9d 100644
--- a/meta/lib/oe/patch.py
+++ b/meta/lib/oe/patch.py
@@ -203,17 +203,23 @@ class GitApplyTree(PatchTree):
         PatchTree.__init__(self, dir, d)
 
     def _applypatch(self, patch, force = False, reverse = False, run = True):
-        shellcmd = ["git", "--git-dir=.", "apply", "-p%s" % patch['strippath']]
+        def _applypatchhelper(shellcmd, patch, force = False, reverse = False, run = True):
+            if reverse:
+                shellcmd.append('-R')
 
-        if reverse:
-            shellcmd.append('-R')
+            shellcmd.append(patch['file'])
 
-        shellcmd.append(patch['file'])
+            if not run:
+                return "sh" + "-c" + " ".join(shellcmd)
 
-        if not run:
-            return "sh" + "-c" + " ".join(shellcmd)
+            return runcmd(["sh", "-c", " ".join(shellcmd)], self.dir)
 
-        return runcmd(["sh", "-c", " ".join(shellcmd)], self.dir)
+        try:
+            shellcmd = ["git", "--work-tree=.", "am", "-3", "-p%s" % patch['strippath']]
+            return _applypatchhelper(shellcmd, patch, force, reverse, run)
+        except CmdError:
+            shellcmd = ["git", "--git-dir=.", "apply", "-p%s" % patch['strippath']]
+            return _applypatchhelper(shellcmd, patch, force, reverse, run)
 
 
 class QuiltTree(PatchSet):
-- 
1.8.5.1

Re: [PATCH] lib/oe/patch.py: Prefer "git am" over "git apply" when applying git patches

[Laszlo Papp]

Ping?

Alternatively, the system could also have an option for further
fine-tuning what to do with git patches

On Tue, Dec 24, 2013 at 12:44 PM, Laszlo Papp <lpapp@kde.org> wrote:
> It is better to use "git am" when possible to preserve the commit messages and
> the mail format in general for patches when those are present. A typical use
> case is when developers would like to keep the changes on top of the latest
> upstream, and they may occasionally need to rebase. This is not possible with
> "git diff" and "diff" generated patches.
>
> Since this is not always the case, the fallback would be the "git apply"
> operation which is currently available.
>
> Signed-off-by: Laszlo Papp <lpapp@kde.org>
> ---
>  meta/lib/oe/patch.py | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/meta/lib/oe/patch.py b/meta/lib/oe/patch.py
> index 59abd0a..b085c9d 100644
> --- a/meta/lib/oe/patch.py
> +++ b/meta/lib/oe/patch.py
> @@ -203,17 +203,23 @@ class GitApplyTree(PatchTree):
>          PatchTree.__init__(self, dir, d)
>
>      def _applypatch(self, patch, force = False, reverse = False, run = True):
> -        shellcmd = ["git", "--git-dir=.", "apply", "-p%s" % patch['strippath']]
> +        def _applypatchhelper(shellcmd, patch, force = False, reverse = False, run = True):
> +            if reverse:
> +                shellcmd.append('-R')
>
> -        if reverse:
> -            shellcmd.append('-R')
> +            shellcmd.append(patch['file'])
>
> -        shellcmd.append(patch['file'])
> +            if not run:
> +                return "sh" + "-c" + " ".join(shellcmd)
>
> -        if not run:
> -            return "sh" + "-c" + " ".join(shellcmd)
> +            return runcmd(["sh", "-c", " ".join(shellcmd)], self.dir)
>
> -        return runcmd(["sh", "-c", " ".join(shellcmd)], self.dir)
> +        try:
> +            shellcmd = ["git", "--work-tree=.", "am", "-3", "-p%s" % patch['strippath']]
> +            return _applypatchhelper(shellcmd, patch, force, reverse, run)
> +        except CmdError:
> +            shellcmd = ["git", "--git-dir=.", "apply", "-p%s" % patch['strippath']]
> +            return _applypatchhelper(shellcmd, patch, force, reverse, run)
>
>
>  class QuiltTree(PatchSet):
> --
> 1.8.5.1
>

Re: [PATCH] lib/oe/patch.py: Prefer "git am" over "git apply" when applying git patches

[Saul Wold]

On 12/31/2013 06:18 AM, Laszlo Papp wrote:
> Ping?
>
> Alternatively, the system could also have an option for further
> fine-tuning what to do with git patches
>
> On Tue, Dec 24, 2013 at 12:44 PM, Laszlo Papp <lpapp@kde.org> wrote:
>> It is better to use "git am" when possible to preserve the commit messages and
>> the mail format in general for patches when those are present. A typical use
>> case is when developers would like to keep the changes on top of the latest
>> upstream, and they may occasionally need to rebase. This is not possible with
>> "git diff" and "diff" generated patches.
>>
>> Since this is not always the case, the fallback would be the "git apply"
>> operation which is currently available.
>>
Looking at this, is it possible to detect a git patch and only then use 
git am?  Since most of the patches carried in oe-core and other layers 
the 'git am' will typically fail and increase the build time since it 
will have to re-run the git apply, we don't want to had more forking and 
work in the main hot path.

I am not the expert in this, neither is RP, so maybe Chris can comment 
further.

Thanks
	Sau!


>> Signed-off-by: Laszlo Papp <lpapp@kde.org>
>> ---
>>   meta/lib/oe/patch.py | 20 +++++++++++++-------
>>   1 file changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/meta/lib/oe/patch.py b/meta/lib/oe/patch.py
>> index 59abd0a..b085c9d 100644
>> --- a/meta/lib/oe/patch.py
>> +++ b/meta/lib/oe/patch.py
>> @@ -203,17 +203,23 @@ class GitApplyTree(PatchTree):
>>           PatchTree.__init__(self, dir, d)
>>
>>       def _applypatch(self, patch, force = False, reverse = False, run = True):
>> -        shellcmd = ["git", "--git-dir=.", "apply", "-p%s" % patch['strippath']]
>> +        def _applypatchhelper(shellcmd, patch, force = False, reverse = False, run = True):
>> +            if reverse:
>> +                shellcmd.append('-R')
>>
>> -        if reverse:
>> -            shellcmd.append('-R')
>> +            shellcmd.append(patch['file'])
>>
>> -        shellcmd.append(patch['file'])
>> +            if not run:
>> +                return "sh" + "-c" + " ".join(shellcmd)
>>
>> -        if not run:
>> -            return "sh" + "-c" + " ".join(shellcmd)
>> +            return runcmd(["sh", "-c", " ".join(shellcmd)], self.dir)
>>
>> -        return runcmd(["sh", "-c", " ".join(shellcmd)], self.dir)
>> +        try:
>> +            shellcmd = ["git", "--work-tree=.", "am", "-3", "-p%s" % patch['strippath']]
>> +            return _applypatchhelper(shellcmd, patch, force, reverse, run)
>> +        except CmdError:
>> +            shellcmd = ["git", "--git-dir=.", "apply", "-p%s" % patch['strippath']]
>> +            return _applypatchhelper(shellcmd, patch, force, reverse, run)
>>
>>
>>   class QuiltTree(PatchSet):
>> --
>> 1.8.5.1
>>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>

Re: [PATCH] lib/oe/patch.py: Prefer "git am" over "git apply" when applying git patches

[Otavio Salvador]

On Mon, Jan 6, 2014 at 8:10 PM, Saul Wold <sgw@linux.intel.com> wrote:
> On 12/31/2013 06:18 AM, Laszlo Papp wrote:
>>
>> Ping?
>>
>> Alternatively, the system could also have an option for further
>> fine-tuning what to do with git patches
>>
>> On Tue, Dec 24, 2013 at 12:44 PM, Laszlo Papp <lpapp@kde.org> wrote:
>>>
>>> It is better to use "git am" when possible to preserve the commit
>>> messages and
>>> the mail format in general for patches when those are present. A typical
>>> use
>>> case is when developers would like to keep the changes on top of the
>>> latest
>>> upstream, and they may occasionally need to rebase. This is not possible
>>> with
>>> "git diff" and "diff" generated patches.
>>>
>>> Since this is not always the case, the fallback would be the "git apply"
>>> operation which is currently available.
>>>
> Looking at this, is it possible to detect a git patch and only then use git
> am?  Since most of the patches carried in oe-core and other layers the 'git
> am' will typically fail and increase the build time since it will have to
> re-run the git apply, we don't want to had more forking and work in the main
> hot path.
>
> I am not the expert in this, neither is RP, so maybe Chris can comment
> further.

I am not sure it'd buy us a lot trying to detect it; in fact using git
am patches easy rework, rebase and upstreaming of those.

To detect it, we'd need to parse the file somehow and I am unsure it
would be faster than just try to apply it.

My 2c.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Re: [PATCH] lib/oe/patch.py: Prefer "git am" over "git apply" when applying git patches

[Laszlo Papp]

On Tue, Jan 7, 2014 at 11:46 AM, Otavio Salvador
<otavio@ossystems.com.br> wrote:
> On Mon, Jan 6, 2014 at 8:10 PM, Saul Wold <sgw@linux.intel.com> wrote:
>> On 12/31/2013 06:18 AM, Laszlo Papp wrote:
>>>
>>> Ping?
>>>
>>> Alternatively, the system could also have an option for further
>>> fine-tuning what to do with git patches
>>>
>>> On Tue, Dec 24, 2013 at 12:44 PM, Laszlo Papp <lpapp@kde.org> wrote:
>>>>
>>>> It is better to use "git am" when possible to preserve the commit
>>>> messages and
>>>> the mail format in general for patches when those are present. A typical
>>>> use
>>>> case is when developers would like to keep the changes on top of the
>>>> latest
>>>> upstream, and they may occasionally need to rebase. This is not possible
>>>> with
>>>> "git diff" and "diff" generated patches.
>>>>
>>>> Since this is not always the case, the fallback would be the "git apply"
>>>> operation which is currently available.
>>>>
>> Looking at this, is it possible to detect a git patch and only then use git
>> am?  Since most of the patches carried in oe-core and other layers the 'git
>> am' will typically fail and increase the build time since it will have to
>> re-run the git apply, we don't want to had more forking and work in the main
>> hot path.
>>
>> I am not the expert in this, neither is RP, so maybe Chris can comment
>> further.
>
> I am not sure it'd buy us a lot trying to detect it; in fact using git
> am patches easy rework, rebase and upstreaming of those.

+1

> To detect it, we'd need to parse the file somehow and I am unsure it
> would be faster than just try to apply it.

Agree; it may even be slower. I think it could be separated out later
into a custom option if any performance issue comes up. IMO, it is a
reasonable assumption if the patchtool set is git, then it is a git
patch, not raw diff...

Re: [PATCH] lib/oe/patch.py: Prefer "git am" over "git apply" when applying git patches

[Koen Kooi]

Op 6 jan. 2014, om 23:10 heeft Saul Wold <sgw@linux.intel.com> het volgende geschreven:

> On 12/31/2013 06:18 AM, Laszlo Papp wrote:
>> Ping?
>> 
>> Alternatively, the system could also have an option for further
>> fine-tuning what to do with git patches
>> 
>> On Tue, Dec 24, 2013 at 12:44 PM, Laszlo Papp <lpapp@kde.org> wrote:
>>> It is better to use "git am" when possible to preserve the commit messages and
>>> the mail format in general for patches when those are present. A typical use
>>> case is when developers would like to keep the changes on top of the latest
>>> upstream, and they may occasionally need to rebase. This is not possible with
>>> "git diff" and "diff" generated patches.
>>> 
>>> Since this is not always the case, the fallback would be the "git apply"
>>> operation which is currently available.
>>> 
> Looking at this, is it possible to detect a git patch and only then use git am?  Since most of the patches carried in oe-core and other layers the 'git am' will typically fail

All the patches I add are git am'able since I use a patch similar to this :) A big timesaver is to check for  a .git/ in $WORKDIR otherwise git am will try to use a top level git tree (e.g. combo repo) and that's not what we want.

regards,

Koen

Re: [PATCH] lib/oe/patch.py: Prefer "git am" over "git apply" when applying git patches

[Laszlo Papp]

On Tue, Jan 7, 2014 at 12:16 PM, Koen Kooi <koen@dominion.thruhere.net> wrote:
>
> Op 6 jan. 2014, om 23:10 heeft Saul Wold <sgw@linux.intel.com> het volgende geschreven:
>
>> On 12/31/2013 06:18 AM, Laszlo Papp wrote:
>>> Ping?
>>>
>>> Alternatively, the system could also have an option for further
>>> fine-tuning what to do with git patches
>>>
>>> On Tue, Dec 24, 2013 at 12:44 PM, Laszlo Papp <lpapp@kde.org> wrote:
>>>> It is better to use "git am" when possible to preserve the commit messages and
>>>> the mail format in general for patches when those are present. A typical use
>>>> case is when developers would like to keep the changes on top of the latest
>>>> upstream, and they may occasionally need to rebase. This is not possible with
>>>> "git diff" and "diff" generated patches.
>>>>
>>>> Since this is not always the case, the fallback would be the "git apply"
>>>> operation which is currently available.
>>>>
>> Looking at this, is it possible to detect a git patch and only then use git am?  Since most of the patches carried in oe-core and other layers the 'git am' will typically fail
>
> All the patches I add are git am'able since I use a patch similar to this :)

Cool.

> A big timesaver is to check for  a .git/ in $WORKDIR otherwise git am will try to use a top level git tree (e.g. combo repo) and that's not what we want.

Hmm, yeah, I agree.

However, I do not have time currently for this, nor is this
high-priority for me since it just works for me, and I am happy with
that. Would it be possible to optimize it later?

Failing that, what is the best practice to keep my changes separate
from Yocto? I am referring to changes that are not upstreamed for
whatever reason, like the maintainer saying that here it could be a
performance regression (although non-measured at this point).

Cheers ...

Re: [PATCH] lib/oe/patch.py: Prefer "git am" over "git apply" when applying git patches

[Laszlo Papp]

Hi everyone,

the patch was sent about three weeks ago, and I have not seen any
objections so far.

What is the issue for progressing? Please let me know.

Cheers, L.

On Thu, Jan 9, 2014 at 4:53 PM, Laszlo Papp <lpapp@kde.org> wrote:
> On Tue, Jan 7, 2014 at 12:16 PM, Koen Kooi <koen@dominion.thruhere.net> wrote:
>>
>> Op 6 jan. 2014, om 23:10 heeft Saul Wold <sgw@linux.intel.com> het volgende geschreven:
>>
>>> On 12/31/2013 06:18 AM, Laszlo Papp wrote:
>>>> Ping?
>>>>
>>>> Alternatively, the system could also have an option for further
>>>> fine-tuning what to do with git patches
>>>>
>>>> On Tue, Dec 24, 2013 at 12:44 PM, Laszlo Papp <lpapp@kde.org> wrote:
>>>>> It is better to use "git am" when possible to preserve the commit messages and
>>>>> the mail format in general for patches when those are present. A typical use
>>>>> case is when developers would like to keep the changes on top of the latest
>>>>> upstream, and they may occasionally need to rebase. This is not possible with
>>>>> "git diff" and "diff" generated patches.
>>>>>
>>>>> Since this is not always the case, the fallback would be the "git apply"
>>>>> operation which is currently available.
>>>>>
>>> Looking at this, is it possible to detect a git patch and only then use git am?  Since most of the patches carried in oe-core and other layers the 'git am' will typically fail
>>
>> All the patches I add are git am'able since I use a patch similar to this :)
>
> Cool.
>
>> A big timesaver is to check for  a .git/ in $WORKDIR otherwise git am will try to use a top level git tree (e.g. combo repo) and that's not what we want.
>
> Hmm, yeah, I agree.
>
> However, I do not have time currently for this, nor is this
> high-priority for me since it just works for me, and I am happy with
> that. Would it be possible to optimize it later?
>
> Failing that, what is the best practice to keep my changes separate
> from Yocto? I am referring to changes that are not upstreamed for
> whatever reason, like the maintainer saying that here it could be a
> performance regression (although non-measured at this point).
>
> Cheers ...