threads / discuss / 38635

Salvaging borked project history

Subject: Salvaging borked project history

## tl;dr

9 messages between Feb 23, 2015 and Mar 4, 2015.

replies: 8people: 2as markdown or json

Mason· Feb 23, 2015, 17:05 UTC · lore
Hello everyone,
Here's the situation:

Back in 2012, we cloned a MIPS repo, which was itself a clone of a 3.4.2 kernel with ~40 MIPS-specific patches applied.

Then the devs started pushing patches; and once in a while, the maintainer would "sync" with the mainline kernel. I don't know what tool he used to sync, but he discarded the original patches doing so, and git blame now shows him as the author of every mainline change since the clone.

Now that we are moving to a more recent kernel, I'm trying to "fix" this situation, i.e. I'd like kernel code to have proper attribution in git blame, while keeping intact the information for "internal" commits.

While I'm at it, I want to minimize history by ignoring patches that are now irrelevant:

1) we don't support MIPS anymore, I want to ignore any internal
change we made in the arch/mips directory
2) for some reason, there was a large amount of create/delete churn
in arch/arm/configs; so I want to ignore our changes, and I'll commit
an acceptable config when the process is complete.
3) ignore some internal platform-specific patches
(NB: All these "ignores" are for internal patches, not mainline patches)

Sooo, using git log and grep, I came up with a list of ~300 patches I'd like to apply on top of the latest 3.14 kernel. For testing purposes, I used git format-patch on the oldest of my 300 patches.

I then made a shiny new clone of linux-stable.git, and tried using git am to apply that patch to the tip of linux-stable, expecting having to solve a few dozens conflicts, and move on (hoping to automate it at some point).

But 'git am' just gave up. Small sample of errors:
error: patch failed: drivers/block/Kconfig:411
error: drivers/block/Kconfig: patch does not apply
error: drivers/tty/serial/8250/8250.c: does not exist in index
error: patch failed: drivers/tty/serial/8250/8250_early.c:121
error: drivers/tty/serial/8250/8250_early.c: patch does not apply
error: patch failed: drivers/video/Kconfig:101
error: drivers/video/Kconfig: patch does not apply
...
Patch failed at 0001 Commit for supporting Sigma Designs' SoCs.
The copy of the patch that failed is found in:
    .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
How do I "resolve this problem".

