Skip to contentSkip to conversation

Salvaging borked project history

DISCUSSION Mason 2 participants 9 messages Jun 15, 2016

lore
Share & tools
lore

Mason

author
#0 lore raw

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.

#1 lore raw
Quoted 17 lines · Mason

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".

Quoted 1 line

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.

author
#2 lore raw
Quoted 1 line · Junio C Hamano

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.

#3 lore raw
Quoted 6 lines · Mason

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'

Quoted 7 lines

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.

Quoted 3 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?

"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).

Quoted 3 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.

author
#4 lore raw
Quoted 12 lines · Junio C Hamano
Quoted 10 lines · Mason

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.

Quoted 5 lines
Quoted 1 line

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.

Quoted 5 lines
Quoted 3 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."

Quoted 5 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).

Quoted 6 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 :-)

Quoted 8 lines
Quoted 3 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.

#5 lore raw
Quoted 8 lines · Mason
Quoted 6 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.

Quoted 6 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".

author
#6 lore raw
Quoted 6 lines · Junio C Hamano
Quoted 2 lines · Mason

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.

#7 lore raw
Quoted 8 lines · Mason
Quoted 6 lines
Quoted 2 lines · Mason

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.

author
#8 lore raw
In reply to Mason
Quoted 3 lines · Mason

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.