On 07/30/2012 03:00 PM, Wolfgang Denk wrote:
Dear Gerlando Falauto,

In message<50164f3a.6050...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to