public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Gerlando Falauto <gerlando.falauto@keymile.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 3/7] powerpc/82xx: merge mgcoge.h and mgcoge3ne.h into km82xx.h
Date: Mon, 30 Jul 2012 20:21:47 +0200	[thread overview]
Message-ID: <5016D0BB.3010000@keymile.com> (raw)
In-Reply-To: <20120730160753.900A720226A@gemini.denx.de>

On 07/30/2012 06:07 PM, Wolfgang Denk wrote:
> Dear Gerlando Falauto,
>
> In message<5016A093.6040102@keymile.com>  you wrote:
>>
>> 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).
>
> Yes, and I fail to understand where your problems could be.
>
>> 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.
>
> I see this (doing it all manually for testing):
>
> ->  patch -p1</tmp/patch
> ->  git rm include/configs/mgcoge.h include/configs/mgcoge3ne.h
> ->  git add include/configs/km82xx.h
> ->  git commit -s -m 'test 1'
> ->  git format-patch -M -C --stdout HEAD^>/tmp/patch
> ->  less /tmp/patch
>  From 1d9ce92a542d139b78291fb4e437e538d647d55b Mon Sep 17 00:00:00 2001
> From: Wolfgang Denk<wd@denx.de>
> Date: Mon, 30 Jul 2012 17:57:53 +0200
> Subject: [PATCH] test 1
>
> Signed-off-by: Wolfgang Denk<wd@denx.de>
> ---
>   include/configs/{mgcoge3ne.h =>  km82xx.h} |   95 ++++++++++++++++++++++-------
>   include/configs/mgcoge.h                  |   93 ----------------------------
>   2 files changed, 74 insertions(+), 114 deletions(-)
>   rename include/configs/{mgcoge3ne.h =>  km82xx.h} (55%)
>   delete mode 100644 include/configs/mgcoge.h
>
> ...
>
> Oops, I forgot to "git add boards.cfg" here, but for this test it
> makes no difference.

It turns out it's a bug/limitation of git 1.7.1.
I upgraded to 1.7.10.4 and 1.7.11.3 and now I get the same results as 
you do (rename detected). See new patch as a follow-up.

[...]

>> 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?
>
> It doesn't matter.  If there are conflicts, and these can be resolved,
> it works just the same.
>
>> 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,
>
> No, on contrary.  This is basicly an atomic change, and we should not
> artificially split it.  git should have no problems with such actions,
> they are really not special in any way.

Renaming, I understand. But merging/splitting files, I guess they should 
be treated differently (i.e., as such!) IMHO, if we want repeatability 
and resilience to small changes.
But git doesn't (yet?) do that, and I think it should be worked around 
some other way.

>
>> the only way I can think of doing this is splitting the commit into 3 parts:
>
> No, please don't.
>
>> 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
>
> Argh.... No, this is not what we're going to do.

Alright, your call.

Thanks for your patience.
Gerlando

  reply	other threads:[~2012-07-30 18:21 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-27 15:16 [U-Boot] [PATCH 0/7] km82xx and mgcogeXxx reworking Gerlando Falauto
2012-07-27 15:16 ` [U-Boot] [PATCH 1/7] powerpc/82xx: remove unused define for mgcoge3ne Gerlando Falauto
2012-07-31 20:29   ` Wolfgang Denk
2012-07-27 15:16 ` [U-Boot] [PATCH 2/7] powerpc/82xx: move mgcoge, mgcoge3ne defines to ease subsequent merge Gerlando Falauto
2012-07-31 20:30   ` Wolfgang Denk
2012-08-02  9:14   ` [U-Boot] [PATCH v2 4/7] powerpc/82xx: move km/km82xx-common.h within km82xx.h Gerlando Falauto
2012-07-27 15:16 ` [U-Boot] [PATCH 3/7] powerpc/82xx: merge mgcoge.h and mgcoge3ne.h into km82xx.h Gerlando Falauto
2012-07-27 17:30   ` Wolfgang Denk
2012-07-30  9:09     ` Gerlando Falauto
2012-07-30 13:00       ` Wolfgang Denk
2012-07-30 14:56         ` Gerlando Falauto
2012-07-30 16:07           ` Wolfgang Denk
2012-07-30 18:21             ` Gerlando Falauto [this message]
2012-07-30 18:22               ` [U-Boot] [PATCH v2 " Gerlando Falauto
2012-07-31 20:30   ` [U-Boot] [PATCH " Wolfgang Denk
2012-07-27 15:16 ` [U-Boot] [PATCH 4/7] powerpc/82xx: move km/km82xx-common.h within km82xx.h Gerlando Falauto
2012-07-27 17:31   ` Wolfgang Denk
2012-07-30 18:28     ` Gerlando Falauto
2012-07-30 19:12       ` Wolfgang Denk
2012-08-02  9:10         ` Gerlando Falauto
2012-07-31 20:31   ` Wolfgang Denk
2012-07-27 15:16 ` [U-Boot] [PATCH 5/7] powerpc/82xx: add SDRAM detection for km82xx Gerlando Falauto
2012-07-31 20:31   ` Wolfgang Denk
2012-07-27 15:16 ` [U-Boot] [PATCH 6/7] powerpc/82xx: use SDRAM detection for mgcoge2ne Gerlando Falauto
2012-07-31 20:31   ` Wolfgang Denk
2012-07-27 15:16 ` [U-Boot] [PATCH 7/7] powerpc/82xx: adapt SDRAM settings for mgcoge3ne Gerlando Falauto
2012-07-31 20:31   ` Wolfgang Denk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5016D0BB.3010000@keymile.com \
    --to=gerlando.falauto@keymile.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox