[U-Boot] [PATCH 3/7] powerpc/82xx: merge mgcoge.h and mgcoge3ne.h into km82xx.h

[Gerlando Falauto]

On 07/30/2012 03:00 PM, Wolfgang Denk wrote:
> Dear Gerlando Falauto,
>
> In message<50164F3A.6050409 at keymile.com>  you wrote:
>>
>>>>    boards.cfg                  |    4 +-
>>>>    include/configs/km82xx.h    |  149 +++++++++++++++++++++++++++++++++++++++++++
>>>>    include/configs/mgcoge.h    |   93 ---------------------------
>>>>    include/configs/mgcoge3ne.h |   93 ---------------------------
>>>>    4 files changed, 151 insertions(+), 188 deletions(-)
>>>>    create mode 100644 include/configs/km82xx.h
>>>>    delete mode 100644 include/configs/mgcoge.h
>>>>    delete mode 100644 include/configs/mgcoge3ne.h
>>>
>>> Can you please try creating this patch with git format-patch with
>>> options "-M" and "-C", please? I think git should do better to
>>> recognize this rename / merge of two files.
>>
>> I tried this but to no avail, the resulting patch is still the same.
>> Same for patch number 4.
>>
>> I guess git gets confused by the fact that we are merging two files into
>> one.
>
> No, git can handle this pretty well if you tell it what you are doing.
> I just retested this; the result is:
>
> 	---
> 	 file.1                | 64 ---------------------------------------------------
> 	 file.2 =>  file.common | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 	 2 files changed, 64 insertions(+), 64 deletions(-)
> 	 delete mode 100644 file.1
> 	 rename file.2 =>  file.common (63%)
>
>> What I could do is to split this commit so that, for instance,
>> first we rename one of the files and then (on a separate commit) we move
>> the content of one into the other.
>
> No, this can and should be done in a single commit, for example like this:
>
> 	1. run: git mv include/configs/mgcoge.h include/configs/km82xx.h
> 	2. merge include/configs/mgcoge3ne.h into include/configs/km82xx.h
> 	3. run: git rm include/configs/mgcoge3ne.h
>
> git format-patch -M -C will then recognize what you did.

The way I understand it, such renaming information is built on the fly 
while building the patch (like you're suggesting, it's a parameter to 
git format-patch, not to the commit itself).

In other words, a renaming is just a remove and an add (see [1]):
--
Git has a rename command git mv, but that is just for convenience. The 
effect is indistinguishable from removing the file and adding another 
with different name and the same content.
--

As a matter of fact, I am able to get the renaming recognized when 
committing:

---
[detached HEAD 85129b8] powerpc/82xx: 1of2 move km/km82xx-common.h 
within km82xx.h
  1 files changed, 148 insertions(+), 0 deletions(-)
  rename include/configs/{km/km82xx-common.h => km82xx.h} (71%)
---

However, I've been struggling to get this same kind of message through 
git-format-patch. No way, I don't know why. I tried with -M, -M -C, 
-M10%, adding "[diff]\n renames = copies" to ~/.gitconfig, with both 
versions below, nothing. Detected as a rename at commit time, it's a 
plain delete/create commit at patch creation time.

$ git --version
git version 1.7.1

$ ~/git/bin-wrappers/git --version
git version 1.7.11.3

Could you please share what GIT version you're running?

>> Question is, is this really worth the effort?
>> Is there a common practice for such reworks?
>
> Yes, if possible we want that git tracks such renames / merges.
> And here it seems easily possible.

Could you please try applying the patch to your tree (namely 3 and 4), 
and then build it again by running:
  git-format-patch -M30% -C

It should get detected as a rename anyway (I mean, even if applied as a 
whole delete/add).

In any case, I have no clue whether git would be able to correctly (i.e. 
intelligently) apply such patch to a slightly different tree (e.g. 
through cherry-pick or rebase).
So for instance, in your example above, what if file.1 (whose contents 
is anyway moved into file.common, regardless of rename detection) is 
slightly different?
Would the patch fail? Or worse, would it silently apply by just deleting 
the "new" file and applying the "old" contents (verbatim from the patch)?

I'm strongly convinced that if we want to track such changes for what 
they are (code moving) so that they can be "easily" re-applied, we 
should mark this explicitly. Even at the cost of creating multiple 
patches if necessary. Since git isn't able to figure it out by itself,
the only way I can think of doing this is splitting the commit into 3 parts:
1) preparation work, adding #include statements to the old files
2) automated code moving through a script like the following (and 
including it in the commit message itself)
3) cleanup changes

================================================================
powerpc/82xx: 2of3 merge mgcoge.h and mgcoge3ne.h into km82xx.h

Since mgcoge and mgcoge3ne are the only km82xx boards, there is no
need to keep them as separate .h config files.
Therefore, make mgcoge3ne.h and mgcoge.h converge into a single
km82xx.h file.
Step 2 of 3: substitute include files through the following script:

INCLUDE_STMT='#include "mgcoge.h"'
INCLUDED=include/configs/mgcoge.h
INCLUDING=include/configs/km82xx.h

[[ -e $INCLUDING ]] &&
[[ -e $INCLUDED ]] &&
grep -F "$INCLUDE_STMT" $INCLUDING &&
( LINE=`grep -nF "$INCLUDE_STMT" $INCLUDING  | cut -d: -f1`
   head -n$((LINE-1)) $INCLUDING
   cat $INCLUDED
   tail -n+$((LINE+1)) $INCLUDING) > /tmp/includemerge.txt &&
mv /tmp/includemerge.txt $INCLUDING &&
git rm $INCLUDED &&
git add $INCLUDING

INCLUDE_STMT='#include "mgcoge3ne.h"'
INCLUDED=include/configs/mgcoge3ne.h
INCLUDING=include/configs/km82xx.h

[[ -e $INCLUDING ]] &&
[[ -e $INCLUDED ]] &&
grep -F "$INCLUDE_STMT" $INCLUDING &&
( LINE=`grep -nF "$INCLUDE_STMT" $INCLUDING  | cut -d: -f1`
   head -n$((LINE-1)) $INCLUDING
   cat $INCLUDED
   tail -n+$((LINE+1)) $INCLUDING) > /tmp/includemerge.txt &&
mv /tmp/includemerge.txt $INCLUDING &&
git rm $INCLUDED &&
git add $INCLUDING
================================================================

Then the patch content itself can be safely ignored.
I know it's not nice, but I think it should work.

Thank you for reading up to this point. :-)
Gerlando

[1] https://git.wiki.kernel.org/index.php/GitFaq