{"thread":{"id":"7388","subject":"[PATCH] Use diff* with --exit-code in git-am, git-rebase and git-merge-ours","startedAt":"2007-03-25T00:56:13Z","lastAt":"2007-03-25T09:33:45Z","messageCount":3,"participants":["Alex Riesen","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"37905","messageId":"20070325005613.GE11507@steel.home","threadId":"7388","inReplyTo":null,"subject":"[PATCH] Use diff* with --exit-code in git-am, git-rebase and git-merge-ours","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-03-25T00:56:13Z","receivedAt":"2007-03-25T00:56:13Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"This simplifies the shell code, reduces its memory footprint, and\nspeeds things up. The performance improvements should be noticable\nwhen git-rebase works on big commits.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n git-am.sh         |   18 +++++++-----------\n git-merge-ours.sh |    2 +-\n git-rebase.sh     |   10 ++++------\n 3 files changed, 12 insertions(+), 18 deletions(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex 88af8dd..e69ecbf 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -408,12 +408,10 @@ do\n \t\t# trust what the user has in the index file and the\n \t\t# working tree.\n \t\tresolved=\n-\t\tchanged=\"$(git-diff-index --cached --name-only HEAD)\"\n-\t\tif test '' = \"$changed\"\n-\t\tthen\n+\t\tgit-diff-index --quiet --cached HEAD && {\n \t\t\techo \"No changes - did you forget to use 'git add'?\"\n \t\t\tstop_here_user_resolve $this\n-\t\tfi\n+\t\t}\n \t\tunmerged=$(git-ls-files -u)\n \t\tif test -n \"$unmerged\"\n \t\tthen\n@@ -435,13 +433,11 @@ do\n \t\tthen\n \t\t    # Applying the patch to an earlier tree and merging the\n \t\t    # result may have produced the same tree as ours.\n-\t\t    changed=\"$(git-diff-index --cached --name-only HEAD)\"\n-\t\t    if test '' = \"$changed\"\n-\t\t    then\n-\t\t\t    echo No changes -- Patch already applied.\n-\t\t\t    go_next\n-\t\t\t    continue\n-\t\t    fi\n+\t\t    git-diff-index --quiet --cached HEAD && {\n+\t\t\techo No changes -- Patch already applied.\n+\t\t\tgo_next\n+\t\t\tcontinue\n+\t\t    }\n \t\t    # clear apply_status -- we have successfully merged.\n \t\t    apply_status=0\n \t\tfi\ndiff --git a/git-merge-ours.sh b/git-merge-ours.sh\nindex 4f3d053..2b6a5c0 100755\n--- a/git-merge-ours.sh\n+++ b/git-merge-ours.sh\n@@ -9,6 +9,6 @@\n # because the current index is what we will be committing as the\n # merge result.\n \n-test \"$(git-diff-index --cached --name-status HEAD)\" = \"\" || exit 2\n+git-diff-index --quiet --cached HEAD || exit 2\n \n exit 0\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex aadd580..1d96f32 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -59,7 +59,7 @@ continue_merge () {\n \t\tdie \"$RESOLVEMSG\"\n \tfi\n \n-\tif test -n \"`git-diff-index HEAD`\"\n+\tif ! git-diff-index --quiet HEAD\n \tthen\n \t\tif ! git-commit -C \"`cat $dotest/current`\"\n \t\tthen\n@@ -124,13 +124,11 @@ while case \"$#\" in 0) break ;; esac\n do\n \tcase \"$1\" in\n \t--continue)\n-\t\tdiff=$(git-diff-files)\n-\t\tcase \"$diff\" in\n-\t\t?*)\techo \"You must edit all merge conflicts and then\"\n+\t\tgit-diff-files --quiet || {\n+\t\t\techo \"You must edit all merge conflicts and then\"\n \t\t\techo \"mark them as resolved using git update-index\"\n \t\t\texit 1\n-\t\t\t;;\n-\t\tesac\n+\t\t}\n \t\tif test -d \"$dotest\"\n \t\tthen\n \t\t\tprev_head=\"`cat $dotest/prev_head`\"\n-- \n1.5.1.rc1.63.g59cc5\n"},{"id":"37911","messageId":"7vwt15c150.fsf@assigned-by-dhcp.cox.net","threadId":"7388","inReplyTo":"20070325005613.GE11507@steel.home","subject":"Re: [PATCH] Use diff* with --exit-code in git-am, git-rebase and git-merge-ours","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-25T06:01:31Z","receivedAt":"2007-03-25T06:01:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen <raa.lkml@gmail.com> writes:\n\n> This simplifies the shell code, reduces its memory footprint, and\n> speeds things up. The performance improvements should be noticable\n> when git-rebase works on big commits.\n\nThanks.\n\nUnlike some optimizations that tilts the tradeoffs to favor\noften-used case by penalizing the less often-used case, this\nchange does not penalize any case while optimizing some cases,\nso there is nothing we are losing.  I'll take the patch for this\nreason.\n\nHowever, I have a few comments.\n\nHave you run any benchmark?  I suspect \"and speeds things up\" is\na gross overstatement.  A qualified \"SOMETIMES speeds things up\"\nwould be a more honest thing to say.\n\nBoth of the changes to git-am are definitely good.  Under normal\nusage, we expect that we would see some diff, and the optimization\napplies to that normal case.\n\n> diff --git a/git-merge-ours.sh b/git-merge-ours.sh\n> index 4f3d053..2b6a5c0 100755\n> --- a/git-merge-ours.sh\n> +++ b/git-merge-ours.sh\n> @@ -9,6 +9,6 @@\n>  # because the current index is what we will be committing as the\n>  # merge result.\n>  \n> -test \"$(git-diff-index --cached --name-status HEAD)\" = \"\" || exit 2\n> +git-diff-index --quiet --cached HEAD || exit 2\n>  \n>  exit 0\n\nI think this does not change the performance profile in the real\nlife.  When the user uses the command correctly, we expect that\nindex and HEAD to match, and in that case the diff would take\nthe same amount of processing with or without --quiet.  The\nchange does not penalize the opposite case, so it is not a wrong\nthing to do, but this is an optimization to error out early.\n\nThe first change to git-rebase optimizes the case to handle\n\"still to be adopted upstream\" case (\"upstream swallowed our\nchange\" case would perform the same as before), so the\noptimization would be more visible by people with slower\nupstream and/or people patches of lessor quality that are\nrejected by upstream often.  In other words, the amount of\nspeeding up by this change really depends on the user (there is\nno slowdown for anybody so that is Ok).\n\nThe other optimizes the \"erroring out early\" uninteresting case.\n"},{"id":"37928","messageId":"20070325093345.GA2563@steel.home","threadId":"7388","inReplyTo":"7vwt15c150.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Use diff* with --exit-code in git-am, git-rebase and git-merge-ours","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-03-25T09:33:45Z","receivedAt":"2007-03-25T09:33:45Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Junio C Hamano, Sun, Mar 25, 2007 08:01:31 +0200:\n> However, I have a few comments.\n> \n> Have you run any benchmark?  I suspect \"and speeds things up\" is\n\nI didn't benchmark any of these, so I can't backup the statement with\nthe number but...\n\n> a gross overstatement.  A qualified \"SOMETIMES speeds things up\"\n> would be a more honest thing to say.\n\nit must speed things up, if only for removed shell-side efforts to\nredirect the output and store it in heap. To overstatement it I should\nhave said: \"greatly speeds things up\".\n"}]}