{"thread":{"id":"14644","subject":"[PATCH] bash completion: Add completion for 'git help'","startedAt":"2008-07-23T23:13:15Z","lastAt":"2008-07-28T06:16:00Z","messageCount":15,"participants":["Lee Marlow","Shawn O. Pearce","Junio C Hamano","Jeff King","Johannes Schindelin","Daniel Barkalow"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"84638","messageId":"1216854795-51155-1-git-send-email-lee.marlow@gmail.com","threadId":"14644","inReplyTo":null,"subject":"[PATCH] bash completion: Add completion for 'git help'","fromName":"Lee Marlow","fromEmail":"lee.marlow@gmail.com","sentAt":"2008-07-23T23:13:15Z","receivedAt":"2008-07-23T23:13:15Z","isPatch":true,"sender":{"key":"lee.marlow@gmail.com","avatar":"https://gravatar.com/avatar/231170f3c8456cf1af984d30d09f35c5509b5f81494c81fa69dc7a1027473fc0?d=mp&s=160"},"body":"Renamed cached __git_commandlist to __git_porcelain_commandlist\nand added __git_all_commandlist that only filters out *--* helpers.\nCompletions for 'git help' will use the __git_all_commandlist, while\n__git_porcelain_commandlist is used for git command completion.\nUsers who actually read man pages may want to see help for plumbing\ncommands.\n\nOptions added: --all --info --man --web\n\nSigned-off-by: Lee Marlow <lee.marlow@gmail.com>\n---\n contrib/completion/git-completion.bash |   46 +++++++++++++++++++++++++++-----\n 1 files changed, 39 insertions(+), 7 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 2edb341..0e37b71 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -349,10 +349,10 @@ __git_complete_revlist ()\n \tesac\n }\n \n-__git_commands ()\n+__git_all_commands ()\n {\n-\tif [ -n \"$__git_commandlist\" ]; then\n-\t\techo \"$__git_commandlist\"\n+\tif [ -n \"$__git_all_commandlist\" ]; then\n+\t\techo \"$__git_all_commandlist\"\n \t\treturn\n \tfi\n \tlocal i IFS=\" \"$'\\n'\n@@ -360,6 +360,24 @@ __git_commands ()\n \tdo\n \t\tcase $i in\n \t\t*--*)             : helper pattern;;\n+\t\t*) echo $i;;\n+\t\tesac\n+\tdone\n+}\n+__git_all_commandlist=\n+__git_all_commandlist=\"$(__git_all_commands 2>/dev/null)\"\n+\n+__git_porcelain_commands ()\n+{\n+\tif [ -n \"$__git_porcelain_commandlist\" ]; then\n+\t\techo \"$__git_porcelain_commandlist\"\n+\t\treturn\n+\tfi\n+\tlocal i IFS=\" \"$'\\n'\n+\tfor i in 'help' $__git_all_commands\n+\tdo\n+\t\tcase $i in\n+\t\t*--*)             : helper pattern;;\n \t\tapplymbox)        : ask gittus;;\n \t\tapplypatch)       : ask gittus;;\n \t\tarchimport)       : import;;\n@@ -427,8 +445,8 @@ __git_commands ()\n \t\tesac\n \tdone\n }\n-__git_commandlist=\n-__git_commandlist=\"$(__git_commands 2>/dev/null)\"\n+__git_porcelain_commandlist=\n+__git_porcelain_commandlist=\"$(__git_porcelain_commands 2>/dev/null)\"\n \n __git_aliases ()\n {\n@@ -767,6 +785,18 @@ _git_gc ()\n \tCOMPREPLY=()\n }\n \n+_git_help ()\n+{\n+\tlocal cur=\"${COMP_WORDS[COMP_CWORD]}\"\n+\tcase \"$cur\" in\n+\t--*)\n+\t\t__gitcomp \"--all --info --man --web\"\n+\t\treturn\n+\t\t;;\n+\tesac\n+\t__gitcomp \"$(__git_all_commands)\"\n+}\n+\n _git_ls_remote ()\n {\n \t__gitcomp \"$(__git_remotes)\"\n@@ -1369,7 +1399,8 @@ _git ()\n \t\tcase \"$i\" in\n \t\t--git-dir=*) __git_dir=\"${i#--git-dir=}\" ;;\n \t\t--bare)      __git_dir=\".\" ;;\n-\t\t--version|--help|-p|--paginate) ;;\n+\t\t--version|-p|--paginate) ;;\n+\t\t--help) command=\"help\"; break ;;\n \t\t*) command=\"$i\"; break ;;\n \t\tesac\n \t\tc=$((++c))\n@@ -1389,7 +1420,7 @@ _git ()\n \t\t\t--help\n \t\t\t\"\n \t\t\t;;\n-\t\t*)     __gitcomp \"$(__git_commands) $(__git_aliases)\" ;;\n+\t\t*)     __gitcomp \"$(__git_porcelain_commands) $(__git_aliases)\" ;;\n \t\tesac\n \t\treturn\n \tfi\n@@ -1414,6 +1445,7 @@ _git ()\n \tfetch)       _git_fetch ;;\n \tformat-patch) _git_format_patch ;;\n \tgc)          _git_gc ;;\n+\thelp)        _git_help ;;\n \tlog)         _git_log ;;\n \tls-remote)   _git_ls_remote ;;\n \tls-tree)     _git_ls_tree ;;\n-- \n1.6.0.rc0.14.g95f8\n"},{"id":"84649","messageId":"1216858043-53646-1-git-send-email-lee.marlow@gmail.com","threadId":"14644","inReplyTo":"1216854795-51155-1-git-send-email-lee.marlow@gmail.com","subject":"[PATCH] bash completion: Add completion for 'git help'","fromName":"Lee Marlow","fromEmail":"lee.marlow@gmail.com","sentAt":"2008-07-24T00:07:23Z","receivedAt":"2008-07-24T00:07:23Z","isPatch":true,"sender":{"key":"lee.marlow@gmail.com","avatar":"https://gravatar.com/avatar/231170f3c8456cf1af984d30d09f35c5509b5f81494c81fa69dc7a1027473fc0?d=mp&s=160"},"body":"Renamed cached __git_commandlist to __git_porcelain_commandlist\nand added __git_all_commandlist that only filters out *--* helpers.\nCompletions for 'git help' will use the __git_all_commandlist, while\n__git_porcelain_commandlist is used for git command completion.\nUsers who actually read man pages may want to see help for plumbing\ncommands.\n\nOptions added: --all --info --man --web\n\nSigned-off-by: Lee Marlow <lee.marlow@gmail.com>\n---\nThis version fixes a small error in the __git_all_commands\n\n contrib/completion/git-completion.bash |   46 +++++++++++++++++++++++++++-----\n 1 files changed, 39 insertions(+), 7 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 2edb341..f3bdea4 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -349,10 +349,10 @@ __git_complete_revlist ()\n \tesac\n }\n \n-__git_commands ()\n+__git_all_commands ()\n {\n-\tif [ -n \"$__git_commandlist\" ]; then\n-\t\techo \"$__git_commandlist\"\n+\tif [ -n \"$__git_all_commandlist\" ]; then\n+\t\techo \"$__git_all_commandlist\"\n \t\treturn\n \tfi\n \tlocal i IFS=\" \"$'\\n'\n@@ -360,6 +360,24 @@ __git_commands ()\n \tdo\n \t\tcase $i in\n \t\t*--*)             : helper pattern;;\n+\t\t*) echo $i;;\n+\t\tesac\n+\tdone\n+}\n+__git_all_commandlist=\n+__git_all_commandlist=\"$(__git_all_commands 2>/dev/null)\"\n+\n+__git_porcelain_commands ()\n+{\n+\tif [ -n \"$__git_porcelain_commandlist\" ]; then\n+\t\techo \"$__git_porcelain_commandlist\"\n+\t\treturn\n+\tfi\n+\tlocal i IFS=\" \"$'\\n'\n+\tfor i in \"help\" $(__git_all_commands)\n+\tdo\n+\t\tcase $i in\n+\t\t*--*)             : helper pattern;;\n \t\tapplymbox)        : ask gittus;;\n \t\tapplypatch)       : ask gittus;;\n \t\tarchimport)       : import;;\n@@ -427,8 +445,8 @@ __git_commands ()\n \t\tesac\n \tdone\n }\n-__git_commandlist=\n-__git_commandlist=\"$(__git_commands 2>/dev/null)\"\n+__git_porcelain_commandlist=\n+__git_porcelain_commandlist=\"$(__git_porcelain_commands 2>/dev/null)\"\n \n __git_aliases ()\n {\n@@ -767,6 +785,18 @@ _git_gc ()\n \tCOMPREPLY=()\n }\n \n+_git_help ()\n+{\n+\tlocal cur=\"${COMP_WORDS[COMP_CWORD]}\"\n+\tcase \"$cur\" in\n+\t--*)\n+\t\t__gitcomp \"--all --info --man --web\"\n+\t\treturn\n+\t\t;;\n+\tesac\n+\t__gitcomp \"$(__git_all_commands)\"\n+}\n+\n _git_ls_remote ()\n {\n \t__gitcomp \"$(__git_remotes)\"\n@@ -1369,7 +1399,8 @@ _git ()\n \t\tcase \"$i\" in\n \t\t--git-dir=*) __git_dir=\"${i#--git-dir=}\" ;;\n \t\t--bare)      __git_dir=\".\" ;;\n-\t\t--version|--help|-p|--paginate) ;;\n+\t\t--version|-p|--paginate) ;;\n+\t\t--help) command=\"help\"; break ;;\n \t\t*) command=\"$i\"; break ;;\n \t\tesac\n \t\tc=$((++c))\n@@ -1389,7 +1420,7 @@ _git ()\n \t\t\t--help\n \t\t\t\"\n \t\t\t;;\n-\t\t*)     __gitcomp \"$(__git_commands) $(__git_aliases)\" ;;\n+\t\t*)     __gitcomp \"$(__git_porcelain_commands) $(__git_aliases)\" ;;\n \t\tesac\n \t\treturn\n \tfi\n@@ -1414,6 +1445,7 @@ _git ()\n \tfetch)       _git_fetch ;;\n \tformat-patch) _git_format_patch ;;\n \tgc)          _git_gc ;;\n+\thelp)        _git_help ;;\n \tlog)         _git_log ;;\n \tls-remote)   _git_ls_remote ;;\n \tls-tree)     _git_ls_tree ;;\n-- \n1.6.0.rc0.14.g95f8\n"},{"id":"84651","messageId":"1216858112-53713-1-git-send-email-lee.marlow@gmail.com","threadId":"14644","inReplyTo":"1216854795-51155-1-git-send-email-lee.marlow@gmail.com","subject":"[PATCH] bash completion: Add completion for 'git help'","fromName":"Lee Marlow","fromEmail":"lee.marlow@gmail.com","sentAt":"2008-07-24T00:08:32Z","receivedAt":"2008-07-24T00:08:32Z","isPatch":true,"sender":{"key":"lee.marlow@gmail.com","avatar":"https://gravatar.com/avatar/231170f3c8456cf1af984d30d09f35c5509b5f81494c81fa69dc7a1027473fc0?d=mp&s=160"},"body":"Renamed cached __git_commandlist to __git_porcelain_commandlist\nand added __git_all_commandlist that only filters out *--* helpers.\nCompletions for 'git help' will use the __git_all_commandlist, while\n__git_porcelain_commandlist is used for git command completion.\nUsers who actually read man pages may want to see help for plumbing\ncommands.\n\nOptions added: --all --info --man --web\n\nSigned-off-by: Lee Marlow <lee.marlow@gmail.com>\n---\nThis version fixes a small error in the __git_all_commands\n\n contrib/completion/git-completion.bash |   46 +++++++++++++++++++++++++++-----\n 1 files changed, 39 insertions(+), 7 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 2edb341..f3bdea4 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -349,10 +349,10 @@ __git_complete_revlist ()\n \tesac\n }\n \n-__git_commands ()\n+__git_all_commands ()\n {\n-\tif [ -n \"$__git_commandlist\" ]; then\n-\t\techo \"$__git_commandlist\"\n+\tif [ -n \"$__git_all_commandlist\" ]; then\n+\t\techo \"$__git_all_commandlist\"\n \t\treturn\n \tfi\n \tlocal i IFS=\" \"$'\\n'\n@@ -360,6 +360,24 @@ __git_commands ()\n \tdo\n \t\tcase $i in\n \t\t*--*)             : helper pattern;;\n+\t\t*) echo $i;;\n+\t\tesac\n+\tdone\n+}\n+__git_all_commandlist=\n+__git_all_commandlist=\"$(__git_all_commands 2>/dev/null)\"\n+\n+__git_porcelain_commands ()\n+{\n+\tif [ -n \"$__git_porcelain_commandlist\" ]; then\n+\t\techo \"$__git_porcelain_commandlist\"\n+\t\treturn\n+\tfi\n+\tlocal i IFS=\" \"$'\\n'\n+\tfor i in \"help\" $(__git_all_commands)\n+\tdo\n+\t\tcase $i in\n+\t\t*--*)             : helper pattern;;\n \t\tapplymbox)        : ask gittus;;\n \t\tapplypatch)       : ask gittus;;\n \t\tarchimport)       : import;;\n@@ -427,8 +445,8 @@ __git_commands ()\n \t\tesac\n \tdone\n }\n-__git_commandlist=\n-__git_commandlist=\"$(__git_commands 2>/dev/null)\"\n+__git_porcelain_commandlist=\n+__git_porcelain_commandlist=\"$(__git_porcelain_commands 2>/dev/null)\"\n \n __git_aliases ()\n {\n@@ -767,6 +785,18 @@ _git_gc ()\n \tCOMPREPLY=()\n }\n \n+_git_help ()\n+{\n+\tlocal cur=\"${COMP_WORDS[COMP_CWORD]}\"\n+\tcase \"$cur\" in\n+\t--*)\n+\t\t__gitcomp \"--all --info --man --web\"\n+\t\treturn\n+\t\t;;\n+\tesac\n+\t__gitcomp \"$(__git_all_commands)\"\n+}\n+\n _git_ls_remote ()\n {\n \t__gitcomp \"$(__git_remotes)\"\n@@ -1369,7 +1399,8 @@ _git ()\n \t\tcase \"$i\" in\n \t\t--git-dir=*) __git_dir=\"${i#--git-dir=}\" ;;\n \t\t--bare)      __git_dir=\".\" ;;\n-\t\t--version|--help|-p|--paginate) ;;\n+\t\t--version|-p|--paginate) ;;\n+\t\t--help) command=\"help\"; break ;;\n \t\t*) command=\"$i\"; break ;;\n \t\tesac\n \t\tc=$((++c))\n@@ -1389,7 +1420,7 @@ _git ()\n \t\t\t--help\n \t\t\t\"\n \t\t\t;;\n-\t\t*)     __gitcomp \"$(__git_commands) $(__git_aliases)\" ;;\n+\t\t*)     __gitcomp \"$(__git_porcelain_commands) $(__git_aliases)\" ;;\n \t\tesac\n \t\treturn\n \tfi\n@@ -1414,6 +1445,7 @@ _git ()\n \tfetch)       _git_fetch ;;\n \tformat-patch) _git_format_patch ;;\n \tgc)          _git_gc ;;\n+\thelp)        _git_help ;;\n \tlog)         _git_log ;;\n \tls-remote)   _git_ls_remote ;;\n \tls-tree)     _git_ls_tree ;;\n-- \n1.6.0.rc0.14.g95f8\n"},{"id":"84945","messageId":"20080725204051.GB23202@spearce.org","threadId":"14644","inReplyTo":"1216858043-53646-1-git-send-email-lee.marlow@gmail.com","subject":"Re: [PATCH] bash completion: Add completion for 'git help'","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-07-25T20:40:51Z","receivedAt":"2008-07-25T20:40:51Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Lee Marlow <lee.marlow@gmail.com> wrote:\n> Renamed cached __git_commandlist to __git_porcelain_commandlist\n> and added __git_all_commandlist that only filters out *--* helpers.\n> Completions for 'git help' will use the __git_all_commandlist, while\n> __git_porcelain_commandlist is used for git command completion.\n> Users who actually read man pages may want to see help for plumbing\n> commands.\n> \n> Options added: --all --info --man --web\n> \n> Signed-off-by: Lee Marlow <lee.marlow@gmail.com>\n\nAcked-by: Shawn O. Pearce <spearce@spearce.org>\n\n> This version fixes a small error in the __git_all_commands\n...\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 2edb341..f3bdea4 100755\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -349,10 +349,10 @@ __git_complete_revlist ()\n>  \tesac\n>  }\n>  \n> -__git_commands ()\n> +__git_all_commands ()\n>  {\n> -\tif [ -n \"$__git_commandlist\" ]; then\n> -\t\techo \"$__git_commandlist\"\n> +\tif [ -n \"$__git_all_commandlist\" ]; then\n> +\t\techo \"$__git_all_commandlist\"\n>  \t\treturn\n>  \tfi\n>  \tlocal i IFS=\" \"$'\\n'\n> @@ -360,6 +360,24 @@ __git_commands ()\n>  \tdo\n>  \t\tcase $i in\n>  \t\t*--*)             : helper pattern;;\n> +\t\t*) echo $i;;\n> +\t\tesac\n> +\tdone\n> +}\n> +__git_all_commandlist=\n> +__git_all_commandlist=\"$(__git_all_commands 2>/dev/null)\"\n> +\n> +__git_porcelain_commands ()\n> +{\n> +\tif [ -n \"$__git_porcelain_commandlist\" ]; then\n> +\t\techo \"$__git_porcelain_commandlist\"\n> +\t\treturn\n> +\tfi\n> +\tlocal i IFS=\" \"$'\\n'\n> +\tfor i in \"help\" $(__git_all_commands)\n> +\tdo\n> +\t\tcase $i in\n> +\t\t*--*)             : helper pattern;;\n>  \t\tapplymbox)        : ask gittus;;\n>  \t\tapplypatch)       : ask gittus;;\n>  \t\tarchimport)       : import;;\n> @@ -427,8 +445,8 @@ __git_commands ()\n>  \t\tesac\n>  \tdone\n>  }\n> -__git_commandlist=\n> -__git_commandlist=\"$(__git_commands 2>/dev/null)\"\n> +__git_porcelain_commandlist=\n> +__git_porcelain_commandlist=\"$(__git_porcelain_commands 2>/dev/null)\"\n>  \n>  __git_aliases ()\n>  {\n> @@ -767,6 +785,18 @@ _git_gc ()\n>  \tCOMPREPLY=()\n>  }\n>  \n> +_git_help ()\n> +{\n> +\tlocal cur=\"${COMP_WORDS[COMP_CWORD]}\"\n> +\tcase \"$cur\" in\n> +\t--*)\n> +\t\t__gitcomp \"--all --info --man --web\"\n> +\t\treturn\n> +\t\t;;\n> +\tesac\n> +\t__gitcomp \"$(__git_all_commands)\"\n> +}\n> +\n>  _git_ls_remote ()\n>  {\n>  \t__gitcomp \"$(__git_remotes)\"\n> @@ -1369,7 +1399,8 @@ _git ()\n>  \t\tcase \"$i\" in\n>  \t\t--git-dir=*) __git_dir=\"${i#--git-dir=}\" ;;\n>  \t\t--bare)      __git_dir=\".\" ;;\n> -\t\t--version|--help|-p|--paginate) ;;\n> +\t\t--version|-p|--paginate) ;;\n> +\t\t--help) command=\"help\"; break ;;\n>  \t\t*) command=\"$i\"; break ;;\n>  \t\tesac\n>  \t\tc=$((++c))\n> @@ -1389,7 +1420,7 @@ _git ()\n>  \t\t\t--help\n>  \t\t\t\"\n>  \t\t\t;;\n> -\t\t*)     __gitcomp \"$(__git_commands) $(__git_aliases)\" ;;\n> +\t\t*)     __gitcomp \"$(__git_porcelain_commands) $(__git_aliases)\" ;;\n>  \t\tesac\n>  \t\treturn\n>  \tfi\n> @@ -1414,6 +1445,7 @@ _git ()\n>  \tfetch)       _git_fetch ;;\n>  \tformat-patch) _git_format_patch ;;\n>  \tgc)          _git_gc ;;\n> +\thelp)        _git_help ;;\n>  \tlog)         _git_log ;;\n>  \tls-remote)   _git_ls_remote ;;\n>  \tls-tree)     _git_ls_tree ;;\n\n-- \nShawn.\n"},{"id":"84947","messageId":"7v1w1hsmnc.fsf@gitster.siamese.dyndns.org","threadId":"14644","inReplyTo":"20080725204051.GB23202@spearce.org","subject":"Re: [PATCH] bash completion: Add completion for 'git help'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-25T21:02:15Z","receivedAt":"2008-07-25T21:02:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks for an Ack, but personally I do not think the completion of \"all\ncommands\" is worth it.\n\nI've been busy with day-job for the past few days, and haven't had chance\nto push things out today, but FYI here is what I already have on 'master'\nprivately, relative to what have already been pushed out.\n\n  Documentation: clarify how to disable elements in core.whitespace (Junio C Hamano)\n  Makefile: fix shell quoting (Junio C Hamano)\n  tests: propagate $(TAR) down from the toplevel Makefile (Junio C Hamano)\n  index-pack.c: correctly initialize appended objects (Björn Steinbrink)\n  send-email: find body-encoding correctly (Peter Valdemar Mørch)\n  document that git-tag can tag more than heads (Jonathan Nieder)\n  perl/Makefile: update NO_PERL_MAKEMAKER section (Brandon Casey)\n  bash: offer only paths after '--' for 'git checkout' (SZEDER Gábor)\n  checkout: mention '--' in the docs (SZEDER Gábor)\n  git-checkout: improve error messages, detect ambiguities. (Pierre Habouzit)\n  update test case to protect am --skip behaviour (Olivier Marin)\n  Teach fsck and prune about the new location of temporary objects (Brandon Casey)\n  git-checkout: fix command line parsing. (Pierre Habouzit)\n\nAt this point immediately before -rc1, I am giving much higher precedence\nto real fixes than clean-ups, \"use parse-opt\", or new features.  Please do\nnot get alarmed if your non-fix patches are left unresponded for a while.\n\nBTW, has anybody taken a look at this one?\n\n  Subject: BUG: fetch incorrect interpretation of globing patterns in refspecs\n  Date: Thu, 24 Jul 2008 09:07:21 +0200\n  Message-ID: <71295b5a0807240007k246973abj1897895d0d67bb6c@mail.gmail.com>\n\nIf not, I think I probably need to take a look at this, reproducing and\npossibly fixing, before applying non-fix patches.\n"},{"id":"85032","messageId":"20080726082405.GA10104@sigill.intra.peff.net","threadId":"14644","inReplyTo":"7v1w1hsmnc.fsf@gitster.siamese.dyndns.org","subject":"fetch refspec foo/* matches foo*","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-07-26T08:24:05Z","receivedAt":"2008-07-26T08:24:05Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 25, 2008 at 02:02:15PM -0700, Junio C Hamano wrote:\n\n> BTW, has anybody taken a look at this one?\n> \n>   Subject: BUG: fetch incorrect interpretation of globing patterns in refspecs\n>   Date: Thu, 24 Jul 2008 09:07:21 +0200\n>   Message-ID: <71295b5a0807240007k246973abj1897895d0d67bb6c@mail.gmail.com>\n> \n> If not, I think I probably need to take a look at this, reproducing and\n> possibly fixing, before applying non-fix patches.\n\nI have been meaning to look at it for days, so I finally took a peek.  I\nwas able to reproduce the problem easily. I think it is (almost) as\nsimple as the patch below. In the refspec parsing, we already require\nglobs to come after '/', so this is the analagous check during match.\n\nUnfortunately, this breaks t1020 (something about failing to clone HEAD\nit looks like, so probably it is some boundary case for matching just\n\"*\"). I don't have time to look further, and I will be out of touch\nuntil probably Sunday evening, so hopefully somebody else can run with\nit.\n\n---\ndiff --git a/remote.c b/remote.c\nindex 0d6020b..3ae0431 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1108,7 +1108,8 @@ static struct ref *get_expanded_map(const struct ref *remote_refs,\n \tfor (ref = remote_refs; ref; ref = ref->next) {\n \t\tif (strchr(ref->name, '^'))\n \t\t\tcontinue; /* a dereference item */\n-\t\tif (!prefixcmp(ref->name, refspec->src)) {\n+\t\tif (!prefixcmp(ref->name, refspec->src)\n+\t\t     && ref->name[remote_prefix_len] == '/') {\n \t\t\tconst char *match;\n \t\t\tstruct ref *cpy = copy_ref(ref);\n \t\t\tmatch = ref->name + remote_prefix_len;\n"},{"id":"85034","messageId":"20080726083356.GB10104@sigill.intra.peff.net","threadId":"14644","inReplyTo":"20080726082405.GA10104@sigill.intra.peff.net","subject":"Re: fetch refspec foo/* matches foo*","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-07-26T08:33:57Z","receivedAt":"2008-07-26T08:33:57Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 26, 2008 at 04:24:05AM -0400, Jeff King wrote:\n\n> > If not, I think I probably need to take a look at this, reproducing and\n> > possibly fixing, before applying non-fix patches.\n> \n> I have been meaning to look at it for days, so I finally took a peek.  I\n> was able to reproduce the problem easily. I think it is (almost) as\n> simple as the patch below. In the refspec parsing, we already require\n> globs to come after '/', so this is the analagous check during match.\n\nAlso, while I have your attention, Junio, here is another bug fix\nthat should go into 1.6.0. I posted the patch as a \"how about this\" deep\nin a thread and got no response (which means no complaints, right?).\n\n-- >8 --\ninit: handle empty \"template\" parameter\n\nIf a user passes \"--template=\", then our template parameter\nis blank. Unfortunately, copy_templates() assumes it has at\nleast one character, and does all sorts of bad things like\nreading from template[-1] and then proceeding to link all of\n'/' into the .git directory.\n\nThis patch just checks for that condition in copy_templates\nand aborts. As a side effect, this means that --template=\nnow has the meaning \"don't copy any templates.\"\n---\nI don't really care about the \"side effect\" behavior, but it seems\nreasonable. The other obvious option is to simply die(). Certainly\neither is better than the current bug.\n\n builtin-init-db.c |    2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-init-db.c b/builtin-init-db.c\nindex 38b4fcb..baf0d09 100644\n--- a/builtin-init-db.c\n+++ b/builtin-init-db.c\n@@ -117,6 +117,8 @@ static void copy_templates(const char *template_dir)\n \t\ttemplate_dir = getenv(TEMPLATE_DIR_ENVIRONMENT);\n \tif (!template_dir)\n \t\ttemplate_dir = system_path(DEFAULT_GIT_TEMPLATE_DIR);\n+\tif (!template_dir[0])\n+\t\treturn;\n \tstrcpy(template_path, template_dir);\n \ttemplate_len = strlen(template_path);\n \tif (template_path[template_len-1] != '/') {\n-- \n1.6.0.rc0.233.gb3fd2\n"},{"id":"85055","messageId":"alpine.DEB.1.00.0807261511490.26810@eeepc-johanness","threadId":"14644","inReplyTo":"20080726083356.GB10104@sigill.intra.peff.net","subject":"Re: [PATCH] init: handle empty \"template\" parameter, was Re: fetch refspec foo/* matches foo*","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-26T13:13:26Z","receivedAt":"2008-07-26T13:13:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 26 Jul 2008, Jeff King wrote:\n\n> Also, while I have your attention, Junio, here is another bug fix\n> that should go into 1.6.0. I posted the patch as a \"how about this\" deep\n> in a thread and got no response (which means no complaints, right?).\n\nAgain it is in a thread...\n\n> This patch just checks for that condition in copy_templates\n> and aborts. As a side effect, this means that --template=\n> now has the meaning \"don't copy any templates.\"\n\nI deem this patch obviously correct, and your reasoning as to what an \nempty parameter should mean makes sense.\n\nCiao,\nDscho\n"},{"id":"85128","messageId":"7vvdysb2na.fsf@gitster.siamese.dyndns.org","threadId":"14644","inReplyTo":"20080726082405.GA10104@sigill.intra.peff.net","subject":"Re: fetch refspec foo/* matches foo*","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-27T00:18:33Z","receivedAt":"2008-07-27T00:18:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Fri, Jul 25, 2008 at 02:02:15PM -0700, Junio C Hamano wrote:\n>\n>> BTW, has anybody taken a look at this one?\n>> \n>>   Subject: BUG: fetch incorrect interpretation of globing patterns in refspecs\n>>   Date: Thu, 24 Jul 2008 09:07:21 +0200\n>>   Message-ID: <71295b5a0807240007k246973abj1897895d0d67bb6c@mail.gmail.com>\n>> \n>> If not, I think I probably need to take a look at this, reproducing and\n>> possibly fixing, before applying non-fix patches.\n>\n> I have been meaning to look at it for days, so I finally took a peek.  I\n> was able to reproduce the problem easily. I think it is (almost) as\n> simple as the patch below. In the refspec parsing, we already require\n> globs to come after '/', so this is the analagous check during match.\n>\n> Unfortunately, this breaks t1020 (something about failing to clone HEAD\n\nYour patch expects that the parsed refspec->{src,dst} omit the terminating\nslash, which is in line with what parse_refspec_internal() in remote.c\ndoes.  The problem is that builtin-clone.c uses a refspec that is\nincompatible from that assumption.  The static \"tag_refspec\" variable\ndefined in remote.c has the same problem.  These two have trailing slash\ne.g. \"refs/heads/\", \"refs/tags/\", that should be dropped for your updated\ncheck to work.\n\nThe attached patch, that includes your one-liner change, makes all tests\npass.\n\nI have a nagging suspicion that it might be a simpler and cleaner change\nto change parse_refspec_internal() to keep the trailing slash, instead of\ndropping it.  Then the check you added is not needed (the trailing slash\nguarantees that the pattern matches at the hierarchy boundary), neither\nany of the change in this patch.\n\n---\n builtin-clone.c        |    9 +++++----\n remote.c               |    7 ++++---\n t/t5513-fetch-track.sh |   30 ++++++++++++++++++++++++++++++\n 3 files changed, 39 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex e086a40..022ffb9 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -440,12 +440,12 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tgit_config(git_default_config, NULL);\n \n \tif (option_bare) {\n-\t\tstrcpy(branch_top, \"refs/heads/\");\n+\t\tstrcpy(branch_top, \"refs/heads\");\n \n \t\tgit_config_set(\"core.bare\", \"true\");\n \t} else {\n \t\tsnprintf(branch_top, sizeof(branch_top),\n-\t\t\t \"refs/remotes/%s/\", option_origin);\n+\t\t\t \"refs/remotes/%s\", option_origin);\n \n \t\t/* Configure the remote */\n \t\tsnprintf(key, sizeof(key), \"remote.%s.url\", option_origin);\n@@ -453,13 +453,13 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \n \t\tsnprintf(key, sizeof(key), \"remote.%s.fetch\", option_origin);\n \t\tsnprintf(value, sizeof(value),\n-\t\t\t\t\"+refs/heads/*:%s*\", branch_top);\n+\t\t\t\t\"+refs/heads/*:%s/*\", branch_top);\n \t\tgit_config_set_multivar(key, value, \"^$\", 0);\n \t}\n \n \trefspec.force = 0;\n \trefspec.pattern = 1;\n-\trefspec.src = \"refs/heads/\";\n+\trefspec.src = \"refs/heads\";\n \trefspec.dst = branch_top;\n \n \tif (path && !is_bundle)\n@@ -514,6 +514,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \n \t\t\tstrbuf_init(&head_ref, 0);\n \t\t\tstrbuf_addstr(&head_ref, branch_top);\n+\t\t\tstrbuf_addch(&head_ref, '/');\n \t\t\tstrbuf_addstr(&head_ref, \"HEAD\");\n \n \t\t\t/* Remote branch link */\ndiff --git a/remote.c b/remote.c\nindex 0d6020b..6b313fb 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -9,8 +9,8 @@ static struct refspec s_tag_refspec = {\n \t0,\n \t1,\n \t0,\n-\t\"refs/tags/\",\n-\t\"refs/tags/\"\n+\t\"refs/tags\",\n+\t\"refs/tags\"\n };\n \n const struct refspec *tag_refspec = &s_tag_refspec;\n@@ -1108,7 +1108,8 @@ static struct ref *get_expanded_map(const struct ref *remote_refs,\n \tfor (ref = remote_refs; ref; ref = ref->next) {\n \t\tif (strchr(ref->name, '^'))\n \t\t\tcontinue; /* a dereference item */\n-\t\tif (!prefixcmp(ref->name, refspec->src)) {\n+\t\tif (!prefixcmp(ref->name, refspec->src) &&\n+\t\t    ref->name[remote_prefix_len] == '/') {\n \t\t\tconst char *match;\n \t\t\tstruct ref *cpy = copy_ref(ref);\n \t\t\tmatch = ref->name + remote_prefix_len;\ndiff --git a/t/t5513-fetch-track.sh b/t/t5513-fetch-track.sh\nnew file mode 100755\nindex 0000000..9e74862\n--- /dev/null\n+++ b/t/t5513-fetch-track.sh\n@@ -0,0 +1,30 @@\n+#!/bin/sh\n+\n+test_description='fetch follows remote tracking branches correctly'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\t>file &&\n+\tgit add . &&\n+\ttest_tick &&\n+\tgit commit -m Initial &&\n+\tgit branch b-0 &&\n+\tgit branch b1 &&\n+\tgit branch b/one &&\n+\ttest_create_repo other &&\n+\t(\n+\t\tcd other &&\n+\t\tgit config remote.origin.url .. &&\n+\t\tgit config remote.origin.fetch \"+refs/heads/b/*:refs/remotes/b/*\"\n+\t)\n+'\n+\n+test_expect_success fetch '\n+\t(\n+\t\tcd other && git fetch origin &&\n+\t\ttest \"$(git for-each-ref --format=\"%(refname)\")\" = refs/remotes/b/one\n+\t)\n+'\n+\n+test_done\n"},{"id":"85145","messageId":"7vsktv3l9k.fsf_-_@gitster.siamese.dyndns.org","threadId":"14644","inReplyTo":"7vvdysb2na.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] make sure parsed wildcard refspec ends with slash","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-27T06:15:51Z","receivedAt":"2008-07-27T06:15:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"A wildcard refspec is internally parsed into a refspec structure with src\nand dst strings.  Many parts of the code assumed that these do not include\nthe trailing \"/*\" when matching the wildcard pattern with an actual ref we\nsee at the remote.  What this meant was that we needed to make sure not\njust that the prefix matched, and also that a slash followed the part that\nmatched.\n\nBut a codepath that scans the result from ls-remote and finds matching\nrefs forgot to check the \"matching part must be followed by a slash\" rule.\nThis resulted in \"refs/heads/b1\" from the remote side to mistakenly match\nthe source side of \"refs/heads/b/*:refs/remotes/b/*\" refspec.\n\nWorse, the refspec crafted internally by \"git-clone\", and a hardcoded\npreparsed refspec that is used to implement \"git-fetch --tags\", violated\nthis \"parsed widcard refspec does not end with slash\" rule; simply adding\nthe \"matching part must be followed by a slash\" rule then would have\nbroken codepaths that use these refspecs.\n\nThis commit changes the rule to require a trailing slash to parsed\nwildcard refspecs.  IOW, \"refs/heads/b/*:refs/remotes/b/*\" is parsed as\nsrc = \"refs/heads/b/\" and dst = \"refs/remotes/b/\".  This allows us to\nsimplify the matching logic because we only need to do a prefixcmp() to\nnotice \"refs/heads/b/one\" matches and \"refs/heads/b1\" does not.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\nJunio C Hamano <gitster@pobox.com> writes:\n\n> I have a nagging suspicion that it might be a simpler and cleaner change\n> to change parse_refspec_internal() to keep the trailing slash, instead of\n> dropping it.  Then the check you added is not needed (the trailing slash\n> guarantees that the pattern matches at the hierarchy boundary), neither\n> any of the change in this patch.\n\nThis is the other variant, and it turns out that I was right.  Among the\n64-18 = 46 new lines, 30 are from the new test file.  Two existing\n\"matching part is followed by '/'\" tests are removed.\n\n remote.c               |   52 +++++++++++++++++++++++++++++++----------------\n t/t5513-fetch-track.sh |   30 +++++++++++++++++++++++++++\n 2 files changed, 64 insertions(+), 18 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 0d6020b..f61a3ab 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -427,6 +427,28 @@ static void read_config(void)\n \talias_all_urls();\n }\n \n+/*\n+ * We need to make sure the tracking branches are well formed, but a\n+ * wildcard refspec in \"struct refspec\" must have a trailing slash. We\n+ * temporarily drop the trailing '/' while calling check_ref_format(),\n+ * and put it back.  The caller knows that a CHECK_REF_FORMAT_ONELEVEL\n+ * error return is Ok for a wildcard refspec.\n+ */\n+static int verify_refname(char *name, int is_glob)\n+{\n+\tint result, len = -1;\n+\n+\tif (is_glob) {\n+\t\tlen = strlen(name);\n+\t\tassert(name[len - 1] == '/');\n+\t\tname[len - 1] = '\\0';\n+\t}\n+\tresult = check_ref_format(name);\n+\tif (is_glob)\n+\t\tname[len - 1] = '/';\n+\treturn result;\n+}\n+\n static struct refspec *parse_refspec_internal(int nr_refspec, const char **refspec, int fetch, int verify)\n {\n \tint i;\n@@ -434,11 +456,11 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \tstruct refspec *rs = xcalloc(sizeof(*rs), nr_refspec);\n \n \tfor (i = 0; i < nr_refspec; i++) {\n-\t\tsize_t llen, rlen;\n+\t\tsize_t llen;\n \t\tint is_glob;\n \t\tconst char *lhs, *rhs;\n \n-\t\tllen = rlen = is_glob = 0;\n+\t\tllen = is_glob = 0;\n \n \t\tlhs = refspec[i];\n \t\tif (*lhs == '+') {\n@@ -458,12 +480,9 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \t\t}\n \n \t\tif (rhs) {\n-\t\t\trhs++;\n-\t\t\trlen = strlen(rhs);\n+\t\t\tsize_t rlen = strlen(++rhs);\n \t\t\tis_glob = (2 <= rlen && !strcmp(rhs + rlen - 2, \"/*\"));\n-\t\t\tif (is_glob)\n-\t\t\t\trlen -= 2;\n-\t\t\trs[i].dst = xstrndup(rhs, rlen);\n+\t\t\trs[i].dst = xstrndup(rhs, rlen - is_glob);\n \t\t}\n \n \t\tllen = (rhs ? (rhs - lhs - 1) : strlen(lhs));\n@@ -471,7 +490,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \t\t\tif ((rhs && !is_glob) || (!rhs && fetch))\n \t\t\t\tgoto invalid;\n \t\t\tis_glob = 1;\n-\t\t\tllen -= 2;\n+\t\t\tllen--;\n \t\t} else if (rhs && is_glob) {\n \t\t\tgoto invalid;\n \t\t}\n@@ -488,7 +507,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \t\t\tif (!*rs[i].src)\n \t\t\t\t; /* empty is ok */\n \t\t\telse {\n-\t\t\t\tst = check_ref_format(rs[i].src);\n+\t\t\t\tst = verify_refname(rs[i].src, is_glob);\n \t\t\t\tif (st && st != CHECK_REF_FORMAT_ONELEVEL)\n \t\t\t\t\tgoto invalid;\n \t\t\t}\n@@ -503,7 +522,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \t\t\t} else if (!*rs[i].dst) {\n \t\t\t\t; /* ok */\n \t\t\t} else {\n-\t\t\t\tst = check_ref_format(rs[i].dst);\n+\t\t\t\tst = verify_refname(rs[i].dst, is_glob);\n \t\t\t\tif (st && st != CHECK_REF_FORMAT_ONELEVEL)\n \t\t\t\t\tgoto invalid;\n \t\t\t}\n@@ -518,7 +537,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \t\t\tif (!*rs[i].src)\n \t\t\t\t; /* empty is ok */\n \t\t\telse if (is_glob) {\n-\t\t\t\tst = check_ref_format(rs[i].src);\n+\t\t\t\tst = verify_refname(rs[i].src, is_glob);\n \t\t\t\tif (st && st != CHECK_REF_FORMAT_ONELEVEL)\n \t\t\t\t\tgoto invalid;\n \t\t\t}\n@@ -532,13 +551,13 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \t\t\t * - otherwise it must be a valid looking ref.\n \t\t\t */\n \t\t\tif (!rs[i].dst) {\n-\t\t\t\tst = check_ref_format(rs[i].src);\n+\t\t\t\tst = verify_refname(rs[i].src, is_glob);\n \t\t\t\tif (st && st != CHECK_REF_FORMAT_ONELEVEL)\n \t\t\t\t\tgoto invalid;\n \t\t\t} else if (!*rs[i].dst) {\n \t\t\t\tgoto invalid;\n \t\t\t} else {\n-\t\t\t\tst = check_ref_format(rs[i].dst);\n+\t\t\t\tst = verify_refname(rs[i].dst, is_glob);\n \t\t\t\tif (st && st != CHECK_REF_FORMAT_ONELEVEL)\n \t\t\t\t\tgoto invalid;\n \t\t\t}\n@@ -687,8 +706,7 @@ int remote_find_tracking(struct remote *remote, struct refspec *refspec)\n \t\tif (!fetch->dst)\n \t\t\tcontinue;\n \t\tif (fetch->pattern) {\n-\t\t\tif (!prefixcmp(needle, key) &&\n-\t\t\t    needle[strlen(key)] == '/') {\n+\t\t\tif (!prefixcmp(needle, key)) {\n \t\t\t\t*result = xmalloc(strlen(value) +\n \t\t\t\t\t\t  strlen(needle) -\n \t\t\t\t\t\t  strlen(key) + 1);\n@@ -966,9 +984,7 @@ static const struct refspec *check_pattern_match(const struct refspec *rs,\n \t\t\tcontinue;\n \t\t}\n \n-\t\tif (rs[i].pattern &&\n-\t\t    !prefixcmp(src->name, rs[i].src) &&\n-\t\t    src->name[strlen(rs[i].src)] == '/')\n+\t\tif (rs[i].pattern && !prefixcmp(src->name, rs[i].src))\n \t\t\treturn rs + i;\n \t}\n \tif (matching_refs != -1)\ndiff --git a/t/t5513-fetch-track.sh b/t/t5513-fetch-track.sh\nnew file mode 100755\nindex 0000000..9e74862\n--- /dev/null\n+++ b/t/t5513-fetch-track.sh\n@@ -0,0 +1,30 @@\n+#!/bin/sh\n+\n+test_description='fetch follows remote tracking branches correctly'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\t>file &&\n+\tgit add . &&\n+\ttest_tick &&\n+\tgit commit -m Initial &&\n+\tgit branch b-0 &&\n+\tgit branch b1 &&\n+\tgit branch b/one &&\n+\ttest_create_repo other &&\n+\t(\n+\t\tcd other &&\n+\t\tgit config remote.origin.url .. &&\n+\t\tgit config remote.origin.fetch \"+refs/heads/b/*:refs/remotes/b/*\"\n+\t)\n+'\n+\n+test_expect_success fetch '\n+\t(\n+\t\tcd other && git fetch origin &&\n+\t\ttest \"$(git for-each-ref --format=\"%(refname)\")\" = refs/remotes/b/one\n+\t)\n+'\n+\n+test_done\n"},{"id":"85180","messageId":"alpine.LNX.1.00.0807271257400.19665@iabervon.org","threadId":"14644","inReplyTo":"7vsktv3l9k.fsf_-_@gitster.siamese.dyndns.org","subject":"Re: [PATCH] make sure parsed wildcard refspec ends with slash","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-07-27T17:20:34Z","receivedAt":"2008-07-27T17:20:34Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Sat, 26 Jul 2008, Junio C Hamano wrote:\n\n> A wildcard refspec is internally parsed into a refspec structure with src\n> and dst strings.  Many parts of the code assumed that these do not include\n> the trailing \"/*\" when matching the wildcard pattern with an actual ref we\n> see at the remote.  What this meant was that we needed to make sure not\n> just that the prefix matched, and also that a slash followed the part that\n> matched.\n> \n> But a codepath that scans the result from ls-remote and finds matching\n> refs forgot to check the \"matching part must be followed by a slash\" rule.\n> This resulted in \"refs/heads/b1\" from the remote side to mistakenly match\n> the source side of \"refs/heads/b/*:refs/remotes/b/*\" refspec.\n> \n> Worse, the refspec crafted internally by \"git-clone\", and a hardcoded\n> preparsed refspec that is used to implement \"git-fetch --tags\", violated\n> this \"parsed widcard refspec does not end with slash\" rule; simply adding\n> the \"matching part must be followed by a slash\" rule then would have\n> broken codepaths that use these refspecs.\n> \n> This commit changes the rule to require a trailing slash to parsed\n> wildcard refspecs.  IOW, \"refs/heads/b/*:refs/remotes/b/*\" is parsed as\n> src = \"refs/heads/b/\" and dst = \"refs/remotes/b/\".  This allows us to\n> simplify the matching logic because we only need to do a prefixcmp() to\n> notice \"refs/heads/b/one\" matches and \"refs/heads/b1\" does not.\n> \n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > I have a nagging suspicion that it might be a simpler and cleaner change\n> > to change parse_refspec_internal() to keep the trailing slash, instead of\n> > dropping it.  Then the check you added is not needed (the trailing slash\n> > guarantees that the pattern matches at the hierarchy boundary), neither\n> > any of the change in this patch.\n> \n> This is the other variant, and it turns out that I was right.  Among the\n> 64-18 = 46 new lines, 30 are from the new test file.  Two existing\n> \"matching part is followed by '/'\" tests are removed.\n\nYeah, the first version of this code included the '/', by virtue of having \nthe '/' not be required in the refspec (i.e., you could have \n\"refs/heads/b*\"); when you told me that refspecs needed to have a '/' \nbefore the '*', I thought that it would be easiest to not include the '/', \nsince it was redundant, but, in retrospect, I'm not too surprised that it \nsimplifies things to include it.\n\nAcked-by: Daniel Barkalow <barkalow@iabervon.org>\n\n>  remote.c               |   52 +++++++++++++++++++++++++++++++----------------\n>  t/t5513-fetch-track.sh |   30 +++++++++++++++++++++++++++\n>  2 files changed, 64 insertions(+), 18 deletions(-)\n> \n> diff --git a/remote.c b/remote.c\n> index 0d6020b..f61a3ab 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -427,6 +427,28 @@ static void read_config(void)\n>  \talias_all_urls();\n>  }\n>  \n> +/*\n> + * We need to make sure the tracking branches are well formed, but a\n> + * wildcard refspec in \"struct refspec\" must have a trailing slash. We\n> + * temporarily drop the trailing '/' while calling check_ref_format(),\n> + * and put it back.  The caller knows that a CHECK_REF_FORMAT_ONELEVEL\n> + * error return is Ok for a wildcard refspec.\n> + */\n> +static int verify_refname(char *name, int is_glob)\n> +{\n> +\tint result, len = -1;\n> +\n> +\tif (is_glob) {\n> +\t\tlen = strlen(name);\n> +\t\tassert(name[len - 1] == '/');\n> +\t\tname[len - 1] = '\\0';\n> +\t}\n> +\tresult = check_ref_format(name);\n> +\tif (is_glob)\n> +\t\tname[len - 1] = '/';\n> +\treturn result;\n> +}\n> +\n\nMaybe check_ref_format() ought to have a return for \"this is valid as a \ndirectory rather than a single ref\", and that would be allowed with globs? \nI'm not too fond of temporarily changing strings for testing. Also, this \ndesign (while it matches the current code), means that check_ref_format() \nsees one fewer level than the refs that match will actually have, which is \na little confusing.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"85239","messageId":"20080728054151.GB7294@sigill.intra.peff.net","threadId":"14644","inReplyTo":"7vsktv3l9k.fsf_-_@gitster.siamese.dyndns.org","subject":"Re: [PATCH] make sure parsed wildcard refspec ends with slash","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-07-28T05:41:51Z","receivedAt":"2008-07-28T05:41:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 26, 2008 at 11:15:51PM -0700, Junio C Hamano wrote:\n\n> > I have a nagging suspicion that it might be a simpler and cleaner change\n> > to change parse_refspec_internal() to keep the trailing slash, instead of\n> > dropping it.  Then the check you added is not needed (the trailing slash\n> > guarantees that the pattern matches at the hierarchy boundary), neither\n> > any of the change in this patch.\n> \n> This is the other variant, and it turns out that I was right.  Among the\n> 64-18 = 46 new lines, 30 are from the new test file.  Two existing\n> \"matching part is followed by '/'\" tests are removed.\n\nLooks like you have already applied it, but I will chime in that of the\ntwo, I think this is the more sensible change. Stripping the '/' felt\nlike a loss of information to me. IIRC, when \"support only /*\" was\ndiscussed, the thinking was \"let's keep it tight now, and we can loosen\nit later.\" Keeping the '/' means that the code is much more sane if\nother globbing rules come in the future.\n\n-Peff\n"},{"id":"85245","messageId":"20080728060204.GC7294@sigill.intra.peff.net","threadId":"14644","inReplyTo":"alpine.DEB.1.00.0807261511490.26810@eeepc-johanness","subject":"Re: [PATCH] init: handle empty \"template\" parameter, was Re: fetch refspec foo/* matches foo*","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-07-28T06:02:04Z","receivedAt":"2008-07-28T06:02:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 26, 2008 at 03:13:26PM +0200, Johannes Schindelin wrote:\n\n> > Also, while I have your attention, Junio, here is another bug fix\n> > that should go into 1.6.0. I posted the patch as a \"how about this\" deep\n> > in a thread and got no response (which means no complaints, right?).\n> \n> Again it is in a thread...\n\nHeh. Point taken.\n\nMy meaning was \"you, Junio, did not see this because it was in another\nthread, so I am pointing it out to you\" but the irony of the rest of the\nstatement was lost on me during the original writing.\n\n> I deem this patch obviously correct, and your reasoning as to what an \n> empty parameter should mean makes sense.\n\nThank you for reviewing, anyway. ;)\n\nHere is a re-post with a test for the new behavior added in (and an\nextra test just to make sure --template works at all. It does get used\nby test-lib, so we were implicitly checking it to some degree, but it's\nentirely possible that it could fail and the tests wouldn't necessarily\nnotice (e.g., if it accidentally used an installed set of templates\ninstead)).\n\n-- >8 --\ninit: handle empty \"template\" parameter\n\nIf a user passes \"--template=\", then our template parameter\nis blank. Unfortunately, copy_templates() assumes it has at\nleast one character, and does all sorts of bad things like\nreading from template[-1] and then proceeding to link all of\n'/' into the .git directory.\n\nThis patch just checks for that condition in copy_templates\nand aborts. As a side effect, this means that --template=\nnow has the meaning \"don't copy any templates.\"\n---\n builtin-init-db.c |    2 ++\n t/t0001-init.sh   |   26 ++++++++++++++++++++++++++\n 2 files changed, 28 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-init-db.c b/builtin-init-db.c\nindex 38b4fcb..baf0d09 100644\n--- a/builtin-init-db.c\n+++ b/builtin-init-db.c\n@@ -117,6 +117,8 @@ static void copy_templates(const char *template_dir)\n \t\ttemplate_dir = getenv(TEMPLATE_DIR_ENVIRONMENT);\n \tif (!template_dir)\n \t\ttemplate_dir = system_path(DEFAULT_GIT_TEMPLATE_DIR);\n+\tif (!template_dir[0])\n+\t\treturn;\n \tstrcpy(template_path, template_dir);\n \ttemplate_len = strlen(template_path);\n \tif (template_path[template_len-1] != '/') {\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex 2a38d98..620da5b 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -141,4 +141,30 @@ test_expect_success 'reinit' '\n \ttest_cmp again/empty again/err2\n '\n \n+test_expect_success 'init with --template' '\n+\tmkdir template-source &&\n+\techo content >template-source/file &&\n+\t(\n+\t\tmkdir template-custom &&\n+\t\tcd template-custom &&\n+\t\tgit init --template=../template-source\n+\t) &&\n+\ttest_cmp template-source/file template-custom/.git/file\n+'\n+\n+test_expect_success 'init with --template (blank)' '\n+\t(\n+\t\tmkdir template-plain &&\n+\t\tcd template-plain &&\n+\t\tgit init\n+\t) &&\n+\ttest -f template-plain/.git/info/exclude &&\n+\t(\n+\t\tmkdir template-blank &&\n+\t\tcd template-blank &&\n+\t\tgit init --template=\n+\t) &&\n+\t! test -f template-blank/.git/info/exclude\n+'\n+\n test_done\n-- \n1.6.0.rc1.155.gd3310\n"},{"id":"85247","messageId":"7vljzmv8mm.fsf@gitster.siamese.dyndns.org","threadId":"14644","inReplyTo":"20080728060204.GC7294@sigill.intra.peff.net","subject":"Re: [PATCH] init: handle empty \"template\" parameter, was Re: fetch refspec foo/* matches foo*","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-28T06:13:37Z","receivedAt":"2008-07-28T06:13:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks, it seems that I keep forgetting to apply this patch after saying\nto myself \"Heh, it obviously is the correct thing to do\".\n"},{"id":"85248","messageId":"20080728061600.GA10885@sigill.intra.peff.net","threadId":"14644","inReplyTo":"7vljzmv8mm.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] init: handle empty \"template\" parameter, was Re: fetch refspec foo/* matches foo*","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-07-28T06:16:00Z","receivedAt":"2008-07-28T06:16:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jul 27, 2008 at 11:13:37PM -0700, Junio C Hamano wrote:\n\n> Thanks, it seems that I keep forgetting to apply this patch after saying\n> to myself \"Heh, it obviously is the correct thing to do\".\n\nThat is why you have other developers to nag you. ;)\n\n-Peff\n"}]}