{"thread":{"id":"22798","subject":"[PATCH 1/2] t5521: fix and modernize","startedAt":"2010-02-24T18:22:05Z","lastAt":"2010-02-24T20:28:23Z","messageCount":7,"participants":["Junio C Hamano","Teemu Likonen","Michael Lukashov"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"135593","messageId":"1267035726-2815-1-git-send-email-gitster@pobox.com","threadId":"22798","inReplyTo":null,"subject":"[PATCH 1/2] t5521: fix and modernize","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-24T18:22:05Z","receivedAt":"2010-02-24T18:22:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"All of these tests were bogus, as they created new directory and tried to\nrun \"git pull\" without even running \"git init\" in there.  They were mucking\nwith the repository in $TEST_DIRECTORY.\n\nWhile fixing it, modernize the style not to chdir around outside of\nsubshell.  Otherwise a failed test will take us to an unexpected directory\nand we need to chdir back to the test directory in each test, which is\nugly and error prone.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t5521-pull-options.sh |   46 ++++++++++++++++++++--------------------------\n 1 files changed, 20 insertions(+), 26 deletions(-)\n\ndiff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh\nindex 83e2e8a..c18d829 100755\n--- a/t/t5521-pull-options.sh\n+++ b/t/t5521-pull-options.sh\n@@ -4,8 +4,6 @@ test_description='pull options'\n \n . ./test-lib.sh\n \n-D=`pwd`\n-\n test_expect_success 'setup' '\n \tmkdir parent &&\n \t(cd parent && git init &&\n@@ -13,48 +11,44 @@ test_expect_success 'setup' '\n \t git commit -m one)\n '\n \n-cd \"$D\"\n-\n test_expect_success 'git pull -q' '\n \tmkdir clonedq &&\n-\tcd clonedq &&\n-\tgit pull -q \"$D/parent\" >out 2>err &&\n-\ttest ! -s out\n+\t(cd clonedq && git init &&\n+\tgit pull -q \"../parent\" >out 2>err &&\n+\ttest ! -s err &&\n+\ttest ! -s out)\n '\n \n-cd \"$D\"\n-\n test_expect_success 'git pull' '\n \tmkdir cloned &&\n-\tcd cloned &&\n-\tgit pull \"$D/parent\" >out 2>err &&\n-\ttest -s out\n+\t(cd cloned && git init &&\n+\tgit pull \"../parent\" >out 2>err &&\n+\ttest -s err &&\n+\ttest ! -s out)\n '\n-cd \"$D\"\n \n test_expect_success 'git pull -v' '\n \tmkdir clonedv &&\n-\tcd clonedv &&\n-\tgit pull -v \"$D/parent\" >out 2>err &&\n-\ttest -s out\n+\t(cd clonedv && git init &&\n+\tgit pull -v \"../parent\" >out 2>err &&\n+\ttest -s err &&\n+\ttest ! -s out)\n '\n \n-cd \"$D\"\n-\n test_expect_success 'git pull -v -q' '\n \tmkdir clonedvq &&\n-\tcd clonedvq &&\n-\tgit pull -v -q \"$D/parent\" >out 2>err &&\n-\ttest ! -s out\n+\t(cd clonedvq && git init &&\n+\tgit pull -v -q \"../parent\" >out 2>err &&\n+\ttest ! -s out &&\n+\ttest ! -s err)\n '\n \n-cd \"$D\"\n-\n test_expect_success 'git pull -q -v' '\n \tmkdir clonedqv &&\n-\tcd clonedqv &&\n-\tgit pull -q -v \"$D/parent\" >out 2>err &&\n-\ttest -s out\n+\t(cd clonedqv && git init &&\n+\tgit pull -q -v \"../parent\" >out 2>err &&\n+\ttest ! -s out &&\n+\ttest -s err)\n '\n \n test_done\n-- \n1.7.0.207.gac4ec\n"},{"id":"135594","messageId":"1267035726-2815-2-git-send-email-gitster@pobox.com","threadId":"22798","inReplyTo":"1267035726-2815-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/2] builtin-fetch --all/--multi: propagate options correctly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-24T18:22:06Z","receivedAt":"2010-02-24T18:22:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When running a subfetch, the code propagated some options but not others.\nPropagate --force, --update-head-ok and --keep options as well.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-fetch.c |    9 ++++++++-\n 1 files changed, 8 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-fetch.c b/builtin-fetch.c\nindex 8654fa7..61b2e40 100644\n--- a/builtin-fetch.c\n+++ b/builtin-fetch.c\n@@ -784,13 +784,19 @@ static int add_remote_or_group(const char *name, struct string_list *list)\n static int fetch_multiple(struct string_list *list)\n {\n \tint i, result = 0;\n-\tconst char *argv[] = { \"fetch\", NULL, NULL, NULL, NULL, NULL, NULL };\n+\tconst char *argv[10] = { \"fetch\" };\n \tint argc = 1;\n \n \tif (dry_run)\n \t\targv[argc++] = \"--dry-run\";\n \tif (prune)\n \t\targv[argc++] = \"--prune\";\n+\tif (update_head_ok)\n+\t\targv[argc++] = \"--update-head-ok\";\n+\tif (force)\n+\t\targv[argc++] = \"--force\";\n+\tif (keep)\n+\t\targv[argc++] = \"--keep\";\n \tif (verbosity >= 2)\n \t\targv[argc++] = \"-v\";\n \tif (verbosity >= 1)\n@@ -801,6 +807,7 @@ static int fetch_multiple(struct string_list *list)\n \tfor (i = 0; i < list->nr; i++) {\n \t\tconst char *name = list->items[i].string;\n \t\targv[argc] = name;\n+\t\targv[argc + 1] = NULL;\n \t\tif (verbosity >= 0)\n \t\t\tprintf(\"Fetching %s\\n\", name);\n \t\tif (run_command_v_opt(argv, RUN_GIT_CMD)) {\n-- \n1.7.0.207.gac4ec\n"},{"id":"135596","messageId":"7vpr3uqwya.fsf_-_@alter.siamese.dyndns.org","threadId":"22798","inReplyTo":"1267035726-2815-2-git-send-email-gitster@pobox.com","subject":"[PATCH 3/3] fetch --all/--multiple: keep all the fetched branch information","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-24T19:02:05Z","receivedAt":"2010-02-24T19:02:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Since \"git fetch\" learned \"--all\" and \"--multiple\" options, it has become\ntempting for users to say \"git pull --all\".  Even though it may fetch from\nremotes that do not need to be fetched from for merging with the current\nbranch, it is handy.\n\n\"git fetch\" however clears the list of fetched branches every time it\ncontacts a different remote.  Unless the current branch is configured to\nmerge with a branch from a remote that happens to be the last in the list\nof remotes that are contacted, \"git pull\" that fetches from multiple\nremotes will not be able to find the branch it should be merging with.\n\nMake \"fetch\" clear FETCH_HEAD (unless --append is given) and then append\nthe list of branches fetched to it (even when --apend is not given).  That\nway, \"pull\" will be able to find the data for the branch being merged in\nFETCH_HEAD no matter where the remote appears in the list of remotes to be\ncontacted by \"git fetch\".\n\nReported-by: Michael Lukashov\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * This obviously builds on top of the earlier two clean-up patches.\n\n builtin-fetch.c         |   29 ++++++++++++++++++++++-------\n t/t5521-pull-options.sh |   18 ++++++++++++++++++\n 2 files changed, 40 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-fetch.c b/builtin-fetch.c\nindex 61b2e40..b059d65 100644\n--- a/builtin-fetch.c\n+++ b/builtin-fetch.c\n@@ -651,6 +651,17 @@ static void check_not_current_branch(struct ref *ref_map)\n \t\t\t    \"of non-bare repository\", current_branch->refname);\n }\n \n+static int truncate_fetch_head(void)\n+{\n+\tchar *filename = git_path(\"FETCH_HEAD\");\n+\tFILE *fp = fopen(filename, \"w\");\n+\n+\tif (!fp)\n+\t\treturn error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n+\tfclose(fp);\n+\treturn 0;\n+}\n+\n static int do_fetch(struct transport *transport,\n \t\t    struct refspec *refs, int ref_count)\n {\n@@ -672,11 +683,9 @@ static int do_fetch(struct transport *transport,\n \n \t/* if not appending, truncate FETCH_HEAD */\n \tif (!append && !dry_run) {\n-\t\tchar *filename = git_path(\"FETCH_HEAD\");\n-\t\tFILE *fp = fopen(filename, \"w\");\n-\t\tif (!fp)\n-\t\t\treturn error(\"cannot open %s: %s\\n\", filename, strerror(errno));\n-\t\tfclose(fp);\n+\t\tint errcode = truncate_fetch_head();\n+\t\tif (errcode)\n+\t\t\treturn errcode;\n \t}\n \n \tref_map = get_ref_map(transport, refs, ref_count, tags, &autotags);\n@@ -784,8 +793,8 @@ static int add_remote_or_group(const char *name, struct string_list *list)\n static int fetch_multiple(struct string_list *list)\n {\n \tint i, result = 0;\n-\tconst char *argv[10] = { \"fetch\" };\n-\tint argc = 1;\n+\tconst char *argv[11] = { \"fetch\", \"--append\" };\n+\tint argc = 2;\n \n \tif (dry_run)\n \t\targv[argc++] = \"--dry-run\";\n@@ -804,6 +813,12 @@ static int fetch_multiple(struct string_list *list)\n \telse if (verbosity < 0)\n \t\targv[argc++] = \"-q\";\n \n+\tif (!append && !dry_run) {\n+\t\tint errcode = truncate_fetch_head();\n+\t\tif (errcode)\n+\t\t\treturn errcode;\n+\t}\n+\n \tfor (i = 0; i < list->nr; i++) {\n \t\tconst char *name = list->items[i].string;\n \t\targv[argc] = name;\ndiff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh\nindex 84059d8..1b06691 100755\n--- a/t/t5521-pull-options.sh\n+++ b/t/t5521-pull-options.sh\n@@ -72,4 +72,22 @@ test_expect_success 'git pull --force' '\n \t)\n '\n \n+test_expect_success 'git pull --all' '\n+\tmkdir clonedmulti &&\n+\t(cd clonedmulti && git init &&\n+\tcat >>.git/config <<-\\EOF &&\n+\t[remote \"one\"]\n+\t\turl = ../parent\n+\t\tfetch = refs/heads/*:refs/remotes/one/*\n+\t[remote \"two\"]\n+\t\turl = ../parent\n+\t\tfetch = refs/heads/*:refs/remotes/two/*\n+\t[branch \"master\"]\n+\t\tremote = one\n+\t\tmerge = refs/heads/master\n+\tEOF\n+\tgit pull --all\n+\t)\n+'\n+\n test_done\n-- \n1.7.0.207.gac4ec\n"},{"id":"135597","messageId":"87y6iizc4j.fsf@mithlond.arda","threadId":"22798","inReplyTo":"7vpr3uqwya.fsf_-_@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] fetch --all/--multiple: keep all the fetched branch information","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2010-02-24T19:07:08Z","receivedAt":"2010-02-24T19:07:08Z","isPatch":true,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"* 2010-02-24 11:02 (-0800), Junio C. Hamano wrote:\n\n> Since \"git fetch\" learned \"--all\" and \"--multiple\" options, it has become\n> tempting for users to say \"git pull --all\".  Even though it may fetch from\n> remotes that do not need to be fetched from for merging with the current\n> branch, it is handy.\n>\n> \"git fetch\" however clears the list of fetched branches every time it\n> contacts a different remote.  Unless the current branch is configured to\n> merge with a branch from a remote that happens to be the last in the list\n> of remotes that are contacted, \"git pull\" that fetches from multiple\n> remotes will not be able to find the branch it should be merging with.\n>\n> Make \"fetch\" clear FETCH_HEAD (unless --append is given) and then append\n> the list of branches fetched to it (even when --apend is not given).  That\n                                                ^^^^^^^\n\nNot very important but there's a typo: \"apend\".\n\n\n> way, \"pull\" will be able to find the data for the branch being merged in\n> FETCH_HEAD no matter where the remote appears in the list of remotes to be\n> contacted by \"git fetch\".\n>\n> Reported-by: Michael Lukashov\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n"},{"id":"135609","messageId":"63cde7731002241154o40ad7e6eh26f20017f7854fc3@mail.gmail.com","threadId":"22798","inReplyTo":"7vpr3uqwya.fsf_-_@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] fetch --all/--multiple: keep all the fetched branch information","fromName":"Michael Lukashov","fromEmail":"michael.lukashov@gmail.com","sentAt":"2010-02-24T19:54:18Z","receivedAt":"2010-02-24T19:54:18Z","isPatch":true,"sender":{"key":"michael.lukashov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/890439?v=4"},"body":"Patch 3 does not apply correctly:\n\nApplying: fetch --all/--multiple: keep all the fetched branch information\nerror: patch failed: t/t5521-pull-options.sh:72\nerror: t/t5521-pull-options.sh: patch does not apply\nPatch failed at 0001 fetch --all/--multiple: keep all the fetched\nbranch information\nWhen you have resolved this problem run \"git am --resolved\".\nIf you would prefer to skip this patch, instead run \"git am --skip\".\nTo restore the original branch and stop patching run \"git am --abort\".\n\nOn Wed, Feb 24, 2010 at 10:02 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Since \"git fetch\" learned \"--all\" and \"--multiple\" options, it has become\n> tempting for users to say \"git pull --all\".  Even though it may fetch from\n> remotes that do not need to be fetched from for merging with the current\n> branch, it is handy.\n>\n"},{"id":"135610","messageId":"7vy6iil81k.fsf@alter.siamese.dyndns.org","threadId":"22798","inReplyTo":"63cde7731002241154o40ad7e6eh26f20017f7854fc3@mail.gmail.com","subject":"Re: [PATCH 3/3] fetch --all/--multiple: keep all the fetched branch information","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-24T19:59:03Z","receivedAt":"2010-02-24T19:59:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Lukashov <michael.lukashov@gmail.com> writes:\n\n> Patch 3 does not apply correctly:\n\nIt probably has a trivial confict, because there is an added test to the\nsecond patch since I sent it out.  Please add this after applying [2/3].\n\ndiff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh\nindex c18d829..84059d8 100755\n--- a/t/t5521-pull-options.sh\n+++ b/t/t5521-pull-options.sh\n@@ -51,4 +51,25 @@ test_expect_success 'git pull -q -v' '\n \ttest -s err)\n '\n \n+test_expect_success 'git pull --force' '\n+\tmkdir clonedoldstyle &&\n+\t(cd clonedoldstyle && git init &&\n+\tcat >>.git/config <<-\\EOF &&\n+\t[remote \"one\"]\n+\t\turl = ../parent\n+\t\tfetch = refs/heads/master:refs/heads/mirror\n+\t[remote \"two\"]\n+\t\turl = ../parent\n+\t\tfetch = refs/heads/master:refs/heads/origin\n+\t[branch \"master\"]\n+\t\tremote = two\n+\t\tmerge = refs/heads/master\n+\tEOF\n+\tgit pull two &&\n+\ttest_commit A &&\n+\tgit branch -f origin &&\n+\tgit pull --all --force\n+\t)\n+'\n+\n test_done\n"},{"id":"135611","messageId":"63cde7731002241228g5ef3154fw5b0c2858cbc168f@mail.gmail.com","threadId":"22798","inReplyTo":"7vy6iil81k.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] fetch --all/--multiple: keep all the fetched branch information","fromName":"Michael Lukashov","fromEmail":"michael.lukashov@gmail.com","sentAt":"2010-02-24T20:28:23Z","receivedAt":"2010-02-24T20:28:23Z","isPatch":true,"sender":{"key":"michael.lukashov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/890439?v=4"},"body":"Well, these patches solve the problem.\nThanks for your work!\n\nOn Wed, Feb 24, 2010 at 10:59 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Michael Lukashov <michael.lukashov@gmail.com> writes:\n>\n>> Patch 3 does not apply correctly:\n>\n> It probably has a trivial confict, because there is an added test to the\n> second patch since I sent it out.  Please add this after applying [2/3].\n>\n> diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh\n> index c18d829..84059d8 100755\n> --- a/t/t5521-pull-options.sh\n> +++ b/t/t5521-pull-options.sh\n> @@ -51,4 +51,25 @@ test_expect_success 'git pull -q -v' '\n>        test -s err)\n>  '\n>\n> +test_expect_success 'git pull --force' '\n> +       mkdir clonedoldstyle &&\n> +       (cd clonedoldstyle && git init &&\n> +       cat >>.git/config <<-\\EOF &&\n> +       [remote \"one\"]\n> +               url = ../parent\n> +               fetch = refs/heads/master:refs/heads/mirror\n> +       [remote \"two\"]\n> +               url = ../parent\n> +               fetch = refs/heads/master:refs/heads/origin\n> +       [branch \"master\"]\n> +               remote = two\n> +               merge = refs/heads/master\n> +       EOF\n> +       git pull two &&\n> +       test_commit A &&\n> +       git branch -f origin &&\n> +       git pull --all --force\n> +       )\n> +'\n> +\n>  test_done\n>\n"}]}