I suppose I am doing this the wrong way. What is the correct way to do it? (I'm using git 2.3)

Should I use git am -3 in the original repo?
Regards.
Junio C Hamano· Feb 23, 2015, 19:36 UTC · re: Mason · lore

Re: Salvaging borked project history

Mason <slash.tmp@free.fr> writes:
Show 18 quoted lines
> But 'git am' just gave up. Small sample of errors:
>
> error: patch failed: drivers/block/Kconfig:411
> error: drivers/block/Kconfig: patch does not apply
> error: drivers/tty/serial/8250/8250.c: does not exist in index
> error: patch failed: drivers/tty/serial/8250/8250_early.c:121
> error: drivers/tty/serial/8250/8250_early.c: patch does not apply
> error: patch failed: drivers/video/Kconfig:101
> error: drivers/video/Kconfig: patch does not apply
> ...
> Patch failed at 0001 Commit for supporting Sigma Designs' SoCs.
> The copy of the patch that failed is found in:
>    .git/rebase-apply/patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
>
> How do I "resolve this problem".

You look at .git/rebase-apply/patch and drivers/block/Kconfig in your editor, and edit the former to make it apply (i.e. adjust the common context lines that begin with SP and preimage lines that begin with '-' to match what you see in the drivers/block/Kconfig where the change wants to go). You do so for all the other files.

And then run "git am" again without any argument, until the patch applies.

One way to "edit .git/rebase-apply/patch and make it apply" that is often necessary when you are applying to codebase that is way more ahead than where you took the original patches is to hand-edit the working tree files (e.g. drivers/block/Kconfig) that the patch wants to touch and change it in the way the patch wants to, while viewing the original .git/rebase-apply/patch in your pager (you do not directly edit this original patch---you only use it as a guide to discern what the patch wants to do). Once the working tree files are in a state that the patch would have wanted to make if it were taken based on the new codebase, overwrite .git/rebase-apply/patch with the output from "git diff HEAD". After that (i.e. you have "edited" .git/rebase-apply/patch, with the help with your editor, the working tree files and "git diff"), do "git reset --hard".

Which brings you to the same state just after "git am" gave up and you correctly edited ".git/rebase-apply/patch:" to make it apply.

So you run "git am" again without any argument. This time the patch would apply and continue.

But I personally think "git am -3" may be easier to handle.
Mason· Feb 26, 2015, 11:54 UTC · re: Junio C Hamano · lore

Re: Salvaging borked project history

Junio C Hamano wrote:
> But I personally think "git am -3" may be easier to handle.
Thanks! At least now, I see the light at the end of the tunnel.

I fetched linux-stable.git inside our repo. I created ~300 patches using git format-patch -1 in a loop. I can now run 'git am --3way $IGNORE *.patch'

IGNORE is used to --exclude the directories I'm not interested in.
Note: it seems --exclude=arch/mips and --exclude=arch/mips/ are
not sufficient, I need to write --exclude=arch/mips/* for git-apply
to ignore changes to files inside arch/mips.
Is that expected behavior?

Another nit: if a patch contains only changes to files inside arch/mips then git-apply will create an "empty commit" (one with no diff). Is there an option to say "skip empty patches"?

One more thing: "regular" diff -q returns 0 when the files are identical, and 1 when they differ. It seems git diff -s does not have that behavior. Is that by design?

If there is no option to skip empty patches, I'm thinking I can script a fixup step to squash all empty commits. What do you think?

Regards.
Junio C Hamano· Feb 26, 2015, 18:56 UTC · re: Mason · lore

Re: Salvaging borked project history

Mason <slash.tmp@free.fr> writes:
Show 13 quoted lines
> Thanks! At least now, I see the light at the end of the tunnel.
>
> I fetched linux-stable.git inside our repo.
> I created ~300 patches using git format-patch -1 in a loop.
> I can now run 'git am --3way $IGNORE *.patch'
>
> IGNORE is used to --exclude the directories I'm not interested in.
>
> Note: it seems --exclude=arch/mips and --exclude=arch/mips/ are
> not sufficient, I need to write --exclude=arch/mips/* for git-apply
> to ignore changes to files inside arch/mips.
>
> Is that expected behavior?

I have no idea; at least to me, "--exclude" option to "git apply" was invented to name individual paths, not patterns, and I wouldn't be surprised if glob working were merely by accident not by design.

> Another nit: if a patch contains only changes to files inside arch/mips
> then git-apply will create an "empty commit" (one with no diff). Is there
> an option to say "skip empty patches"?
"git am --skip" perhaps?

"git am" may pass the "--exclude/--include" options to "git apply" but I wouldn't be surprised if that support was added without thinking. Perhaps the reason why you discovered that it needed a lot more thinking to properly integrate these options to "git am" only now is because hardly anybody uses it ;-). Not just passing these options, the code in "git am" to react to the result of patch application to avoid the issue you observed when these options are passed need to be adjusted by changes that started passing them, but I do not think they did, cf. 77e9e496 (am: pass exclude down to apply, 2011-08-03).

> One more thing: "regular" diff -q returns 0 when the files are identical,
> and 1 when they differ. It seems git diff -s does not have that behavior.
> Is that by design?

"diff -s" may be accepted but it is an idiotic thing for a user to say. The "-s" option is to squelch output from "log" and friends, and it is exposed to "diff" only because these two families of commands share the command line parser.

Mason· Feb 26, 2015, 20:25 UTC · re: Junio C Hamano · lore

Re: Salvaging borked project history

Junio C Hamano wrote:
Show 17 quoted lines
> Mason wrote:
> 
>> I fetched linux-stable.git inside our repo.
>> I created ~300 patches using git format-patch -1 in a loop.
>> I can now run 'git am --3way $IGNORE *.patch'
>>
>> IGNORE is used to --exclude the directories I'm not interested in.
>>
>> Note: it seems --exclude=arch/mips and --exclude=arch/mips/ are
>> not sufficient, I need to write --exclude=arch/mips/* for git-apply
>> to ignore changes to files inside arch/mips.
>>
>> Is that expected behavior?
> 
> I have no idea; at least to me, "--exclude" option to "git apply"
> was invented to name individual paths, not patterns, and I wouldn't
> be surprised if glob working were merely by accident not by design.

According to the docs, IIUC globbing is by design: http://git-scm.com/docs/git-apply

--exclude=<path-pattern>
  Don't apply changes to files matching the given path pattern.
  This can be useful when importing patchsets, where you want to
  exclude certain files or directories.
This lead me to believe that --exclude=arch/mips should work.
Show 5 quoted lines
>> Another nit: if a patch contains only changes to files inside arch/mips
>> then git-apply will create an "empty commit" (one with no diff). Is there
>> an option to say "skip empty patches"?
> 
> "git am --skip" perhaps?

Nah, as the doc states, --skip "is only meaningful when restarting an aborted patch."

Show 5 quoted lines
> "git am" may pass the "--exclude/--include" options to "git apply"
> but I wouldn't be surprised if that support was added without
> thinking.  Perhaps the reason why you discovered that it needed a
> lot more thinking to properly integrate these options to "git am"
> only now is because hardly anybody uses it ;-).

I do have a rather obscure use-case (and if I do it right, I will never have to do it again).

Show 6 quoted lines
> Not just passing
> these options, the code in "git am" to react to the result of patch
> application to avoid the issue you observed when these options are
> passed need to be adjusted by changes that started passing them, but
> I do not think they did, cf. 77e9e496 (am: pass exclude down to
> apply, 2011-08-03).
Sorry, I could not parse that paragraph :-)
Show 8 quoted lines
>> One more thing: "regular" diff -q returns 0 when the files are identical,
>> and 1 when they differ. It seems git diff -s does not have that behavior.
>> Is that by design?
> 
> "diff -s" may be accepted but it is an idiotic thing for a user to
> say.  The "-s" option is to squelch output from "log" and friends,
> and it is exposed to "diff" only because these two families of
> commands share the command line parser.
Here is the use-case:
if diff -q A B; then do_X; else do_Y; fi
It makes sense to prevent diff from writing to stdout.

I was planning to write 'git diff -q commit^ commit' to test for empty commits. Looks like I'll be needing 'git diff commit^ commit | wc -l' instead?

Regards.
Junio C Hamano· Feb 26, 2015, 21:28 UTC · re: Mason · lore

Re: Salvaging borked project history

Mason <slash.tmp@free.fr> writes:
Show 8 quoted lines
>> Not just passing
>> these options, the code in "git am" to react to the result of patch
>> application to avoid the issue you observed when these options are
>> passed need to be adjusted by changes that started passing them, but
>> I do not think they did, cf. 77e9e496 (am: pass exclude down to
>> apply, 2011-08-03).
>
> Sorry, I could not parse that paragraph :-)
Heh, paraphrasing.

77e9e496 and others tried to teach --exclude/--include to "git am". For "git am" to be able to claim that these options are properly supported, you need two things:

 - The options can be given from the command line and they are
   passed to the underlying "git apply", instead of complaining with
   "no such option".
 - After calling "git mailsplit", "git mailinfo", and "git apply"
   and then these helper programs return, "git am" needs to inspect
   what happened and react.  The patch application may have failed
   due to conflicts, in which case it may have to ask the user to
   resolve.  A patch application may have resulted in no-change,
   which often is a sign that there is something wrong and "am"
   would want to stop and ask the user for confirmation.  If use of
   --include/--exclude introduces a new failure mode (e.g. mailsplit
   and mailinfo may find a patch text and happy without complaint,
   but passing --exclude to apply may cause the remaining patch to
   become essentially empty, which never happens before "am" started
   accepting these options), that codepath needs to be updated to
   cope with the new failure mode it has introduced.

And I think 77e9e496 and the other one that added --include only did the former without doing the latter.

Show 6 quoted lines
> Here is the use-case:
>
> if diff -q A B; then do_X; else do_Y; fi
> ...
> I was planning to write 'git diff -q commit^ commit'
> to test for empty commits.
s/-q/--quiet/ and all is well, no?

"git diff --quiet" doesn't abbreviate down to "git diff -q" primarily because "-q" for the underlying "git diff-files" means something different from "--quiet".

Mason· Feb 26, 2015, 23:47 UTC · re: Junio C Hamano · lore

Re: Salvaging borked project history

Junio C Hamano wrote:
Show 6 quoted lines
> Mason wrote:
> 
>> I was planning to write 'git diff -q commit^ commit'
>> to test for empty commits.
> 
> s/-q/--quiet/ and all is well, no?
Doh! I've no idea how I missed these...
--exit-code
  Make the program exit with codes similar to diff(1). That is, it
  exits with 1 if there were differences and 0 means no differences.
--quiet
  Disable all output of the program. Implies --exit-code.
Thanks for walking me through it.
Regards.
Junio C Hamano· Feb 26, 2015, 23:54 UTC · re: Mason · lore

Re: Salvaging borked project history

Mason <slash.tmp@free.fr> writes:
Show 8 quoted lines
>> Mason wrote:
>> 
>>> I was planning to write 'git diff -q commit^ commit'
>>> to test for empty commits.
>> 
>> s/-q/--quiet/ and all is well, no?
>
> Doh! I've no idea how I missed these...

Yeah, this is one of the unfortunate corners of Git that I can apologize but cannot do much more than that about (in other words, making "git diff -q" as a short-hand for "git diff --quiet" is not acceptable) due to backward compatibility and consistency concerns.

Mason· Mar 4, 2015, 14:49 UTC · re: Mason · lore

Re: Salvaging borked project history

Mason wrote:
> I was planning to write 'git diff -q commit^ commit'
> to test for empty commits. Looks like I'll be needing
> 'git diff commit^ commit | wc -l' instead?
I wrote a script to generate the list of empty commits.
git log --format="%h" --reverse 413cb08.. | while read H
do
   if git diff --quiet $H^ $H; then echo $H; fi
done >empty_commits

But it turns out all this is unnecessary, as git-rebase will automatically filter empty commits! :-) Unless given the --keep-empty option, I presume.

Thanks to everyone involved in making this great tool.
Regards.

← back to recent threads