{"thread":{"id":"23070","subject":"[BUG] merge-recursive call in git-am -3 chokes, autocrlf issue?","startedAt":"2010-03-19T00:49:02Z","lastAt":"2010-05-25T05:37:22Z","messageCount":6,"participants":["Thomas Rast","Scott R. Godin","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"137170","messageId":"201003190149.03025.trast@student.ethz.ch","threadId":"23070","inReplyTo":null,"subject":"[BUG] merge-recursive call in git-am -3 chokes, autocrlf issue?","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2010-03-19T00:49:02Z","receivedAt":"2010-03-19T00:49:02Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Hi everyone,\n\nI helped Scott R. \"WebDragon\" Godin on IRC[1] with a bug internal to\ngit-rebase.  It manifests like this:\n\n  $ git rebase --stat develop\n  First, rewinding head to replay your work on top of it...\n   .gitmeta                                           |    2 +-\n   Products/index.php                                 |    1 +\n   index.php                                          |   11 -----\n   res/includes/featured/featured-TEMPLATE.php        |    6 +-\n   res/includes/featured/featured-aerotube.php        |    2 +-\n   res/includes/featured/featured-beltfeederclock.php |    2 +-\n   res/includes/featured/featured-fiberglasstanks.php |    2 +-\n   res/includes/featured/featured-handypolaris2.php   |    2 +-\n   res/includes/featured/featured-hydrotech.php       |    2 +-\n   .../featured/featured-uv_sterilization.php         |    2 +-\n   res/includes/inc_meta.php                          |    1 +\n   res/includes/inc_nav.php                           |   42 ++++++++++++++++---\n   res/java/featurebox.js                             |    2 +-\n   13 files changed, 48 insertions(+), 29 deletions(-)\n  Applying: Begin restyling/content work on new footer\n  Using index info to reconstruct a base tree...\n  Falling back to patching base and 3-way merge...\n  error: Your local changes to 'res/css/stylehome.css' would be overwritten by merge.  Aborting.\n  Please, commit your changes or stash them before you can merge.\n  Failed to merge in the changes.\n  [...]\n\nI don't know much about the merging machinery, but I figured I could\nhelp him poke around, so here's what we gathered:\n\n* He uses 1.7.0.1 on Fedora [2]\n\n* The repo is fairly ordinary except for[3]: setgitperms.perl contrib\n  hooks, core.autocrlf = true\n\n* Editing git-am to use git-merge-resolve instead fixes the issue.\n\n* With GIT_MERGE_VERBOSITY=5 it says [I don't think there's anything\n  useful in there, but who knows]:\n\n    $ git rebase develop\n    First, rewinding head to replay your work on top of it...\n    Applying: Begin restyling/content work on new footer\n    Using index info to reconstruct a base tree...\n    Falling back to patching base and 3-way merge...\n    Merging HEAD with Begin restyling/content work on new footer\n    Merging:\n    9e2793f remove innerbox sizing js, as no longer necessary: matching bg color obviates need for equal sized boxes\n    virtual Begin restyling/content work on new footer\n    found 1 common ancestor(s):\n    virtual cef31479147bd3ba2922c3506ec1c70ee5b22729\n    error: Your local changes to 'res/css/stylehome.css' would be overwritten by merge.  Aborting.\n    Please, commit your changes or stash them before you can merge.\n    fatal: merging of trees fe2928d5311cfcf5e668b13cd85608589928f848 and b2ad8208510f713acf1be1c9e62856c51175c6d0 failed\n    Failed to merge in the changes.\n\n* Immediately before the git-merge-{recursive,resolve} call, the\n  following outputs may be relevant:\n\n-- 8< -- git diff-files --patch-with-raw\n:100644 100644 c1ff94058b86004de4dc1693b6dcd2fccfb28d52 0000000000000000000000000000000000000000 M     res/css/stylehome.css\n:100644 100644 b653183cc466262996c319ce21fbc29d1164b942 0000000000000000000000000000000000000000 M res/includes/inc_footerhome.php\n:100644 100644 29182f14b8e54525649685cdfe718a9a4204ab0e 0000000000000000000000000000000000000000 M       res/includes/inc_meta.php\n:100644 100644 5910568e733b631d3e3cc73b74ee89a71e4140a2 0000000000000000000000000000000000000000 M     res/includes/inc_validate.php\n \ndiff --git a/res/css/stylehome.css b/res/css/stylehome.css\ndiff --git a/res/includes/inc_footerhome.php b/res/includes/inc_footerhome.php\ndiff --git a/res/includes/inc_meta.php b/res/includes/inc_meta.php\ndiff --git a/res/includes/inc_validate.php b/res/includes/inc_validate.php\n-- >8 --\n\n-- 8< -- git diff-index --patch-with-raw HEAD\n:100644 100644 c1ff94058b86004de4dc1693b6dcd2fccfb28d52 0000000000000000000000000000000000000000 M    res/css/stylehome.css\n:100644 100644 b653183cc466262996c319ce21fbc29d1164b942 0000000000000000000000000000000000000000 M res/includes/inc_footerhome.php\n:100644 100644 29182f14b8e54525649685cdfe718a9a4204ab0e 0000000000000000000000000000000000000000 M       res/includes/inc_meta.php\n:100644 100644 5910568e733b631d3e3cc73b74ee89a71e4140a2 0000000000000000000000000000000000000000 M     res/includes/inc_validate.php\n \ndiff --git a/res/css/stylehome.css b/res/css/stylehome.css\ndiff --git a/res/includes/inc_footerhome.php b/res/includes/inc_footerhome.php\ndiff --git a/res/includes/inc_meta.php b/res/includes/inc_meta.php\ndiff --git a/res/includes/inc_validate.php b/res/includes/inc_validate.php\n-- >8 --\n\nNot sure if it's relevant, but the differences shown here and in the\ndiffstat for the rebased patch above both list\n'res/includes/inc_meta.php'.\n\n[I only just noticed that we probably should have used diff-index\n--cached for the second snippet; I hope that this doesn't make the\ndata worthless...]\n\nWe tried the differences between $base_tree and {$his_tree,HEAD} too,\nbut they were too large to be practical.\n\n\nSince there is some difference between files and index, but neither\nthe modes nor the contents actually show any, my best guess is that\nit's autocrlf's fault.  Then again, who knows.\n\nI did try a theory, but it worked well[4] so that's not it:\n\n  * base has a file foo\n  * side..master recodes foo to crlf and changes bar\n  * master..side changes bar differently to trigger the 3-way logic\n  Then rebase side on master.\n\nIn code:\n\n  git init foo\n  cd foo\n  seq 1 20 > foo\n  git add foo\n  git commit -m initial\n  seq 100 120 > bar\n  git add bar\n  git commit -m bar\n  seq 1 20 | sed 's/$/\\r/' > foo\n  git add foo\n  git commit -m crlf\n  sed -i 's/101/a/;s/102/b/;s/103/c/' bar\n  git add bar\n  git commit -m 'change bar'\n  git checkout -b side HEAD~2\n  sed -i 's/105/d/;s/106/e/;s/107/f/' bar\n  git add bar\n  git commit -m 'change bar differently'\n  git config core.autocrlf true\n  git rebase master\n\n\n[1] http://colabti.org/irclogger/irclogger_log/git?date=2010-03-18#l3682\n[2] http://colabti.org/irclogger/irclogger_log/git?date=2010-03-18#l3818\n[3] http://colabti.org/irclogger/irclogger_log/git?date=2010-03-18#l3898\n[4] modulo the slight problem that I can't get rid of the false dirty\n    state of foo after that, but if I understood autocrlf right that's\n    expected?\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"138346","messageId":"hp2jkj$mu0$1@dough.gmane.org","threadId":"23070","inReplyTo":"201003190149.03025.trast@student.ethz.ch","subject":"Re: [BUG] [RESOLVED] merge-recursive call in git-am -3 chokes, autocrlf issue?","fromName":"Scott R. Godin","fromEmail":"scottg.wp-hackers@mhg2.com","sentAt":"2010-04-01T17:03:15Z","receivedAt":"2010-04-01T17:03:15Z","isPatch":false,"sender":{"key":"scottg.wp-hackers@mhg2.com","avatar":"https://gravatar.com/avatar/766980733f46a32860153479ca94372e4fff5c4632e8586dbbfb143051d231ce?d=mp&s=160"},"body":"On 03/18/2010 08:49 PM, Thomas Rast wrote:\n> Hi everyone,\n>\n> I helped Scott R. \"WebDragon\" Godin on IRC[1] with a bug internal to\n> git-rebase.  It manifests like this:\n\nIt turns out it's not actually a bug in git-am/git-rebase.\n\nwhile changing merge-recursive to merge-resolve in git-am solved the \nplain git rebase branch1 branch2 issue, I still ran into trouble when \ntrying to do a rebase -i so I could squash a commit or two together.\n\nIlari stepped up to the plate [1] and it was quickly determined that the \nworking copy cache is somehow left dirty after checkout.\n\ndoener stepped back in, and while pulling the hooks out that used \nsetgitperms.perl, temporarily, it became obvious that one of them was \nthe culprit, and further testing at this point would allow the rebase to \ncontinue, albiet without permissions being set. using git update-index \n--refresh allowed things to go on normally, when added to the \npost-checkout hook calling setgitperms.perl [2].\n\nSo my recommendation at this point is to patch the instructions within \nsetgitperms.perl to add 'git update-index --refresh' to the end of the \npost-checkout hook.\n\nI've since reset git-am to use recursive again (instead of resolve) and \ndone several rebases (both with and without -i) and all seems well and \nnormal, and this has made my day.\n\npatch follows:\n\n--8<--\n\nSubject: [PATCH] revise setgitperms.perl hook script description to fix \nrebase issue\n\nadd index-refresh command to post-checkout post-merge script hooks to \nkeep working tree from being marked dirty during a rebase action\n---\n  contrib/hooks/setgitperms.perl |    1 +\n  1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/contrib/hooks/setgitperms.perl b/contrib/hooks/setgitperms.perl\nindex a577ad0..286835d 100644\n--- a/contrib/hooks/setgitperms.perl\n+++ b/contrib/hooks/setgitperms.perl\n@@ -17,6 +17,7 @@\n  #      #!/bin/sh\n  #     SUBDIRECTORY_OK=1 . git-sh-setup\n  #     $GIT_DIR/hooks/setgitperms.perl -w\n+#     git update-index --refresh\n  #\n  use strict;\n  use Getopt::Long;\n\n--8<--\n\n[1] http://colabti.org/irclogger/irclogger_log/git?date=2010-03-31#l1556\n[2] http://colabti.org/irclogger/irclogger_log/git?date=2010-03-31#l1743\n\n-- \n(please respond to the list as opposed to my email box directly,\nunless you are supplying private information you don't want public\non the list)\n"},{"id":"138347","messageId":"7vbpe3qe09.fsf@alter.siamese.dyndns.org","threadId":"23070","inReplyTo":"hp2jkj$mu0$1@dough.gmane.org","subject":"Re: [BUG] [RESOLVED] merge-recursive call in git-am -3 chokes, autocrlf issue?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-01T17:27:50Z","receivedAt":"2010-04-01T17:27:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Scott R. Godin\" <scottg.wp-hackers@mhg2.com> writes:\n\n> So my recommendation at this point is to patch the instructions within\n> setgitperms.perl to add 'git update-index --refresh' to the end of the\n> post-checkout hook.\n>\n> I've since reset git-am to use recursive again (instead of resolve)\n> and done several rebases (both with and without -i) and all seems well\n> and normal, and this has made my day.\n\nAhh.  If you muck with work tree files and the index in pre-commit,\npost-merge, or post-checkout hook (especially if you make an up-to-date\nwork tree file stat-dirty), I can imagine that you would need to \"refresh\"\nso that unchanged paths would appear unchanged in the index not to confuse\nyour caller.\n\nI however think the patch probably \"fixes\" the issue at the worst point.\nWouldn't either of these alternatives be better?\n\n (1) Perhaps the caller of \"pre-commit/post-merge/post-checkout\" hook\n     should instead refresh the index when the hook returns, _iff_ we\n     expect that majority of these hooks are used to munge the work tree\n     or the index; or\n\n (2) Because you already established that setgitperms script is the\n     culprit that leaves the index unrefreshed, instead of forcing all the\n     callers of the script, it should do the refresh for its callers\n     before it exits.\n"},{"id":"140942","messageId":"4BE095D9.6090403@mhg2.com","threadId":"23070","inReplyTo":"7vbpe3qe09.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG] [RESOLVED] merge-recursive call in git-am -3 chokes, autocrlf issue?","fromName":"Scott R. Godin","fromEmail":"scottg.wp-hackers@mhg2.com","sentAt":"2010-05-04T21:47:05Z","receivedAt":"2010-05-04T21:47:05Z","isPatch":false,"sender":{"key":"scottg.wp-hackers@mhg2.com","avatar":"https://gravatar.com/avatar/766980733f46a32860153479ca94372e4fff5c4632e8586dbbfb143051d231ce?d=mp&s=160"},"body":"On 04/01/2010 01:27 PM, Junio C Hamano wrote:\n> I however think the patch probably \"fixes\" the issue at the worst point.\n> Wouldn't either of these alternatives be better?\n>\n>   (1) Perhaps the caller of \"pre-commit/post-merge/post-checkout\" hook\n>       should instead refresh the index when the hook returns, _iff_ we\n>       expect that majority of these hooks are used to munge the work tree\n>       or the index; or\n>\n>   (2) Because you already established that setgitperms script is the\n>       culprit that leaves the index unrefreshed, instead of forcing all the\n>       callers of the script, it should do the refresh for its callers\n>       before it exits.\n\nGood call.\n\nI talked it over with Todd Zullinger and he came up with the following \npatch, which I tested on my end to my complete satisfaction, rebases and \nmerges go smoothly.\n\nit's still necessary however, to --no-commit on merges so that you can \nfix the permissions before your umask blots them out and they wind up in \nthe commit and saved in the gitmeta file\n\nAs a result, my usual modus operandi currently is:\n\tgit checkout master\n\tgit merge --no-ff --no-commit develop\n\tfind . -perm 0600 -or -perm 0700 |grep -v .git/\n\t...fix perms back to where they should be\n\tgit add -A\n\tgit commit\n\nwhich is somewhat less than optimal, but otherwise setgitperms.perl is \ndoing what it should.\n\nRevised patch follows:\n--8<--\nSubject: [PATCH] Revise setgitperms.perl to fix dirty tree problem when \nrebasing/merging\n\nreference:\nhttp://comments.gmane.org/gmane.comp.version-control.git/142548\n\nNote that it will be necessary to not only copy the changed\nsetgitperms.perl from /usr/share/git-core/contrib/hooks/ to\n/usr/share/git-core/templates/hooks/ but additionally every git\nrepository you currently use this script with, will also need to be\nupdated with the new version. This process is regrettably not automatic \nsimply\nbecause git was updated on your system.\n---\n  contrib/hooks/setgitperms.perl |    4 ++++\n  1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/contrib/hooks/setgitperms.perl b/contrib/hooks/setgitperms.perl\nindex a577ad0..e571560 100644\n--- a/contrib/hooks/setgitperms.perl\n+++ b/contrib/hooks/setgitperms.perl\n@@ -91,6 +91,10 @@ if ($write_mode) {\n         }\n      }\n      close IN;\n+\n+    # Make sure the index isn't left dirty\n+    # http://comments.gmane.org/gmane.comp.version-control.git/142548\n+    system(\"git update-index --refresh\");\n  }\n  elsif ($read_mode) {\n      # Handle merge conflicts in the .gitperms file\n-- \n1.7.1\n\n--8<--\n\n-- \n(please respond to the list as opposed to my email box directly,\nunless you are supplying private information you don't want public\non the list)\n"},{"id":"142209","messageId":"htebs9$ann$1@dough.gmane.org","threadId":"23070","inReplyTo":"4BE095D9.6090403@mhg2.com","subject":"[PATCH] setgitperms.perl dirty index problem (was Re: [BUG] [RESOLVED] merge-recursive call in git-am -3 chokes, autocrlf issue?)","fromName":"Scott R. Godin","fromEmail":"scottg.wp-hackers@mhg2.com","sentAt":"2010-05-24T17:09:28Z","receivedAt":"2010-05-24T17:09:28Z","isPatch":true,"sender":{"key":"scottg.wp-hackers@mhg2.com","avatar":"https://gravatar.com/avatar/766980733f46a32860153479ca94372e4fff5c4632e8586dbbfb143051d231ce?d=mp&s=160"},"body":"On 05/04/2010 05:47 PM, Scott R. Godin wrote:\n\nHadn't seen any response to this so I'm reposting in the hopes that this \nwill make it to the next release.\n\n> On 04/01/2010 01:27 PM, Junio C Hamano wrote:\n>> I however think the patch probably \"fixes\" the issue at the worst point.\n>> Wouldn't either of these alternatives be better?\n>>\n>> (1) Perhaps the caller of \"pre-commit/post-merge/post-checkout\" hook\n>> should instead refresh the index when the hook returns, _iff_ we\n>> expect that majority of these hooks are used to munge the work tree\n>> or the index; or\n>>\n>> (2) Because you already established that setgitperms script is the\n>> culprit that leaves the index unrefreshed, instead of forcing all the\n>> callers of the script, it should do the refresh for its callers\n>> before it exits.\n>\n> Good call.\n>\n> I talked it over with Todd Zullinger and he came up with the following\n> patch, which I tested on my end to my complete satisfaction, rebases and\n> merges go smoothly.\n>\n> it's still necessary however, to --no-commit on merges so that you can\n> fix the permissions before your umask blots them out and they wind up in\n> the commit and saved in the gitmeta file\n>\n> As a result, my usual modus operandi currently is:\n> git checkout master\n> git merge --no-ff --no-commit develop\n> find . -perm 0600 -or -perm 0700 |grep -v .git/\n> ...fix perms back to where they should be\n> git add -A\n> git commit\n>\n> which is somewhat less than optimal, but otherwise setgitperms.perl is\n> doing what it should.\n>\n> Revised patch follows:\n> --8<--\n> Subject: [PATCH] Revise setgitperms.perl to fix dirty tree problem when\n> rebasing/merging\n>\n> reference:\n> http://comments.gmane.org/gmane.comp.version-control.git/142548\n>\n> Note that it will be necessary to not only copy the changed\n> setgitperms.perl from /usr/share/git-core/contrib/hooks/ to\n> /usr/share/git-core/templates/hooks/ but additionally every git\n> repository you currently use this script with, will also need to be\n> updated with the new version. This process is regrettably not automatic\n> simply\n> because git was updated on your system.\n> ---\n> contrib/hooks/setgitperms.perl | 4 ++++\n> 1 files changed, 4 insertions(+), 0 deletions(-)\n>\n> diff --git a/contrib/hooks/setgitperms.perl\n> b/contrib/hooks/setgitperms.perl\n> index a577ad0..e571560 100644\n> --- a/contrib/hooks/setgitperms.perl\n> +++ b/contrib/hooks/setgitperms.perl\n> @@ -91,6 +91,10 @@ if ($write_mode) {\n> }\n> }\n> close IN;\n> +\n> + # Make sure the index isn't left dirty\n> + # http://comments.gmane.org/gmane.comp.version-control.git/142548\n> + system(\"git update-index --refresh\");\n> }\n> elsif ($read_mode) {\n> # Handle merge conflicts in the .gitperms file\n\n\n-- \n(please respond to the list as opposed to my email box directly,\nunless you are supplying private information you don't want public\non the list)\n"},{"id":"142234","messageId":"7v632cwnhp.fsf@alter.siamese.dyndns.org","threadId":"23070","inReplyTo":"htebs9$ann$1@dough.gmane.org","subject":"Re: [PATCH] setgitperms.perl dirty index problem","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-05-25T05:37:22Z","receivedAt":"2010-05-25T05:37:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Scott R. Godin\" <scottg.wp-hackers@mhg2.com> writes:\n\n> On 05/04/2010 05:47 PM, Scott R. Godin wrote:\n>\n> Hadn't seen any response to this so I'm reposting in the hopes that\n> this will make it to the next release.\n\nCould you fix the \"format=flowed\" and also sign-off the patch?\n"}]}