{"thread":{"id":"9568","subject":"file disappears after git rebase (missing one commit)","startedAt":"2007-08-18T19:37:34Z","lastAt":"2007-08-18T22:52:55Z","messageCount":6,"participants":["Torgil Svensson","Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"50980","messageId":"e7bda7770708181237u34253bf1h7c3fe0987d13d3b3@mail.gmail.com","threadId":"9568","inReplyTo":null,"subject":"file disappears after git rebase (missing one commit)","fromName":"Torgil Svensson","fromEmail":"torgil.svensson@gmail.com","sentAt":"2007-08-18T19:37:34Z","receivedAt":"2007-08-18T19:37:34Z","isPatch":false,"sender":{"key":"torgil.svensson@gmail.com","avatar":null},"body":"Hi,\n\nI'm trying to rebase a branch (\"msmtp\") on another branch (\"devel\").\nThe msmtp has a number of commits that are already in the devel branch\n(but with different history) and one new commit that adds one file.\n\n$ git clone git://repo.or.cz/msysgit.git\n$ cd msysgit\n$ git rev-parse origin/msmtp\nb11cf4ce6262a7c3b243e3cfdc70e6b44682cb59\n$ git rev-parse origin/devel\n57aa8405103856106ec0e31453089c33c899c98b\n$ git -b checkout devel origin/devel\n$ git -b checkout msmtp origin/msmtp\n$ git show-branch msmtp devel\n* [msmtp] Added msmtp.exe SMTP client\n ! [devel] Add disk summarize tool (du.exe)\n--\n + [devel] Add disk summarize tool (du.exe)\n + [devel^] gdb updated to v6.6\n + [devel~2] w32api updated to v3.10\n + [devel~3] Updated gcc to v3.4.5\n + [devel~4] Updated binutils to v2.17.50\n + [devel~5] Remove remnants of the c++ compiler\n + [devel~6] GitMe: only fetch 'master' of msysgit.git\n + [devel~7] GitMe: inline 7z's install script\n + [devel~8] GitMe: avoid dependency on cmd.exe\n + [devel~9] msysGit: adjust for submodule layout\n + [devel~10] msysGit: we have 7zip installed in /share/7-Zip/ now\n + [devel~11] msysGit: 7z cannot update existing installers\n + [devel~12] msysGit: move scripts to /share/msysGit/\n + [devel~13] WinGit: do not pack builtins, but copy them when unpacking\n + [devel~14] Update TODO: Marius squashed two\n + [devel~15] WinGit: strip executables (Issue 25)\n + [devel~16^2] GitMe: fix HTTP transport\n + [devel~16^2^] Undo hacky she-bang fixup\n + [devel~16^2~2] Issue 21: core.autocrlf should be set to true (at\nleast for end-users)\n + [devel~16^2~3] Add clear script, and remove the clear=clsb alias in profile\n + [devel~16^2~4] Make 7z functional\n + [devel~16^2~5] msys: support for Windows XP x64\n + [devel~16^2~6] Removed all SuperGitMe functionality Also propagated\nlatest fixes and made installer even smaller\n + [devel~16^2~7] Fixed Issue 37: Errors during install because repo\nnow has tags\n + [devel~18] Issue 21: core.autocrlf should be set to true (at least\nfor end-users)\n + [devel~19] Add clear script, and remove the clear=clsb alias in profile\n + [devel~20] Make 7z functional\n + [devel~21] msys: support for Windows XP x64\n + [devel~22] GitMe SuperFetch\n + [devel~23] Latest git submodule\n*  [msmtp] Added msmtp.exe SMTP client\n*  [msmtp^] gdb updated to v6.6\n*  [msmtp~2] w32api updated to v3.10\n*  [msmtp~3] Updated gcc to v3.4.5\n*  [msmtp~4] Updated binutils to v2.17.50\n*  [msmtp~5] Remove remnants of the c++ compiler\n*+ [devel~24] GitMe: check if cygwin is in PATH; if so abort installer.\n$ find bin -name \"msmtp.exe\"\nbin/msmtp.exe\n\nNote! that this file is added with the commit \"*  [msmtp] Added\nmsmtp.exe SMTP client\". After rebase I expect that this commit will be\non top of the devel branch.\n\n$ cat .git/HEAD\nref: refs/heads/msmtp\n$ git rebase devel\nFirst, rewinding head to replay your work on top of it...\nHEAD is now at 57aa840... Add disk summarize tool (du.exe)\nNothing to do.\n$ git show-branch msmtp devel\n* [msmtp] Add disk summarize tool (du.exe)\n ! [devel] Add disk summarize tool (du.exe)\n--\n*+ [msmtp] Add disk summarize tool (du.exe)\n$ find bin -name \"msmtp.exe\"\n\nAnd the msmtp commit + file is lost.\n\nI've tested on windows (4msysgit.git):\n$ git --version\ngit version 1.5.3.rc4.mingw.2.49.g3314\n\nAnd on linux (git.git):\n$ git --version\ngit version 1.5.2.5.g0734d\n\nAnd on linux with next branch (git.git)\n$ git version\ngit version 1.5.3.rc5.843.gdac75\n\n\nIs this a bug?  Any ideas?\n\nBest regards,\n\n//Torgil\n"},{"id":"50981","messageId":"alpine.LFD.0.999.0708181247330.30176@woody.linux-foundation.org","threadId":"9568","inReplyTo":"e7bda7770708181237u34253bf1h7c3fe0987d13d3b3@mail.gmail.com","subject":"Re: file disappears after git rebase (missing one commit)","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-08-18T20:01:05Z","receivedAt":"2007-08-18T20:01:05Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 18 Aug 2007, Torgil Svensson wrote:\n>\n> $ git rebase devel\n> First, rewinding head to replay your work on top of it...\n> HEAD is now at 57aa840... Add disk summarize tool (du.exe)\n> Nothing to do.\n\nOk. \"git rebase\" really does believe that there's nothing to do.\n\nThe reason, I think, is that I suspect that the newly added file is a \nbinary file, no? That, in turn, will mean that the *patch* will have no \npatch ID (or rather, it will have an empty patch ID) - which in turn will \nmake it invisible to \"--ignore-if-in-upstream\" if there are already some \n*other* patches that also just adds a binary file (which I think there is: \nI think upstream has \"Add disk summarize tool (du.exe)\" which I assume has \nexactly the same patch fingerprint).\n\nIn other words, \"git rebase\" really is just a series of cherry-picks, but \nit avoids patches that have the same patch ID as something that is already \nupstream. That helps *enormously*, but it so happens that the patch ID's \ndon't work really well for binary diffs.\n\nTry this patch - see if it helps. Totally untested! It will enable patch \nID's on binary diffs too, which should avoid this issue.\n\n\t\tLinus\n\n---\ndiff --git a/patch-ids.c b/patch-ids.c\nindex a288fac..4a3432e 100644\n--- a/patch-ids.c\n+++ b/patch-ids.c\n@@ -122,6 +122,7 @@ int init_patch_ids(struct patch_ids *ids)\n \tmemset(ids, 0, sizeof(*ids));\n \tdiff_setup(&ids->diffopts);\n \tids->diffopts.recursive = 1;\n+\tids->diffopts.binary = 1;\n \tif (diff_setup_done(&ids->diffopts) < 0)\n \t\treturn error(\"diff_setup_done failed\");\n \treturn 0;\n"},{"id":"50982","messageId":"e7bda7770708181329i7a64e613y88187a608c323a07@mail.gmail.com","threadId":"9568","inReplyTo":"alpine.LFD.0.999.0708181247330.30176@woody.linux-foundation.org","subject":"Re: file disappears after git rebase (missing one commit)","fromName":"Torgil Svensson","fromEmail":"torgil.svensson@gmail.com","sentAt":"2007-08-18T20:29:52Z","receivedAt":"2007-08-18T20:29:52Z","isPatch":false,"sender":{"key":"torgil.svensson@gmail.com","avatar":null},"body":"On 8/18/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:\n>\n> The reason, I think, is that I suspect that the newly added file is a\n> binary file, no?\n\nYes, that's correct.\n\n\n> That, in turn, will mean that the *patch* will have no\n> patch ID (or rather, it will have an empty patch ID) - which in turn will\n> make it invisible to \"--ignore-if-in-upstream\" if there are already some\n> *other* patches that also just adds a binary file (which I think there is:\n> I think upstream has \"Add disk summarize tool (du.exe)\" which I assume has\n> exactly the same patch fingerprint).\n\nThe \"du.exe\" is only in the devel branch but there is five other\npatches that meets your criteria.\n\n\n> In other words, \"git rebase\" really is just a series of cherry-picks, but\n> it avoids patches that have the same patch ID as something that is already\n> upstream. That helps *enormously*, but it so happens that the patch ID's\n> don't work really well for binary diffs.\n\nGit cherry-pick seems to work on that particular patch:\n\n$ git cherry-pick b11cf4ce6262a7c3b243e3cfdc70e6b44682cb59\nFinished one cherry-pick.\nCreated commit 92a58d3: Added msmtp.exe SMTP client\n 1 files changed, 0 insertions(+), 0 deletions(-)\n create mode 100644 bin/msmtp.exe\n\n\n> Try this patch - see if it helps. Totally untested! It will enable patch\n> ID's on binary diffs too, which should avoid this issue.\n\nThat didn't help. Same symptom.\n\n//Torgil\n"},{"id":"50983","messageId":"alpine.LFD.0.999.0708181334200.30176@woody.linux-foundation.org","threadId":"9568","inReplyTo":"e7bda7770708181329i7a64e613y88187a608c323a07@mail.gmail.com","subject":"Re: file disappears after git rebase (missing one commit)","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-08-18T20:55:08Z","receivedAt":"2007-08-18T20:55:08Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 18 Aug 2007, Torgil Svensson wrote:\n> \n> > In other words, \"git rebase\" really is just a series of cherry-picks, \n> > but it avoids patches that have the same patch ID as something that is \n> > already upstream. That helps *enormously*, but it so happens that the \n> > patch ID's don't work really well for binary diffs.\n> \n> Git cherry-pick seems to work on that particular patch:\n\nYes, cherry-picking itself works, it's just that \"git rebase\" probably \nwon't even *try* to cherry-pick it because it thinks it is already \napplied.\n\n> > Try this patch - see if it helps. Totally untested! It will enable \n> > patch ID's on binary diffs too, which should avoid this issue.\n> \n> That didn't help. Same symptom.\n\nYeah, I was thinking about the external \"git-patch-id\" program, which \nactually takes the diff and looks at it from there. But \n\"--ignore-if-in-upstream\" does its own binary file testing, and doesn't \nuse the generic diff code at all. \n\nSo the following patch is likely much better..\n\n\t\tLinus\n\n---\n diff.c |    4 ----\n 1 files changed, 0 insertions(+), 4 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 97cc5bc..a7e7671 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2919,10 +2919,6 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)\n \t\t\t\tfill_mmfile(&mf2, p->two) < 0)\n \t\t\treturn error(\"unable to read files to diff\");\n \n-\t\t/* Maybe hash p->two? into the patch id? */\n-\t\tif (diff_filespec_is_binary(p->two))\n-\t\t\tcontinue;\n-\n \t\tlen1 = remove_space(p->one->path, strlen(p->one->path));\n \t\tlen2 = remove_space(p->two->path, strlen(p->two->path));\n \t\tif (p->one->mode == 0)\n"},{"id":"50984","messageId":"e7bda7770708181411v67730b57ibcd8df44695e036f@mail.gmail.com","threadId":"9568","inReplyTo":"alpine.LFD.0.999.0708181334200.30176@woody.linux-foundation.org","subject":"Re: file disappears after git rebase (missing one commit)","fromName":"Torgil Svensson","fromEmail":"torgil.svensson@gmail.com","sentAt":"2007-08-18T21:11:36Z","receivedAt":"2007-08-18T21:11:36Z","isPatch":false,"sender":{"key":"torgil.svensson@gmail.com","avatar":null},"body":"On 8/18/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:\n\n> Yeah, I was thinking about the external \"git-patch-id\" program, which\n> actually takes the diff and looks at it from there. But\n> \"--ignore-if-in-upstream\" does its own binary file testing, and doesn't\n> use the generic diff code at all.\n>\n> So the following patch is likely much better..\n\nThis patch made the difference and solved the issue for me. Thanks\n\n//Torgil\n"},{"id":"50986","messageId":"alpine.LFD.0.999.0708181547400.30176@woody.linux-foundation.org","threadId":"9568","inReplyTo":"e7bda7770708181411v67730b57ibcd8df44695e036f@mail.gmail.com","subject":"Take binary diffs into account for \"git rebase\"","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-08-18T22:52:55Z","receivedAt":"2007-08-18T22:52:55Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nWe used to not generate a patch ID for binary diffs, but that means that \nsome commits may be skipped as being identical to already-applied diffs \nwhen doing a rebase.\n\nSo just delete the code that skips the binary diff. At the very least, \nwe'd want the filenames to be part of the patch ID, but we might also want \nto generate some hash for the binary diff itself too.\n\nThis fixes an issue noticed by Torgil Svensson.\n\nTested-by: Torgil Svensson <torgil.svensson@gmail.com>\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\n\nJunio, you might want to do as the comment says, instead of just hashing \nwhatever random binary patch. Your call.\n\nOn Sat, 18 Aug 2007, Torgil Svensson wrote:\n> \n> This patch made the difference and solved the issue for me. Thanks\n\n diff.c |    4 ----\n 1 files changed, 0 insertions(+), 4 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 97cc5bc..a7e7671 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2919,10 +2919,6 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)\n \t\t\t\tfill_mmfile(&mf2, p->two) < 0)\n \t\t\treturn error(\"unable to read files to diff\");\n \n-\t\t/* Maybe hash p->two? into the patch id? */\n-\t\tif (diff_filespec_is_binary(p->two))\n-\t\t\tcontinue;\n-\n \t\tlen1 = remove_space(p->one->path, strlen(p->one->path));\n \t\tlen2 = remove_space(p->two->path, strlen(p->two->path));\n \t\tif (p->one->mode == 0)\n"}]}