{"thread":{"id":"6924","subject":"[PATCHv3] git-fetch: Split fetch and merge logic","startedAt":"2007-02-23T08:49:31Z","lastAt":"2007-02-25T05:07:38Z","messageCount":6,"participants":["Santi Béjar","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"35309","messageId":"87wt29i7hg.fsf@gmail.com","threadId":"6924","inReplyTo":null,"subject":"[PATCHv3] git-fetch: Split fetch and merge logic","fromName":"Santi Béjar","fromEmail":"sbejar@gmail.com","sentAt":"2007-02-23T08:49:31Z","receivedAt":"2007-02-23T08:49:31Z","isPatch":false,"sender":{"key":"santi@agolina.net","avatar":null},"body":"\nIt makes git-parse-remote.sh and almost all git-fetch independent\nof the merge logic.\n\ngit-fetch fetches the branches from the remote and saves this\ninformation in .git/FETCH_FETCHED, and at the end it generates\nthe file .git/FETCH_HEAD.\n\nThe current merge behaviour is unchanged.\n\nSigned-off-by: Santi Béjar <sbejar@gmail.com>\n---\nHi *,\n\n This version removes the behaviour changes.\n\n Now to rebase this on top of 'jc/fetch' (a branch of 'pu').\n\n Santi\n\n git-fetch.sh        |   79 +++++++++++++++++++++++++++++++--------------------\n git-parse-remote.sh |   64 ++++++++++++-----------------------------\n t/t5510-fetch.sh    |   16 ++++++++++\n 3 files changed, 83 insertions(+), 76 deletions(-)\n\ndiff --git a/git-fetch.sh b/git-fetch.sh\nindex d230995..637d732 100755\n--- a/git-fetch.sh\n+++ b/git-fetch.sh\n@@ -77,9 +77,9 @@ do\n \tshift\n done\n \n+origin=$(get_default_remote)\n case \"$#\" in\n 0)\n-\torigin=$(get_default_remote)\n \ttest -n \"$(get_remote_url ${origin})\" ||\n \t\tdie \"Where do you want to fetch from today?\"\n \tset x $origin ; shift ;;\n@@ -101,6 +101,7 @@ if test \"\" = \"$append\"\n then\n \t: >\"$GIT_DIR/FETCH_HEAD\"\n fi\n+: >\"$GIT_DIR/FETCH_FETCHED\"\n \n # Global that is reused later\n ls_remote_result=$(git ls-remote $exec \"$remote\") ||\n@@ -112,10 +113,6 @@ append_fetch_head () {\n     remote_name_=\"$3\"\n     remote_nick_=\"$4\"\n     local_name_=\"$5\"\n-    case \"$6\" in\n-    t) not_for_merge_='not-for-merge' ;;\n-    '') not_for_merge_= ;;\n-    esac\n \n     # remote-nick is the URL given on the command line (or a shorthand)\n     # remote-name is the $GIT_DIR relative refs/ path we computed\n@@ -146,9 +143,7 @@ append_fetch_head () {\n     if git-cat-file commit \"$head_\" >/dev/null 2>&1\n     then\n \theadc_=$(git-rev-parse --verify \"$head_^0\") || exit\n-\techo \"$headc_\t$not_for_merge_\t$note_\" >>\"$GIT_DIR/FETCH_HEAD\"\n-    else\n-\techo \"$head_\tnot-for-merge\t$note_\" >>\"$GIT_DIR/FETCH_HEAD\"\n+\techo \"$headc_\t$remote_name_:$local_name_\t$note_\" >>\"$GIT_DIR/FETCH_FETCHED\"\n     fi\n \n     update_local_ref \"$local_name_\" \"$head_\" \"$note_\"\n@@ -265,7 +260,7 @@ then\n \t\t  git-show-ref --exclude-existing=refs/tags/ |\n \t          while read sha1 name\n \t\t  do\n-\t\t\techo \".${name}:${name}\"\n+\t\t\techo \"${name}:${name}\"\n \t\t  done` || exit\n \tif test \"$#\" -gt 1\n \tthen\n@@ -288,13 +283,6 @@ fetch_main () {\n \n       # These are relative path from $GIT_DIR, typically starting at refs/\n       # but may be HEAD\n-      if expr \"z$ref\" : 'z\\.' >/dev/null\n-      then\n-\t  not_for_merge=t\n-\t  ref=$(expr \"z$ref\" : 'z\\.\\(.*\\)')\n-      else\n-\t  not_for_merge=\n-      fi\n       if expr \"z$ref\" : 'z+' >/dev/null\n       then\n \t  single_force=t\n@@ -375,7 +363,7 @@ fetch_main () {\n       esac\n \n       append_fetch_head \"$head\" \"$remote\" \\\n-\t  \"$remote_name\" \"$remote_nick\" \"$local_name\" \"$not_for_merge\" || exit\n+\t  \"$remote_name\" \"$remote_nick\" \"$local_name\" || exit\n \n   done\n \n@@ -418,28 +406,16 @@ fetch_main () {\n \t      case \"$ref\" in\n \t      +$remote_name:*)\n \t\t  single_force=t\n-\t\t  not_for_merge=\n-\t\t  found=\"$ref\"\n-\t\t  break ;;\n-\t      .+$remote_name:*)\n-\t\t  single_force=t\n-\t\t  not_for_merge=t\n-\t\t  found=\"$ref\"\n-\t\t  break ;;\n-\t      .$remote_name:*)\n-\t\t  not_for_merge=t\n \t\t  found=\"$ref\"\n \t\t  break ;;\n \t      $remote_name:*)\n-\t\t  not_for_merge=\n \t\t  found=\"$ref\"\n \t\t  break ;;\n \t      esac\n \t  done\n \t  local_name=$(expr \"z$found\" : 'z[^:]*:\\(.*\\)')\n \t  append_fetch_head \"$sha1\" \"$remote\" \\\n-\t\t  \"$remote_name\" \"$remote_nick\" \"$local_name\" \\\n-\t\t  \"$not_for_merge\" || exit\n+\t\t  \"$remote_name\" \"$remote_nick\" \"$local_name\" || exit\n         done\n       )\n     ) || exit ;;\n@@ -463,7 +439,7 @@ case \"$no_tags$tags\" in\n \t\tdo\n \t\t\tgit-cat-file -t \"$sha1\" >/dev/null 2>&1 || continue\n \t\t\techo >&2 \"Auto-following $name\"\n-\t\t\techo \".${name}:${name}\"\n+\t\t\techo \"${name}:${name}\"\n \t\tdone)\n \tesac\n \tcase \"$taglist\" in\n@@ -491,3 +467,44 @@ case \"$orig_head\" in\n \tfi\n \t;;\n esac\n+\n+# Generate $GIT_DIR/FETCH_HEAD\n+case \",$#,$remote_nick,\" in\n+,1,$origin,)\n+\tcurr_branch=$(git-symbolic-ref -q HEAD | sed -e 's|^refs/heads/||')\n+\tmerge_branches=$(git-repo-config \\\n+\t\t--get-all \"branch.${curr_branch}.merge\" | sort -u)\n+\tfetch_branches=$(get_remote_default_refs_for_fetch -n $remote_nick |\n+\t\tsed 's/:.*$//g;s/^+//' | sort -u)\n+\ttest \"$merge_branches\" && test \"$fetch_branches\" &&\n+\tmerge_branches=$(echo -e \"$merge_branches\\n$fetch_branches\" | sort | uniq -d)\n+\t[ -z \"$merge_branches\" ] ||\n+\ttest \"$(get_data_source $remote_nick)\" = branches && merge_first=yes;;\n+,1,$remote,) merge_branches=HEAD;;\n+,1,*)merge_first=yes ;;\n+*)\n+\tshift\n+\tmerge_branches=$(for ref; do\n+\t\texpr \"z$ref\" : 'z.*:' >/dev/null || ref=\"${ref}:\"\n+\t\techo \"$(expr \"z$ref\" : 'z\\([^:]*\\):')\"; done);;\n+esac\n+\n+test \"$merge_first\" = \"yes\" &&\n+test \"$(get_remote_default_refs_for_fetch -t $remote_nick)\" != \"explicit\" &&\n+merge_branches= && merge_first=\n+\n+merge_branches=$(canon_refs_list_for_fetch $merge_branches | sed 's/:.*$//g')\n+\n+cat \"$GIT_DIR\"/FETCH_FETCHED | while IFS='\t' read sha1 ref note ; do\n+\tremote_branch=$(expr \"z$ref\" : 'z\\([^:]*\\):')\n+\tfor merge_branch in $merge_branches ; do\n+\t\t[ \"$merge_branch\" == \"$remote_branch\" ] &&\n+\t\t\techo \"$sha1\t\t$note\" && continue 2\n+\tdone\n+\tif ! test \"$merge_first\" || test \"$merge_first\" == \"done\" ; then\n+\t\techo \"$sha1\tnot-for-merge\t$note\"\n+\telse\n+\t\techo \"$sha1\t\t$note\"\n+\t\tmerge_first=done\n+\tfi\n+done >> \"$GIT_DIR/FETCH_HEAD\"\ndiff --git a/git-parse-remote.sh b/git-parse-remote.sh\nindex 5208ee6..691d46c 100755\n--- a/git-parse-remote.sh\n+++ b/git-parse-remote.sh\n@@ -70,8 +70,7 @@ get_remote_default_refs_for_push () {\n \tesac\n }\n \n-# Called from canon_refs_list_for_fetch -d \"$remote\", which\n-# is called from get_remote_default_refs_for_fetch to grok\n+# Called from get_remote_default_refs_for_fetch to grok\n # refspecs that are retrieved from the configuration, but not\n # from get_remote_refs_for_fetch when it deals with refspecs\n # supplied on the command line.  $ls_remote_result has the list\n@@ -130,30 +129,6 @@ expand_refs_wildcard () {\n \n # Subroutine to canonicalize remote:local notation.\n canon_refs_list_for_fetch () {\n-\t# If called from get_remote_default_refs_for_fetch\n-\t# leave the branches in branch.${curr_branch}.merge alone,\n-\t# or the first one otherwise; add prefix . to the rest\n-\t# to prevent the secondary branches to be merged by default.\n-\tmerge_branches=\n-\tcurr_branch=\n-\tif test \"$1\" = \"-d\"\n-\tthen\n-\t\tshift ; remote=\"$1\" ; shift\n-\t\tset $(expand_refs_wildcard \"$remote\" \"$@\")\n-\t\tis_explicit=\"$1\"\n-\t\tshift\n-\t\tif test \"$remote\" = \"$(get_default_remote)\"\n-\t\tthen\n-\t\t\tcurr_branch=$(git-symbolic-ref -q HEAD | \\\n-\t\t\t    sed -e 's|^refs/heads/||')\n-\t\t\tmerge_branches=$(git-config \\\n-\t\t\t    --get-all \"branch.${curr_branch}.merge\")\n-\t\tfi\n-\t\tif test -z \"$merge_branches\" && test $is_explicit != explicit\n-\t\tthen\n-\t\t\tmerge_branches=..this.will.never.match.any.ref..\n-\t\tfi\n-\tfi\n \tfor ref\n \tdo\n \t\tforce=\n@@ -166,18 +141,6 @@ canon_refs_list_for_fetch () {\n \t\texpr \"z$ref\" : 'z.*:' >/dev/null || ref=\"${ref}:\"\n \t\tremote=$(expr \"z$ref\" : 'z\\([^:]*\\):')\n \t\tlocal=$(expr \"z$ref\" : 'z[^:]*:\\(.*\\)')\n-\t\tdot_prefix=.\n-\t\tif test -z \"$merge_branches\"\n-\t\tthen\n-\t\t\tmerge_branches=$remote\n-\t\t\tdot_prefix=\n-\t\telse\n-\t\t\tfor merge_branch in $merge_branches\n-\t\t\tdo\n-\t\t\t    [ \"$remote\" = \"$merge_branch\" ] &&\n-\t\t\t    dot_prefix= && break\n-\t\t\tdone\n-\t\tfi\n \t\tcase \"$remote\" in\n \t\t'' | HEAD ) remote=HEAD ;;\n \t\trefs/heads/* | refs/tags/* | refs/remotes/*) ;;\n@@ -196,32 +159,43 @@ canon_refs_list_for_fetch () {\n \t\t   git-check-ref-format \"$local_ref_name\" ||\n \t\t   die \"* refusing to create funny ref '$local_ref_name' locally\"\n \t\tfi\n-\t\techo \"${dot_prefix}${force}${remote}:${local}\"\n+\t\techo \"${force}${remote}:${local}\"\n \tdone\n }\n \n # Returns list of src: (no store), or src:dst (store)\n get_remote_default_refs_for_fetch () {\n+\ttest \"$1\" == -t && type=yes && shift\n+\ttest \"$1\" == -n && canon=no && shift\n \tdata_source=$(get_data_source \"$1\")\n \tcase \"$data_source\" in\n \t'')\n-\t\techo \"HEAD:\" ;;\n+\t\tset explicit \"HEAD:\" ;;\n \tconfig)\n-\t\tcanon_refs_list_for_fetch -d \"$1\" \\\n-\t\t\t$(git-config --get-all \"remote.$1.fetch\") ;;\n+\t\tset $(expand_refs_wildcard \"$1\" \\\n+\t\t\t$(git-repo-config --get-all \"remote.$1.fetch\")) ;;\n \tbranches)\n \t\tremote_branch=$(sed -ne '/#/s/.*#//p' \"$GIT_DIR/branches/$1\")\n \t\tcase \"$remote_branch\" in '') remote_branch=master ;; esac\n-\t\techo \"refs/heads/${remote_branch}:refs/heads/$1\"\n+\t\tset explicit \"refs/heads/${remote_branch}:refs/heads/$1\"\n \t\t;;\n \tremotes)\n-\t\tcanon_refs_list_for_fetch -d \"$1\" $(sed -ne '/^Pull: */{\n+\t\tset $(expand_refs_wildcard \"$1\" $(sed -ne '/^Pull: */{\n \t\t\t\t\t\ts///p\n-\t\t\t\t\t}' \"$GIT_DIR/remotes/$1\")\n+\t\t\t\t\t}' \"$GIT_DIR/remotes/$1\"))\n \t\t;;\n \t*)\n \t\tdie \"internal error: get-remote-default-ref-for-push $1\" ;;\n \tesac\n+\tif test \"$type\" = yes ; then\n+\t\techo $1\n+\telif test \"$canon\" = no ; then\n+\t\tshift\n+\t\tfor ref ; do echo \"$ref\" ; done\n+\telse\n+\t\tshift\n+\t\tcanon_refs_list_for_fetch \"$@\"\n+\tfi\n }\n \n get_remote_refs_for_push () {\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 50c6485..0a19a7d 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -50,6 +50,22 @@ test_expect_success \"fetch test\" '\n \ttest \"z$mine\" = \"z$his\"\n '\n \n+test_expect_success \"fetch test fetched\" '\n+\tcd \"$D\" &&\n+\tcd three &&\n+\tgit fetch &&\n+\ttest -f .git/refs/heads/two &&\n+\ttest -f .git/refs/heads/one &&\n+\tmaster_in_two=`cd ../two && git rev-parse master` &&\n+\tone_in_two=`cd ../two && git rev-parse one` &&\n+\t{\n+\t\techo \"$master_in_two\trefs/heads/master:refs/heads/two\"\n+\t\techo \"$one_in_two\trefs/heads/one:refs/heads/one\"\n+\t} >expected &&\n+\tcut -f -2 .git/FETCH_FETCHED >actual &&\n+\tdiff expected actual'\n+\n+\n test_expect_success \"fetch test for-merge\" '\n \tcd \"$D\" &&\n \tcd three &&\n-- \n1.5.0.1.576.g78541\n"},{"id":"35314","messageId":"7vabz56vyq.fsf@assigned-by-dhcp.cox.net","threadId":"6924","inReplyTo":"87wt29i7hg.fsf@gmail.com","subject":"Re: [PATCHv3] git-fetch: Split fetch and merge logic","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-02-23T09:53:49Z","receivedAt":"2007-02-23T09:53:49Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Santi Béjar <sbejar@gmail.com> writes:\n\n> It makes git-parse-remote.sh and almost all git-fetch independent\n> of the merge logic.\n>\n> git-fetch fetches the branches from the remote and saves this\n> information in .git/FETCH_FETCHED, and at the end it generates\n> the file .git/FETCH_HEAD.\n\nI might have some more comments after actually applying this and\nreviewing it with wider contexts, but it looks nice overall.\n\nI am wondering if FETCH_FETCHED is purely for internal use by\ngit-fetch (it appears so), and if so if it is worth trying to do\nwithout the temporary file, but that is a minor detail.\n\n>  git-fetch.sh        |   79 +++++++++++++++++++++++++++++++--------------------\n>  git-parse-remote.sh |   64 ++++++++++++-----------------------------\n>  t/t5510-fetch.sh    |   16 ++++++++++\n>  3 files changed, 83 insertions(+), 76 deletions(-)\n\nLoses more lines than it adds (+16 lines to test does not\ncount), which is a very good sign.\n\n> diff --git a/git-fetch.sh b/git-fetch.sh\n> index d230995..637d732 100755\n> --- a/git-fetch.sh\n> +++ b/git-fetch.sh\n> @@ -491,3 +467,44 @@ case \"$orig_head\" in\n>  \tfi\n>  \t;;\n>  esac\n> +\n> +# Generate $GIT_DIR/FETCH_HEAD\n> +case \",$#,$remote_nick,\" in\n> +,1,$origin,)\n> +\tcurr_branch=$(git-symbolic-ref -q HEAD | sed -e 's|^refs/heads/||')\n> +\tmerge_branches=$(git-repo-config \\\n> +\t\t--get-all \"branch.${curr_branch}.merge\" | sort -u)\n\nWhy \"sort -u\" (instead of erroring out if the repository is\nmisconfigured)?\n\n> +\tfetch_branches=$(get_remote_default_refs_for_fetch -n $remote_nick |\n> +\t\tsed 's/:.*$//g;s/^+//' | sort -u)\n\nGNU sed users do not have problems with this, but I recall that\nwe had to rewrite our sed scripts not to use multiple commands\nconcatenated with ';' for portability, like:\n\n\tsed -e 's/:.*$//g' -e 's/^+//'\n\nAgain why \"sort -u\"?\n\n> +\ttest \"$merge_branches\" && test \"$fetch_branches\" &&\n\nWe probably would want to be defensive by saying \"test -n\".\n\n> +\tmerge_branches=$(echo -e \"$merge_branches\\n$fetch_branches\" | sort | uniq -d)\n\nI appreciate the cleverness of the intersection.  However, is\n\"echo -e\" portable?  I think we have avoided it so far (we have\navoided even \"echo -n\" which is traditionally much more\navailable).\n\n> +cat \"$GIT_DIR\"/FETCH_FETCHED | while IFS='\t' read sha1 ref note ; do\n> +\tremote_branch=$(expr \"z$ref\" : 'z\\([^:]*\\):')\n> +\tfor merge_branch in $merge_branches ; do\n> +\t\t[ \"$merge_branch\" == \"$remote_branch\" ] &&\n> +\t\t\techo \"$sha1\t\t$note\" && continue 2\n> +\tdone\n> +\tif ! test \"$merge_first\" || test \"$merge_first\" == \"done\" ; then\n> +\t\techo \"$sha1\tnot-for-merge\t$note\"\n> +\telse\n> +\t\techo \"$sha1\t\t$note\"\n> +\t\tmerge_first=done\n> +\tfi\n> +done >> \"$GIT_DIR/FETCH_HEAD\"\n\nYou can do:\n\n\twhile ...\n        do\n        done < \"$GIT_DIR/FETCH_FETCHED\"\n\nwhich is easier on the eye.  \n\nI often see a buggy shell script that expects assignment in a\nwhile loop to survive after the loop finished, when the loop is\non the downstream side of a pipe (e.g. the loop is run in a\nsubshell so merge_first after this loop is finished will never\nbe 'done').  You do not use the variable after the loop so your\nscript is not buggy, but avoiding a pipe into while loop is a\ngood habit to get into.\n\n> diff --git a/git-parse-remote.sh b/git-parse-remote.sh\n> index 5208ee6..691d46c 100755\n> --- a/git-parse-remote.sh\n> +++ b/git-parse-remote.sh\n> @@ -196,32 +159,43 @@ canon_refs_list_for_fetch () {\n\n>  \tconfig)\n> -\t\tcanon_refs_list_for_fetch -d \"$1\" \\\n> -\t\t\t$(git-config --get-all \"remote.$1.fetch\") ;;\n> +\t\tset $(expand_refs_wildcard \"$1\" \\\n> +\t\t\t$(git-repo-config --get-all \"remote.$1.fetch\")) ;;\n\nOops?  It is not buggy but it's better to set an example by\nusing git-config consistenty.  You have another mention of\nrepo-config above.\n\n>  \tremotes)\n> -\t\tcanon_refs_list_for_fetch -d \"$1\" $(sed -ne '/^Pull: */{\n> +\t\tset $(expand_refs_wildcard \"$1\" $(sed -ne '/^Pull: */{\n>  \t\t\t\t\t\ts///p\n> -\t\t\t\t\t}' \"$GIT_DIR/remotes/$1\")\n> +\t\t\t\t\t}' \"$GIT_DIR/remotes/$1\"))\n\nHmph.  I wonder why the original author did not do '/^Pull: */s///p'...\n"},{"id":"35317","messageId":"8aa486160702230242r4059811ewbc4cb5c6d33500df@mail.gmail.com","threadId":"6924","inReplyTo":"7vabz56vyq.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCHv3] git-fetch: Split fetch and merge logic","fromName":"Santi Béjar","fromEmail":"sbejar@gmail.com","sentAt":"2007-02-23T10:42:05Z","receivedAt":"2007-02-23T10:42:05Z","isPatch":false,"sender":{"key":"santi@agolina.net","avatar":null},"body":"On 2/23/07, Junio C Hamano <junkio@cox.net> wrote:\n> Santi Béjar <sbejar@gmail.com> writes:\n>\n> > It makes git-parse-remote.sh and almost all git-fetch independent\n> > of the merge logic.\n> >\n> > git-fetch fetches the branches from the remote and saves this\n> > information in .git/FETCH_FETCHED, and at the end it generates\n> > the file .git/FETCH_HEAD.\n>\n> I might have some more comments after actually applying this and\n> reviewing it with wider contexts, but it looks nice overall.\n\nThanks.\n\n>\n> I am wondering if FETCH_FETCHED is purely for internal use by\n> git-fetch (it appears so), and if so if it is worth trying to do\n> without the temporary file, but that is a minor detail.\n\nYes, it's purely internal. With \"without the temporary file\" you mean\nto put the content in a variable or removing at the end?\n\n> > diff --git a/git-fetch.sh b/git-fetch.sh\n> > index d230995..637d732 100755\n> > --- a/git-fetch.sh\n> > +++ b/git-fetch.sh\n> > @@ -491,3 +467,44 @@ case \"$orig_head\" in\n> >       fi\n> >       ;;\n> >  esac\n> > +\n> > +# Generate $GIT_DIR/FETCH_HEAD\n> > +case \",$#,$remote_nick,\" in\n> > +,1,$origin,)\n> > +     curr_branch=$(git-symbolic-ref -q HEAD | sed -e 's|^refs/heads/||')\n> > +     merge_branches=$(git-repo-config \\\n> > +             --get-all \"branch.${curr_branch}.merge\" | sort -u)\n>\n> Why \"sort -u\" (instead of erroring out if the repository is\n> misconfigured)?\n\nIt is consistent with the current code and I needed it for the\nintersection below. The check could be added later.\n\n>\n> > +     fetch_branches=$(get_remote_default_refs_for_fetch -n $remote_nick |\n> > +             sed 's/:.*$//g;s/^+//' | sort -u)\n>\n> GNU sed users do not have problems with this, but I recall that\n> we had to rewrite our sed scripts not to use multiple commands\n> concatenated with ';' for portability, like:\n>\n>         sed -e 's/:.*$//g' -e 's/^+//'\n\nOK.\n\n>\n> Again why \"sort -u\"?\n\nFor the intersection below. Moreover, it make sense because\nremotes.${remote}.fetch could be of the form:\n\n[remote \"origin\"]\nurl=...\nfetch= refs/heads/master:refs/remotes/origin/master\nfetch= refs/heads/*:refs/remotes/origin/*\n\nto have a well defined first refspec. In this case refs/heads/master\nappears twice.\n\n>\n> > +     test \"$merge_branches\" && test \"$fetch_branches\" &&\n>\n> We probably would want to be defensive by saying \"test -n\".\n>\n\nOk.\n\n> > +     merge_branches=$(echo -e \"$merge_branches\\n$fetch_branches\" | sort | uniq -d)\n>\n> I appreciate the cleverness of the intersection.  However, is\n> \"echo -e\" portable?  I think we have avoided it so far (we have\n> avoided even \"echo -n\" which is traditionally much more\n> available).\n\nprintf '%s\\n%s' \"$merge_branches\" \"$fetch_branches\"\n\nis OK?\n\n>\n> > +cat \"$GIT_DIR\"/FETCH_FETCHED | while IFS='   ' read sha1 ref note ; do\n> > +     remote_branch=$(expr \"z$ref\" : 'z\\([^:]*\\):')\n> > +     for merge_branch in $merge_branches ; do\n> > +             [ \"$merge_branch\" == \"$remote_branch\" ] &&\n> > +                     echo \"$sha1             $note\" && continue 2\n> > +     done\n> > +     if ! test \"$merge_first\" || test \"$merge_first\" == \"done\" ; then\n> > +             echo \"$sha1     not-for-merge   $note\"\n> > +     else\n> > +             echo \"$sha1             $note\"\n> > +             merge_first=done\n> > +     fi\n> > +done >> \"$GIT_DIR/FETCH_HEAD\"\n>\n> You can do:\n>\n>         while ...\n>         do\n>         done < \"$GIT_DIR/FETCH_FETCHED\"\n>\n> which is easier on the eye.\n>\n> I often see a buggy shell script that expects assignment in a\n> while loop to survive after the loop finished, when the loop is\n> on the downstream side of a pipe (e.g. the loop is run in a\n> subshell so merge_first after this loop is finished will never\n> be 'done').  You do not use the variable after the loop so your\n> script is not buggy, but avoiding a pipe into while loop is a\n> good habit to get into.\n\nOK, and thanks for the explanation.\n\n>\n> > diff --git a/git-parse-remote.sh b/git-parse-remote.sh\n> > index 5208ee6..691d46c 100755\n> > --- a/git-parse-remote.sh\n> > +++ b/git-parse-remote.sh\n> > @@ -196,32 +159,43 @@ canon_refs_list_for_fetch () {\n>\n> >       config)\n> > -             canon_refs_list_for_fetch -d \"$1\" \\\n> > -                     $(git-config --get-all \"remote.$1.fetch\") ;;\n> > +             set $(expand_refs_wildcard \"$1\" \\\n> > +                     $(git-repo-config --get-all \"remote.$1.fetch\")) ;;\n>\n> Oops?  It is not buggy but it's better to set an example by\n> using git-config consistenty.  You have another mention of\n> repo-config above.\n\nYes, of course.\n\n>\n> >       remotes)\n> > -             canon_refs_list_for_fetch -d \"$1\" $(sed -ne '/^Pull: */{\n> > +             set $(expand_refs_wildcard \"$1\" $(sed -ne '/^Pull: */{\n> >                                               s///p\n> > -                                     }' \"$GIT_DIR/remotes/$1\")\n> > +                                     }' \"$GIT_DIR/remotes/$1\"))\n>\n> Hmph.  I wonder why the original author did not do '/^Pull: */s///p'...\n\nI don't know, I'll do.\n\nSanti\n"},{"id":"35389","messageId":"7vd53zzyvo.fsf@assigned-by-dhcp.cox.net","threadId":"6924","inReplyTo":"8aa486160702230242r4059811ewbc4cb5c6d33500df@mail.gmail.com","subject":"Re: [PATCHv3] git-fetch: Split fetch and merge logic","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-02-24T09:30:19Z","receivedAt":"2007-02-24T09:30:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Santi Béjar\" <sbejar@gmail.com> writes:\n\n> On 2/23/07, Junio C Hamano <junkio@cox.net> wrote:\n>\n>> I am wondering if FETCH_FETCHED is purely for internal use by\n>> git-fetch (it appears so), and if so if it is worth trying to do\n>> without the temporary file, but that is a minor detail.\n>\n> Yes, it's purely internal. With \"without the temporary file\" you mean\n> to put the content in a variable or removing at the end?\n\nIf a variable suffices that would be quite nice, but it is not a\nbig deal.  As the script does fair amount of computation in\nsubprocess, I suspect it may not be worth trying to use variable,\nonly to get rid of the temporary file.\n\n>> I appreciate the cleverness of the intersection.  However, is\n>> \"echo -e\" portable?  I think we have avoided it so far (we have\n>> avoided even \"echo -n\" which is traditionally much more\n>> available).\n>\n> printf '%s\\n%s' \"$merge_branches\" \"$fetch_branches\"\n>\n> is OK?\n\nYes.  I think printf is what people who rewrote my \"echo -n\"\nhave done elsewhere.\n"},{"id":"35394","messageId":"7v1wkfzqgk.fsf@assigned-by-dhcp.cox.net","threadId":"6924","inReplyTo":"87wt29i7hg.fsf@gmail.com","subject":"Re: [PATCHv3] git-fetch: Split fetch and merge logic","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-02-24T12:32:11Z","receivedAt":"2007-02-24T12:32:11Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Santi Béjar <sbejar@gmail.com> writes:\n\n> It makes git-parse-remote.sh and almost all git-fetch independent\n> of the merge logic.\n>\n> git-fetch fetches the branches from the remote and saves this\n> information in .git/FETCH_FETCHED, and at the end it generates\n> the file .git/FETCH_HEAD.\n>\n> The current merge behaviour is unchanged.\n\nI overlooked your earlier huge testcase patch, but I noticed it\ndoes not include any test that fetches an annotated tag that\npoints at a commit or a non-commit, nor an unannotated tag that\nis a tree or a blob.  I am too tired to check them right now,\nbut are you sure you are not regressing with them?\n\nIt is sometimes handy to be able to say:\n\n\t$ git fetch git-gui tag spearce-gpg-pub\n        $ git cat-file -p FETCH_HEAD\n"},{"id":"35414","messageId":"7vlkimx1t1.fsf@assigned-by-dhcp.cox.net","threadId":"6924","inReplyTo":"7v1wkfzqgk.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCHv3] git-fetch: Split fetch and merge logic","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-02-25T05:07:38Z","receivedAt":"2007-02-25T05:07:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n> Santi Béjar <sbejar@gmail.com> writes:\n> ...\n>> The current merge behaviour is unchanged.\n>\n> I overlooked your earlier huge testcase patch, but I noticed it\n> does not include any test that fetches an annotated tag that\n> points at a commit or a non-commit, nor an unannotated tag that\n> is a tree or a blob.\n\nA couple of other things I think the users of information in\nFETCH_HEAD rely on need to be checked by the test suite.\n\n * Automated following of tags when a branch head is fetched.\n   When this happens, I think the users (not just git-pull\n   script but end users) expect the branch head to be listed\n   first in the FETCH_HEAD file, so that:\n\n\t$ git fetch git://some.repo/\n\t$ git show FETCH_HEAD\n\n   gives stable and useful result.\n\n * When two refs are listed on the command line explicitly, are\n   they listed in the order given by the user in the resulting\n   FETCH_HEAD file?\n\n\t$ git fetch git://some.repo/ one two\n\t$ git fetch git://some.repo/ two one\n\n   Again, \"git show FETCH_HEAD\" after that expects the first one\n   to show 'one' and the second one 'two'.  Replacing 'fetch'\n   with 'pull' in the example would affect the parent order in\n   the resulting Octopus.\n"}]}