{"thread":{"id":"39267","subject":"[PATCH v2 00/12] Improve git-pull test coverage","startedAt":"2015-05-07T08:43:56Z","lastAt":"2015-05-12T13:43:04Z","messageCount":49,"participants":["Paul Tan","Torsten Bögershausen","Junio C Hamano","Stefan Beller","Johannes Schindelin","Eric Sunshine","Johannes Sixt","Thomas Gummerer"],"isPatch":true,"patchVersion":2,"patchTotal":12},"messages":[{"id":"260705","messageId":"1430988248-18285-1-git-send-email-pyokagan@gmail.com","threadId":"39267","inReplyTo":null,"subject":"[PATCH v2 00/12] Improve git-pull test coverage","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-07T08:43:56Z","receivedAt":"2015-05-07T08:43:56Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"This patch series improves test coverage of git-pull.sh.\n\nThis is part of my GSoC project to rewrite git-pull into a builtin. Improving\ntest coverage helps to prevent regressions that could occur due to the rewrite.\n\nThe previous patch series can be found at [1]. Note that it is now based on\njc/merge in pu.\n\nBesides fixing issues raised in the last round, some failing tests have been\nadded to demonstrate some bugs in git-pull.sh, and some tests are modified to\nreduce their dependence on git-pull's functionality so that irrelevant test\nsuites will not break during the rewrite.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/268231\n\nPaul Tan (12):\n t5520: implement tests for 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 t4013: call git-merge instead of git-pull\n t5520: ensure origin refs are updated\n t7406: use \"git pull\" instead of \"git pull --rebase\"\n t5520: failing test for pull --all with no configured upstream\n t5524: test --log=1 limits shortlog length\n t5520: check reflog action in fast-forward merge\n\n t/t4013-diff-various.sh | 2 +-\n t/t5520-pull.sh | 148 +++++++++++++++++++++++++++++++++++++++++++-\n t/t5521-pull-options.sh | 13 ++++\n t/t5524-pull-msg.sh | 17 +++++\n t/t7406-submodule-update.sh | 2 +-\n 5 files changed, 177 insertions(+), 5 deletions(-)\n\n-- \n2.1.4\n"},{"id":"260708","messageId":"1430988248-18285-2-git-send-email-pyokagan@gmail.com","threadId":"39267","inReplyTo":"1430988248-18285-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 01/12] t5520: implement tests for no merge candidates cases","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-07T08:43:57Z","receivedAt":"2015-05-07T08:43:57Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Commit a8c9bef4 fully established the current advices given by git-pull\nfor the different cases where git-fetch will not have anything marked\nfor 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 \n---\n\nNotes:\n * Re-worded commit message to match the logic used in git-pull.sh's\n error_on_no_merge_candidates().\n \n * The tests have thus also been reordered to match the commit message.\n \n * Non-hackish solution for case 3a.\n \n * Add more checks to ensure that git-pull does not touch any files it\n should not be touching on failure.\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 7efd45b..5add900 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 \ttest `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+\ttest `cat file` = file &&\n+\ttest_must_fail git pull . \"refs/nonexisting1/*:refs/nonexisting2/*\" 2>out &&\n+\ttest_i18ngrep \"no candidates for merging\" out &&\n+\ttest `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+\ttest `cat file` = file &&\n+\ttest_config branch.test.remote origin &&\n+\ttest_must_fail git pull test_remote 2>out &&\n+\ttest_i18ngrep \"specify a branch on the command line\" out &&\n+\ttest `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+\ttest `cat file` = file &&\n+\ttest_must_fail git pull 2>out &&\n+\ttest_i18ngrep \"not currently on a branch\" out &&\n+\ttest `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+\ttest `cat file` = file &&\n+\ttest_must_fail git pull 2>out &&\n+\ttest_i18ngrep \"no tracking information\" out &&\n+\ttest `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+\ttest `cat file` = file &&\n+\ttest_must_fail git pull 2>out &&\n+\ttest_i18ngrep \"no such ref was fetched\" out &&\n+\ttest `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":"260707","messageId":"1430988248-18285-3-git-send-email-pyokagan@gmail.com","threadId":"39267","inReplyTo":"1430988248-18285-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 02/12] t5520: test for failure if index has unresolved entries","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-07T08:43:58Z","receivedAt":"2015-05-07T08:43:58Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Commit d38a30df (Be more user-friendly when refusing to do something\nbecause of conflict) introduced code paths to git-pull which will error\nout with user-friendly advices if the user is in the middle of a merge\nor 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 \n---\n\nNotes:\n * Test that on merge conflict, git-pull will not reset conflict status,\n or modify the conflicted file.\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 5add900..37ff45f 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -164,6 +164,27 @@ test_expect_success 'fail if upstream branch does not exist' '\n \ttest `cat file` = file\n '\n \n+test_expect_success 'fail if the index has unresolved entries' '\n+\tgit checkout -b third master^ &&\n+\ttest_when_finished \"git checkout -f copy && git branch -D third\" &&\n+\techo file >expected &&\n+\ttest_cmp expected file &&\n+\techo modified2 >file &&\n+\tgit commit -a -m modified2 &&\n+\ttest -z \"$(git ls-files -u)\" &&\n+\ttest_must_fail git pull . second &&\n+\ttest -n \"$(git ls-files -u)\" &&\n+\tcp file expected &&\n+\ttest_must_fail git pull . second 2>out &&\n+\ttest_i18ngrep \"Pull is not possible because you have unmerged files\" out &&\n+\ttest_cmp expected file &&\n+\tgit add file &&\n+\ttest -z \"$(git ls-files -u)\" &&\n+\ttest_must_fail git pull . second 2>out &&\n+\ttest_i18ngrep \"You have not concluded your merge\" out &&\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":"260709","messageId":"1430988248-18285-4-git-send-email-pyokagan@gmail.com","threadId":"39267","inReplyTo":"1430988248-18285-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 03/12] t5520: test work tree fast-forward when fetch updates head","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-07T08:43:59Z","receivedAt":"2015-05-07T08:43:59Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Since commit b10ac50f (Fix pulling into the same branch), 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 \n---\n\nNotes:\n * Ensure that on fast-forward failure, if there is a conflict, the work\n tree should not be touched.\n\n t/t5520-pull.sh | 22 ++++++++++++++++++++++\n 1 file changed, 22 insertions(+)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 37ff45f..99b6f67 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -185,6 +185,28 @@ 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 master^ &&\n+\ttest_when_finished \"git checkout -f copy && git branch -D third\" &&\n+\techo file >expected &&\n+\ttest_cmp expected file &&\n+\tgit pull . second:third 2>out &&\n+\ttest_i18ngrep \"fetch updated the current branch head\" out &&\n+\techo modified >expected &&\n+\ttest_cmp expected file\n+'\n+\n+test_expect_success 'fast-forward fails with conflicting work tree' '\n+\tgit checkout -b third master^ &&\n+\ttest_when_finished \"git checkout -f copy && git branch -D third\" &&\n+\techo file >expected &&\n+\ttest_cmp expected file &&\n+\techo conflict >file &&\n+\ttest_must_fail git pull . second:third 2>out &&\n+\ttest_i18ngrep \"Cannot fast-forward your working tree\" out &&\n+\ttest `cat file` = conflict\n+'\n+\n test_expect_success '--rebase' '\n \tgit branch to-rebase &&\n \techo modified again > file &&\n-- \n2.1.4\n"},{"id":"260706","messageId":"1430988248-18285-5-git-send-email-pyokagan@gmail.com","threadId":"39267","inReplyTo":"1430988248-18285-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 04/12] t5520: test --rebase with multiple branches","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-07T08:44:00Z","receivedAt":"2015-05-07T08:44:00Z","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 commit 51b2ead0 (\"disallow providing multiple upstream branches\nto rebase, pull --rebase\"), 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 \n---\n\nNotes:\n * Check that when git-pull fails, HEAD was not moved.\n \n * Removed the not-really-required `test_when_finished \"rm -f out\"`.\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 99b6f67..05a92a2 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -220,6 +220,15 @@ test_expect_success '--rebase' '\n \ttest $(git rev-parse HEAD^) = $(git rev-parse copy) &&\n \ttest 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>out &&\n+\ttest $(git rev-parse HEAD) = $(git rev-parse before-rebase) &&\n+\ttest_i18ngrep \"Cannot rebase onto multiple branches\" out &&\n+\ttest 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":"260712","messageId":"1430988248-18285-6-git-send-email-pyokagan@gmail.com","threadId":"39267","inReplyTo":"1430988248-18285-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 05/12] t5520: test --rebase failure on unborn branch with index","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-07T08:44:01Z","receivedAt":"2015-05-07T08:44:01Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Commit 19a7fcbf (allow pull --rebase on branch yet to be born) special\ncases git-pull on an unborn branch in a different code path such that\ngit-pull --rebase is still valid even though there is 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 \n---\n\nNotes:\n * Test for error message.\n \n * Ensure that when git-pull does not modify the index.\n \n * Moved test_when_finished\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 05a92a2..9107991 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -415,6 +415,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\ttest \"$(git ls-files)\" = staged-file &&\n+\t\ttest_must_fail git pull --rebase .. master 2>../out &&\n+\t\ttest \"$(git ls-files)\" = staged-file &&\n+\t\ttest \"$(git show :staged-file)\" = staged-file\n+\t) &&\n+\ttest_i18ngrep \"unborn branch with changes added to the index\" out\n+'\n+\n test_expect_success 'setup for detecting upstreamed changes' '\n \tmkdir src &&\n \t(cd src &&\n-- \n2.1.4\n"},{"id":"260710","messageId":"1430988248-18285-7-git-send-email-pyokagan@gmail.com","threadId":"39267","inReplyTo":"1430988248-18285-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 06/12] t5521: test --dry-run does not make any changes","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-07T08:44:02Z","receivedAt":"2015-05-07T08:44:02Z","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 \n---\n\nNotes:\n * Moved test_when_finished to beginning of test\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..21b1dbe 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":"260713","messageId":"1430988248-18285-8-git-send-email-pyokagan@gmail.com","threadId":"39267","inReplyTo":"1430988248-18285-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 07/12] t4013: call git-merge instead of git-pull","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-07T08:44:03Z","receivedAt":"2015-05-07T08:44:03Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Since \"git fetch .\" does not update any refs, \"git pull . side\" is\nequivalent to calling \"git merge side\".\n\nAs such, replace the call to git-pull with a call to git-merge to reduce\nthe dependence on git-pull's functionality to reduce irrelevant test\nbreakage when git-pull is rewritten to C.\n\nSigned-off-by: Paul Tan \n---\n\nNotes:\n * This is a new patch in the patch series.\n\n t/t4013-diff-various.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\nindex 6ec6072..48f2fe2 100755\n--- a/t/t4013-diff-various.sh\n+++ b/t/t4013-diff-various.sh\n@@ -64,7 +64,7 @@ test_expect_success setup '\n \texport GIT_AUTHOR_DATE GIT_COMMITTER_DATE &&\n \n \tgit checkout master &&\n-\tgit pull -s ours . side &&\n+\tgit merge -s ours side &&\n \n \tGIT_AUTHOR_DATE=\"2006-06-26 00:05:00 +0000\" &&\n \tGIT_COMMITTER_DATE=\"2006-06-26 00:05:00 +0000\" &&\n-- \n2.1.4\n"},{"id":"260711","messageId":"1430988248-18285-9-git-send-email-pyokagan@gmail.com","threadId":"39267","inReplyTo":"1430988248-18285-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 08/12] t5520: ensure origin refs are updated","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-07T08:44:04Z","receivedAt":"2015-05-07T08:44:04Z","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 \n---\n\nNotes:\n * This is a new patch in the patch series.\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 9107991..d97a575 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -461,6 +461,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":"260714","messageId":"1430988248-18285-10-git-send-email-pyokagan@gmail.com","threadId":"39267","inReplyTo":"1430988248-18285-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 09/12] t7406: use \"git pull\" instead of \"git pull --rebase\"","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-07T08:44:05Z","receivedAt":"2015-05-07T08:44:05Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"At this point, the HEAD of super/submodule/ is a direct descendent of\nsubmodule/ and thus a fast-forward merge can occur. There is no need to\nrebase.\n\nCall \"git pull\" instead of \"git pull --rebase\" in order to reduce\ndependence on git-pull's functionality, and thus prevent the whole test suite\nfrom failing when git-pull is rewritten to C.\n\nSigned-off-by: Paul Tan \n---\n\nNotes:\n * This is a new patch in the patch series.\n\n t/t7406-submodule-update.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex dda3929..e38d830 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -44,7 +44,7 @@ test_expect_success 'setup a submodule tree' '\n \t) &&\n \t(cd super &&\n \t (cd submodule &&\n-\t git pull --rebase origin\n+\t git pull origin\n \t ) &&\n \t git add submodule &&\n \t git commit -m \"submodule update\"\n-- \n2.1.4\n"},{"id":"260715","messageId":"1430988248-18285-11-git-send-email-pyokagan@gmail.com","threadId":"39267","inReplyTo":"1430988248-18285-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 10/12] t5520: failing test for pull --all with no configured upstream","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-07T08:44:06Z","receivedAt":"2015-05-07T08:44:06Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"error_on_no_merge_candidates() does not consider the case where \"$#\"\nincludes command-line flags that are passed to git-fetch.\n\nAs such, when the current branch has no configured upstream, and there\nare no merge candidates because of that, git-pull --all erroneously reports\nthat we are pulling from \"--all\", as it believes that the first argument\nis the remote name.\n\nAdd a failing test that shows this case.\n\nReported-by: Stephen Robin \nSigned-off-by: Paul Tan \n---\n\nNotes:\n * Added this test to the patch series.\n\n t/t5520-pull.sh | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex d97a575..b93b735 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -153,6 +153,18 @@ test_expect_success 'fail if no configuration for current branch' '\n \ttest `cat file` = file\n '\n \n+test_expect_failure 'pull --all: 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+\ttest `cat file` = file &&\n+\ttest_must_fail git pull --all 2>out &&\n+\ttest_i18ngrep \"There is no tracking information\" out &&\n+\ttest `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-- \n2.1.4\n"},{"id":"260717","messageId":"1430988248-18285-12-git-send-email-pyokagan@gmail.com","threadId":"39267","inReplyTo":"1430988248-18285-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 11/12] t5524: test --log=1 limits shortlog length","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-07T08:44:07Z","receivedAt":"2015-05-07T08:44:07Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"While git-pull supports --log and passes the switch to git-merge, it\ndoes not support --log=, ignoring the value .\n\nThis is not only at odds with the documentation of git-pull, it's also a\nundesirable limitation as could simply be passed to git-merge as\nwell.\n\nImplement a failing test that demonstrates this.\n\nSigned-off-by: Paul Tan \n---\n\nNotes:\n * Added this test to the patch series\n\n t/t5524-pull-msg.sh | 17 +++++++++++++++++\n 1 file changed, 17 insertions(+)\n\ndiff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh\nindex 8cccecc..5b7af07 100755\n--- a/t/t5524-pull-msg.sh\n+++ b/t/t5524-pull-msg.sh\n@@ -17,6 +17,9 @@ test_expect_success setup '\n \t\tgit commit -m \"add bfile\"\n \t) &&\n \ttest_tick && test_tick &&\n+\techo \"second\" >afile &&\n+\tgit add afile &&\n+\tgit commit -m \"second commit\" &&\n \techo \"original $dollar\" >afile &&\n \tgit add afile &&\n \tgit commit -m \"do not clobber $dollar signs\"\n@@ -32,4 +35,18 @@ test_expect_success pull '\n )\n '\n \n+test_expect_failure '--log=1 limits shortlog length' '\n+(\n+\tcd cloned &&\n+\tgit reset --hard HEAD^ &&\n+\ttest `cat afile` = original &&\n+\ttest `cat bfile` = added &&\n+\tgit pull --log &&\n+\tgit log -3 &&\n+\tgit cat-file commit HEAD >result &&\n+\tgrep Dollar result &&\n+\t! grep \"second commit\" result\n+)\n+'\n+\n test_done\n-- \n2.1.4\n"},{"id":"260716","messageId":"1430988248-18285-13-git-send-email-pyokagan@gmail.com","threadId":"39267","inReplyTo":"1430988248-18285-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 12/12] t5520: check reflog action in fast-forward merge","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-07T08:44:08Z","receivedAt":"2015-05-07T08:44:08Z","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 \n---\n\nNotes:\n * Added this test to the patch series.\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 b93b735..6045491 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 \ttest `cat file` = file &&\n \tgit pull &&\n-\ttest `cat file` = updated\n+\ttest `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 \ttest `cat file` = file &&\n \tgit pull . second &&\n-\ttest `cat file` = modified\n+\ttest `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":"260719","messageId":"554B2AEB.3020608@web.de","threadId":"39267","inReplyTo":"1430988248-18285-2-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 01/12] t5520: implement tests for no merge candidates cases","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-05-07T09:05:47Z","receivedAt":"2015-05-07T09:05:47Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 05/07/2015 10:43 AM, Paul Tan wrote:\n> Commit a8c9bef4 fully established the current advices given by git-pull\n> for the different cases where git-fetch will not have anything marked\n> for merge:\n>\n> 1. 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>\n> 2. 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>\n> 3. 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>\n> 4. 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>\n> Implement tests for the above 5 cases to ensure that the correct code\n> paths are triggered for each of these cases.\n>\n> Signed-off-by: Paul Tan \n> ---\n>\n> Notes:\n> * Re-worded commit message to match the logic used in git-pull.sh's\n> error_on_no_merge_candidates().\n> \n> * The tests have thus also been reordered to match the commit message.\n> \n> * Non-hackish solution for case 3a.\n> \n> * Add more checks to ensure that git-pull does not touch any files it\n> should not be touching on failure.\n>\n> t/t5520-pull.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n> 1 file changed, 55 insertions(+)\n>\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index 7efd45b..5add900 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> \ttest `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> +\ttest `cat file` = file &&\nMinor nit, please see Documentation/CodingGuidelines:\n - We prefer $( ... ) for command substitution; unlike ``, it\n properly nests. It should have been the way Bourne spelled\n it from day one, but unfortunately isn't.\nIn other words:\ntest $(cat file) = file &&\n"},{"id":"260726","messageId":"xmqqh9roscty.fsf@gitster.dls.corp.google.com","threadId":"39267","inReplyTo":"554B2AEB.3020608@web.de","subject":"Re: [PATCH v2 01/12] t5520: implement tests for no merge candidates cases","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-07T14:04:57Z","receivedAt":"2015-05-07T14:04:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen writes:\n\n> In other words:\n> test $(cat file) = file &&\n\nIs there a guarantee that file has a single word? Can it be empty?\nCan it contain \"foo bar\\n\"?\n"},{"id":"260728","messageId":"CACRoPnSCvitGfVKyqV7PJd_TzLJEs8U_mjYhaRy3KOFrWjBEwA@mail.gmail.com","threadId":"39267","inReplyTo":"xmqqh9roscty.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 01/12] t5520: implement tests for no merge candidates cases","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-07T14:47:02Z","receivedAt":"2015-05-07T14:47:02Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Junio,\n\nOn Thu, May 7, 2015 at 10:04 PM, Junio C Hamano wrote:\n> Torsten Bögershausen writes:\n>\n>> In other words:\n>> test $(cat file) = file &&\n>\n> Is there a guarantee that file has a single word? Can it be empty?\n> Can it contain \"foo bar\\n\"?\n\nIt can, but it should not ;-). But yes, this will need to be quoted as\nwell to be safe. Whoops.\n\nThanks,\nPaul\n"},{"id":"260729","messageId":"554B7D0E.5040808@web.de","threadId":"39267","inReplyTo":"xmqqh9roscty.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 01/12] t5520: implement tests for no merge candidates cases","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-05-07T14:56:14Z","receivedAt":"2015-05-07T14:56:14Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"\n\nOn 07/05/15 16:04, Junio C Hamano wrote:\n> Torsten Bögershausen writes:\n>\n>> In other words:\n>> test $(cat file) = file &&\n> Is there a guarantee that file has a single word? Can it be empty?\n> Can it contain \"foo bar\\n\"?\nSo true, will this work ?\n\ntest \"$(cat file)\" = file &&\n"},{"id":"260731","messageId":"xmqq4mnos7ns.fsf@gitster.dls.corp.google.com","threadId":"39267","inReplyTo":"CACRoPnSCvitGfVKyqV7PJd_TzLJEs8U_mjYhaRy3KOFrWjBEwA@mail.gmail.com","subject":"Re: [PATCH v2 01/12] t5520: implement tests for no merge candidates cases","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-07T15:56:39Z","receivedAt":"2015-05-07T15:56:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan writes:\n\n> Hi Junio,\n>\n> On Thu, May 7, 2015 at 10:04 PM, Junio C Hamano wrote:\n>> Torsten Bögershausen writes:\n>>\n>>> In other words:\n>>> test $(cat file) = file &&\n>>\n>> Is there a guarantee that file has a single word? Can it be empty?\n>> Can it contain \"foo bar\\n\"?\n>\n> It can, but it should not ;-). But yes, this will need to be quoted as\n> well to be safe. Whoops.\n\nYup. I see that existing test (this script is ancient, isn't it?)\nhas the same issue of using backticks and not quoting sufficiently.\n\nPerhaps we would want a clean-up-and-modernise step before this\npatch to reduce patch noise.\n\nThanks.\n"},{"id":"260736","messageId":"CAGZ79kY88=r83x_eZ+RgUb9CAFTN4uukB11RwJhY+GNKL3Tq0Q@mail.gmail.com","threadId":"39267","inReplyTo":"1430988248-18285-4-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 03/12] t5520: test work tree fast-forward when fetch updates head","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-05-07T16:23:22Z","receivedAt":"2015-05-07T16:23:22Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, May 7, 2015 at 1:43 AM, Paul Tan wrote:\n> Since commit b10ac50f (Fix pulling into the same branch), git-pull,\n> upon detecting that git-fetch updated the current head, will\n> fast-forward the working tree to the updated head commit.\n>\n> Implement tests to ensure that the fast-forward occurs in such a case,\n> as well as to ensure that the user-friendly advice is printed upon\n> failure.\n>\n> Signed-off-by: Paul Tan \n> ---\n>\n> Notes:\n> * Ensure that on fast-forward failure, if there is a conflict, the work\n> tree should not be touched.\n>\n> t/t5520-pull.sh | 22 ++++++++++++++++++++++\n> 1 file changed, 22 insertions(+)\n>\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index 37ff45f..99b6f67 100755\n> --- a/t/t5520-pull.sh\n> +++ b/t/t5520-pull.sh\n> @@ -185,6 +185,28 @@ test_expect_success 'fail if the index has unresolved entries' '\n> test_cmp expected file\n> '\n>\n> +test_expect_success 'fast-forwards working tree if branch head is updated' '\n> + git checkout -b third master^ &&\n> + test_when_finished \"git checkout -f copy && git branch -D third\" &&\n> + echo file >expected &&\n> + test_cmp expected file &&\n> + git pull . second:third 2>out &&\n> + test_i18ngrep \"fetch updated the current branch head\" out &&\n> + echo modified >expected &&\n> + test_cmp expected file\n> +'\n> +\n> +test_expect_success 'fast-forward fails with conflicting work tree' '\n> + git checkout -b third master^ &&\n> + test_when_finished \"git checkout -f copy && git branch -D third\" &&\n> + echo file >expected &&\n> + test_cmp expected file &&\n> + echo conflict >file &&\n> + test_must_fail git pull . second:third 2>out &&\n> + test_i18ngrep \"Cannot fast-forward your working tree\" out &&\n> + test `cat file` = conflict\n\nsame comments as in patch 1 apply here.\n\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":"260737","messageId":"a2c6df723ca5237c094ab4002e45a834@www.dscho.org","threadId":"39267","inReplyTo":"1430988248-18285-8-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 07/12] t4013: call git-merge instead of git-pull","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-05-07T16:26:34Z","receivedAt":"2015-05-07T16:26:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nsorry for being so slow on reviewing your patches, but I saw that plenty of excellent feedback has made the second round of patches even better than the first one.\n\nI am particularly impressed by the thoroughness of the commit messages, they make reviewing much more pleasant.\n\nOn 2015-05-07 10:44, Paul Tan wrote:\n\n> As such, replace the call to git-pull with a call to git-merge to reduce\n> the dependence on git-pull's functionality to reduce irrelevant test\n> breakage when git-pull is rewritten to C.\n\nBoth this patch and 9/12 change `git pull` invocations to equivalent non-pull ones, but I wonder whether it would not be a better idea to leave them as-are, so that we can make sure that scripts out there that might use similar `git pull` invocations would be unaffected by the rewrite?\n\nCiao,\nDscho\n"},{"id":"260739","messageId":"05adfac12cb9a7ad183281974d991e00@www.dscho.org","threadId":"39267","inReplyTo":"1430988248-18285-12-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 11/12] t5524: test --log=1 limits shortlog length","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-05-07T16:28:56Z","receivedAt":"2015-05-07T16:28:56Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn 2015-05-07 10:44, Paul Tan wrote:\n> @@ -32,4 +35,18 @@ test_expect_success pull '\n> )\n> '\n> \n> +test_expect_failure '--log=1 limits shortlog length' '\n> +(\n> +\tcd cloned &&\n> +\tgit reset --hard HEAD^ &&\n> +\ttest `cat afile` = original &&\n> +\ttest `cat bfile` = added &&\n> +\tgit pull --log &&\n> +\tgit log -3 &&\n> +\tgit cat-file commit HEAD >result &&\n> +\tgrep Dollar result &&\n> +\t! grep \"second commit\" result\n> +)\n\nI think it might be better to use `test_must_fail` here, just for consistency (the `!` operator would also pass if `grep` itself could not be executed correctly, quite academic, I know, given that `grep` is exercised plenty of times by the test suite, but still...)\n\nWhat do you think?\n\nCiao,\nDscho\n\nP.S.: I missed 12/12 but the rest of the patches looked fine to these old eyes. Thanks!\n"},{"id":"260740","messageId":"CAGZ79kY7gstS1xPcNu4HqmmXA+qg9BKpXZwaHeQ91n7vL2Sa-g@mail.gmail.com","threadId":"39267","inReplyTo":"1430988248-18285-6-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 05/12] t5520: test --rebase failure on unborn branch with index","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-05-07T16:32:22Z","receivedAt":"2015-05-07T16:32:22Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, May 7, 2015 at 1:44 AM, Paul Tan wrote:\n> Commit 19a7fcbf (allow pull --rebase on branch yet to be born) special\n> cases git-pull on an unborn branch in a different code path such that\n> git-pull --rebase is still valid even though there is no HEAD yet.\n>\n> This code path still ensures that there is no index in order not to lose\n> any staged changes. Implement a test to ensure that this check is\n> triggered.\n>\n> Signed-off-by: Paul Tan \n> ---\n>\n> Notes:\n> * Test for error message.\n>\n> * Ensure that when git-pull does not modify the index.\n>\n> * Moved test_when_finished\n>\n> t/t5520-pull.sh | 15 +++++++++++++++\n> 1 file changed, 15 insertions(+)\n>\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index 05a92a2..9107991 100755\n> --- a/t/t5520-pull.sh\n> +++ b/t/t5520-pull.sh\n> @@ -415,6 +415,21 @@ test_expect_success 'pull --rebase works on branch yet to be born' '\n> test_cmp expect actual\n> '\n>\n> +test_expect_success 'pull --rebase fails on unborn branch with staged changes' '\n> + test_when_finished \"rm -rf empty_repo2\" &&\n> + git init empty_repo2 &&\n> + (\n> + cd empty_repo2 &&\n> + echo staged-file >staged-file &&\n> + git add staged-file &&\n> + test \"$(git ls-files)\" = staged-file &&\n\nI think usually people use\n\n git ls-files >actual\n echo staged-file >expected && # you have this already in your 2nd\n # line in the paragraph\n test_cmp staged-file actual\n\nto make debugging easier as you can inspect the files (actual, expected)\nafter the test has failed.\n\nPersonally I don't mind the difference as when it comes to debugging\nusing the test suite I haven't found the silver bullet yet.\n\n\n> + test_must_fail git pull --rebase .. master 2>../out &&\n> + test \"$(git ls-files)\" = staged-file &&\n> + test \"$(git show :staged-file)\" = staged-file\n> + ) &&\n> + test_i18ngrep \"unborn branch with changes added to the index\" out\n> +'\n> +\n> test_expect_success 'setup for detecting upstreamed changes' '\n> mkdir src &&\n> (cd src &&\n> --\n> 2.1.4\n>\n"},{"id":"260742","messageId":"CAGZ79kaUCp=8JKSspqetzEGFMF7FVvCvFwO9v+A_XtiEd_V=fw@mail.gmail.com","threadId":"39267","inReplyTo":"1430988248-18285-13-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 12/12] t5520: check reflog action in fast-forward merge","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-05-07T16:39:23Z","receivedAt":"2015-05-07T16:39:23Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, May 7, 2015 at 1:44 AM, Paul Tan wrote:\n> When testing a fast-forward merge with git-pull, check to see if the\n> reflog action is \"pull\" with the arguments passed to git-pull.\n>\n> While we are in the vicinity, remove the empty line as well.\n>\n> Signed-off-by: Paul Tan \n> ---\n>\n> Notes:\n> * Added this test to the patch series.\n>\n> t/t5520-pull.sh | 13 ++++++++++---\n> 1 file changed, 10 insertions(+), 3 deletions(-)\n>\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index b93b735..6045491 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> git branch copy master &&\n> git config branch.copy.remote . &&\n> git config branch.copy.merge refs/heads/master &&\n> @@ -95,7 +94,11 @@ test_expect_success 'test . as a remote' '\n> git checkout copy &&\n> test `cat file` = file &&\n> git pull &&\n> - test `cat file` = updated\n> + test `cat file` = updated &&\n\nsame as in patch 1\n\n> + git reflog -1 >reflog.actual &&\n> + sed \"s/$_x05[0-9a-f]*/OBJID/g\" reflog.actual >reflog.fuzzy &&\n> + echo \"OBJID HEAD@{0}: pull: Fast-forward\" >reflog.expected &&\n> + test_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> git reset --hard HEAD^ &&\n> test `cat file` = file &&\n> git pull . second &&\n> - test `cat file` = modified\n> + test `cat file` = modified &&\n> + git reflog -1 >reflog.actual &&\n> + sed \"s/$_x05[0-9a-f]*/OBJID/g\" reflog.actual >reflog.fuzzy &&\n> + echo \"OBJID HEAD@{0}: pull . second: Fast-forward\" >reflog.expected &&\n> + test_cmp reflog.expected reflog.fuzzy\n> '\n>\n> test_expect_success 'fail if wildcard spec does not match any refs' '\n> --\n> 2.1.4\n>\n\n\nThe series looks good to me apart from the minor nits.\n\nThanks,\nStefan\n"},{"id":"260745","messageId":"CACRoPnTMgZBfeneMCZSJVowmSCGf2Ufpt-J461+w9Lh1aN-rhA@mail.gmail.com","threadId":"39267","inReplyTo":"a2c6df723ca5237c094ab4002e45a834@www.dscho.org","subject":"Re: [PATCH v2 07/12] t4013: call git-merge instead of git-pull","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-07T16:55:51Z","receivedAt":"2015-05-07T16:55:51Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Dscho,\n\nOn Fri, May 8, 2015 at 12:26 AM, Johannes Schindelin\n wrote:\n> Both this patch and 9/12 change `git pull` invocations to equivalent non-pull ones, but I wonder whether it would not be a better idea to leave them as-are, so that we can make sure that scripts out there that might use similar `git pull` invocations would be unaffected by the rewrite?\n\nIn the current state[1], I'm aiming for the git-pull rewrite patch\nseries to break all git-pull tests in the first patch, and then\nsubsequently make them pass again in later smaller patches by\nimplementing back the old features. This will make reviewing the code\nmuch easier, as opposed to dumping a huge patch every single re-roll\n;-).\n\nFor both patches and test suites, if the \"setup\" tests fail, the whole\ntest suite fails. Given that the test suites are about testing the\ndiff formatting options and submodules update implementation, which is\nmostly irrelevant to git-pull, I think it would be better if the test\nsuite was not affected by the rewrite, especially since it only\nrequires changing one line.\n\n[1] https://github.com/pyokagan/git/commit/bfdf5039d1627c9599051faf2ce34b007d4bfbea\n\nThanks,\nPaul\n"},{"id":"260748","messageId":"CACRoPnRfqQMRJ5N=oL84SMw=FC=Eg-Co-De_9E9cpWCaaSZAxw@mail.gmail.com","threadId":"39267","inReplyTo":"05adfac12cb9a7ad183281974d991e00@www.dscho.org","subject":"Re: [PATCH v2 11/12] t5524: test --log=1 limits shortlog length","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-07T17:06:18Z","receivedAt":"2015-05-07T17:06:18Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Dscho,\n\nOn Fri, May 8, 2015 at 12:28 AM, Johannes Schindelin\n wrote:\n> Hi Paul,\n>\n> On 2015-05-07 10:44, Paul Tan wrote:\n>> @@ -32,4 +35,18 @@ test_expect_success pull '\n>> )\n>> '\n>>\n>> +test_expect_failure '--log=1 limits shortlog length' '\n>> +(\n>> + cd cloned &&\n>> + git reset --hard HEAD^ &&\n>> + test `cat afile` = original &&\n>> + test `cat bfile` = added &&\n>> + git pull --log &&\n>> + git log -3 &&\n>> + git cat-file commit HEAD >result &&\n>> + grep Dollar result &&\n>> + ! grep \"second commit\" result\n>> +)\n>\n> I think it might be better to use `test_must_fail` here, just for consistency (the `!` operator would also pass if `grep` itself could not be executed correctly, quite academic, I know, given that `grep` is exercised plenty of times by the test suite, but still...)\n>\n> What do you think?\n\nYep, it's definitely better. Sometimes I forget about the existence of\nsome test utility functions :-/.\n\nThanks,\nPaul\n"},{"id":"260749","messageId":"xmqqoalwqpk4.fsf@gitster.dls.corp.google.com","threadId":"39267","inReplyTo":"1430988248-18285-4-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 03/12] t5520: test work tree fast-forward when fetch updates head","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-07T17:12:59Z","receivedAt":"2015-05-07T17:12:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan writes:\n\n> +test_expect_success 'fast-forward fails with conflicting work tree' '\n> +\tgit checkout -b third master^ &&\n> +\ttest_when_finished \"git checkout -f copy && git branch -D third\" &&\n> +\techo file >expected &&\n> +\ttest_cmp expected file &&\n> +\techo conflict >file &&\n> +\ttest_must_fail git pull . second:third 2>out &&\n> +\ttest_i18ngrep \"Cannot fast-forward your working tree\" out &&\n> +\ttest `cat file` = conflict\n\nAt this point, HEAD would match either master^ (as initially checked\nout) or second (as fetch fast-forwarded), but I cannot read what this\ntest is expecting to happen. \n\nShould the HEAD move or stay?\n\n> +'\n> +\n> test_expect_success '--rebase' '\n> \tgit branch to-rebase &&\n> \techo modified again > file &&\n"},{"id":"260750","messageId":"xmqqk2wkqpc9.fsf@gitster.dls.corp.google.com","threadId":"39267","inReplyTo":"a2c6df723ca5237c094ab4002e45a834@www.dscho.org","subject":"Re: [PATCH v2 07/12] t4013: call git-merge instead of git-pull","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-07T17:17:42Z","receivedAt":"2015-05-07T17:17:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Both this patch and 9/12 change `git pull` invocations to equivalent\n> non-pull ones, but I wonder whether it would not be a better idea to\n> leave them as-are, so that we can make sure that scripts out there\n> that might use similar `git pull` invocations would be unaffected by\n> the rewrite?\n\nYes. I do not mind changing t4013 that is not about testing 'git pull'\nto lose 'git pull . other-branch', but then we would want to have a\ncorresponding test somewhere in the test script that is about\ntesting 'git pull' to cover that (ancient) pattern of use.\n\nThanks.\n"},{"id":"260751","messageId":"xmqqfv78qp13.fsf@gitster.dls.corp.google.com","threadId":"39267","inReplyTo":"1430988248-18285-10-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 09/12] t7406: use \"git pull\" instead of \"git pull --rebase\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-07T17:24:24Z","receivedAt":"2015-05-07T17:24:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan writes:\n\n> At this point, the HEAD of super/submodule/ is a direct descendent of\n> submodule/ and thus a fast-forward merge can occur. There is no need to\n> rebase.\n>\n> Call \"git pull\" instead of \"git pull --rebase\" in order to reduce\n> dependence on git-pull's functionality, and thus prevent the whole test suite\n> from failing when git-pull is rewritten to C.\n\nAlmost the same comment as 07/12 applies here. \"when the pull would\nresult in fast-forward, 'pull' and 'pull --rebase' results in\nexactly the same history\" is something we would want to have in the\ntest suite for 'git pull', and it is perfectly fine to lose the\ninvocation of 'pull --rebase' from here.\n\nHaving said all that.\n\nIf 'git pull' gets broken, it will break this test _anyway_. Unless\nthe operating assumption is \"it is OK to break 'git pull --rebase',\nas long as we do not break 'git pull', while rewriting it\", I am not\nsure the value of the change in this patch. We'd need to keep both\nform working, no?\n\n>\n> Signed-off-by: Paul Tan \n> ---\n>\n> Notes:\n> * This is a new patch in the patch series.\n>\n> t/t7406-submodule-update.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\n> index dda3929..e38d830 100755\n> --- a/t/t7406-submodule-update.sh\n> +++ b/t/t7406-submodule-update.sh\n> @@ -44,7 +44,7 @@ test_expect_success 'setup a submodule tree' '\n> \t) &&\n> \t(cd super &&\n> \t (cd submodule &&\n> -\t git pull --rebase origin\n> +\t git pull origin\n> \t ) &&\n> \t git add submodule &&\n> \t git commit -m \"submodule update\"\n"},{"id":"260755","messageId":"xmqqbnhwqoue.fsf@gitster.dls.corp.google.com","threadId":"39267","inReplyTo":"1430988248-18285-12-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 11/12] t5524: test --log=1 limits shortlog length","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-07T17:28:25Z","receivedAt":"2015-05-07T17:28:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan writes:\n\n> While git-pull supports --log and passes the switch to git-merge, it\n> does not support --log=, ignoring the value .\n>\n> This is not only at odds with the documentation of git-pull, it's also a\n> undesirable limitation as could simply be passed to git-merge as\n> well.\n\nA cleaner alternative may be to fix that while git-pull is still a\nscript, as you seem to already know what is broken and what in the\ncurrent code needs to be fixed in what way exactly. Perhaps do that\nat the earlier part of (or even as an independent patch outside)\nthis series and add this test to protect the fix from getting broken\nlater (with expect-failure flipped to expect-success)?\n\nThanks.\n\n>\n> Implement a failing test that demonstrates this.\n\n\n\n>\n> Signed-off-by: Paul Tan \n> ---\n>\n> Notes:\n> * Added this test to the patch series\n>\n> t/t5524-pull-msg.sh | 17 +++++++++++++++++\n> 1 file changed, 17 insertions(+)\n>\n> diff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh\n> index 8cccecc..5b7af07 100755\n> --- a/t/t5524-pull-msg.sh\n> +++ b/t/t5524-pull-msg.sh\n> @@ -17,6 +17,9 @@ test_expect_success setup '\n> \t\tgit commit -m \"add bfile\"\n> \t) &&\n> \ttest_tick && test_tick &&\n> +\techo \"second\" >afile &&\n> +\tgit add afile &&\n> +\tgit commit -m \"second commit\" &&\n> \techo \"original $dollar\" >afile &&\n> \tgit add afile &&\n> \tgit commit -m \"do not clobber $dollar signs\"\n> @@ -32,4 +35,18 @@ test_expect_success pull '\n> )\n> '\n> \n> +test_expect_failure '--log=1 limits shortlog length' '\n> +(\n> +\tcd cloned &&\n> +\tgit reset --hard HEAD^ &&\n> +\ttest `cat afile` = original &&\n> +\ttest `cat bfile` = added &&\n> +\tgit pull --log &&\n> +\tgit log -3 &&\n> +\tgit cat-file commit HEAD >result &&\n> +\tgrep Dollar result &&\n> +\t! grep \"second commit\" result\n> +)\n> +'\n> +\n> test_done\n"},{"id":"260756","messageId":"CACRoPnR_T95p+FeLLiQViBiE5z9H3XAEUA5pokuG4B5nns0H0A@mail.gmail.com","threadId":"39267","inReplyTo":"xmqqoalwqpk4.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 03/12] t5520: test work tree fast-forward when fetch updates head","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-07T17:32:47Z","receivedAt":"2015-05-07T17:32:47Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Junio,\n\nOn Fri, May 8, 2015 at 1:12 AM, Junio C Hamano wrote:\n> Paul Tan writes:\n>\n>> +test_expect_success 'fast-forward fails with conflicting work tree' '\n>> + git checkout -b third master^ &&\n>> + test_when_finished \"git checkout -f copy && git branch -D third\" &&\n>> + echo file >expected &&\n>> + test_cmp expected file &&\n>> + echo conflict >file &&\n>> + test_must_fail git pull . second:third 2>out &&\n>> + test_i18ngrep \"Cannot fast-forward your working tree\" out &&\n>> + test `cat file` = conflict\n>\n> At this point, HEAD would match either master^ (as initially checked\n> out) or second (as fetch fast-forwarded), but I cannot read what this\n> test is expecting to happen.\n>\n> Should the HEAD move or stay?\n\nUgh, it was probably me getting confused by all the test dependencies :-(.\n\nAnyway, second^ == master^ == the initial commit at this point, so it\nwould be clearer to write \"git checkout -b third second^\", I think.\n\nThe HEAD has already moved because git-fetch updated it. I should\ndefinitely add a check to make that clear.\n\nWhether it should or not is something I have an answer to yet though.\n\nThanks,\nPaul\n"},{"id":"260757","messageId":"xmqq3838qoeb.fsf@gitster.dls.corp.google.com","threadId":"39267","inReplyTo":"1430988248-18285-11-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 10/12] t5520: failing test for pull --all with no configured upstream","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-07T17:38:04Z","receivedAt":"2015-05-07T17:38:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan writes:\n\n> error_on_no_merge_candidates() does not consider the case where \"$#\"\n> includes command-line flags that are passed to git-fetch.\n>\n> As such, when the current branch has no configured upstream, and there\n> are no merge candidates because of that, git-pull --all erroneously reports\n> that we are pulling from \"--all\", as it believes that the first argument\n> is the remote name.\n\nInteresting.\n\nI do not think \"pull [origin] --all\" makes much sense for the same\nreason why we error out when you say \"pull [origin] --tag\", so I am\nnot sure \"There is no tracking information\" is the right diag we\nwould want to give the user, but I agree that \"--all\" is not a\nremote name.\n\nDoes the same comment as 11/12 applies to this as well?\n\n>\n> Add a failing test that shows this case.\n>\n> Reported-by: Stephen Robin \n> Signed-off-by: Paul Tan \n> ---\n>\n> Notes:\n> * Added this test to the patch series.\n>\n> t/t5520-pull.sh | 12 ++++++++++++\n> 1 file changed, 12 insertions(+)\n>\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index d97a575..b93b735 100755\n> --- a/t/t5520-pull.sh\n> +++ b/t/t5520-pull.sh\n> @@ -153,6 +153,18 @@ test_expect_success 'fail if no configuration for current branch' '\n> \ttest `cat file` = file\n> '\n> \n> +test_expect_failure 'pull --all: 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> +\ttest `cat file` = file &&\n> +\ttest_must_fail git pull --all 2>out &&\n> +\ttest_i18ngrep \"There is no tracking information\" out &&\n> +\ttest `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"},{"id":"260758","messageId":"CACRoPnQzAjY0bPmHrw2o4Mdzn1Q4Dd587hLXkEPA-qYcYrVGEg@mail.gmail.com","threadId":"39267","inReplyTo":"CAGZ79kY7gstS1xPcNu4HqmmXA+qg9BKpXZwaHeQ91n7vL2Sa-g@mail.gmail.com","subject":"Re: [PATCH v2 05/12] t5520: test --rebase failure on unborn branch with index","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-07T17:44:47Z","receivedAt":"2015-05-07T17:44:47Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi,\n\nOn Fri, May 8, 2015 at 12:32 AM, Stefan Beller wrote:\n> On Thu, May 7, 2015 at 1:44 AM, Paul Tan wrote:\n>> +test_expect_success 'pull --rebase fails on unborn branch with staged changes' '\n>> + test_when_finished \"rm -rf empty_repo2\" &&\n>> + git init empty_repo2 &&\n>> + (\n>> + cd empty_repo2 &&\n>> + echo staged-file >staged-file &&\n>> + git add staged-file &&\n>> + test \"$(git ls-files)\" = staged-file &&\n>\n> I think usually people use\n>\n> git ls-files >actual\n> echo staged-file >expected && # you have this already in your 2nd\n> # line in the paragraph\n> test_cmp staged-file actual\n>\n> to make debugging easier as you can inspect the files (actual, expected)\n> after the test has failed.\n>\n> Personally I don't mind the difference as when it comes to debugging\n> using the test suite I haven't found the silver bullet yet.\n\nEhh, but using test_cmp will litter the test with lots of \"echo X\n>expected\" lines which I find quite distracting.\n\nJust thinking aloud, but it would be great if there was a function to\ncompare a string and a file, or a string and a string.\n\nBut yeah, I guess if the patches are verified to be correct, then I\nshould change these comparisons to use test_cmp.\n\nThanks,\nPaul\n"},{"id":"260760","messageId":"CACRoPnRXJOxUkT5sKQf2bLnWVQ-+xFPu8D2PbX2b_9XVEGwQnA@mail.gmail.com","threadId":"39267","inReplyTo":"xmqq3838qoeb.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 10/12] t5520: failing test for pull --all with no configured upstream","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-07T18:05:35Z","receivedAt":"2015-05-07T18:05:35Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi,\n\nOn Fri, May 8, 2015 at 1:38 AM, Junio C Hamano wrote:\n> Paul Tan writes:\n>\n>> error_on_no_merge_candidates() does not consider the case where \"$#\"\n>> includes command-line flags that are passed to git-fetch.\n>>\n>> As such, when the current branch has no configured upstream, and there\n>> are no merge candidates because of that, git-pull --all erroneously reports\n>> that we are pulling from \"--all\", as it believes that the first argument\n>> is the remote name.\n>\n> Interesting.\n>\n> I do not think \"pull [origin] --all\" makes much sense for the same\n> reason why we error out when you say \"pull [origin] --tag\", so I am\n> not sure \"There is no tracking information\" is the right diag we\n> would want to give the user, but I agree that \"--all\" is not a\n> remote name.\n>\n> Does the same comment as 11/12 applies to this as well?\n\nThis is actually only just one symptom of the problem of git-pull\nassuming in many places that \"$1\" is the remote. There are other\npossible ones, such as git pull --rebase failing silently at finding\nthe upstream branch to pass to git-rebase when git-fetch flags are\npassed as well (see git-pull.sh:253), though crafting a test case for\nthat would be a bit more involved. Maybe if I have extra time to look\ninto it.\n\nThen there is the problem where command line flag parsing stops when\nan unidentified flag is encountered, assuming that it is for\ngit-fetch. E.g. git-pull --all --dry-run will pass --dry-run to\ngit-fetch, but will still run git-merge. Now, I haven't written this\nup as a test case yet because I haven't the time yet, and it could be\nargued that this is expected behavior, as the git-pull docs say that\noptions meant for git-fetch must come after git-pull.\n\nHowever, this is ultimately really confusing for end-users, so I\npropose that we just drop the distinction between git-pull's flags and\ngit-fetch's flags, and just parse them all at git-pull. This can be\ndone with the current git-pull.sh, but would be easier with the\nsuperior C parse-options API.\n\nThanks,\nPaul\n"},{"id":"260762","messageId":"CACRoPnQ3zYsv63pNUtasS6y2HnWMYqwssF=yFA3OHVvF2Yb5Sw@mail.gmail.com","threadId":"39267","inReplyTo":"xmqqfv78qp13.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 09/12] t7406: use \"git pull\" instead of \"git pull --rebase\"","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-07T18:17:48Z","receivedAt":"2015-05-07T18:17:48Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi,\n\nOn Fri, May 8, 2015 at 1:24 AM, Junio C Hamano wrote:\n> Paul Tan writes:\n> If 'git pull' gets broken, it will break this test _anyway_.\n\nYou have a point, perhaps this should be changed to git-fetch + git-merge?\n\nWell my reasoning is that it's because git-pull --rebase requires more\ncode to be implemented than git-pull. So I'm thinking that git-pull\n--rebase is more likely to break than git-pull.\n\n> Unless\n> the operating assumption is \"it is OK to break 'git pull --rebase',\n> as long as we do not break 'git pull', while rewriting it\", I am not\n> sure the value of the change in this patch. We'd need to keep both\n> form working, no?\n\nYes, ultimately the git-pull rewrite must re-implement everything, but\nif this test suite is affected by any git-pull (--rebase) breakage,\nthen there will be lots of patch noise as this test suite's tests get\ndisabled/re-enabled in the git-pull rewrite patches.\n\nBut if the patch noise is okay, then I'm fine with dropping this patch\nand 07/12.\n\nThanks,\nPaul\n"},{"id":"260764","messageId":"CAPig+cT6dom8vMoy4eObwQOuERYh4nfLgFHfmofn+C7pD-PnCw@mail.gmail.com","threadId":"39267","inReplyTo":"CACRoPnQzAjY0bPmHrw2o4Mdzn1Q4Dd587hLXkEPA-qYcYrVGEg@mail.gmail.com","subject":"Re: [PATCH v2 05/12] t5520: test --rebase failure on unborn branch with index","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-05-07T18:20:36Z","receivedAt":"2015-05-07T18:20:36Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, May 7, 2015 at 1:44 PM, Paul Tan wrote:\n> On Fri, May 8, 2015 at 12:32 AM, Stefan Beller wrote:\n>> On Thu, May 7, 2015 at 1:44 AM, Paul Tan wrote:\n>>> +test_expect_success 'pull --rebase fails on unborn branch with staged changes' '\n>>> + test_when_finished \"rm -rf empty_repo2\" &&\n>>> + git init empty_repo2 &&\n>>> + (\n>>> + cd empty_repo2 &&\n>>> + echo staged-file >staged-file &&\n>>> + git add staged-file &&\n>>> + test \"$(git ls-files)\" = staged-file &&\n>>\n>> I think usually people use\n>>\n>> git ls-files >actual\n>> echo staged-file >expected && # you have this already in your 2nd\n>> # line in the paragraph\n>> test_cmp staged-file actual\n>>\n>> to make debugging easier as you can inspect the files (actual, expected)\n>> after the test has failed.\n>>\n>> Personally I don't mind the difference as when it comes to debugging\n>> using the test suite I haven't found the silver bullet yet.\n>\n> Ehh, but using test_cmp will litter the test with lots of \"echo X\n>>expected\" lines which I find quite distracting.\n>\n> Just thinking aloud, but it would be great if there was a function to\n> compare a string and a file, or a string and a string.\n>\n> But yeah, I guess if the patches are verified to be correct, then I\n> should change these comparisons to use test_cmp.\n\nCheck out verbose() in test-lib-functions.sh:643. It might be just\nwhat you want. t0020-crlf.sh has a bunch of examples of its use.\n"},{"id":"260766","messageId":"CAPig+cRBrcC+Zud=mQoCco9E3DC66gZkX5C0wTWxQ4B-Oje3eQ@mail.gmail.com","threadId":"39267","inReplyTo":"1430988248-18285-3-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 02/12] t5520: test for failure if index has unresolved entries","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-05-07T18:28:03Z","receivedAt":"2015-05-07T18:28:03Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"A couple very minor comments applying to the entire patch series...\n\nOn Thu, May 7, 2015 at 4:43 AM, Paul Tan wrote:\n> Commit d38a30df (Be more user-friendly when refusing to do something\n> because of conflict) introduced code paths to git-pull which will error\n\nCustom for citing a commit is also to include the date:\n\n d38a30df (Be more user-friendly...conflict, 2010-01-12)\n\nSome people use this git alias to help automate:\n\n whatis = show -s --pretty='tformat:%h (%s, %ad)' --date=short\n\n> out with user-friendly advices if the user is in the middle of a merge\n> or has unmerged files.\n>\n> Implement tests to ensure that git-pull will not run, and will print\n> these advices, if the user is in the middle of a merge or has unmerged\n> files in the index.\n>\n> Signed-off-by: Paul Tan \n> ---\n> t/t5520-pull.sh | 21 +++++++++++++++++++++\n> 1 file changed, 21 insertions(+)\n>\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index 5add900..37ff45f 100755\n> --- a/t/t5520-pull.sh\n> +++ b/t/t5520-pull.sh\n> @@ -164,6 +164,27 @@ test_expect_success 'fail if upstream branch does not exist' '\n> test `cat file` = file\n> '\n>\n> +test_expect_success 'fail if the index has unresolved entries' '\n> + git checkout -b third master^ &&\n> + test_when_finished \"git checkout -f copy && git branch -D third\" &&\n> + echo file >expected &&\n> + test_cmp expected file &&\n> + echo modified2 >file &&\n> + git commit -a -m modified2 &&\n> + test -z \"$(git ls-files -u)\" &&\n> + test_must_fail git pull . second &&\n> + test -n \"$(git ls-files -u)\" &&\n> + cp file expected &&\n> + test_must_fail git pull . second 2>out &&\n\nPerhaps call this stderr capture file 'err' rather than 'out' to\nclarify its nature and to distinguish it from a stdout capture which\nsomeone might add in the future?\n\n> + test_i18ngrep \"Pull is not possible because you have unmerged files\" out &&\n> + test_cmp expected file &&\n> + git add file &&\n> + test -z \"$(git ls-files -u)\" &&\n> + test_must_fail git pull . second 2>out &&\n> + test_i18ngrep \"You have not concluded your merge\" out &&\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"},{"id":"260773","messageId":"xmqq3838p6jw.fsf@gitster.dls.corp.google.com","threadId":"39267","inReplyTo":"CACRoPnRXJOxUkT5sKQf2bLnWVQ-+xFPu8D2PbX2b_9XVEGwQnA@mail.gmail.com","subject":"Re: [PATCH v2 10/12] t5520: failing test for pull --all with no configured upstream","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-07T18:48:51Z","receivedAt":"2015-05-07T18:48:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan writes:\n\n> .... This can be\n> done with the current git-pull.sh, but would be easier with the\n> superior C parse-options API.\n\nJust this part, but I thought parse-options API is available to\nscripts via \"rev-parse --parseopt\".\n"},{"id":"260775","messageId":"xmqqy4l0nrdv.fsf@gitster.dls.corp.google.com","threadId":"39267","inReplyTo":"1430988248-18285-1-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 00/12] Improve git-pull test coverage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-07T19:01:48Z","receivedAt":"2015-05-07T19:01:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan writes:\n\n> This patch series improves test coverage of git-pull.sh.\n>\n> This is part of my GSoC project to rewrite git-pull into a builtin. Improving\n> test coverage helps to prevent regressions that could occur due to the rewrite.\n\nOverall, I like what I saw in this series. Good first progress ;-)\n"},{"id":"260776","messageId":"554BB93A.3090000@kdbg.org","threadId":"39267","inReplyTo":"CACRoPnRfqQMRJ5N=oL84SMw=FC=Eg-Co-De_9E9cpWCaaSZAxw@mail.gmail.com","subject":"Re: [PATCH v2 11/12] t5524: test --log=1 limits shortlog length","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2015-05-07T19:12:58Z","receivedAt":"2015-05-07T19:12:58Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 07.05.2015 um 19:06 schrieb Paul Tan:\n> Hi Dscho,\n>\n> On Fri, May 8, 2015 at 12:28 AM, Johannes Schindelin\n> wrote:\n>> Hi Paul,\n>>\n>> On 2015-05-07 10:44, Paul Tan wrote:\n>>> @@ -32,4 +35,18 @@ test_expect_success pull '\n>>> )\n>>> '\n>>>\n>>> +test_expect_failure '--log=1 limits shortlog length' '\n>>> +(\n>>> + cd cloned &&\n>>> + git reset --hard HEAD^ &&\n>>> + test `cat afile` = original &&\n>>> + test `cat bfile` = added &&\n>>> + git pull --log &&\n>>> + git log -3 &&\n>>> + git cat-file commit HEAD >result &&\n>>> + grep Dollar result &&\n>>> + ! grep \"second commit\" result\n>>> +)\n>>\n>> I think it might be better to use `test_must_fail` here, just for\n>> consistency (the `!` operator would also pass if `grep` itself could not\n>> be executed correctly, quite academic, I know, given that `grep` is\n>> exercised plenty of times by the test suite, but still...)\n>>\n>> What do you think?\n>\n> Yep, it's definitely better. Sometimes I forget about the existence of\n> some test utility functions :-/.\n\nNope, it's not better. test_must_fail is explicitly only for git \ninvocations. We do not expect 'grep' to segfault or something.\n\nCf. eg. \nhttp://thread.gmane.org/gmane.comp.version-control.git/258725/focus=258752\n\n-- Hannes\n"},{"id":"260779","messageId":"xmqqpp6cnnyr.fsf@gitster.dls.corp.google.com","threadId":"39267","inReplyTo":"CACRoPnQ3zYsv63pNUtasS6y2HnWMYqwssF=yFA3OHVvF2Yb5Sw@mail.gmail.com","subject":"Re: [PATCH v2 09/12] t7406: use \"git pull\" instead of \"git pull --rebase\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-07T20:15:40Z","receivedAt":"2015-05-07T20:15:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan writes:\n\n> Hi,\n>\n> On Fri, May 8, 2015 at 1:24 AM, Junio C Hamano wrote:\n>> Paul Tan writes:\n>> If 'git pull' gets broken, it will break this test _anyway_.\n>\n> You have a point, perhaps this should be changed to git-fetch + git-merge?\n>\n> Well my reasoning is that it's because git-pull --rebase requires more\n> code to be implemented than git-pull. So I'm thinking that git-pull\n> --rebase is more likely to break than git-pull.\n>\n>> Unless\n>> the operating assumption is \"it is OK to break 'git pull --rebase',\n>> as long as we do not break 'git pull', while rewriting it\", I am not\n>> sure the value of the change in this patch. We'd need to keep both\n>> form working, no?\n>\n> Yes, ultimately the git-pull rewrite must re-implement everything, but\n> if this test suite is affected by any git-pull (--rebase) breakage,\n> then there will be lots of patch noise as this test suite's tests get\n> disabled/re-enabled in the git-pull rewrite patches.\n>\n> But if the patch noise is okay, then I'm fine with dropping this patch\n> and 07/12.\n\nI am not sure what you mean by \"patch noise\".\n\nI do not think touching this test which does not have anything to do\nwith \"git pull\" in your series is sensible at all, and you shouldn't\nflip test_expect_success temporarily to _expect_failure, if that is\nwhat you have in mind.\n\nJust don't run unrelated tests while your series is in flux.\n"},{"id":"260805","messageId":"8e34bc9ea27d147f86de0cf60141687a@www.dscho.org","threadId":"39267","inReplyTo":"554BB93A.3090000@kdbg.org","subject":"Re: [PATCH v2 11/12] t5524: test --log=1 limits shortlog length","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-05-08T10:07:29Z","receivedAt":"2015-05-08T10:07:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Hannes,\n\nOn 2015-05-07 21:12, Johannes Sixt wrote:\n> Am 07.05.2015 um 19:06 schrieb Paul Tan:\n>\n>> On Fri, May 8, 2015 at 12:28 AM, Johannes Schindelin\n>> wrote:\n>>\n>>> On 2015-05-07 10:44, Paul Tan wrote:\n>>>> @@ -32,4 +35,18 @@ test_expect_success pull '\n>>>> )\n>>>> '\n>>>>\n>>>> +test_expect_failure '--log=1 limits shortlog length' '\n>>>> +(\n>>>> + cd cloned &&\n>>>> + git reset --hard HEAD^ &&\n>>>> + test `cat afile` = original &&\n>>>> + test `cat bfile` = added &&\n>>>> + git pull --log &&\n>>>> + git log -3 &&\n>>>> + git cat-file commit HEAD >result &&\n>>>> + grep Dollar result &&\n>>>> + ! grep \"second commit\" result\n>>>> +)\n>>>\n>>> I think it might be better to use `test_must_fail` here, just for\n>>> consistency (the `!` operator would also pass if `grep` itself could not\n>>> be executed correctly, quite academic, I know, given that `grep` is\n>>> exercised plenty of times by the test suite, but still...)\n>>>\n>>> What do you think?\n>>\n>> Yep, it's definitely better. Sometimes I forget about the existence of\n>> some test utility functions :-/.\n> \n> Nope, it's not better. test_must_fail is explicitly only for git\n> invocations. We do not expect 'grep' to segfault or something.\n> \n> Cf. eg.\n> http://thread.gmane.org/gmane.comp.version-control.git/258725/focus=258752\n\nThat link leads to a patch that changes `! grep` to a `test_must_fail grep` and is not contested, at least not in the thread visible on GMane. Would you have a link with a more convincing argument for me?\n\nThank you,\nJohannes\n"},{"id":"260807","messageId":"87r3qrwd1j.fsf@hank.tgummerer.com","threadId":"39267","inReplyTo":"8e34bc9ea27d147f86de0cf60141687a@www.dscho.org","subject":"Re: [PATCH v2 11/12] t5524: test --log=1 limits shortlog length","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2015-05-08T10:59:04Z","receivedAt":"2015-05-08T10:59:04Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Johannes Schindelin writes:\n\n> Hi Hannes,\n>\n> On 2015-05-07 21:12, Johannes Sixt wrote:\n>> Am 07.05.2015 um 19:06 schrieb Paul Tan:\n>>\n>>> On Fri, May 8, 2015 at 12:28 AM, Johannes Schindelin\n>>> wrote:\n>>>\n>>>> On 2015-05-07 10:44, Paul Tan wrote:\n>>>>> @@ -32,4 +35,18 @@ test_expect_success pull '\n>>>>> )\n>>>>> '\n>>>>>\n>>>>> +test_expect_failure '--log=1 limits shortlog length' '\n>>>>> +(\n>>>>> + cd cloned &&\n>>>>> + git reset --hard HEAD^ &&\n>>>>> + test `cat afile` = original &&\n>>>>> + test `cat bfile` = added &&\n>>>>> + git pull --log &&\n>>>>> + git log -3 &&\n>>>>> + git cat-file commit HEAD >result &&\n>>>>> + grep Dollar result &&\n>>>>> + ! grep \"second commit\" result\n>>>>> +)\n>>>>\n>>>> I think it might be better to use `test_must_fail` here, just for\n>>>> consistency (the `!` operator would also pass if `grep` itself could not\n>>>> be executed correctly, quite academic, I know, given that `grep` is\n>>>> exercised plenty of times by the test suite, but still...)\n>>>>\n>>>> What do you think?\n>>>\n>>> Yep, it's definitely better. Sometimes I forget about the existence of\n>>> some test utility functions :-/.\n>>\n>> Nope, it's not better. test_must_fail is explicitly only for git\n>> invocations. We do not expect 'grep' to segfault or something.\n>>\n>> Cf. eg.\n>> http://thread.gmane.org/gmane.comp.version-control.git/258725/focus=258752\n>\n> That link leads to a patch that changes `! grep` to a `test_must_fail grep` and is not contested, at least not in the thread visible on GMane. Would you have a link with a more convincing argument for me?\n\nt/README states:\n\n On the other hand, don't use test_must_fail for running regular\n platform commands; just use '! cmd'. We are not in the business\n of verifying that the world given to us sanely works.\n\nExcept for a few cases that is also respected in the test scripts.\n\n$ git grep \"! grep\" | wc -l\n203\n$ git grep \"test_must_fail grep\" | wc -l\n19\n\nSo I think using ! grep is the right way to go.\n\n> Thank you,\n> Johannes\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"},{"id":"260811","messageId":"51074f84c26d99ceeb7f7d19aa88e7a7@www.dscho.org","threadId":"39267","inReplyTo":"87r3qrwd1j.fsf@hank.tgummerer.com","subject":"Re: [PATCH v2 11/12] t5524: test --log=1 limits shortlog length","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-05-08T13:09:11Z","receivedAt":"2015-05-08T13:09:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Thomas,\n\nOn 2015-05-08 12:59, Thomas Gummerer wrote:\n \n> t/README states:\n> \n> On the other hand, don't use test_must_fail for running regular\n> platform commands; just use '! cmd'. We are not in the business\n> of verifying that the world given to us sanely works.\n\nOkay, thanks for the reality check.\n\nCiao,\nDscho\n"},{"id":"260812","messageId":"cdd3a0b88d731a536735192317fff32b@www.dscho.org","threadId":"39267","inReplyTo":"CACRoPnTMgZBfeneMCZSJVowmSCGf2Ufpt-J461+w9Lh1aN-rhA@mail.gmail.com","subject":"Re: [PATCH v2 07/12] t4013: call git-merge instead of git-pull","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-05-08T13:12:20Z","receivedAt":"2015-05-08T13:12:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn 2015-05-07 18:55, Paul Tan wrote:\n> Hi Dscho,\n> \n> On Fri, May 8, 2015 at 12:26 AM, Johannes Schindelin\n> wrote:\n>> Both this patch and 9/12 change `git pull` invocations to equivalent non-pull ones, but I wonder whether it would not be a better idea to leave them as-are, so that we can make sure that scripts out there that might use similar `git pull` invocations would be unaffected by the rewrite?\n> \n> In the current state[1], I'm aiming for the git-pull rewrite patch\n> series to break all git-pull tests in the first patch, and then\n> subsequently make them pass again in later smaller patches by\n> implementing back the old features. This will make reviewing the code\n> much easier, as opposed to dumping a huge patch every single re-roll\n> ;-).\n> \n> For both patches and test suites, if the \"setup\" tests fail, the whole\n> test suite fails. Given that the test suites are about testing the\n> diff formatting options and submodules update implementation, which is\n> mostly irrelevant to git-pull, I think it would be better if the test\n> suite was not affected by the rewrite, especially since it only\n> requires changing one line.\n\nAh, that makes sense. I just failed to understand what you were telling me.\n\nMaybe it would be better to make that patch a part of the builtin pull instead, so that we do not forget to revert it with the commit that introduces support for `-s ours` in builtin pull?\n\nCiao,\nDscho\n"},{"id":"260823","messageId":"554CF03A.7030503@kdbg.org","threadId":"39267","inReplyTo":"8e34bc9ea27d147f86de0cf60141687a@www.dscho.org","subject":"Re: [PATCH v2 11/12] t5524: test --log=1 limits shortlog length","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2015-05-08T17:19:54Z","receivedAt":"2015-05-08T17:19:54Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 08.05.2015 um 12:07 schrieb Johannes Schindelin:\n> On 2015-05-07 21:12, Johannes Sixt wrote:\n>> Nope, it's not better. test_must_fail is explicitly only for git\n>> invocations. We do not expect 'grep' to segfault or something.\n>>\n>> Cf. eg.\n>> http://thread.gmane.org/gmane.comp.version-control.git/258725/focus=258752\n>\n> That link leads to a patch that changes `! grep` to a `test_must_fail\n> grep` and is not contested, at least not in the thread visible on\n> GMane. Would you have a link with a more convincing argument for me?\n\nGah! Sorry for sending you in circles. I see that others have brought \nforward sufficient arguments. Just to get my own argument straight, here \nis the message I wanted to direct you to:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/258725/focus=258792\n\n-- Hannes\n"},{"id":"260827","messageId":"xmqq3837m0uk.fsf@gitster.dls.corp.google.com","threadId":"39267","inReplyTo":"554CF03A.7030503@kdbg.org","subject":"Re: [PATCH v2 11/12] t5524: test --log=1 limits shortlog length","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-08T17:32:35Z","receivedAt":"2015-05-08T17:32:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 08.05.2015 um 12:07 schrieb Johannes Schindelin:\n>> On 2015-05-07 21:12, Johannes Sixt wrote:\n>>> Nope, it's not better. test_must_fail is explicitly only for git\n>>> invocations. We do not expect 'grep' to segfault or something.\n>>>\n>>> Cf. eg.\n>>> http://thread.gmane.org/gmane.comp.version-control.git/258725/focus=258752\n>>\n>> That link leads to a patch that changes `! grep` to a `test_must_fail\n>> grep` and is not contested, at least not in the thread visible on\n>> GMane. Would you have a link with a more convincing argument for me?\n>\n> Gah! Sorry for sending you in circles. I see that others have brought\n> forward sufficient arguments. Just to get my own argument straight,\n> here is the message I wanted to direct you to:\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/258725/focus=258792\n\nFWIW, I think Michael Gruber gave the rationale better than I did\nin that thread.\n"},{"id":"260889","messageId":"CACRoPnT-9Z-GY9Orid+-nUgTGENakOA8DDD8uUBJGMBw_gbqWQ@mail.gmail.com","threadId":"39267","inReplyTo":"xmqq3838p6jw.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 10/12] t5520: failing test for pull --all with no configured upstream","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-09T03:10:46Z","receivedAt":"2015-05-09T03:10:46Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Junio,\n\nOn Fri, May 8, 2015 at 2:48 AM, Junio C Hamano wrote:\n> Paul Tan writes:\n>\n>> .... This can be\n>> done with the current git-pull.sh, but would be easier with the\n>> superior C parse-options API.\n>\n> Just this part, but I thought parse-options API is available to\n> scripts via \"rev-parse --parseopt\".\n\nYes, it kind of is, but my impression is that it only validates\ncommand-line arguments -- we still have to loop over each of them\nwhich means more code has to be written as compared to the C version\n(especially if we want to validate all the git-fetch switches as\nwell).\n\nThanks,\nPaul\n"},{"id":"260922","messageId":"CACRoPnQdmaoNW_CoZmTz24SgLrQZjyTGWRpCKmWh3bTH92939g@mail.gmail.com","threadId":"39267","inReplyTo":"xmqqpp6cnnyr.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 09/12] t7406: use \"git pull\" instead of \"git pull --rebase\"","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-10T08:19:48Z","receivedAt":"2015-05-10T08:19:48Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Junio,\n\nOn Fri, May 8, 2015 at 4:15 AM, Junio C Hamano wrote:\n> I do not think touching this test which does not have anything to do\n> with \"git pull\" in your series is sensible at all, and you shouldn't\n> flip test_expect_success temporarily to _expect_failure, if that is\n> what you have in mind.\n>\n> Just don't run unrelated tests while your series is in flux.\n\nYes, you are right, these patches are not related to the topic at all,\nand I will drop them.\n\nJust to make myself clear though, the issue I have is that\ngit-submodule and the various diff commands do not depend on\ngit-pull's functionality at all. If only git-pull breaks, these\ncommand will continue to work perfectly. However, if all the tests in\nt7406 and t4013 break as well, I would consider that misrepresenting\nthe problem.\n\nNot that this is really a big problem anyway, the entire failing test\nsuites just surprised me, that's all. Sorry for the noise.\n\nThanks,\nPaul\n"},{"id":"261049","messageId":"CACRoPnR7f713ABa=o2f-VQnDMjGvxKyUmn2Ap-ZCQ_Q447DOLA@mail.gmail.com","threadId":"39267","inReplyTo":"CAPig+cRBrcC+Zud=mQoCco9E3DC66gZkX5C0wTWxQ4B-Oje3eQ@mail.gmail.com","subject":"Re: [PATCH v2 02/12] t5520: test for failure if index has unresolved entries","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-12T13:43:04Z","receivedAt":"2015-05-12T13:43:04Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Eric,\n\nOn Fri, May 8, 2015 at 2:28 AM, Eric Sunshine wrote:\n> A couple very minor comments applying to the entire patch series...\n>\n> On Thu, May 7, 2015 at 4:43 AM, Paul Tan wrote:\n>> Commit d38a30df (Be more user-friendly when refusing to do something\n>> because of conflict) introduced code paths to git-pull which will error\n>\n> Custom for citing a commit is also to include the date:\n>\n> d38a30df (Be more user-friendly...conflict, 2010-01-12)\n>\n> Some people use this git alias to help automate:\n>\n> whatis = show -s --pretty='tformat:%h (%s, %ad)' --date=short\n\nThis is really useful, thanks!\n\n>> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n>> index 5add900..37ff45f 100755\n>> --- a/t/t5520-pull.sh\n>> +++ b/t/t5520-pull.sh\n>> @@ -164,6 +164,27 @@ test_expect_success 'fail if upstream branch does not exist' '\n>> test `cat file` = file\n>> '\n>>\n>> +test_expect_success 'fail if the index has unresolved entries' '\n>> + git checkout -b third master^ &&\n>> + test_when_finished \"git checkout -f copy && git branch -D third\" &&\n>> + echo file >expected &&\n>> + test_cmp expected file &&\n>> + echo modified2 >file &&\n>> + git commit -a -m modified2 &&\n>> + test -z \"$(git ls-files -u)\" &&\n>> + test_must_fail git pull . second &&\n>> + test -n \"$(git ls-files -u)\" &&\n>> + cp file expected &&\n>> + test_must_fail git pull . second 2>out &&\n>\n> Perhaps call this stderr capture file 'err' rather than 'out' to\n> clarify its nature and to distinguish it from a stdout capture which\n> someone might add in the future?\n\nWill fix.\n\nRegards,\nPaul\n"}]}