{"thread":{"id":"35414","subject":"[PATCH] subtree: fix argument validation in add/pull/push","startedAt":"2013-11-27T18:34:09Z","lastAt":"2013-11-27T20:30:51Z","messageCount":3,"participants":["Anthony Baire","Thomas Rast"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"231200","messageId":"1385577249-29269-1-git-send-email-Anthony.Baire@irisa.fr","threadId":"35414","inReplyTo":null,"subject":"[PATCH] subtree: fix argument validation in add/pull/push","fromName":"Anthony Baire","fromEmail":"anthony.baire@irisa.fr","sentAt":"2013-11-27T18:34:09Z","receivedAt":"2013-11-27T18:34:09Z","isPatch":true,"sender":{"key":"anthony.baire@irisa.fr","avatar":"https://avatars.githubusercontent.com/u/6051986?v=4"},"body":"When working with a remote repository add/pull/push do not accept a\n as parameter but just a . They should accept any\nwell-formatted ref name.\n\nThis patch:\n - relaxes the check the argument in \"git subtree add \"\n (previous code would not accept a ref name that does not exist\n locally too, new code only ensures that the ref is well formatted)\n\n - add the same check in \"git subtree pull/push\" + check the number of\n parameters\n\n - update the doc to use instead of \n\nSigned-off-by: Anthony Baire \n---\n contrib/subtree/git-subtree.sh | 22 ++++++++++++++++------\n contrib/subtree/git-subtree.txt | 14 +++++++-------\n 2 files changed, 23 insertions(+), 13 deletions(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex 7d7af03..dc59a91 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -9,10 +9,10 @@ if [ $# -eq 0 ]; then\n fi\n OPTS_SPEC=\"\\\n git subtree add --prefix= \n-git subtree add --prefix= \n+git subtree add --prefix= \n git subtree merge --prefix= \n-git subtree pull --prefix= \n-git subtree push --prefix= \n+git subtree pull --prefix= \n+git subtree push --prefix= \n git subtree split --prefix= \n --\n h,help show the help\n@@ -489,6 +489,12 @@ ensure_clean()\n \tfi\n }\n \n+ensure_valid_ref_format()\n+{\n+\tgit check-ref-format \"refs/heads/$1\" ||\n+\t die \"'$1' does not look like a ref\"\n+}\n+\n cmd_add()\n {\n \tif [ -e \"$dir\" ]; then\n@@ -508,8 +514,7 @@ cmd_add()\n \t # specified directory. Allowing a refspec might be\n \t # misleading because we won't do anything with any other\n \t # branches fetched via the refspec.\n-\t git rev-parse -q --verify \"$2^{commit}\" >/dev/null ||\n-\t die \"'$2' does not refer to a commit\"\n+\t ensure_valid_ref_format \"$2\"\n \n \t \"cmd_add_repository\" \"$@\"\n \telse\n@@ -699,7 +704,11 @@ cmd_merge()\n \n cmd_pull()\n {\n+\tif [ $# -ne 2 ]; then\n+\t die \"You must provide \"\n+\tfi\n \tensure_clean\n+\tensure_valid_ref_format \"$2\"\n \tgit fetch \"$@\" || exit $?\n \trevs=FETCH_HEAD\n \tset -- $revs\n@@ -709,8 +718,9 @@ cmd_pull()\n cmd_push()\n {\n \tif [ $# -ne 2 ]; then\n-\t die \"You must provide \"\n+\t die \"You must provide \"\n \tfi\n+\tensure_valid_ref_format \"$2\"\n \tif [ -e \"$dir\" ]; then\n \t repository=$1\n \t refspec=$2\ndiff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt\nindex e0957ee..02669b1 100644\n--- a/contrib/subtree/git-subtree.txt\n+++ b/contrib/subtree/git-subtree.txt\n@@ -9,10 +9,10 @@ git-subtree - Merge subtrees together and split repository into subtrees\n SYNOPSIS\n --------\n [verse]\n-'git subtree' add -P \n-'git subtree' add -P \n-'git subtree' pull -P \n-'git subtree' push -P \n+'git subtree' add -P \n+'git subtree' add -P \n+'git subtree' pull -P \n+'git subtree' push -P \n 'git subtree' merge -P \n 'git subtree' split -P [OPTIONS] []\n \n@@ -68,7 +68,7 @@ COMMANDS\n --------\n add::\n \tCreate the subtree by importing its contents\n-\tfrom the given or and remote .\n+\tfrom the given or and remote .\n \tA new commit is created\tautomatically, joining the imported\n \tproject's history with your own. With '--squash', imports\n \tonly a single commit from the subproject, rather than its\n@@ -90,13 +90,13 @@ merge::\n \t\n pull::\n \tExactly like 'merge', but parallels 'git pull' in that\n-\tit fetches the given commit from the specified remote\n+\tit fetches the given ref from the specified remote\n \trepository.\n \t\n push::\n \tDoes a 'split' (see below) using the supplied\n \tand then does a 'git push' to push the result to the \n-\trepository and refspec. This can be used to push your\n+\trepository and ref. This can be used to push your\n \tsubtree to different branches of the remote repository.\n \n split::\n-- \n1.7.10.4\n"},{"id":"231207","messageId":"87siuhfy9q.fsf@thomasrast.ch","threadId":"35414","inReplyTo":"1385577249-29269-1-git-send-email-Anthony.Baire@irisa.fr","subject":"Re: [PATCH] subtree: fix argument validation in add/pull/push","fromName":"Thomas Rast","fromEmail":"tr@thomasrast.ch","sentAt":"2013-11-27T19:19:29Z","receivedAt":"2013-11-27T19:19:29Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Anthony Baire writes:\n\n> When working with a remote repository add/pull/push do not accept a\n> as parameter but just a . They should accept any\n> well-formatted ref name.\n[...]\n> - update the doc to use instead of \n[...]\n> OPTS_SPEC=\"\\\n> git subtree add --prefix= \n> -git subtree add --prefix= \n> +git subtree add --prefix= \n> git subtree merge --prefix= \n> -git subtree pull --prefix= \n> -git subtree push --prefix= \n> +git subtree pull --prefix= \n> +git subtree push --prefix= \n> git subtree split --prefix= \n[...]\n> @@ -68,7 +68,7 @@ COMMANDS\n> --------\n> add::\n> \tCreate the subtree by importing its contents\n> -\tfrom the given or and remote .\n> +\tfrom the given or and remote .\n\nAFAICS you are changing refspec->commit in the manpage, but commit->ref\nin the usage message for 'subtree add'? How does this line up?\n\n-- \nThomas Rast\ntr@thomasrast.ch\n"},{"id":"231211","messageId":"5296567B.5080704@irisa.fr","threadId":"35414","inReplyTo":"87siuhfy9q.fsf@thomasrast.ch","subject":"Re: [PATCH] subtree: fix argument validation in add/pull/push","fromName":"Anthony Baire","fromEmail":"anthony.baire@irisa.fr","sentAt":"2013-11-27T20:30:51Z","receivedAt":"2013-11-27T20:30:51Z","isPatch":true,"sender":{"key":"anthony.baire@irisa.fr","avatar":"https://avatars.githubusercontent.com/u/6051986?v=4"},"body":"On 27/11/2013 20:19, Thomas Rast wrote:\n> Anthony Baire writes:\n>\n>> When working with a remote repository add/pull/push do not accept a\n>> as parameter but just a . They should accept any\n>> well-formatted ref name.\n> [...]\n>> - update the doc to use instead of \n> [...]\n>> OPTS_SPEC=\"\\\n>> git subtree add --prefix= \n>> -git subtree add --prefix= \n>> +git subtree add --prefix= \n>> git subtree merge --prefix= \n>> -git subtree pull --prefix= \n>> -git subtree push --prefix= \n>> +git subtree pull --prefix= \n>> +git subtree push --prefix= \n>> git subtree split --prefix= \n> [...]\n>> @@ -68,7 +68,7 @@ COMMANDS\n>> --------\n>> add::\n>> \tCreate the subtree by importing its contents\n>> -\tfrom the given or and remote .\n>> +\tfrom the given or and remote .\n> AFAICS you are changing refspec->commit in the manpage, but commit->ref\n> in the usage message for 'subtree add'? How does this line up?\n>\n'git subtree add' accepts a commit when working with the local \nrepository and a ref when working with a remote repository:\n\n\tgit subtree add --prefix= \n\tgit subtree add --prefix= \n\nbut the manpage was no in sync with the code. I fixed it for the local \ncase too.\nAnthony\n"}]}