{"thread":{"id":"39323","subject":"[PATCH v3 0/9] Improve git-pull test coverage","startedAt":"2015-05-13T09:08:47Z","lastAt":"2015-05-18T13:09:47Z","messageCount":25,"participants":["Paul Tan","Matthieu Moy","Junio C Hamano","Michael Blume"],"isPatch":true,"patchVersion":3,"patchTotal":9},"messages":[{"id":"261145","messageId":"1431508136-15313-1-git-send-email-pyokagan@gmail.com","threadId":"39323","inReplyTo":null,"subject":"[PATCH v3 0/9] Improve git-pull test coverage","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-13T09:08:47Z","receivedAt":"2015-05-13T09:08:47Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"This is a re-roll of [1]. This series depends on jc/merge.\n\nThis patch series improves test coverage of git-pull.sh, and is part of my\nGSoC project to rewrite git-pull into a builtin. Improving test coverage\nhelps to prevent regressions that could occur due to the rewrite.\n\nThis re-roll includes the following changes across all patches:\n\n* Added clean-up patch to fix file content comparisons in t5520\n\n* All file content comparisons are quoted to prevent word splitting and use\n  verbose() to provide better error messages.\n\n* Style: References to commits are standardized with their summary and date.\n\n* Failing tests have been removed. This series will be solely used to extend\n  test coverage of already working functionality. Bugs will be fixed in their\n  own patch series'.\n\n* Style: Instead of redirecting stderr to a file named \"out\", redirect it to a\n  file named \"err\".\n\nThanks Eric, Torsten, Junio, Stefan, Dscho and Johannes for your reviews last\nround.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/268510/\n\nPaul Tan (9):\n  t5520: fixup file contents comparisons\n  t5520: ensure origin refs are updated\n  t5520: test no merge candidates cases\n  t5520: test for failure if index has unresolved entries\n  t5520: test work tree fast-forward when fetch updates head\n  t5520: test --rebase with multiple branches\n  t5520: test --rebase failure on unborn branch with index\n  t5521: test --dry-run does not make any changes\n  t5520: check reflog action in fast-forward merge\n\n t/t5520-pull.sh         | 214 ++++++++++++++++++++++++++++++++++++++----------\n t/t5521-pull-options.sh |  13 +++\n 2 files changed, 184 insertions(+), 43 deletions(-)\n\n-- \n2.1.4\n"},{"id":"261146","messageId":"1431508136-15313-2-git-send-email-pyokagan@gmail.com","threadId":"39323","inReplyTo":"1431508136-15313-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v3 1/9] t5520: fixup file contents comparisons","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-13T09:08:48Z","receivedAt":"2015-05-13T09:08:48Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Many tests in t5520 used the following to test the contents of files:\n\n\ttest `cat file` = expected\n\nor\n\n\ttest $(cat file) = expected\n\nThese 2 forms, however, will be affected by field splitting and,\ndepending on the value of $IFS, may be split into multiple arguments,\nmaking the test fail in mysterious ways.\n\nReplace the above 2 forms with:\n\n\tverbose test \"$(cat file)\" = expected\n\nas quoting the command substitution will prevent field splitting, and\nthe verbose function will print the failed test command on failure for\neasier debugging.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n* This is a new patch\n\n t/t5520-pull.sh | 84 ++++++++++++++++++++++++++++-----------------------------\n 1 file changed, 42 insertions(+), 42 deletions(-)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 7efd45b..20ad373 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -81,7 +81,7 @@ test_expect_success 'pulling into void must not create an octopus' '\n \t(\n \t\tcd cloned-octopus &&\n \t\ttest_must_fail git pull .. master master &&\n-\t\t! test -f file\n+\t\tverbose test ! -f file\n \t)\n '\n \n@@ -93,9 +93,9 @@ test_expect_success 'test . as a remote' '\n \techo updated >file &&\n \tgit commit -a -m updated &&\n \tgit checkout copy &&\n-\ttest `cat file` = file &&\n+\tverbose test \"$(cat file)\" = file &&\n \tgit pull &&\n-\ttest `cat file` = updated\n+\tverbose test \"$(cat file)\" = updated\n '\n \n test_expect_success 'the default remote . should not break explicit pull' '\n@@ -104,9 +104,9 @@ test_expect_success 'the default remote . should not break explicit pull' '\n \tgit commit -a -m modified &&\n \tgit checkout copy &&\n \tgit reset --hard HEAD^ &&\n-\ttest `cat file` = file &&\n+\tverbose test \"$(cat file)\" = file &&\n \tgit pull . second &&\n-\ttest `cat file` = modified\n+\tverbose test \"$(cat file)\" = modified\n '\n \n test_expect_success '--rebase' '\n@@ -119,23 +119,23 @@ test_expect_success '--rebase' '\n \tgit commit -m \"new file\" &&\n \tgit tag before-rebase &&\n \tgit pull --rebase . copy &&\n-\ttest $(git rev-parse HEAD^) = $(git rev-parse copy) &&\n-\ttest new = $(git show HEAD:file2)\n+\tverbose test \"$(git rev-parse HEAD^)\" = \"$(git rev-parse copy)\" &&\n+\tverbose test new = \"$(git show HEAD:file2)\"\n '\n test_expect_success 'pull.rebase' '\n \tgit reset --hard before-rebase &&\n \ttest_config pull.rebase true &&\n \tgit pull . copy &&\n-\ttest $(git rev-parse HEAD^) = $(git rev-parse copy) &&\n-\ttest new = $(git show HEAD:file2)\n+\tverbose test \"$(git rev-parse HEAD^)\" = \"$(git rev-parse copy)\" &&\n+\tverbose test new = \"$(git show HEAD:file2)\"\n '\n \n test_expect_success 'branch.to-rebase.rebase' '\n \tgit reset --hard before-rebase &&\n \ttest_config branch.to-rebase.rebase true &&\n \tgit pull . copy &&\n-\ttest $(git rev-parse HEAD^) = $(git rev-parse copy) &&\n-\ttest new = $(git show HEAD:file2)\n+\tverbose test \"$(git rev-parse HEAD^)\" = \"$(git rev-parse copy)\" &&\n+\tverbose test new = \"$(git show HEAD:file2)\"\n '\n \n test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '\n@@ -143,8 +143,8 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '\n \ttest_config pull.rebase true &&\n \ttest_config branch.to-rebase.rebase false &&\n \tgit pull . copy &&\n-\ttest $(git rev-parse HEAD^) != $(git rev-parse copy) &&\n-\ttest new = $(git show HEAD:file2)\n+\tverbose test \"$(git rev-parse HEAD^)\" != \"$(git rev-parse copy)\" &&\n+\tverbose test new = \"$(git show HEAD:file2)\"\n '\n \n # add a feature branch, keep-merge, that is merged into master, so the\n@@ -163,33 +163,33 @@ test_expect_success 'pull.rebase=false create a new merge commit' '\n \tgit reset --hard before-preserve-rebase &&\n \ttest_config pull.rebase false &&\n \tgit pull . copy &&\n-\ttest $(git rev-parse HEAD^1) = $(git rev-parse before-preserve-rebase) &&\n-\ttest $(git rev-parse HEAD^2) = $(git rev-parse copy) &&\n-\ttest file3 = $(git show HEAD:file3.t)\n+\tverbose test \"$(git rev-parse HEAD^1)\" = \"$(git rev-parse before-preserve-rebase)\" &&\n+\tverbose test \"$(git rev-parse HEAD^2)\" = \"$(git rev-parse copy)\" &&\n+\tverbose test file3 = \"$(git show HEAD:file3.t)\"\n '\n \n test_expect_success 'pull.rebase=true flattens keep-merge' '\n \tgit reset --hard before-preserve-rebase &&\n \ttest_config pull.rebase true &&\n \tgit pull . copy &&\n-\ttest $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n-\ttest file3 = $(git show HEAD:file3.t)\n+\tverbose test \"$(git rev-parse HEAD^^)\" = \"$(git rev-parse copy)\" &&\n+\tverbose test file3 = \"$(git show HEAD:file3.t)\"\n '\n \n test_expect_success 'pull.rebase=1 is treated as true and flattens keep-merge' '\n \tgit reset --hard before-preserve-rebase &&\n \ttest_config pull.rebase 1 &&\n \tgit pull . copy &&\n-\ttest $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n-\ttest file3 = $(git show HEAD:file3.t)\n+\tverbose test \"$(git rev-parse HEAD^^)\" = \"$(git rev-parse copy)\" &&\n+\tverbose test file3 = \"$(git show HEAD:file3.t)\"\n '\n \n test_expect_success 'pull.rebase=preserve rebases and merges keep-merge' '\n \tgit reset --hard before-preserve-rebase &&\n \ttest_config pull.rebase preserve &&\n \tgit pull . copy &&\n-\ttest $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n-\ttest $(git rev-parse HEAD^2) = $(git rev-parse keep-merge)\n+\tverbose test \"$(git rev-parse HEAD^^)\" = \"$(git rev-parse copy)\" &&\n+\tverbose test \"$(git rev-parse HEAD^2)\" = \"$(git rev-parse keep-merge)\"\n '\n \n test_expect_success 'pull.rebase=invalid fails' '\n@@ -202,25 +202,25 @@ test_expect_success '--rebase=false create a new merge commit' '\n \tgit reset --hard before-preserve-rebase &&\n \ttest_config pull.rebase true &&\n \tgit pull --rebase=false . copy &&\n-\ttest $(git rev-parse HEAD^1) = $(git rev-parse before-preserve-rebase) &&\n-\ttest $(git rev-parse HEAD^2) = $(git rev-parse copy) &&\n-\ttest file3 = $(git show HEAD:file3.t)\n+\tverbose test \"$(git rev-parse HEAD^1)\" = \"$(git rev-parse before-preserve-rebase)\" &&\n+\tverbose test \"$(git rev-parse HEAD^2)\" = \"$(git rev-parse copy)\" &&\n+\tverbose test file3 = \"$(git show HEAD:file3.t)\"\n '\n \n test_expect_success '--rebase=true rebases and flattens keep-merge' '\n \tgit reset --hard before-preserve-rebase &&\n \ttest_config pull.rebase preserve &&\n \tgit pull --rebase=true . copy &&\n-\ttest $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n-\ttest file3 = $(git show HEAD:file3.t)\n+\tverbose test \"$(git rev-parse HEAD^^)\" = \"$(git rev-parse copy)\" &&\n+\tverbose test file3 = \"$(git show HEAD:file3.t)\"\n '\n \n test_expect_success '--rebase=preserve rebases and merges keep-merge' '\n \tgit reset --hard before-preserve-rebase &&\n \ttest_config pull.rebase true &&\n \tgit pull --rebase=preserve . copy &&\n-\ttest $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n-\ttest $(git rev-parse HEAD^2) = $(git rev-parse keep-merge)\n+\tverbose test \"$(git rev-parse HEAD^^)\" = \"$(git rev-parse copy)\" &&\n+\tverbose test \"$(git rev-parse HEAD^2)\" = \"$(git rev-parse keep-merge)\"\n '\n \n test_expect_success '--rebase=invalid fails' '\n@@ -232,8 +232,8 @@ test_expect_success '--rebase overrides pull.rebase=preserve and flattens keep-m\n \tgit reset --hard before-preserve-rebase &&\n \ttest_config pull.rebase preserve &&\n \tgit pull --rebase . copy &&\n-\ttest $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n-\ttest file3 = $(git show HEAD:file3.t)\n+\tverbose test \"$(git rev-parse HEAD^^)\" = \"$(git rev-parse copy)\" &&\n+\tverbose test file3 = \"$(git show HEAD:file3.t)\"\n '\n \n test_expect_success '--rebase with rebased upstream' '\n@@ -249,8 +249,8 @@ test_expect_success '--rebase with rebased upstream' '\n \tgit commit -m to-rebase file2 &&\n \tgit tag to-rebase-orig &&\n \tgit pull --rebase me copy &&\n-\ttest \"conflicting modification\" = \"$(cat file)\" &&\n-\ttest file = $(cat file2)\n+\tverbose test \"conflicting modification\" = \"$(cat file)\" &&\n+\tverbose test file = \"$(cat file2)\"\n \n '\n \n@@ -260,8 +260,8 @@ test_expect_success '--rebase with rebased default upstream' '\n \tgit checkout --track -b to-rebase2 me/copy &&\n \tgit reset --hard to-rebase-orig &&\n \tgit pull --rebase &&\n-\ttest \"conflicting modification\" = \"$(cat file)\" &&\n-\ttest file = $(cat file2)\n+\tverbose test \"conflicting modification\" = \"$(cat file)\" &&\n+\tverbose test file = \"$(cat file2)\"\n \n '\n \n@@ -282,7 +282,7 @@ test_expect_success 'pull --rebase dies early with dirty working directory' '\n \n \tgit checkout to-rebase &&\n \tgit update-ref refs/remotes/me/copy copy^ &&\n-\tCOPY=$(git rev-parse --verify me/copy) &&\n+\tCOPY=\"$(git rev-parse --verify me/copy)\" &&\n \tgit rebase --onto $COPY copy &&\n \ttest_config branch.to-rebase.remote me &&\n \ttest_config branch.to-rebase.merge refs/heads/copy &&\n@@ -290,10 +290,10 @@ test_expect_success 'pull --rebase dies early with dirty working directory' '\n \techo dirty >> file &&\n \tgit add file &&\n \ttest_must_fail git pull &&\n-\ttest $COPY = $(git rev-parse --verify me/copy) &&\n+\tverbose test \"$COPY\" = \"$(git rev-parse --verify me/copy)\" &&\n \tgit checkout HEAD -- file &&\n \tgit pull &&\n-\ttest $COPY != $(git rev-parse --verify me/copy)\n+\tverbose test \"$COPY\" != \"$(git rev-parse --verify me/copy)\"\n \n '\n \n@@ -332,7 +332,7 @@ test_expect_success 'setup for detecting upstreamed changes' '\n test_expect_success 'git pull --rebase detects upstreamed changes' '\n \t(cd dst &&\n \t git pull --rebase &&\n-\t test -z \"$(git ls-files -u)\"\n+\t verbose test -z \"$(git ls-files -u)\"\n \t)\n '\n \n@@ -361,15 +361,15 @@ test_expect_success 'setup for avoiding reapplying old patches' '\n test_expect_success 'git pull --rebase does not reapply old patches' '\n \t(cd dst &&\n \t test_must_fail git pull --rebase &&\n-\t test 1 = $(find .git/rebase-apply -name \"000*\" | wc -l)\n+\t verbose test 1 = \"$(find .git/rebase-apply -name \"000*\" | wc -l)\"\n \t)\n '\n \n test_expect_success 'git pull --rebase against local branch' '\n \tgit checkout -b copy2 to-rebase-orig &&\n \tgit pull --rebase . to-rebase &&\n-\ttest \"conflicting modification\" = \"$(cat file)\" &&\n-\ttest file = \"$(cat file2)\"\n+\tverbose test \"conflicting modification\" = \"$(cat file)\" &&\n+\tverbose test file = \"$(cat file2)\"\n '\n \n test_done\n-- \n2.1.4\n"},{"id":"261150","messageId":"1431508136-15313-3-git-send-email-pyokagan@gmail.com","threadId":"39323","inReplyTo":"1431508136-15313-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v3 2/9] t5520: ensure origin refs are updated","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-13T09:08:49Z","receivedAt":"2015-05-13T09:08:49Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Should all of the tests before \"setup for avoiding reapplying old\npatches\" fail or be skipped, the repo \"dst\" will not have fetched the\nupdated refs from origin. To be resilient against such failures, run\n\"git fetch origin\".\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n* Hmm, no reviews the last round?\n\n t/t5520-pull.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 20ad373..14a9280 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -339,6 +339,7 @@ test_expect_success 'git pull --rebase detects upstreamed changes' '\n test_expect_success 'setup for avoiding reapplying old patches' '\n \t(cd dst &&\n \t test_might_fail git rebase --abort &&\n+\t git fetch origin &&\n \t git reset --hard origin/master\n \t) &&\n \tgit clone --bare src src-replace.git &&\n-- \n2.1.4\n"},{"id":"261147","messageId":"1431508136-15313-4-git-send-email-pyokagan@gmail.com","threadId":"39323","inReplyTo":"1431508136-15313-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v3 3/9] t5520: test no merge candidates cases","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-13T09:08:50Z","receivedAt":"2015-05-13T09:08:50Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"a8c9bef (pull: improve advice for unconfigured error case, 2009-10-05)\nfully established the current advices given by git-pull for the\ndifferent cases where git-fetch will not have anything marked for merge:\n\n1. We fetched from a specific remote, and a refspec was given, but it\n   ended up not fetching anything. This is usually because the user\n   provided a wildcard refspec which had no matches on the remote end.\n\n2. We fetched from a non-default remote, but didn't specify a branch to\n   merge. We can't use the configured one because it applies to the\n   default remote, and thus the user must specify the branches to merge.\n\n3. We fetched from the branch's or repo's default remote, but:\n\n   a. We are not on a branch, so there will never be a configured branch\n      to merge with.\n\n   b. We are on a branch, but there is no configured branch to merge\n      with.\n\n4. We fetched from the branch's or repo's default remote, but the\n   configured branch to merge didn't get fetched (either it doesn't\n   exist, or wasn't part of the configured fetch refspec)\n\nImplement tests for the above 5 cases to ensure that the correct code\npaths are triggered for each of these cases.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n\n* File content comparisons are now quoted.\n\n\n t/t5520-pull.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 55 insertions(+)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 14a9280..e53d8e9 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -109,6 +109,61 @@ test_expect_success 'the default remote . should not break explicit pull' '\n \tverbose test \"$(cat file)\" = modified\n '\n \n+test_expect_success 'fail if wildcard spec does not match any refs' '\n+\tgit checkout -b test copy^ &&\n+\ttest_when_finished \"git checkout -f copy && git branch -D test\" &&\n+\tverbose test \"$(cat file)\" = file &&\n+\ttest_must_fail git pull . \"refs/nonexisting1/*:refs/nonexisting2/*\" 2>err &&\n+\ttest_i18ngrep \"no candidates for merging\" err &&\n+\tverbose test \"$(cat file)\" = file\n+'\n+\n+test_expect_success 'fail if no branches specified with non-default remote' '\n+\tgit remote add test_remote . &&\n+\ttest_when_finished \"git remote remove test_remote\" &&\n+\tgit checkout -b test copy^ &&\n+\ttest_when_finished \"git checkout -f copy && git branch -D test\" &&\n+\tverbose test \"$(cat file)\" = file &&\n+\ttest_config branch.test.remote origin &&\n+\ttest_must_fail git pull test_remote 2>err &&\n+\ttest_i18ngrep \"specify a branch on the command line\" err &&\n+\tverbose test \"$(cat file)\" = file\n+'\n+\n+test_expect_success 'fail if not on a branch' '\n+\tgit remote add origin . &&\n+\ttest_when_finished \"git remote remove origin\" &&\n+\tgit checkout HEAD^ &&\n+\ttest_when_finished \"git checkout -f copy\" &&\n+\tverbose test \"$(cat file)\" = file &&\n+\ttest_must_fail git pull 2>err &&\n+\ttest_i18ngrep \"not currently on a branch\" err &&\n+\tverbose test \"$(cat file)\" = file\n+'\n+\n+test_expect_success 'fail if no configuration for current branch' '\n+\tgit remote add test_remote . &&\n+\ttest_when_finished \"git remote remove test_remote\" &&\n+\tgit checkout -b test copy^ &&\n+\ttest_when_finished \"git checkout -f copy && git branch -D test\" &&\n+\ttest_config branch.test.remote test_remote &&\n+\tverbose test \"$(cat file)\" = file &&\n+\ttest_must_fail git pull 2>err &&\n+\ttest_i18ngrep \"no tracking information\" err &&\n+\tverbose test \"$(cat file)\" = file\n+'\n+\n+test_expect_success 'fail if upstream branch does not exist' '\n+\tgit checkout -b test copy^ &&\n+\ttest_when_finished \"git checkout -f copy && git branch -D test\" &&\n+\ttest_config branch.test.remote . &&\n+\ttest_config branch.test.merge refs/heads/nonexisting &&\n+\tverbose test \"$(cat file)\" = file &&\n+\ttest_must_fail git pull 2>err &&\n+\ttest_i18ngrep \"no such ref was fetched\" err &&\n+\tverbose test \"$(cat file)\" = file\n+'\n+\n test_expect_success '--rebase' '\n \tgit branch to-rebase &&\n \techo modified again > file &&\n-- \n2.1.4\n"},{"id":"261148","messageId":"1431508136-15313-5-git-send-email-pyokagan@gmail.com","threadId":"39323","inReplyTo":"1431508136-15313-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v3 4/9] t5520: test for failure if index has unresolved entries","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-13T09:08:51Z","receivedAt":"2015-05-13T09:08:51Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Commit d38a30d (Be more user-friendly when refusing to do something\nbecause of conflict., 2010-01-12) introduced code paths to git-pull\nwhich will error out with user-friendly advices if the user is in the\nmiddle of a merge or has unmerged files.\n\nImplement tests to ensure that git-pull will not run, and will print\nthese advices, if the user is in the middle of a merge or has unmerged\nfiles in the index.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n\n* Command substitution is now quoted.\n\n t/t5520-pull.sh | 20 ++++++++++++++++++++\n 1 file changed, 20 insertions(+)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex e53d8e9..8e95402 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -164,6 +164,26 @@ test_expect_success 'fail if upstream branch does not exist' '\n \tverbose test \"$(cat file)\" = file\n '\n \n+test_expect_success 'fail if the index has unresolved entries' '\n+\tgit checkout -b third second^ &&\n+\ttest_when_finished \"git checkout -f copy && git branch -D third\" &&\n+\tverbose test \"$(cat file)\" = file &&\n+\techo modified2 >file &&\n+\tgit commit -a -m modified2 &&\n+\tverbose test -z \"$(git ls-files -u)\" &&\n+\ttest_must_fail git pull . second &&\n+\tverbose test -n \"$(git ls-files -u)\" &&\n+\tcp file expected &&\n+\ttest_must_fail git pull . second 2>err &&\n+\ttest_i18ngrep \"you have unmerged files\" err &&\n+\ttest_cmp expected file &&\n+\tgit add file &&\n+\tverbose test -z \"$(git ls-files -u)\" &&\n+\ttest_must_fail git pull . second 2>err &&\n+\ttest_i18ngrep \"have not concluded your merge\" err &&\n+\ttest_cmp expected file\n+'\n+\n test_expect_success '--rebase' '\n \tgit branch to-rebase &&\n \techo modified again > file &&\n-- \n2.1.4\n"},{"id":"261149","messageId":"1431508136-15313-6-git-send-email-pyokagan@gmail.com","threadId":"39323","inReplyTo":"1431508136-15313-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v3 5/9] t5520: test work tree fast-forward when fetch updates head","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-13T09:08:52Z","receivedAt":"2015-05-13T09:08:52Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Since b10ac50 (Fix pulling into the same branch., 2005-08-25), git-pull,\nupon detecting that git-fetch updated the current head, will\nfast-forward the working tree to the updated head commit.\n\nImplement tests to ensure that the fast-forward occurs in such a case,\nas well as to ensure that the user-friendly advice is printed upon\nfailure.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n\n* Added git rev-parse checks to make it explicit that the branch head has\n  been updated.\n\n* \"git checkout -b third second^\" to make it explicit we are fast-forwarding.\n\n t/t5520-pull.sh | 21 +++++++++++++++++++++\n 1 file changed, 21 insertions(+)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 8e95402..954e581 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -184,6 +184,27 @@ test_expect_success 'fail if the index has unresolved entries' '\n \ttest_cmp expected file\n '\n \n+test_expect_success 'fast-forwards working tree if branch head is updated' '\n+\tgit checkout -b third second^ &&\n+\ttest_when_finished \"git checkout -f copy && git branch -D third\" &&\n+\tverbose test \"$(cat file)\" = file &&\n+\tgit pull . second:third 2>err &&\n+\ttest_i18ngrep \"fetch updated the current branch head\" err &&\n+\tverbose test \"$(cat file)\" = modified &&\n+\tverbose test \"$(git rev-parse third)\" = \"$(git rev-parse second)\"\n+'\n+\n+test_expect_success 'fast-forward fails with conflicting work tree' '\n+\tgit checkout -b third second^ &&\n+\ttest_when_finished \"git checkout -f copy && git branch -D third\" &&\n+\tverbose test \"$(cat file)\" = file &&\n+\techo conflict >file &&\n+\ttest_must_fail git pull . second:third 2>err &&\n+\ttest_i18ngrep \"Cannot fast-forward your working tree\" err &&\n+\tverbose test \"$(cat file)\" = conflict &&\n+\tverbose test \"$(git rev-parse third)\" = \"$(git rev-parse second)\"\n+'\n+\n test_expect_success '--rebase' '\n \tgit branch to-rebase &&\n \techo modified again > file &&\n-- \n2.1.4\n"},{"id":"261152","messageId":"1431508136-15313-7-git-send-email-pyokagan@gmail.com","threadId":"39323","inReplyTo":"1431508136-15313-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v3 6/9] t5520: test --rebase with multiple branches","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-13T09:08:53Z","receivedAt":"2015-05-13T09:08:53Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Since rebasing on top of multiple upstream branches does not make sense,\nsince 51b2ead (disallow providing multiple upstream branches to rebase,\npull --rebase, 2009-02-18), git-pull explicitly disallowed specifying\nmultiple branches in the rebase case.\n\nImplement tests to ensure that git-pull fails and prints out the\nuser-friendly error message in such a case.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n\n* Quoted file content comparisons.\n\n t/t5520-pull.sh | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 954e581..e957368 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -218,6 +218,15 @@ test_expect_success '--rebase' '\n \tverbose test \"$(git rev-parse HEAD^)\" = \"$(git rev-parse copy)\" &&\n \tverbose test new = \"$(git show HEAD:file2)\"\n '\n+\n+test_expect_success '--rebase fails with multiple branches' '\n+\tgit reset --hard before-rebase &&\n+\ttest_must_fail git pull --rebase . copy master 2>err &&\n+\tverbose test \"$(git rev-parse HEAD)\" = \"$(git rev-parse before-rebase)\" &&\n+\ttest_i18ngrep \"Cannot rebase onto multiple branches\" err &&\n+\tverbose test modified = \"$(git show HEAD:file)\"\n+'\n+\n test_expect_success 'pull.rebase' '\n \tgit reset --hard before-rebase &&\n \ttest_config pull.rebase true &&\n-- \n2.1.4\n"},{"id":"261151","messageId":"1431508136-15313-8-git-send-email-pyokagan@gmail.com","threadId":"39323","inReplyTo":"1431508136-15313-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v3 7/9] t5520: test --rebase failure on unborn branch with index","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-13T09:08:54Z","receivedAt":"2015-05-13T09:08:54Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Commit 19a7fcb (allow pull --rebase on branch yet to be born,\n2009-08-11) special cases git-pull on an unborn branch in a different\ncode path such that git-pull --rebase is still valid even though there\nis no HEAD yet.\n\nThis code path still ensures that there is no index in order not to lose\nany staged changes. Implement a test to ensure that this check is\ntriggered.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n\n* Renamed \"out\" to \"err\"\n\n* Quoted command substitution and file content comparisons.\n\n t/t5520-pull.sh | 15 +++++++++++++++\n 1 file changed, 15 insertions(+)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex e957368..96d2e7c 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -413,6 +413,21 @@ test_expect_success 'pull --rebase works on branch yet to be born' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'pull --rebase fails on unborn branch with staged changes' '\n+\ttest_when_finished \"rm -rf empty_repo2\" &&\n+\tgit init empty_repo2 &&\n+\t(\n+\t\tcd empty_repo2 &&\n+\t\techo staged-file >staged-file &&\n+\t\tgit add staged-file &&\n+\t\tverbose test \"$(git ls-files)\" = staged-file &&\n+\t\ttest_must_fail git pull --rebase .. master 2>../err &&\n+\t\tverbose test \"$(git ls-files)\" = staged-file &&\n+\t\tverbose test \"$(git show :staged-file)\" = staged-file\n+\t) &&\n+\ttest_i18ngrep \"unborn branch with changes added to the index\" err\n+'\n+\n test_expect_success 'setup for detecting upstreamed changes' '\n \tmkdir src &&\n \t(cd src &&\n-- \n2.1.4\n"},{"id":"261154","messageId":"1431508136-15313-9-git-send-email-pyokagan@gmail.com","threadId":"39323","inReplyTo":"1431508136-15313-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v3 8/9] t5521: test --dry-run does not make any changes","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-13T09:08:55Z","receivedAt":"2015-05-13T09:08:55Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Test that when --dry-run is provided to git-pull, it does not make any\nchanges, namely:\n\n* --dry-run gets passed to git-fetch, so no FETCH_HEAD will be created\n  and no refs will be fetched.\n\n* The index and work tree will not be modified.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n t/t5521-pull-options.sh | 13 +++++++++++++\n 1 file changed, 13 insertions(+)\n\ndiff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh\nindex 453aba5..37d6db6 100755\n--- a/t/t5521-pull-options.sh\n+++ b/t/t5521-pull-options.sh\n@@ -117,4 +117,17 @@ test_expect_success 'git pull --all' '\n \t)\n '\n \n+test_expect_success 'git pull --dry-run' '\n+\ttest_when_finished \"rm -rf clonedry\" &&\n+\tgit init clonedry &&\n+\t(\n+\t\tcd clonedry &&\n+\t\tgit pull --dry-run ../parent &&\n+\t\ttest_path_is_missing .git/FETCH_HEAD &&\n+\t\ttest_path_is_missing .git/refs/heads/master &&\n+\t\ttest_path_is_missing .git/index &&\n+\t\ttest_path_is_missing file\n+\t)\n+'\n+\n test_done\n-- \n2.1.4\n"},{"id":"261153","messageId":"1431508136-15313-10-git-send-email-pyokagan@gmail.com","threadId":"39323","inReplyTo":"1431508136-15313-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v3 9/9] t5520: check reflog action in fast-forward merge","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-13T09:08:56Z","receivedAt":"2015-05-13T09:08:56Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"When testing a fast-forward merge with git-pull, check to see if the\nreflog action is \"pull\" with the arguments passed to git-pull.\n\nWhile we are in the vicinity, remove the empty line as well.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n t/t5520-pull.sh | 13 ++++++++++---\n 1 file changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 96d2e7c..da120b2 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -86,7 +86,6 @@ test_expect_success 'pulling into void must not create an octopus' '\n '\n \n test_expect_success 'test . as a remote' '\n-\n \tgit branch copy master &&\n \tgit config branch.copy.remote . &&\n \tgit config branch.copy.merge refs/heads/master &&\n@@ -95,7 +94,11 @@ test_expect_success 'test . as a remote' '\n \tgit checkout copy &&\n \tverbose test \"$(cat file)\" = file &&\n \tgit pull &&\n-\tverbose test \"$(cat file)\" = updated\n+\tverbose test \"$(cat file)\" = updated &&\n+\tgit reflog -1 >reflog.actual &&\n+\tsed \"s/$_x05[0-9a-f]*/OBJID/g\" reflog.actual >reflog.fuzzy &&\n+\techo \"OBJID HEAD@{0}: pull: Fast-forward\" >reflog.expected &&\n+\ttest_cmp reflog.expected reflog.fuzzy\n '\n \n test_expect_success 'the default remote . should not break explicit pull' '\n@@ -106,7 +109,11 @@ test_expect_success 'the default remote . should not break explicit pull' '\n \tgit reset --hard HEAD^ &&\n \tverbose test \"$(cat file)\" = file &&\n \tgit pull . second &&\n-\tverbose test \"$(cat file)\" = modified\n+\tverbose test \"$(cat file)\" = modified &&\n+\tgit reflog -1 >reflog.actual &&\n+\tsed \"s/$_x05[0-9a-f]*/OBJID/g\" reflog.actual >reflog.fuzzy &&\n+\techo \"OBJID HEAD@{0}: pull . second: Fast-forward\" >reflog.expected &&\n+\ttest_cmp reflog.expected reflog.fuzzy\n '\n \n test_expect_success 'fail if wildcard spec does not match any refs' '\n-- \n2.1.4\n"},{"id":"261159","messageId":"vpqy4ks257w.fsf@anie.imag.fr","threadId":"39323","inReplyTo":"1431508136-15313-5-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v3 4/9] t5520: test for failure if index has unresolved entries","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-05-13T09:32:03Z","receivedAt":"2015-05-13T09:32:03Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> +test_expect_success 'fail if the index has unresolved entries' '\n> +\tgit checkout -b third second^ &&\n> +\ttest_when_finished \"git checkout -f copy && git branch -D third\" &&\n> +\tverbose test \"$(cat file)\" = file &&\n> +\techo modified2 >file &&\n> +\tgit commit -a -m modified2 &&\n> +\tverbose test -z \"$(git ls-files -u)\" &&\n> +\ttest_must_fail git pull . second &&\n> +\tverbose test -n \"$(git ls-files -u)\" &&\n> +\tcp file expected &&\n> +\ttest_must_fail git pull . second 2>err &&\n> +\ttest_i18ngrep \"you have unmerged files\" err &&\n> +\ttest_cmp expected file &&\n> +\tgit add file &&\n> +\tverbose test -z \"$(git ls-files -u)\" &&\n> +\ttest_must_fail git pull . second 2>err &&\n> +\ttest_i18ngrep \"have not concluded your merge\" err &&\n\nReading this, I'm actually thinking that the message may have been\nbetter written as \"You have not concluded your previous merge\". But\nthat's definitely out of the scope of this patch.\n\nAnyway, the patch looks good to me, thanks.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"261178","messageId":"xmqqk2wcbmq5.fsf@gitster.dls.corp.google.com","threadId":"39323","inReplyTo":"1431508136-15313-2-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v3 1/9] t5520: fixup file contents comparisons","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-13T14:01:22Z","receivedAt":"2015-05-13T14:01:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> Many tests in t5520 used the following to test the contents of files:\n>\n> \ttest `cat file` = expected\n>\n> or\n>\n> \ttest $(cat file) = expected\n>\n> These 2 forms, however, will be affected by field splitting and,\n> depending on the value of $IFS, may be split into multiple arguments,\n> making the test fail in mysterious ways.\n>\n> Replace the above 2 forms with:\n>\n> \tverbose test \"$(cat file)\" = expected\n\nQuoting is very much a good idea, but I am not enthused by the\nvision of having to write verbose everywhere in our script.\n\nAfter seeing a script fail, you can run it again with -i -x options;\nwouldn't it be sufficient?\n"},{"id":"261179","messageId":"xmqqegmkbliw.fsf@gitster.dls.corp.google.com","threadId":"39323","inReplyTo":"1431508136-15313-3-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v3 2/9] t5520: ensure origin refs are updated","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-13T14:27:19Z","receivedAt":"2015-05-13T14:27:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> Should all of the tests before \"setup for avoiding reapplying old\n> patches\" fail or be skipped, the repo \"dst\" will not have fetched the\n> updated refs from origin. To be resilient against such failures, run\n> \"git fetch origin\".\n>\n> Signed-off-by: Paul Tan <pyokagan@gmail.com>\n> ---\n> * Hmm, no reviews the last round?\n\nIt is not unusual when the change is trivially correct.\n\nI do not think this hurts, but I do not think that this is vastly\nbetter, either.  If you suspect that the previous one may fail but\nyou at the same time are so trusting that the one before that one\nwould succeed, yes, this will help in that case.  But if you suspect\nthe previous one may fail, the one before that may have also failed,\nin which case this may not be sufficient (the result of fetching may\nnot match what this test expects to see).\n\nIt all depends on where our paranoia ends. The current code is very\nmuch trusting all previous ones equally, and accepts \"upon the first\nerror, all bets are off for the later ones\". With this patch, it\nbecomes slightly less trusting.\n\nIf everything else were equal, I would say this change is a \"Meh\" to\nme, but I think the change improves this test in a different way.\n\nIt begins with \"Run 'git rebase --abort', just in case\"; which is a\nsignal that it does consider that the previous one may have failed\nand attempts to prepare for that possibility, while trusting the one\nbefore that would have succeeded.  And under that assumption, what\nit currently does is _not_ consistent; the previous \"pull --rebase\"\nmay have failed in the \"rebase\" phase, in which case \"abort just in\ncase\" is a good measure to go back to the clean state, but it may\nhave failed in the \"fetch\" phase, in which case \"abort\" does not\nhelp.  And this patch is needed to fix that inconsistency.\n\nIf justified in that way in the log message, then I wouldn't have\nsaid \"I do not think that this is vastly better\", I think.\n\n>  t/t5520-pull.sh | 1 +\n>  1 file changed, 1 insertion(+)\n>\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index 20ad373..14a9280 100755\n> --- a/t/t5520-pull.sh\n> +++ b/t/t5520-pull.sh\n> @@ -339,6 +339,7 @@ test_expect_success 'git pull --rebase detects upstreamed changes' '\n>  test_expect_success 'setup for avoiding reapplying old patches' '\n>  \t(cd dst &&\n>  \t test_might_fail git rebase --abort &&\n> +\t git fetch origin &&\n>  \t git reset --hard origin/master\n>  \t) &&\n>  \tgit clone --bare src src-replace.git &&\n"},{"id":"261180","messageId":"xmqqa8x8bkuc.fsf@gitster.dls.corp.google.com","threadId":"39323","inReplyTo":"xmqqk2wcbmq5.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 1/9] t5520: fixup file contents comparisons","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-13T14:42:03Z","receivedAt":"2015-05-13T14:42:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Paul Tan <pyokagan@gmail.com> writes:\n>\n>> Replace the above 2 forms with:\n>>\n>> \tverbose test \"$(cat file)\" = expected\n>\n> Quoting is very much a good idea, but I am not enthused by the\n> vision of having to write verbose everywhere in our script.\n>\n> After seeing a script fail, you can run it again with -i -x options;\n> wouldn't it be sufficient?\n\nJust to avoid misunderstanding, I am unhappy if we have to keep\nwriting \"verbose test\" in that exact form.\n\nJust like we invented to help debugging by wrapping \"cmp\" with\n\"test_cmp\" (i.e. we want to see if the file contents are the same,\nbut a person who debugs can be helped by seeing the differences when\nthe expectation is not met), I do not mind if we had a shorter and\ncleanly-named wrapper that we can use consistently.  E.g. I do not\nmind something like this in test-lib-functions.sh\n\n\ttest_file_contents () {\n\t\tif test \"$(cat \"$1\")\" != \"$2\"\n\t\tthen\n\t\t\techo \"Contents of file $1 is not $2\"\n                        false\n\t\tfi\n\t}\n\nand used like so:\n\n\ttest_file_contents file expected_string\n"},{"id":"261247","messageId":"CAO2U3QgD0-tAwGnMeeMR5aqbUuqDsdWy0Sw8dQBPUpUNwJZpHg@mail.gmail.com","threadId":"39323","inReplyTo":"xmqqa8x8bkuc.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 1/9] t5520: fixup file contents comparisons","fromName":"Michael Blume","fromEmail":"blume.mike@gmail.com","sentAt":"2015-05-14T17:29:34Z","receivedAt":"2015-05-14T17:29:34Z","isPatch":true,"sender":{"key":"blume.mike@gmail.com","avatar":"https://gravatar.com/avatar/1a7b440e1d942425ff4098ac7fc15b86b30cecaa56e1692a7ef8b5939ba25ea7?d=mp&s=160"},"body":"On Wed, May 13, 2015 at 7:42 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Paul Tan <pyokagan@gmail.com> writes:\n>>\n>>> Replace the above 2 forms with:\n>>>\n>>>      verbose test \"$(cat file)\" = expected\n>>\n>> Quoting is very much a good idea, but I am not enthused by the\n>> vision of having to write verbose everywhere in our script.\n>>\n>> After seeing a script fail, you can run it again with -i -x options;\n>> wouldn't it be sufficient?\n>\n> Just to avoid misunderstanding, I am unhappy if we have to keep\n> writing \"verbose test\" in that exact form.\n>\n> Just like we invented to help debugging by wrapping \"cmp\" with\n> \"test_cmp\" (i.e. we want to see if the file contents are the same,\n> but a person who debugs can be helped by seeing the differences when\n> the expectation is not met), I do not mind if we had a shorter and\n> cleanly-named wrapper that we can use consistently.  E.g. I do not\n> mind something like this in test-lib-functions.sh\n>\n>         test_file_contents () {\n>                 if test \"$(cat \"$1\")\" != \"$2\"\n>                 then\n>                         echo \"Contents of file $1 is not $2\"\n>                         false\n>                 fi\n>         }\n>\n> and used like so:\n>\n>         test_file_contents file expected_string\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n\nMy build starts breaking from this commit, I'm on a mac.\n\n\nexpecting success:\n    (cd dst &&\n     test_must_fail git pull --rebase &&\n     verbose test 1 = \"$(find .git/rebase-apply -name \"000*\" | wc -l)\"\n    )\n\nFirst, rewinding head to replay your work on top of it...\nApplying: Modified Change 4\nUsing index info to reconstruct a base tree...\nM    stuff\nFalling back to patching base and 3-way merge...\nMerging HEAD with Modified Change 4\nMerging:\n814f1c3 Change 4\nvirtual Modified Change 4\nfound 1 common ancestor:\nvirtual e369e858ec1aa73d497d02c4f23e5cf4ae2d3c3b\nAuto-merging stuff\nCONFLICT (content): Merge conflict in stuff\nFailed to merge in the changes.\nPatch failed at 0001 Modified Change 4\nThe copy of the patch that failed is found in:\n   /Users/michael.blume/workspace/git/t/trash\ndirectory.t5520-pull/dst/.git/rebase-apply/patch\n\nWhen you have resolved this problem, run \"git rebase --continue\".\nIf you prefer to skip this patch, run \"git rebase --skip\" instead.\nTo check out the original branch and stop rebasing, run \"git rebase --abort\".\n\ncommand failed:  'test' '1' '=' '       1'\nnot ok 33 - git pull --rebase does not reapply old patches\n#\n#        (cd dst &&\n#         test_must_fail git pull --rebase &&\n#         verbose test 1 = \"$(find .git/rebase-apply -name \"000*\" | wc -l)\"\n#        )\n#\n"},{"id":"261251","messageId":"xmqq4mnf8358.fsf@gitster.dls.corp.google.com","threadId":"39323","inReplyTo":"CAO2U3QgD0-tAwGnMeeMR5aqbUuqDsdWy0Sw8dQBPUpUNwJZpHg@mail.gmail.com","subject":"Re: [PATCH v3 1/9] t5520: fixup file contents comparisons","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-14T17:44:51Z","receivedAt":"2015-05-14T17:44:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Blume <blume.mike@gmail.com> writes:\n\n> On Wed, May 13, 2015 at 7:42 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Junio C Hamano <gitster@pobox.com> writes:\n>>\n>>> Paul Tan <pyokagan@gmail.com> writes:\n>>>\n>>>> Replace the above 2 forms with:\n>>>>\n>>>>      verbose test \"$(cat file)\" = expected\n>>>\n>>> Quoting is very much a good idea, but I am not enthused by the\n>>> vision of having to write verbose everywhere in our script.\n>>> \n>>> After seeing a script fail, you can run it again with -i -x options;\n>>> wouldn't it be sufficient?\n>> ...\n>\n> My build starts breaking from this commit, I'm on a mac.\n>\n>\n> expecting success:\n>     (cd dst &&\n>      test_must_fail git pull --rebase &&\n>      verbose test 1 = \"$(find .git/rebase-apply -name \"000*\" | wc -l)\"\n>     )\n>\n> First, rewinding head to replay your work on top of it...\n> ...\n> To check out the original branch and stop rebasing, run \"git rebase --abort\".\n>\n> command failed:  'test' '1' '=' '       1'\n> not ok 33 - git pull --rebase does not reapply old patches\n\nChange that 'verbose test' line to\n\n\tverbose test 1 = $(find .git/rebase-apply -name \"000*\" | wc -l)\n\ni.e. losing the double-quotes around $().\n\nBy the way, thanks for a fine demonstration that the 'verbose test'\nis not very useful.\n\nThis output\n\n> command failed:  'test' '1' '=' '       1'\n\nand that you said \"on a mac\", _I_ can immediately guess that there\nis \"wc -l\" involved, because it has been a frequent source of\nportability headache.\n\nBut \"verbose\" is not helping very much to show there is \"wc -l\";\nunless the person debugging the output has a pretty good idea what\ncan go wrong, that is.\n"},{"id":"261327","messageId":"CACRoPnR_tKfSPpRDnQ6_z+XECe7phaN1zRWDZxoaRAz6egGcNw@mail.gmail.com","threadId":"39323","inReplyTo":"1431508136-15313-5-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v3 4/9] t5520: test for failure if index has unresolved entries","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-15T08:25:02Z","receivedAt":"2015-05-15T08:25:02Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"On Wed, May 13, 2015 at 5:08 PM, Paul Tan <pyokagan@gmail.com> wrote:\n> +test_expect_success 'fail if the index has unresolved entries' '\n> +       git checkout -b third second^ &&\n> +       test_when_finished \"git checkout -f copy && git branch -D third\" &&\n> +       verbose test \"$(cat file)\" = file &&\n> +       echo modified2 >file &&\n> +       git commit -a -m modified2 &&\n> +       verbose test -z \"$(git ls-files -u)\" &&\n> +       test_must_fail git pull . second &&\n> +       verbose test -n \"$(git ls-files -u)\" &&\n> +       cp file expected &&\n> +       test_must_fail git pull . second 2>err &&\n> +       test_i18ngrep \"you have unmerged files\" err &&\n\nHmm, it appears that this is too loose, as git-merge will throw \"merge\nis not possible because you have unmerged files\".\n\nSo it looks like we will have to go back to the stricter \"Pull is not\npossible because you have unmerged files\".\n\n> +       test_cmp expected file &&\n> +       git add file &&\n> +       verbose test -z \"$(git ls-files -u)\" &&\n> +       test_must_fail git pull . second 2>err &&\n> +       test_i18ngrep \"have not concluded your merge\" err &&\n> +       test_cmp expected file\n> +'\n> +\n>  test_expect_success '--rebase' '\n>         git branch to-rebase &&\n>         echo modified again > file &&\n> --\n> 2.1.4\n>\n"},{"id":"261334","messageId":"CACRoPnSbekLANNiGOyxN70TCUd1c=wcrU_6Gfew5pp5EBpSEsA@mail.gmail.com","threadId":"39323","inReplyTo":"xmqq4mnf8358.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 1/9] t5520: fixup file contents comparisons","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-15T11:41:39Z","receivedAt":"2015-05-15T11:41:39Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Junio,\n\nOn Fri, May 15, 2015 at 1:44 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Change that 'verbose test' line to\n>\n>         verbose test 1 = $(find .git/rebase-apply -name \"000*\" | wc -l)\n>\n> i.e. losing the double-quotes around $().\n\nNoted and fixed. Interesting quirk though :-).\n\n> By the way, thanks for a fine demonstration that the 'verbose test'\n> is not very useful.\n>\n> This output\n>\n>> command failed:  'test' '1' '=' '       1'\n\nPersonally, I find that the quoting provided by \"verbose\" helps make\nit clear that it's a whitespace issue, which might be a bit harder to\nspot with the output of set -x I think.\n\nOther than that, I'm also convinced that \"verbose\" doesn't really\noffer much. Will remove in the re-roll.\n\nThanks,\nPaul\n"},{"id":"261366","messageId":"xmqq7fs9hekc.fsf@gitster.dls.corp.google.com","threadId":"39323","inReplyTo":"CACRoPnSbekLANNiGOyxN70TCUd1c=wcrU_6Gfew5pp5EBpSEsA@mail.gmail.com","subject":"Re: [PATCH v3 1/9] t5520: fixup file contents comparisons","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-15T18:37:55Z","receivedAt":"2015-05-15T18:37:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> Hi Junio,\n>\n> On Fri, May 15, 2015 at 1:44 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Change that 'verbose test' line to\n>>\n>>         verbose test 1 = $(find .git/rebase-apply -name \"000*\" | wc -l)\n>>\n>> i.e. losing the double-quotes around $().\n>\n> Noted and fixed. Interesting quirk though :-).\n>\n>> By the way, thanks for a fine demonstration that the 'verbose test'\n>> is not very useful.\n>>\n>> This output\n>>\n>>> command failed:  'test' '1' '=' '       1'\n>\n> Personally, I find that the quoting provided by \"verbose\" helps make\n> it clear that it's a whitespace issue, which might be a bit harder to\n> spot with the output of set -x I think.\n\nTo be fair, yes, because of the leading SP in the RHS, I immediately\nknew that this was a \"wc -l\" from that without running the test one\nmore time with \"-i -v -x\".  The \"rev-parse --sq-quote\" did help.\nWithout the --sq-quote trick, i.e. \"command failed: test 1 = 1\",\nwould actually make the debugger suspect that there would be a\nquoting issue anyway, so it is not a very big deal, though.\n\nIn any case, that \"test 1 = 1\" (with or without quoting) helped only\nbecause I had to deal with \"wc -l\" issues in the past.  Without\ntelling how that ' 1' ended up compared with '1' by showing \"wc -l\"\non that 'command failed:' line, it wouldn't have helped much if the\ndebugger were not me.\n\n> Other than that, I'm also convinced that \"verbose\" doesn't really\n> offer much. Will remove in the re-roll.\n\nJust to avoid misunderstanding, please do not remove 'verbose '\nblindly without thinking while doing so, as you already did 1/3 of\nthe necessary job to make things better.\n\nYou might have noticed, while adding them, there were something\ncommon that we currently do with a bare 'test' only because we\nhaven't identified common needs.  As I already said, it may be that\nwe often try to see a file has a known single line content (I didn't\ncheck if that were the case; I am just giving you an example) and\nonly because there is no ready-made test_file_contents helper to be\nused, the current tests say\n\n\ttest expected_string = \"$(cat file)\"\n\nAnd if that were the case, it is a good thing to have a new helper\nlike this\n\n\ttest_file_contents () {\n\t\tif test \"$(cat \"$1\")\" != \"$2\"\n\t\tthen\n\t\t\techo \"Contents of file '$1' is not '$2'\"\n                        false\n\t\tfi\n\t}\n\nin t/test-lib-functions.sh and convert them to say\n\n\ttest_file_contents file expected_string\n\nThat would be an improvement (and that is the remaining 2/3 ;-).\n\nThanks.\n"},{"id":"261369","messageId":"xmqqzj55fxyf.fsf@gitster.dls.corp.google.com","threadId":"39323","inReplyTo":"xmqq7fs9hekc.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 1/9] t5520: fixup file contents comparisons","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-15T19:22:00Z","receivedAt":"2015-05-15T19:22:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> You might have noticed, while adding them, there were something\n> common that we currently do with a bare 'test' only because we\n> haven't identified common needs....\n> ...\n> in t/test-lib-functions.sh and convert them to say\n>\n> \ttest_file_contents file expected_string\n>\n> That would be an improvement (and that is the remaining 2/3 ;-).\n\nI haven't made up my mind on this other example, but since I started\nwriting it...\n\nIt may be that counting the number of lines in output, \"cmd | wc -l\",\nis a common pattern.  We already have test_line_count to check the\nnumber of lines in a file, but having test_output_count may help.\n\n t/test-lib-functions.sh | 11 +++++++++++\n t/t0000-basic.sh        |  3 +--\n t/t0030-stripspace.sh   | 11 +++++------\n 3 files changed, 17 insertions(+), 8 deletions(-)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 0d93e33..624a8c5 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -538,6 +538,17 @@ test_line_count () {\n \tfi\n }\n \n+test_output_count () {\n+\tif test $# != 3\n+\tthen\n+\t\terror \"bug in the test script: not 3 parameters to test_output_count\"\n+\telif ! test $(eval \"$2\" | wc -l) \"$1\" \"$3\"\n+\tthen\n+\t\techo \"test_output_count: line count for output from '$2' !$1 $3\"\n+\t\treturn 1\n+\tfi\n+}\n+\n # This is not among top-level (test_expect_success | test_expect_failure)\n # but is a prefix that can be used in the test script, like:\n #\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex f10ba4a..bd5930d 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -1039,8 +1039,7 @@ test_expect_success 'update-index D/F conflict' '\n \tmv path2 path0 &&\n \tmv tmp path2 &&\n \tgit update-index --add --replace path2 path0/file2 &&\n-\tnumpath0=$(git ls-files path0 | wc -l) &&\n-\ttest $numpath0 = 1\n+\ttest_output_count = \"git ls-files path0\" 1\n '\n \n test_expect_success 'very long name in the index handled sanely' '\ndiff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh\nindex 0333dd9..9502938 100755\n--- a/t/t0030-stripspace.sh\n+++ b/t/t0030-stripspace.sh\n@@ -223,12 +223,11 @@ test_expect_success \\\n     test_cmp expect actual\n '\n \n-test_expect_success \\\n-    'text without newline at end should end with newline' '\n-    test $(printf \"$ttt\" | git stripspace | wc -l) -gt 0 &&\n-    test $(printf \"$ttt$ttt\" | git stripspace | wc -l) -gt 0 &&\n-    test $(printf \"$ttt$ttt$ttt\" | git stripspace | wc -l) -gt 0 &&\n-    test $(printf \"$ttt$ttt$ttt$ttt\" | git stripspace | wc -l) -gt 0\n+test_expect_success 'text without newline at end should end with newline' '\n+\ttest_output_count -gt '\\''printf \"$ttt\" | git stripspace'\\'' 0 &&\n+\ttest_output_count -gt '\\''printf \"$ttt$ttt\" | git stripspace'\\'' 0 &&\n+\ttest_output_count -gt '\\''printf \"$ttt$ttt$ttt\" | git stripspace'\\'' 0 &&\n+\ttest_output_count -gt '\\''printf \"$ttt$ttt$ttt$ttt\" | git stripspace'\\'' 0\n '\n \n # text plus spaces at the end:\n"},{"id":"261390","messageId":"CACRoPnSP9xfyW47ZqU7QO5o4tyzROh4hGRPqG9g9OB5cquS+uw@mail.gmail.com","threadId":"39323","inReplyTo":"xmqq7fs9hekc.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 1/9] t5520: fixup file contents comparisons","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-16T13:49:52Z","receivedAt":"2015-05-16T13:49:52Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Junio,\n\nOn Sat, May 16, 2015 at 2:37 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Just to avoid misunderstanding, please do not remove 'verbose '\n> blindly without thinking while doing so, as you already did 1/3 of\n> the necessary job to make things better.\n\nEh? I thought we established that using \"verbose\" does not provide\nanything more than what \"set -x\" already provides. So at the very\nleast, its use should be removed completely.\n\nHere is my understanding of the current situation, please correct me\nif I am wrong:\n\nWhen a test fails, it would be useful to know the exact, unexpanded,\nunsubstituted command which failed (additionally with a nice stack\ntrace and line numbers). However, the output of -v and -x (and by\nextension, the \"verbose\" function) is not very helpful, as it still\nrequires the debugger to understand the test script.\n\n-v will print out the stdout and stderr of the executed commands, but\nit requires the debugger to match up the output of the commands with\nthe test script to understand where the test failed. It is also not\nvery helpful in the case of \"test\", which does not print anything when\nit fails.\n\n-x will trace the commands being executed, but the tracing output is\nso verbose it still requires the debugger to understand the test\nscript. e.g:\n\n     test \"$(cat file)\" \"expected\"\n\nIf the above test fails (e.g. the content of the file is\n\"unexpected\"), the tracing output will be:\n\n     + cat file\n     + test unexpected = expected\n     error: last command exited with $?=1\n\nFurthermore, the format of the tracing output is not specified by\nPOSIX, so we can't count on it being consistent among all shells (e.g.\nfor users who submit bug reports with the test output)\n\n> You might have noticed, while adding them, there were something\n> common that we currently do with a bare 'test' only because we\n> haven't identified common needs.  As I already said, it may be that\n> we often try to see a file has a known single line content (I didn't\n> check if that were the case; I am just giving you an example) and\n> only because there is no ready-made test_file_contents helper to be\n> used, the current tests say\n>\n>         test expected_string = \"$(cat file)\"\n>\n> And if that were the case, it is a good thing to have a new helper\n> like this\n>\n>         test_file_contents () {\n>                 if test \"$(cat \"$1\")\" != \"$2\"\n>                 then\n>                         echo \"Contents of file '$1' is not '$2'\"\n>                         false\n>                 fi\n>         }\n>\n> in t/test-lib-functions.sh and convert them to say\n>\n>         test_file_contents file expected_string\n>\n> That would be an improvement (and that is the remaining 2/3 ;-).\n\nYeah, this kind of comparison with file contents is something that is\ndone often in t5520, so I agree with adding it.\n\nHowever, what about these kind of tests:\n\n     test new = \"$(git show HEAD:file2)\"\n\nor these:\n\n     test $(git rev-parse HEAD^2) = $(git rev-parse keep-merge)\n\nSo, perhaps we could introduce a generic function like:\n\n    # Compares that the output of $1 eval'ed is identical to $2.\n    test_output () {\n        output=$(eval $1)\n        if \"$output\" != \"$2\"\n        then\n             echo >&2 \"Output of '$1' ('$output') != '$2'\"\n             false\n        fi\n    }\n\nSo the first example would be:\n\n    test_output \"git show HEAD:file2\" new\n\nAnd the error output will thus be:\n\n     Output of 'git show HEAD:file2' ('some unexpected output') != 'new'\n\nSo we know the exact comparison that failed, and we know how the\nexpected and actual output differs.\n\nWhat do you think?\n\nThanks,\nPaul\n"},{"id":"261393","messageId":"xmqq617sfj05.fsf@gitster.dls.corp.google.com","threadId":"39323","inReplyTo":"CACRoPnSP9xfyW47ZqU7QO5o4tyzROh4hGRPqG9g9OB5cquS+uw@mail.gmail.com","subject":"Re: [PATCH v3 1/9] t5520: fixup file contents comparisons","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-16T18:57:14Z","receivedAt":"2015-05-16T18:57:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> Hi Junio,\n>\n> On Sat, May 16, 2015 at 2:37 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Just to avoid misunderstanding, please do not remove 'verbose '\n>> blindly without thinking while doing so, as you already did 1/3 of\n>> the necessary job to make things better.\n>\n> Eh? I thought we established that using \"verbose\" does not provide\n> anything more than what \"set -x\" already provides. So at the very\n> least, its use should be removed completely.\n\nI did not mean \"do not remove and keep them\".  I meant \"do not\nremove without thinking; instead, take mental notes on patterns\nthese silent ones may have while removing them\".\n\n>> You might have noticed, while adding them, there were something\n>> common that we currently do with a bare 'test' only because we\n>> haven't identified common needs.  As I already said,...\n>> ...\n>> That would be an improvement (and that is the remaining 2/3 ;-).\n>\n> Yeah, this kind of comparison with file contents is something that is\n> done often in t5520, so I agree with adding it.\n>\n> However, what about these kind of tests:\n>\n>      test new = \"$(git show HEAD:file2)\"\n>\n> or these:\n>\n>      test $(git rev-parse HEAD^2) = $(git rev-parse keep-merge)\n>\n> So, perhaps we could introduce a generic function like:\n\nIt all depends on how common they are.\n\n> So the first example would be:\n>\n>     test_output \"git show HEAD:file2\" new\n\nSimple things like that look fine, but when a variable is involved,\nuse of eval combined with the fact that the test body is inside sq,\nmakes the callers unnecessarily ugly.\n\n\ttest_expect_success 'some title' '\n\t\tvar=$(...) &&\n\t\ttest_output \"git show \\$var:file2 | sed -e \\\"s/$old/$new/\\\"\" new\n\t'\n\nWhich is the concern this shares with the other one I sent about\ncounting the number of lines in the output from a command that made\nme hesitate to suggest it.\n\nSo I dunno.\n"},{"id":"261398","messageId":"xmqqoalkdrop.fsf@gitster.dls.corp.google.com","threadId":"39323","inReplyTo":"xmqq617sfj05.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 1/9] t5520: fixup file contents comparisons","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-16T23:32:38Z","receivedAt":"2015-05-16T23:32:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Paul Tan <pyokagan@gmail.com> writes:\n>\n>> So the first example would be:\n>>\n>>     test_output \"git show HEAD:file2\" new\n>\n> Simple things like that look fine, but when a variable is involved,\n> use of eval combined with the fact that the test body is inside sq,\n> makes the callers unnecessarily ugly.\n>\n> \ttest_expect_success 'some title' '\n> \t\tvar=$(...) &&\n> \t\ttest_output \"git show \\$var:file2 | sed -e \\\"s/$old/$new/\\\"\" new\n> \t'\n>\n> Which is the concern this shares with the other one I sent about\n> counting the number of lines in the output from a command that made\n> me hesitate to suggest it.\n>\n> So I dunno.\n\nI actually think that \"test\" that compares output from command and a\nconstant string, and \"test\" that compares outputs from two commands\nare lazyily written forms of these:\n\n        echo constant string >expect &&\n\tcommand >actual &&\n        test_cmp expect actual\n\n\tcommand1 >expect &&\n        command2 >actual &&\n        test_cmp expect actual\n\nThe examples you gave in the earlier message were\n\n>\n>      test new = \"$(git show HEAD:file2)\"\n>\n> or these:\n>\n>      test $(git rev-parse HEAD^2) = $(git rev-parse keep-merge)\n>\n\nand I suspect they match my observation.\n\nMy earlier test_output_count was probably in the same \"lazy\"\ncategory.  \"test $(command | wc -l) = 20\" is better written\nas\n\n\tcommand >output &&\n        test_line_count = 20 output\n\ninstead of using the hypothetical\n\n\ttest_output_count = 20 \"command\"\n\nthat evals the command argument, not only because the quoting of\n'command part will become complex for real world uses, but because\nthe output itself would be the first thing we would want to inspect\nonce the command fails.  For that reason, I'd rather not to add the\ntest_output_count I suggested earlier, so that we would encourage\nthe more straight-forward form, i.e.\n\n\tcommand >output &&\n        test_line_count = 20 output\n\nto be used.\n"},{"id":"261411","messageId":"CACRoPnTLaJ+4Vc0Dg2rZxO2P3WUxF8++OUi24An00xNSbvTk6A@mail.gmail.com","threadId":"39323","inReplyTo":"xmqq617sfj05.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 1/9] t5520: fixup file contents comparisons","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-17T07:47:51Z","receivedAt":"2015-05-17T07:47:51Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Junio,\n\nOn Sun, May 17, 2015 at 2:57 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Paul Tan <pyokagan@gmail.com> writes:\n>\n>> Hi Junio,\n>>\n>> On Sat, May 16, 2015 at 2:37 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> Just to avoid misunderstanding, please do not remove 'verbose '\n>>> blindly without thinking while doing so, as you already did 1/3 of\n>>> the necessary job to make things better.\n>>\n>> Eh? I thought we established that using \"verbose\" does not provide\n>> anything more than what \"set -x\" already provides. So at the very\n>> least, its use should be removed completely.\n>\n> I did not mean \"do not remove and keep them\".  I meant \"do not\n> remove without thinking; instead, take mental notes on patterns\n> these silent ones may have while removing them\".\n\nOkay. Just to keep things moving, for now I will send a re-roll to\nremove \"verbose\" and to fix other issues like [1] and [2].\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/268950/focus=269052\n[2] http://thread.gmane.org/gmane.comp.version-control.git/268950/focus=269132\n\nThanks,\nPaul\n"},{"id":"261427","messageId":"CACRoPnR9Bgg2OXh4any6RihcDTDNZy_tmukGW-4ADr-G28egeg@mail.gmail.com","threadId":"39323","inReplyTo":"xmqqegmkbliw.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/9] t5520: ensure origin refs are updated","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-18T13:09:47Z","receivedAt":"2015-05-18T13:09:47Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Junio,\n\nOn Wed, May 13, 2015 at 10:27 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> It is not unusual when the change is trivially correct.\n>\n> I do not think this hurts, but I do not think that this is vastly\n> better, either.  If you suspect that the previous one may fail but\n> you at the same time are so trusting that the one before that one\n> would succeed, yes, this will help in that case.  But if you suspect\n> the previous one may fail, the one before that may have also failed,\n> in which case this may not be sufficient (the result of fetching may\n> not match what this test expects to see).\n>\n> It all depends on where our paranoia ends. The current code is very\n> much trusting all previous ones equally, and accepts \"upon the first\n> error, all bets are off for the later ones\". With this patch, it\n> becomes slightly less trusting.\n>\n> If everything else were equal, I would say this change is a \"Meh\" to\n> me,\n\nYeah, thinking about it, it's a \"meh\" to me too. While this patch will\nimprove consistency in the attempt to recover from failure, I don't\nthink it will be useful in practice, and it's also not relevant to the\ngoal of this series.\n\nWill drop this patch.\n\n> but I think the change improves this test in a different way.\n>\n> It begins with \"Run 'git rebase --abort', just in case\"; which is a\n> signal that it does consider that the previous one may have failed\n> and attempts to prepare for that possibility, while trusting the one\n> before that would have succeeded.  And under that assumption, what\n> it currently does is _not_ consistent; the previous \"pull --rebase\"\n> may have failed in the \"rebase\" phase, in which case \"abort just in\n> case\" is a good measure to go back to the clean state, but it may\n> have failed in the \"fetch\" phase, in which case \"abort\" does not\n> help.  And this patch is needed to fix that inconsistency.\n>\n> If justified in that way in the log message, then I wouldn't have\n> said \"I do not think that this is vastly better\", I think.\n>\n\nThanks,\nPaul\n"}]}