{"thread":{"id":"41482","subject":"[PATCH] contrib/subtree: add repo url to commit messages","startedAt":"2016-02-23T10:25:59Z","lastAt":"2016-05-31T12:19:20Z","messageCount":7,"participants":["Mathias Nyman","Eric Sunshine","David A. Greene"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"278991","messageId":"20160223102559.GA18668@iki.fi","threadId":"41482","inReplyTo":null,"subject":"[PATCH] contrib/subtree: add repo url to commit messages","fromName":"Mathias Nyman","fromEmail":"mathias.nyman@iki.fi","sentAt":"2016-02-23T10:25:59Z","receivedAt":"2016-02-23T10:25:59Z","isPatch":true,"sender":{"key":"mathias.nyman@iki.fi","avatar":null},"body":"For recalling where a subtree came from; git-subtree operations 'add'\nand 'pull', when called with the <repository> parameter add this to the\ncommit message:\n    git-subtree-repo: <repo_url>\n\nOther operations that don't have the <repository> information, like\n'merge' and 'add' without <repository>, are unchanged. Users with such a\nworkflow will continue to be on their own with the --message parameter,\nif they'd like to record where the subtree came from.\n\nSigned-off-by: Mathias Nyman <mathias.nyman@iki.fi>\nBased-on-patch-by: Nicola Paolucci <npaolucci@atlassian.com>\n---\n contrib/subtree/git-subtree.sh | 73 ++++++++++++++++++++++++++++--------------\n 1 file changed, 49 insertions(+), 24 deletions(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex 7a39b30..7cf73c0 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -335,18 +335,21 @@ add_msg()\n \tdir=\"$1\"\n \tlatest_old=\"$2\"\n \tlatest_new=\"$3\"\n+\trepo=\"$4\" # optional\n \tif [ -n \"$message\" ]; then\n \t\tcommit_message=\"$message\"\n \telse\n \t\tcommit_message=\"Add '$dir/' from commit '$latest_new'\"\n \tfi\n-\tcat <<-EOF\n-\t\t$commit_message\n-\t\t\n-\t\tgit-subtree-dir: $dir\n-\t\tgit-subtree-mainline: $latest_old\n-\t\tgit-subtree-split: $latest_new\n-\tEOF\n+\techo $commit_message\n+\techo\n+\techo git-subtree-dir: $dir\n+\techo git-subtree-mainline: $latest_old\n+\techo git-subtree-split: $latest_new\n+\tif [ -n \"$repo\" ]; then\n+\t\trepo_url=$(get_repository_url \"$repo\")\n+\t\techo \"git-subtree-repo: $repo_url\"\n+\tfi\n }\n \n add_squashed_msg()\n@@ -382,8 +385,9 @@ squash_msg()\n \tdir=\"$1\"\n \toldsub=\"$2\"\n \tnewsub=\"$3\"\n+\trepo=\"$4\" # optional\n \tnewsub_short=$(git rev-parse --short \"$newsub\")\n-\t\n+\n \tif [ -n \"$oldsub\" ]; then\n \t\toldsub_short=$(git rev-parse --short \"$oldsub\")\n \t\techo \"Squashed '$dir/' changes from $oldsub_short..$newsub_short\"\n@@ -397,6 +401,10 @@ squash_msg()\n \techo\n \techo \"git-subtree-dir: $dir\"\n \techo \"git-subtree-split: $newsub\"\n+\tif [ -n \"$repo\" ]; then\n+\t\trepo_url=$(get_repository_url \"$repo\")\n+\t\techo \"git-subtree-repo: $repo_url\"\n+\tfi\n }\n \n toptree_for_commit()\n@@ -440,12 +448,13 @@ new_squash_commit()\n \told=\"$1\"\n \toldsub=\"$2\"\n \tnewsub=\"$3\"\n+\trepo=\"$4\" # optional\n \ttree=$(toptree_for_commit $newsub) || exit $?\n \tif [ -n \"$old\" ]; then\n-\t\tsquash_msg \"$dir\" \"$oldsub\" \"$newsub\" | \n+\t\tsquash_msg \"$dir\" \"$oldsub\" \"$newsub\" \"$repo\" |\n \t\t\tgit commit-tree \"$tree\" -p \"$old\" || exit $?\n \telse\n-\t\tsquash_msg \"$dir\" \"\" \"$newsub\" |\n+\t\tsquash_msg \"$dir\" \"\" \"$newsub\" \"$repo\" |\n \t\t\tgit commit-tree \"$tree\" || exit $?\n \tfi\n }\n@@ -517,6 +526,16 @@ ensure_valid_ref_format()\n \t    die \"'$1' does not look like a ref\"\n }\n \n+get_repository_url()\n+{\n+\trepo=$1\n+\trepo_url=$(git config --get remote.$repo.url)\n+\tif [ -z \"$repo_url\" ]; then\n+\t\trepo_url=$repo\n+\tfi\n+\techo $repo_url\n+}\n+\n cmd_add()\n {\n \tif [ -e \"$dir\" ]; then\n@@ -548,19 +567,18 @@ cmd_add()\n cmd_add_repository()\n {\n \techo \"git fetch\" \"$@\"\n-\trepository=$1\n+\trepo=$1\n \trefspec=$2\n \tgit fetch \"$@\" || exit $?\n \trevs=FETCH_HEAD\n-\tset -- $revs\n+\tset -- $revs $repo\n \tcmd_add_commit \"$@\"\n }\n \n cmd_add_commit()\n {\n-\trevs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n-\tset -- $revs\n-\trev=\"$1\"\n+\trev=$(git rev-parse $default --revs-only \"$1\") || exit $?\n+\trepo=\"$2\" # optional\n \t\n \tdebug \"Adding $dir as '$rev'...\"\n \tgit read-tree --prefix=\"$dir\" $rev || exit $?\n@@ -575,12 +593,12 @@ cmd_add_commit()\n \tfi\n \t\n \tif [ -n \"$squash\" ]; then\n-\t\trev=$(new_squash_commit \"\" \"\" \"$rev\") || exit $?\n+\t\trev=$(new_squash_commit \"\" \"\" \"$rev\" \"$repo\") || exit $?\n \t\tcommit=$(add_squashed_msg \"$rev\" \"$dir\" |\n \t\t\t git commit-tree $tree $headp -p \"$rev\") || exit $?\n \telse\n \t\trevp=$(peel_committish \"$rev\") &&\n-\t\tcommit=$(add_msg \"$dir\" \"$headrev\" \"$rev\" |\n+\t\tcommit=$(add_msg \"$dir\" \"$headrev\" \"$rev\" \"$repo\" |\n \t\t\t git commit-tree $tree $headp -p \"$revp\") || exit $?\n \tfi\n \tgit reset \"$commit\" || exit $?\n@@ -609,7 +627,8 @@ cmd_split()\n \telse\n \t\tunrevs=\"$(find_existing_splits \"$dir\" \"$revs\")\"\n \tfi\n-\t\n+e\n+\trev=\"$1\"\n \t# We can't restrict rev-list to only $dir here, because some of our\n \t# parents have the $dir contents the root, and those won't match.\n \t# (and rev-list --follow doesn't seem to solve this)\n@@ -683,15 +702,20 @@ cmd_split()\n \n cmd_merge()\n {\n-\trevs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n+\trevs=$(git rev-parse $default --revs-only \"$1\") || exit $?\n \tensure_clean\n-\t\n \tset -- $revs\n \tif [ $# -ne 1 ]; then\n \t\tdie \"You must provide exactly one revision.  Got: '$revs'\"\n \tfi\n+\tdo_merge \"$@\"\n+}\n+\n+do_merge()\n+{\n \trev=\"$1\"\n-\t\n+\trepo=\"$2\" # optional\n+\n \tif [ -n \"$squash\" ]; then\n \t\tfirst_split=\"$(find_latest_squash \"$dir\")\"\n \t\tif [ -z \"$first_split\" ]; then\n@@ -704,7 +728,7 @@ cmd_merge()\n \t\t\tsay \"Subtree is already at commit $rev.\"\n \t\t\texit 0\n \t\tfi\n-\t\tnew=$(new_squash_commit \"$old\" \"$sub\" \"$rev\") || exit $?\n+\t\tnew=$(new_squash_commit \"$old\" \"$sub\" \"$rev\" \"$repo\") || exit $?\n \t\tdebug \"New squash commit: $new\"\n \t\trev=\"$new\"\n \tfi\n@@ -730,12 +754,13 @@ cmd_pull()\n \tif [ $# -ne 2 ]; then\n \t    die \"You must provide <repository> <ref>\"\n \tfi\n+\trepo=$1\n \tensure_clean\n \tensure_valid_ref_format \"$2\"\n \tgit fetch \"$@\" || exit $?\n \trevs=FETCH_HEAD\n-\tset -- $revs\n-\tcmd_merge \"$@\"\n+\tset -- $revs $repo\n+\tdo_merge \"$@\"\n }\n \n cmd_push()\n-- \n2.7.1\n"},{"id":"279393","messageId":"CAPig+cSwQmbvZYbk3T-XYDfMYaMdJ=bFbDwEUtaR121pBrYJOQ@mail.gmail.com","threadId":"41482","inReplyTo":"20160223102559.GA18668@iki.fi","subject":"Re: [PATCH] contrib/subtree: add repo url to commit messages","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-02-25T22:23:16Z","receivedAt":"2016-02-25T22:23:16Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Feb 23, 2016 at 5:25 AM, Mathias Nyman <mathias.nyman@iki.fi> wrote:\n> For recalling where a subtree came from; git-subtree operations 'add'\n> and 'pull', when called with the <repository> parameter add this to the\n> commit message:\n>     git-subtree-repo: <repo_url>\n>\n> Other operations that don't have the <repository> information, like\n> 'merge' and 'add' without <repository>, are unchanged. Users with such a\n> workflow will continue to be on their own with the --message parameter,\n> if they'd like to record where the subtree came from.\n\nI'm not a subtree user, so review comments below are superficial...\n\n> Signed-off-by: Mathias Nyman <mathias.nyman@iki.fi>\n> Based-on-patch-by: Nicola Paolucci <npaolucci@atlassian.com>\n> ---\n> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\n> @@ -335,18 +335,21 @@ add_msg()\n>         dir=\"$1\"\n>         latest_old=\"$2\"\n>         latest_new=\"$3\"\n> +       repo=\"$4\" # optional\n>         if [ -n \"$message\" ]; then\n>                 commit_message=\"$message\"\n>         else\n>                 commit_message=\"Add '$dir/' from commit '$latest_new'\"\n>         fi\n> -       cat <<-EOF\n> -               $commit_message\n> -\n> -               git-subtree-dir: $dir\n> -               git-subtree-mainline: $latest_old\n> -               git-subtree-split: $latest_new\n> -       EOF\n> +       echo $commit_message\n> +       echo\n> +       echo git-subtree-dir: $dir\n> +       echo git-subtree-mainline: $latest_old\n> +       echo git-subtree-split: $latest_new\n\nIt's not clear why this code was changed to use a series of echo's in\nplace of the single cat. Although the net result is the same, this\nappears to be mere code churn. If your intention was to make it\nsimilar to how squash_msg() uses a series of echo's, then that might\nmake sense, however, rejoin_msg() uses the same single 'cat' as\nadd_msg(), so inconsistency remains. Thus, it's not clear what the\nintention is.\n\n> +       if [ -n \"$repo\" ]; then\n> +               repo_url=$(get_repository_url \"$repo\")\n> +               echo \"git-subtree-repo: $repo_url\"\n> +       fi\n>  }\n>\n>  add_squashed_msg()\n> @@ -382,8 +385,9 @@ squash_msg()\n>         dir=\"$1\"\n>         oldsub=\"$2\"\n>         newsub=\"$3\"\n> +       repo=\"$4\" # optional\n>         newsub_short=$(git rev-parse --short \"$newsub\")\n> -\n> +\n\nOkay, this change is removing an unnecessary tab. Perhaps the commit\nmessage can say that the patch fixes a few whitespace inconsistencies\nwhile touching nearby code.\n\nMore below...\n\n>         if [ -n \"$oldsub\" ]; then\n>                 oldsub_short=$(git rev-parse --short \"$oldsub\")\n>                 echo \"Squashed '$dir/' changes from $oldsub_short..$newsub_short\"\n> @@ -397,6 +401,10 @@ squash_msg()\n>         echo\n>         echo \"git-subtree-dir: $dir\"\n>         echo \"git-subtree-split: $newsub\"\n> +       if [ -n \"$repo\" ]; then\n> +               repo_url=$(get_repository_url \"$repo\")\n> +               echo \"git-subtree-repo: $repo_url\"\n> +       fi\n>  }\n>\n>  toptree_for_commit()\n> @@ -440,12 +448,13 @@ new_squash_commit()\n>         old=\"$1\"\n>         oldsub=\"$2\"\n>         newsub=\"$3\"\n> +       repo=\"$4\" # optional\n>         tree=$(toptree_for_commit $newsub) || exit $?\n>         if [ -n \"$old\" ]; then\n> -               squash_msg \"$dir\" \"$oldsub\" \"$newsub\" |\n> +               squash_msg \"$dir\" \"$oldsub\" \"$newsub\" \"$repo\" |\n>                         git commit-tree \"$tree\" -p \"$old\" || exit $?\n>         else\n> -               squash_msg \"$dir\" \"\" \"$newsub\" |\n> +               squash_msg \"$dir\" \"\" \"$newsub\" \"$repo\" |\n>                         git commit-tree \"$tree\" || exit $?\n>         fi\n>  }\n> @@ -517,6 +526,16 @@ ensure_valid_ref_format()\n>             die \"'$1' does not look like a ref\"\n>  }\n>\n> +get_repository_url()\n> +{\n> +       repo=$1\n> +       repo_url=$(git config --get remote.$repo.url)\n> +       if [ -z \"$repo_url\" ]; then\n> +               repo_url=$repo\n> +       fi\n> +       echo $repo_url\n> +}\n> +\n>  cmd_add()\n>  {\n>         if [ -e \"$dir\" ]; then\n> @@ -548,19 +567,18 @@ cmd_add()\n>  cmd_add_repository()\n>  {\n>         echo \"git fetch\" \"$@\"\n> -       repository=$1\n> +       repo=$1\n\nHmm, so 'repository' was present already but unused in this function,\nand now you're using it. I suppose you renamed it 'repo' for\nconsistency with other 'repo' variable the patch introduces elsewhere.\n\n>         refspec=$2\n>         git fetch \"$@\" || exit $?\n>         revs=FETCH_HEAD\n> -       set -- $revs\n> +       set -- $revs $repo\n>         cmd_add_commit \"$@\"\n\nThe original code intentionally allowed passing a set of revs to\ncmd_add_commit(), however, you've repurposed it (below) so that it\naccepts one rev and an (optional) repo. Therefore, there doesn't seem\nto be much value anymore to using \"set --\" when you could just do:\n\n    cmd_add_commit $revs $repo\n\nOr am I missing something obvious?\n\n(Of course, the original code unconditionally used \"set --\" even while\nsetting 'revs' to hardcoded FETCH_HEAD, so I suppose this isn't any\nworse, but still...)\n\n>  }\n>\n>  cmd_add_commit()\n>  {\n> -       revs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n> -       set -- $revs\n> -       rev=\"$1\"\n> +       rev=$(git rev-parse $default --revs-only \"$1\") || exit $?\n\nAn audit of call callers of cmd_add_commit() shows that it was only\never invoked with a single rev, so this change to make it accept a\nsingle rev plus an optional repo seems safe. However, I wonder if it\nwould make sense to keep the more flexible interface (in case future\ncallers might need the functionality) by passing repo in as the first\nargument (using an empty string, for instance, for the optional bit)\nand then taking all subsequent arguments as revs, but perhaps that's\noverkill since it doesn't seem to care about revs other than the first\none.\n\ncmd_merge() still goes through the \"set --\" dance which you've removed\nhere, even though an audit of all its callers pass in only a single\nrev, so that seems inconsistent...\n\n> +       repo=\"$2\" # optional\n>\n>         debug \"Adding $dir as '$rev'...\"\n>         git read-tree --prefix=\"$dir\" $rev || exit $?\n> @@ -575,12 +593,12 @@ cmd_add_commit()\n>         fi\n>\n>         if [ -n \"$squash\" ]; then\n> -               rev=$(new_squash_commit \"\" \"\" \"$rev\") || exit $?\n> +               rev=$(new_squash_commit \"\" \"\" \"$rev\" \"$repo\") || exit $?\n>                 commit=$(add_squashed_msg \"$rev\" \"$dir\" |\n>                          git commit-tree $tree $headp -p \"$rev\") || exit $?\n>         else\n>                 revp=$(peel_committish \"$rev\") &&\n> -               commit=$(add_msg \"$dir\" \"$headrev\" \"$rev\" |\n> +               commit=$(add_msg \"$dir\" \"$headrev\" \"$rev\" \"$repo\" |\n>                          git commit-tree $tree $headp -p \"$revp\") || exit $?\n>         fi\n>         git reset \"$commit\" || exit $?\n> @@ -609,7 +627,8 @@ cmd_split()\n>         else\n>                 unrevs=\"$(find_existing_splits \"$dir\" \"$revs\")\"\n>         fi\n> -\n> +e\n\nSo, you're replacing a line containing a single tab with a line\ncontaining a single 'e'. Seems fishy.\n\n> +       rev=\"$1\"\n>         # We can't restrict rev-list to only $dir here, because some of our\n>         # parents have the $dir contents the root, and those won't match.\n>         # (and rev-list --follow doesn't seem to solve this)\n> @@ -683,15 +702,20 @@ cmd_split()\n>\n>  cmd_merge()\n>  {\n> -       revs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n> +       revs=$(git rev-parse $default --revs-only \"$1\") || exit $?\n\nWhy is this variable still named 'revs' (plural) since you're only\npassing in $1 now rather than $@?\n\n>         ensure_clean\n> -\n>         set -- $revs\n\nDo you still need this \"set --\" or am I missing something?\n\n>         if [ $# -ne 1 ]; then\n>                 die \"You must provide exactly one revision.  Got: '$revs'\"\n>         fi\n\nDitto with the conditional, considering that you only ever look at $1\nnow rather than $@.\n\n> +       do_merge \"$@\"\n> +}\n> +\n> +do_merge()\n> +{\n>         rev=\"$1\"\n> -\n> +       repo=\"$2\" # optional\n> +\n>         if [ -n \"$squash\" ]; then\n>                 first_split=\"$(find_latest_squash \"$dir\")\"\n>                 if [ -z \"$first_split\" ]; then\n> @@ -704,7 +728,7 @@ cmd_merge()\n>                         say \"Subtree is already at commit $rev.\"\n>                         exit 0\n>                 fi\n> -               new=$(new_squash_commit \"$old\" \"$sub\" \"$rev\") || exit $?\n> +               new=$(new_squash_commit \"$old\" \"$sub\" \"$rev\" \"$repo\") || exit $?\n>                 debug \"New squash commit: $new\"\n>                 rev=\"$new\"\n>         fi\n> @@ -730,12 +754,13 @@ cmd_pull()\n>         if [ $# -ne 2 ]; then\n>             die \"You must provide <repository> <ref>\"\n>         fi\n> +       repo=$1\n>         ensure_clean\n>         ensure_valid_ref_format \"$2\"\n>         git fetch \"$@\" || exit $?\n>         revs=FETCH_HEAD\n> -       set -- $revs\n> -       cmd_merge \"$@\"\n> +       set -- $revs $repo\n> +       do_merge \"$@\"\n\nSame question as above. Is \"set --\" still buying you anything over just:\n\n    do_merge $revs $repo\n\n?\n\n>  }\n>\n>  cmd_push()\n> --\n> 2.7.1\n"},{"id":"279499","messageId":"20160226082828.GA5960@iki.fi","threadId":"41482","inReplyTo":"CAPig+cSwQmbvZYbk3T-XYDfMYaMdJ=bFbDwEUtaR121pBrYJOQ@mail.gmail.com","subject":"Re: [PATCH] contrib/subtree: add repo url to commit messages","fromName":"Mathias Nyman","fromEmail":"m.nyman@iki.fi","sentAt":"2016-02-26T08:28:28Z","receivedAt":"2016-02-26T08:28:28Z","isPatch":true,"sender":{"key":"m.nyman@iki.fi","avatar":null},"body":"On 2016-02-25 17:23-0500, Eric Sunshine wrote:\n>On Tue, Feb 23, 2016 at 5:25 AM, Mathias Nyman <mathias.nyman@iki.fi> wrote:\n>> For recalling where a subtree came from; git-subtree operations 'add'\n>> and 'pull', when called with the <repository> parameter add this to the\n>> commit message:\n>>     git-subtree-repo: <repo_url>\n>>\n>> Other operations that don't have the <repository> information, like\n>> 'merge' and 'add' without <repository>, are unchanged. Users with such a\n>> workflow will continue to be on their own with the --message parameter,\n>> if they'd like to record where the subtree came from.\n>\n>I'm not a subtree user, so review comments below are superficial...\n>\n\nThank you for reviewing; will send PATCH v2 shortly.\n\n>> Signed-off-by: Mathias Nyman <mathias.nyman@iki.fi>\n>> Based-on-patch-by: Nicola Paolucci <npaolucci@atlassian.com>\n>> ---\n>> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\n>> @@ -335,18 +335,21 @@ add_msg()\n>>         dir=\"$1\"\n>>         latest_old=\"$2\"\n>>         latest_new=\"$3\"\n>> +       repo=\"$4\" # optional\n>>         if [ -n \"$message\" ]; then\n>>                 commit_message=\"$message\"\n>>         else\n>>                 commit_message=\"Add '$dir/' from commit '$latest_new'\"\n>>         fi\n>> -       cat <<-EOF\n>> -               $commit_message\n>> -\n>> -               git-subtree-dir: $dir\n>> -               git-subtree-mainline: $latest_old\n>> -               git-subtree-split: $latest_new\n>> -       EOF\n>> +       echo $commit_message\n>> +       echo\n>> +       echo git-subtree-dir: $dir\n>> +       echo git-subtree-mainline: $latest_old\n>> +       echo git-subtree-split: $latest_new\n>\n>It's not clear why this code was changed to use a series of echo's in\n>place of the single cat. Although the net result is the same, this\n>appears to be mere code churn. If your intention was to make it\n>similar to how squash_msg() uses a series of echo's, then that might\n>make sense, however, rejoin_msg() uses the same single 'cat' as\n>add_msg(), so inconsistency remains. Thus, it's not clear what the\n>intention is.\n>\n\nUsing a mixutre of heredoc and echo felt messy. But I'll change it\nback to heredoc here, and through out the commit aim for near-zero\nrefactoring.\n\n>> +       if [ -n \"$repo\" ]; then\n>> +               repo_url=$(get_repository_url \"$repo\")\n>> +               echo \"git-subtree-repo: $repo_url\"\n>> +       fi\n>>  }\n>>\n>>  add_squashed_msg()\n>> @@ -382,8 +385,9 @@ squash_msg()\n>>         dir=\"$1\"\n>>         oldsub=\"$2\"\n>>         newsub=\"$3\"\n>> +       repo=\"$4\" # optional\n>>         newsub_short=$(git rev-parse --short \"$newsub\")\n>> -\n>> +\n>\n>Okay, this change is removing an unnecessary tab. Perhaps the commit\n>message can say that the patch fixes a few whitespace inconsistencies\n>while touching nearby code.\n>\n>More below...\n>\n\nWill undo the whitespace fixing.\n\n>>         if [ -n \"$oldsub\" ]; then\n>>                 oldsub_short=$(git rev-parse --short \"$oldsub\")\n>>                 echo \"Squashed '$dir/' changes from $oldsub_short..$newsub_short\"\n>> @@ -397,6 +401,10 @@ squash_msg()\n>>         echo\n>>         echo \"git-subtree-dir: $dir\"\n>>         echo \"git-subtree-split: $newsub\"\n>> +       if [ -n \"$repo\" ]; then\n>> +               repo_url=$(get_repository_url \"$repo\")\n>> +               echo \"git-subtree-repo: $repo_url\"\n>> +       fi\n>>  }\n>>\n>>  toptree_for_commit()\n>> @@ -440,12 +448,13 @@ new_squash_commit()\n>>         old=\"$1\"\n>>         oldsub=\"$2\"\n>>         newsub=\"$3\"\n>> +       repo=\"$4\" # optional\n>>         tree=$(toptree_for_commit $newsub) || exit $?\n>>         if [ -n \"$old\" ]; then\n>> -               squash_msg \"$dir\" \"$oldsub\" \"$newsub\" |\n>> +               squash_msg \"$dir\" \"$oldsub\" \"$newsub\" \"$repo\" |\n>>                         git commit-tree \"$tree\" -p \"$old\" || exit $?\n>>         else\n>> -               squash_msg \"$dir\" \"\" \"$newsub\" |\n>> +               squash_msg \"$dir\" \"\" \"$newsub\" \"$repo\" |\n>>                         git commit-tree \"$tree\" || exit $?\n>>         fi\n>>  }\n>> @@ -517,6 +526,16 @@ ensure_valid_ref_format()\n>>             die \"'$1' does not look like a ref\"\n>>  }\n>>\n>> +get_repository_url()\n>> +{\n>> +       repo=$1\n>> +       repo_url=$(git config --get remote.$repo.url)\n>> +       if [ -z \"$repo_url\" ]; then\n>> +               repo_url=$repo\n>> +       fi\n>> +       echo $repo_url\n>> +}\n>> +\n>>  cmd_add()\n>>  {\n>>         if [ -e \"$dir\" ]; then\n>> @@ -548,19 +567,18 @@ cmd_add()\n>>  cmd_add_repository()\n>>  {\n>>         echo \"git fetch\" \"$@\"\n>> -       repository=$1\n>> +       repo=$1\n>\n>Hmm, so 'repository' was present already but unused in this function,\n>and now you're using it. I suppose you renamed it 'repo' for\n>consistency with other 'repo' variable the patch introduces elsewhere.\n>\n\nYes.\n\n>>         refspec=$2\n>>         git fetch \"$@\" || exit $?\n>>         revs=FETCH_HEAD\n>> -       set -- $revs\n>> +       set -- $revs $repo\n>>         cmd_add_commit \"$@\"\n>\n>The original code intentionally allowed passing a set of revs to\n>cmd_add_commit(), however, you've repurposed it (below) so that it\n>accepts one rev and an (optional) repo. Therefore, there doesn't seem\n>to be much value anymore to using \"set --\" when you could just do:\n>\n>    cmd_add_commit $revs $repo\n>\n>Or am I missing something obvious?\n>\n>(Of course, the original code unconditionally used \"set --\" even while\n>setting 'revs' to hardcoded FETCH_HEAD, so I suppose this isn't any\n>worse, but still...)\n>\n\nWill leave this as is; your suggestion would mean refactoring the 'set\n--' quirks, which I tried not to do.\n\n>>  }\n>>\n>>  cmd_add_commit()\n>>  {\n>> -       revs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n>> -       set -- $revs\n>> -       rev=\"$1\"\n>> +       rev=$(git rev-parse $default --revs-only \"$1\") || exit $?\n>\n>An audit of call callers of cmd_add_commit() shows that it was only\n>ever invoked with a single rev, so this change to make it accept a\n>single rev plus an optional repo seems safe. However, I wonder if it\n>would make sense to keep the more flexible interface (in case future\n>callers might need the functionality) by passing repo in as the first\n>argument (using an empty string, for instance, for the optional bit)\n>and then taking all subsequent arguments as revs, but perhaps that's\n>overkill since it doesn't seem to care about revs other than the first\n>one.\n>\n\nI think it makes sense to refactor the general 'set --' dance in\ngit-subtree.sh all together.\n\n>cmd_merge() still goes through the \"set --\" dance which you've removed\n>here, even though an audit of all its callers pass in only a single\n>rev, so that seems inconsistent...\n>\n\nI'll readd this 'set --' dance here for consistency.\n\n>> +       repo=\"$2\" # optional\n>>\n>>         debug \"Adding $dir as '$rev'...\"\n>>         git read-tree --prefix=\"$dir\" $rev || exit $?\n>> @@ -575,12 +593,12 @@ cmd_add_commit()\n>>         fi\n>>\n>>         if [ -n \"$squash\" ]; then\n>> -               rev=$(new_squash_commit \"\" \"\" \"$rev\") || exit $?\n>> +               rev=$(new_squash_commit \"\" \"\" \"$rev\" \"$repo\") || exit $?\n>>                 commit=$(add_squashed_msg \"$rev\" \"$dir\" |\n>>                          git commit-tree $tree $headp -p \"$rev\") || exit $?\n>>         else\n>>                 revp=$(peel_committish \"$rev\") &&\n>> -               commit=$(add_msg \"$dir\" \"$headrev\" \"$rev\" |\n>> +               commit=$(add_msg \"$dir\" \"$headrev\" \"$rev\" \"$repo\" |\n>>                          git commit-tree $tree $headp -p \"$revp\") || exit $?\n>>         fi\n>>         git reset \"$commit\" || exit $?\n>> @@ -609,7 +627,8 @@ cmd_split()\n>>         else\n>>                 unrevs=\"$(find_existing_splits \"$dir\" \"$revs\")\"\n>>         fi\n>> -\n>> +e\n>\n>So, you're replacing a line containing a single tab with a line\n>containing a single 'e'. Seems fishy.\n>\n\nGreat typo find!\n\n>> +       rev=\"$1\"\n>>         # We can't restrict rev-list to only $dir here, because some of our\n>>         # parents have the $dir contents the root, and those won't match.\n>>         # (and rev-list --follow doesn't seem to solve this)\n>> @@ -683,15 +702,20 @@ cmd_split()\n>>\n>>  cmd_merge()\n>>  {\n>> -       revs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n>> +       revs=$(git rev-parse $default --revs-only \"$1\") || exit $?\n>\n>Why is this variable still named 'revs' (plural) since you're only\n>passing in $1 now rather than $@?\n>\n\nBecause technically the result can still be more then one rev I guess.\nConsider 'git rev-parse HEAD~1..HEAD', which would return two hashes.\n\n>>         ensure_clean\n>> -\n>>         set -- $revs\n>\n>Do you still need this \"set --\" or am I missing something?\n>\n>>         if [ $# -ne 1 ]; then\n>>                 die \"You must provide exactly one revision.  Got: '$revs'\"\n>>         fi\n>\n>Ditto with the conditional, considering that you only ever look at $1\n>now rather than $@.\n>\n\nThis will handle the case where 'git rev-parse' caught more than one\nhash earlier.\n\n>> +       do_merge \"$@\"\n>> +}\n>> +\n>> +do_merge()\n>> +{\n>>         rev=\"$1\"\n>> -\n>> +       repo=\"$2\" # optional\n>> +\n>>         if [ -n \"$squash\" ]; then\n>>                 first_split=\"$(find_latest_squash \"$dir\")\"\n>>                 if [ -z \"$first_split\" ]; then\n>> @@ -704,7 +728,7 @@ cmd_merge()\n>>                         say \"Subtree is already at commit $rev.\"\n>>                         exit 0\n>>                 fi\n>> -               new=$(new_squash_commit \"$old\" \"$sub\" \"$rev\") || exit $?\n>> +               new=$(new_squash_commit \"$old\" \"$sub\" \"$rev\" \"$repo\") || exit $?\n>>                 debug \"New squash commit: $new\"\n>>                 rev=\"$new\"\n>>         fi\n>> @@ -730,12 +754,13 @@ cmd_pull()\n>>         if [ $# -ne 2 ]; then\n>>             die \"You must provide <repository> <ref>\"\n>>         fi\n>> +       repo=$1\n>>         ensure_clean\n>>         ensure_valid_ref_format \"$2\"\n>>         git fetch \"$@\" || exit $?\n>>         revs=FETCH_HEAD\n>> -       set -- $revs\n>> -       cmd_merge \"$@\"\n>> +       set -- $revs $repo\n>> +       do_merge \"$@\"\n>\n>Same question as above. Is \"set --\" still buying you anything over just:\n>\n>    do_merge $revs $repo\n>\n>?\n>\n\nNo. But I will not deviate from the function parameter passing method\n('set --') used throughout git-subtree.sh in this commit. I do think\nparameter passing in git-subtree.sh deserves a separate\nno-functional-changes refactoring commit though.\n\n>>  }\n>>\n>>  cmd_push()\n>> --\n>> 2.7.1\n"},{"id":"279559","messageId":"CAPig+cRTz_VYt-2q9y0+COhVCezMs-7Sm-v=jvhUxUSRhiN93g@mail.gmail.com","threadId":"41482","inReplyTo":"20160226082828.GA5960@iki.fi","subject":"Re: [PATCH] contrib/subtree: add repo url to commit messages","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-02-26T19:49:42Z","receivedAt":"2016-02-26T19:49:42Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Feb 26, 2016 at 3:28 AM, Mathias Nyman <m.nyman@iki.fi> wrote:\n> On 2016-02-25 17:23-0500, Eric Sunshine wrote:\n>> On Tue, Feb 23, 2016 at 5:25 AM, Mathias Nyman <mathias.nyman@iki.fi>\n>> wrote:\n>>> -       cat <<-EOF\n>>> -               $commit_message\n>>> -\n>>> -               git-subtree-dir: $dir\n>>> -               git-subtree-mainline: $latest_old\n>>> -               git-subtree-split: $latest_new\n>>> -       EOF\n>>> +       echo $commit_message\n>>> +       echo\n>>> +       echo git-subtree-dir: $dir\n>>> +       echo git-subtree-mainline: $latest_old\n>>> +       echo git-subtree-split: $latest_new\n>>\n>> It's not clear why this code was changed to use a series of echo's in\n>> place of the single cat. Although the net result is the same, this\n>> appears to be mere code churn. If your intention was to make it\n>> similar to how squash_msg() uses a series of echo's, then that might\n>> make sense, however, rejoin_msg() uses the same single 'cat' as\n>> add_msg(), so inconsistency remains. Thus, it's not clear what the\n>> intention is.\n>\n> Using a mixutre of heredoc and echo felt messy. But I'll change it\n> back to heredoc here, and through out the commit aim for near-zero\n> refactoring.\n\nAn alternative would be to have a preparatory patch which unifies the\nheredoc vs. echo issue across add_msg(), squash_msg(), rejoin_msg(),\nbut I wouldn't insist upon it (that's just more work for you). Leaving\nthis bit alone is a reasonable choice.\n\n>>> +       repo=\"$4\" # optional\n>>>         newsub_short=$(git rev-parse --short \"$newsub\")\n>>> -\n>>> +\n>>\n>>\n>> Okay, this change is removing an unnecessary tab. Perhaps the commit\n>> message can say that the patch fixes a few whitespace inconsistencies\n>> while touching nearby code.\n>\n> Will undo the whitespace fixing.\n\nOh, I wasn't insisting that you should undo the whitespace fix.\nTypically, you'd make such fixes in a preparatory cleanup patch, but\nsince there are only two cases here that you've fixed, it probably\nwouldn't hurt to retain them (if that's all there are in the file).\nThe reason I suggested mentioning the whitespace fixes in the commit\nmessage is to let the reviewer know that they weren't cases of you\naccidentally making unwanted whitespace changes (like inserting tabs\nrather than removing them). As it was, as a reviewer, I had to go\nthrough extra effort to determine whether you had made a fix or had\naccidentally botched something.\n\n>>>  cmd_merge()\n>>>  {\n>>> -       revs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n>>> +       revs=$(git rev-parse $default --revs-only \"$1\") || exit $?\n>>\n>> Why is this variable still named 'revs' (plural) since you're only\n>> passing in $1 now rather than $@?\n>\n> Because technically the result can still be more then one rev I guess.\n> Consider 'git rev-parse HEAD~1..HEAD', which would return two hashes.\n\nOkay, so I was missing something obvious.\n"},{"id":"280108","messageId":"20160303114222.GA9814@iki.fi","threadId":"41482","inReplyTo":"CAPig+cRTz_VYt-2q9y0+COhVCezMs-7Sm-v=jvhUxUSRhiN93g@mail.gmail.com","subject":"Re: [PATCH] contrib/subtree: add repo url to commit messages","fromName":"Mathias Nyman","fromEmail":"m.nyman@iki.fi","sentAt":"2016-03-03T11:42:22Z","receivedAt":"2016-03-03T11:42:22Z","isPatch":true,"sender":{"key":"m.nyman@iki.fi","avatar":null},"body":"On 2016-02-26 14:49-0500, Eric Sunshine wrote:\n>On Fri, Feb 26, 2016 at 3:28 AM, Mathias Nyman <m.nyman@iki.fi> wrote:\n>> On 2016-02-25 17:23-0500, Eric Sunshine wrote:\n>>> On Tue, Feb 23, 2016 at 5:25 AM, Mathias Nyman <mathias.nyman@iki.fi>\n>>> wrote:\n>>>> -       cat <<-EOF\n>>>> -               $commit_message\n>>>> -\n>>>> -               git-subtree-dir: $dir\n>>>> -               git-subtree-mainline: $latest_old\n>>>> -               git-subtree-split: $latest_new\n>>>> -       EOF\n>>>> +       echo $commit_message\n>>>> +       echo\n>>>> +       echo git-subtree-dir: $dir\n>>>> +       echo git-subtree-mainline: $latest_old\n>>>> +       echo git-subtree-split: $latest_new\n>>>\n>>> It's not clear why this code was changed to use a series of echo's in\n>>> place of the single cat. Although the net result is the same, this\n>>> appears to be mere code churn. If your intention was to make it\n>>> similar to how squash_msg() uses a series of echo's, then that might\n>>> make sense, however, rejoin_msg() uses the same single 'cat' as\n>>> add_msg(), so inconsistency remains. Thus, it's not clear what the\n>>> intention is.\n>>\n>> Using a mixutre of heredoc and echo felt messy. But I'll change it\n>> back to heredoc here, and through out the commit aim for near-zero\n>> refactoring.\n>\n>An alternative would be to have a preparatory patch which unifies the\n>heredoc vs. echo issue across add_msg(), squash_msg(), rejoin_msg(),\n>but I wouldn't insist upon it (that's just more work for you). Leaving\n>this bit alone is a reasonable choice.\n>\n>>>> +       repo=\"$4\" # optional\n>>>>         newsub_short=$(git rev-parse --short \"$newsub\")\n>>>> -\n>>>> +\n>>>\n>>>\n>>> Okay, this change is removing an unnecessary tab. Perhaps the commit\n>>> message can say that the patch fixes a few whitespace inconsistencies\n>>> while touching nearby code.\n>>\n>> Will undo the whitespace fixing.\n>\n>Oh, I wasn't insisting that you should undo the whitespace fix.\n>Typically, you'd make such fixes in a preparatory cleanup patch, but\n>since there are only two cases here that you've fixed, it probably\n>wouldn't hurt to retain them (if that's all there are in the file).\n>The reason I suggested mentioning the whitespace fixes in the commit\n>message is to let the reviewer know that they weren't cases of you\n>accidentally making unwanted whitespace changes (like inserting tabs\n>rather than removing them). As it was, as a reviewer, I had to go\n>through extra effort to determine whether you had made a fix or had\n>accidentally botched something.\n>\n\nNot fixing any whitespace inconsistencies felt like the consistent way to\ngo in this commit.\n\nWould be great to have some feedback on the core idea of this change,\nbefore spending more time on it. Anyone?\n\n>>>>  cmd_merge()\n>>>>  {\n>>>> -       revs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n>>>> +       revs=$(git rev-parse $default --revs-only \"$1\") || exit $?\n>>>\n>>> Why is this variable still named 'revs' (plural) since you're only\n>>> passing in $1 now rather than $@?\n>>\n>> Because technically the result can still be more then one rev I guess.\n>> Consider 'git rev-parse HEAD~1..HEAD', which would return two hashes.\n>\n>Okay, so I was missing something obvious.\n"},{"id":"287173","messageId":"87twhrowh0.fsf@waller.obbligato.org","threadId":"41482","inReplyTo":"20160223102559.GA18668@iki.fi","subject":"Re: [PATCH] contrib/subtree: add repo url to commit messages","fromName":"David A. Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-05-21T22:52:11Z","receivedAt":"2016-05-21T22:52:11Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"Mathias Nyman <mathias.nyman@iki.fi> writes:\n\n> For recalling where a subtree came from; git-subtree operations 'add'\n> and 'pull', when called with the <repository> parameter add this to the\n> commit message:\n>     git-subtree-repo: <repo_url>\n\nI am sorry it tooks a couple of months to respond.  I am finally coming\nup for air at work.\n\nWhat is the future intent of this?  I've toyed with the idea of adding\nsomething like this either as commit message metadata or in .gitconfig\nbut every time I get ready to pull the trigger, I question what it will\nbe used for.\n\nHaving been using git-subtree in anger for a couple of years now, I\nfrequently pull subtree updates from multiple sources, so noting a\nparticular repository is not only mostly meaningless, it may actually be\nmisleading in that a quick perusal of the logs may lead one to think\ncommits were draw from fewer places than they actually were.\n\nI don't think it would be a good idea, for example, to have git-subtree\nuse this information to auto-guess from where to pull future commits.\nAgain, I think that would be misleading behavior.\n\n                         -David\n\n> Other operations that don't have the <repository> information, like\n> 'merge' and 'add' without <repository>, are unchanged. Users with such a\n> workflow will continue to be on their own with the --message parameter,\n> if they'd like to record where the subtree came from.\n>\n> Signed-off-by: Mathias Nyman <mathias.nyman@iki.fi>\n> Based-on-patch-by: Nicola Paolucci <npaolucci@atlassian.com>\n> ---\n>  contrib/subtree/git-subtree.sh | 73 ++++++++++++++++++++++++++++--------------\n>  1 file changed, 49 insertions(+), 24 deletions(-)\n>\n> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\n> index 7a39b30..7cf73c0 100755\n> --- a/contrib/subtree/git-subtree.sh\n> +++ b/contrib/subtree/git-subtree.sh\n> @@ -335,18 +335,21 @@ add_msg()\n>  \tdir=\"$1\"\n>  \tlatest_old=\"$2\"\n>  \tlatest_new=\"$3\"\n> +\trepo=\"$4\" # optional\n>  \tif [ -n \"$message\" ]; then\n>  \t\tcommit_message=\"$message\"\n>  \telse\n>  \t\tcommit_message=\"Add '$dir/' from commit '$latest_new'\"\n>  \tfi\n> -\tcat <<-EOF\n> -\t\t$commit_message\n> -\t\t\n> -\t\tgit-subtree-dir: $dir\n> -\t\tgit-subtree-mainline: $latest_old\n> -\t\tgit-subtree-split: $latest_new\n> -\tEOF\n> +\techo $commit_message\n> +\techo\n> +\techo git-subtree-dir: $dir\n> +\techo git-subtree-mainline: $latest_old\n> +\techo git-subtree-split: $latest_new\n> +\tif [ -n \"$repo\" ]; then\n> +\t\trepo_url=$(get_repository_url \"$repo\")\n> +\t\techo \"git-subtree-repo: $repo_url\"\n> +\tfi\n>  }\n>  \n>  add_squashed_msg()\n> @@ -382,8 +385,9 @@ squash_msg()\n>  \tdir=\"$1\"\n>  \toldsub=\"$2\"\n>  \tnewsub=\"$3\"\n> +\trepo=\"$4\" # optional\n>  \tnewsub_short=$(git rev-parse --short \"$newsub\")\n> -\t\n> +\n>  \tif [ -n \"$oldsub\" ]; then\n>  \t\toldsub_short=$(git rev-parse --short \"$oldsub\")\n>  \t\techo \"Squashed '$dir/' changes from $oldsub_short..$newsub_short\"\n> @@ -397,6 +401,10 @@ squash_msg()\n>  \techo\n>  \techo \"git-subtree-dir: $dir\"\n>  \techo \"git-subtree-split: $newsub\"\n> +\tif [ -n \"$repo\" ]; then\n> +\t\trepo_url=$(get_repository_url \"$repo\")\n> +\t\techo \"git-subtree-repo: $repo_url\"\n> +\tfi\n>  }\n>  \n>  toptree_for_commit()\n> @@ -440,12 +448,13 @@ new_squash_commit()\n>  \told=\"$1\"\n>  \toldsub=\"$2\"\n>  \tnewsub=\"$3\"\n> +\trepo=\"$4\" # optional\n>  \ttree=$(toptree_for_commit $newsub) || exit $?\n>  \tif [ -n \"$old\" ]; then\n> -\t\tsquash_msg \"$dir\" \"$oldsub\" \"$newsub\" | \n> +\t\tsquash_msg \"$dir\" \"$oldsub\" \"$newsub\" \"$repo\" |\n>  \t\t\tgit commit-tree \"$tree\" -p \"$old\" || exit $?\n>  \telse\n> -\t\tsquash_msg \"$dir\" \"\" \"$newsub\" |\n> +\t\tsquash_msg \"$dir\" \"\" \"$newsub\" \"$repo\" |\n>  \t\t\tgit commit-tree \"$tree\" || exit $?\n>  \tfi\n>  }\n> @@ -517,6 +526,16 @@ ensure_valid_ref_format()\n>  \t    die \"'$1' does not look like a ref\"\n>  }\n>  \n> +get_repository_url()\n> +{\n> +\trepo=$1\n> +\trepo_url=$(git config --get remote.$repo.url)\n> +\tif [ -z \"$repo_url\" ]; then\n> +\t\trepo_url=$repo\n> +\tfi\n> +\techo $repo_url\n> +}\n> +\n>  cmd_add()\n>  {\n>  \tif [ -e \"$dir\" ]; then\n> @@ -548,19 +567,18 @@ cmd_add()\n>  cmd_add_repository()\n>  {\n>  \techo \"git fetch\" \"$@\"\n> -\trepository=$1\n> +\trepo=$1\n>  \trefspec=$2\n>  \tgit fetch \"$@\" || exit $?\n>  \trevs=FETCH_HEAD\n> -\tset -- $revs\n> +\tset -- $revs $repo\n>  \tcmd_add_commit \"$@\"\n>  }\n>  \n>  cmd_add_commit()\n>  {\n> -\trevs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n> -\tset -- $revs\n> -\trev=\"$1\"\n> +\trev=$(git rev-parse $default --revs-only \"$1\") || exit $?\n> +\trepo=\"$2\" # optional\n>  \t\n>  \tdebug \"Adding $dir as '$rev'...\"\n>  \tgit read-tree --prefix=\"$dir\" $rev || exit $?\n> @@ -575,12 +593,12 @@ cmd_add_commit()\n>  \tfi\n>  \t\n>  \tif [ -n \"$squash\" ]; then\n> -\t\trev=$(new_squash_commit \"\" \"\" \"$rev\") || exit $?\n> +\t\trev=$(new_squash_commit \"\" \"\" \"$rev\" \"$repo\") || exit $?\n>  \t\tcommit=$(add_squashed_msg \"$rev\" \"$dir\" |\n>  \t\t\t git commit-tree $tree $headp -p \"$rev\") || exit $?\n>  \telse\n>  \t\trevp=$(peel_committish \"$rev\") &&\n> -\t\tcommit=$(add_msg \"$dir\" \"$headrev\" \"$rev\" |\n> +\t\tcommit=$(add_msg \"$dir\" \"$headrev\" \"$rev\" \"$repo\" |\n>  \t\t\t git commit-tree $tree $headp -p \"$revp\") || exit $?\n>  \tfi\n>  \tgit reset \"$commit\" || exit $?\n> @@ -609,7 +627,8 @@ cmd_split()\n>  \telse\n>  \t\tunrevs=\"$(find_existing_splits \"$dir\" \"$revs\")\"\n>  \tfi\n> -\t\n> +e\n> +\trev=\"$1\"\n>  \t# We can't restrict rev-list to only $dir here, because some of our\n>  \t# parents have the $dir contents the root, and those won't match.\n>  \t# (and rev-list --follow doesn't seem to solve this)\n> @@ -683,15 +702,20 @@ cmd_split()\n>  \n>  cmd_merge()\n>  {\n> -\trevs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n> +\trevs=$(git rev-parse $default --revs-only \"$1\") || exit $?\n>  \tensure_clean\n> -\t\n>  \tset -- $revs\n>  \tif [ $# -ne 1 ]; then\n>  \t\tdie \"You must provide exactly one revision.  Got: '$revs'\"\n>  \tfi\n> +\tdo_merge \"$@\"\n> +}\n> +\n> +do_merge()\n> +{\n>  \trev=\"$1\"\n> -\t\n> +\trepo=\"$2\" # optional\n> +\n>  \tif [ -n \"$squash\" ]; then\n>  \t\tfirst_split=\"$(find_latest_squash \"$dir\")\"\n>  \t\tif [ -z \"$first_split\" ]; then\n> @@ -704,7 +728,7 @@ cmd_merge()\n>  \t\t\tsay \"Subtree is already at commit $rev.\"\n>  \t\t\texit 0\n>  \t\tfi\n> -\t\tnew=$(new_squash_commit \"$old\" \"$sub\" \"$rev\") || exit $?\n> +\t\tnew=$(new_squash_commit \"$old\" \"$sub\" \"$rev\" \"$repo\") || exit $?\n>  \t\tdebug \"New squash commit: $new\"\n>  \t\trev=\"$new\"\n>  \tfi\n> @@ -730,12 +754,13 @@ cmd_pull()\n>  \tif [ $# -ne 2 ]; then\n>  \t    die \"You must provide <repository> <ref>\"\n>  \tfi\n> +\trepo=$1\n>  \tensure_clean\n>  \tensure_valid_ref_format \"$2\"\n>  \tgit fetch \"$@\" || exit $?\n>  \trevs=FETCH_HEAD\n> -\tset -- $revs\n> -\tcmd_merge \"$@\"\n> +\tset -- $revs $repo\n> +\tdo_merge \"$@\"\n>  }\n>  \n>  cmd_push()\n"},{"id":"287892","messageId":"20160531121920.GA8180@iki.fi","threadId":"41482","inReplyTo":"87twhrowh0.fsf@waller.obbligato.org","subject":"Re: [PATCH] contrib/subtree: add repo url to commit messages","fromName":"Mathias Nyman","fromEmail":"m.nyman@iki.fi","sentAt":"2016-05-31T12:19:20Z","receivedAt":"2016-05-31T12:19:20Z","isPatch":true,"sender":{"key":"m.nyman@iki.fi","avatar":null},"body":"On 2016-05-21 17:52-0500, David A. Greene wrote:\n>Mathias Nyman <mathias.nyman@iki.fi> writes:\n>\n>> For recalling where a subtree came from; git-subtree operations 'add'\n>> and 'pull', when called with the <repository> parameter add this to the\n>> commit message:\n>>     git-subtree-repo: <repo_url>\n>\n>I am sorry it tooks a couple of months to respond.  I am finally coming\n>up for air at work.\n>\n>What is the future intent of this?  I've toyed with the idea of adding\n>something like this either as commit message metadata or in .gitconfig\n>but every time I get ready to pull the trigger, I question what it will\n>be used for.\n>\n>Having been using git-subtree in anger for a couple of years now, I\n>frequently pull subtree updates from multiple sources, so noting a\n>particular repository is not only mostly meaningless, it may actually be\n>misleading in that a quick perusal of the logs may lead one to think\n>commits were draw from fewer places than they actually were.\n>\n>I don't think it would be a good idea, for example, to have git-subtree\n>use this information to auto-guess from where to pull future commits.\n>Again, I think that would be misleading behavior.\n>\n>                         -David\n>\n\nI don't have future features in mind which would build upon this.\n\nFor me this would be very helpful for identifying where to pull in\nupdates from; I'm specifically thinking of the use case where\ngit-subtree is used for managing third party dependencies. In\nparticular when you want to bump versions, it would be convenient to\nhave the history in the history :). So, having git-subtree\nautomatically document that in the commit message seemed like a good\nidea to me.\n\n\n:Mathias\n\n>> Other operations that don't have the <repository> information, like\n>> 'merge' and 'add' without <repository>, are unchanged. Users with such a\n>> workflow will continue to be on their own with the --message parameter,\n>> if they'd like to record where the subtree came from.\n>>\n>> Signed-off-by: Mathias Nyman <mathias.nyman@iki.fi>\n>> Based-on-patch-by: Nicola Paolucci <npaolucci@atlassian.com>\n>> ---\n>>  contrib/subtree/git-subtree.sh | 73 ++++++++++++++++++++++++++++--------------\n>>  1 file changed, 49 insertions(+), 24 deletions(-)\n>>\n>> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\n>> index 7a39b30..7cf73c0 100755\n>> --- a/contrib/subtree/git-subtree.sh\n>> +++ b/contrib/subtree/git-subtree.sh\n>> @@ -335,18 +335,21 @@ add_msg()\n>>  \tdir=\"$1\"\n>>  \tlatest_old=\"$2\"\n>>  \tlatest_new=\"$3\"\n>> +\trepo=\"$4\" # optional\n>>  \tif [ -n \"$message\" ]; then\n>>  \t\tcommit_message=\"$message\"\n>>  \telse\n>>  \t\tcommit_message=\"Add '$dir/' from commit '$latest_new'\"\n>>  \tfi\n>> -\tcat <<-EOF\n>> -\t\t$commit_message\n>> -\t\t\n>> -\t\tgit-subtree-dir: $dir\n>> -\t\tgit-subtree-mainline: $latest_old\n>> -\t\tgit-subtree-split: $latest_new\n>> -\tEOF\n>> +\techo $commit_message\n>> +\techo\n>> +\techo git-subtree-dir: $dir\n>> +\techo git-subtree-mainline: $latest_old\n>> +\techo git-subtree-split: $latest_new\n>> +\tif [ -n \"$repo\" ]; then\n>> +\t\trepo_url=$(get_repository_url \"$repo\")\n>> +\t\techo \"git-subtree-repo: $repo_url\"\n>> +\tfi\n>>  }\n>>\n>>  add_squashed_msg()\n>> @@ -382,8 +385,9 @@ squash_msg()\n>>  \tdir=\"$1\"\n>>  \toldsub=\"$2\"\n>>  \tnewsub=\"$3\"\n>> +\trepo=\"$4\" # optional\n>>  \tnewsub_short=$(git rev-parse --short \"$newsub\")\n>> -\t\n>> +\n>>  \tif [ -n \"$oldsub\" ]; then\n>>  \t\toldsub_short=$(git rev-parse --short \"$oldsub\")\n>>  \t\techo \"Squashed '$dir/' changes from $oldsub_short..$newsub_short\"\n>> @@ -397,6 +401,10 @@ squash_msg()\n>>  \techo\n>>  \techo \"git-subtree-dir: $dir\"\n>>  \techo \"git-subtree-split: $newsub\"\n>> +\tif [ -n \"$repo\" ]; then\n>> +\t\trepo_url=$(get_repository_url \"$repo\")\n>> +\t\techo \"git-subtree-repo: $repo_url\"\n>> +\tfi\n>>  }\n>>\n>>  toptree_for_commit()\n>> @@ -440,12 +448,13 @@ new_squash_commit()\n>>  \told=\"$1\"\n>>  \toldsub=\"$2\"\n>>  \tnewsub=\"$3\"\n>> +\trepo=\"$4\" # optional\n>>  \ttree=$(toptree_for_commit $newsub) || exit $?\n>>  \tif [ -n \"$old\" ]; then\n>> -\t\tsquash_msg \"$dir\" \"$oldsub\" \"$newsub\" |\n>> +\t\tsquash_msg \"$dir\" \"$oldsub\" \"$newsub\" \"$repo\" |\n>>  \t\t\tgit commit-tree \"$tree\" -p \"$old\" || exit $?\n>>  \telse\n>> -\t\tsquash_msg \"$dir\" \"\" \"$newsub\" |\n>> +\t\tsquash_msg \"$dir\" \"\" \"$newsub\" \"$repo\" |\n>>  \t\t\tgit commit-tree \"$tree\" || exit $?\n>>  \tfi\n>>  }\n>> @@ -517,6 +526,16 @@ ensure_valid_ref_format()\n>>  \t    die \"'$1' does not look like a ref\"\n>>  }\n>>\n>> +get_repository_url()\n>> +{\n>> +\trepo=$1\n>> +\trepo_url=$(git config --get remote.$repo.url)\n>> +\tif [ -z \"$repo_url\" ]; then\n>> +\t\trepo_url=$repo\n>> +\tfi\n>> +\techo $repo_url\n>> +}\n>> +\n>>  cmd_add()\n>>  {\n>>  \tif [ -e \"$dir\" ]; then\n>> @@ -548,19 +567,18 @@ cmd_add()\n>>  cmd_add_repository()\n>>  {\n>>  \techo \"git fetch\" \"$@\"\n>> -\trepository=$1\n>> +\trepo=$1\n>>  \trefspec=$2\n>>  \tgit fetch \"$@\" || exit $?\n>>  \trevs=FETCH_HEAD\n>> -\tset -- $revs\n>> +\tset -- $revs $repo\n>>  \tcmd_add_commit \"$@\"\n>>  }\n>>\n>>  cmd_add_commit()\n>>  {\n>> -\trevs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n>> -\tset -- $revs\n>> -\trev=\"$1\"\n>> +\trev=$(git rev-parse $default --revs-only \"$1\") || exit $?\n>> +\trepo=\"$2\" # optional\n>>  \t\n>>  \tdebug \"Adding $dir as '$rev'...\"\n>>  \tgit read-tree --prefix=\"$dir\" $rev || exit $?\n>> @@ -575,12 +593,12 @@ cmd_add_commit()\n>>  \tfi\n>>  \t\n>>  \tif [ -n \"$squash\" ]; then\n>> -\t\trev=$(new_squash_commit \"\" \"\" \"$rev\") || exit $?\n>> +\t\trev=$(new_squash_commit \"\" \"\" \"$rev\" \"$repo\") || exit $?\n>>  \t\tcommit=$(add_squashed_msg \"$rev\" \"$dir\" |\n>>  \t\t\t git commit-tree $tree $headp -p \"$rev\") || exit $?\n>>  \telse\n>>  \t\trevp=$(peel_committish \"$rev\") &&\n>> -\t\tcommit=$(add_msg \"$dir\" \"$headrev\" \"$rev\" |\n>> +\t\tcommit=$(add_msg \"$dir\" \"$headrev\" \"$rev\" \"$repo\" |\n>>  \t\t\t git commit-tree $tree $headp -p \"$revp\") || exit $?\n>>  \tfi\n>>  \tgit reset \"$commit\" || exit $?\n>> @@ -609,7 +627,8 @@ cmd_split()\n>>  \telse\n>>  \t\tunrevs=\"$(find_existing_splits \"$dir\" \"$revs\")\"\n>>  \tfi\n>> -\t\n>> +e\n>> +\trev=\"$1\"\n>>  \t# We can't restrict rev-list to only $dir here, because some of our\n>>  \t# parents have the $dir contents the root, and those won't match.\n>>  \t# (and rev-list --follow doesn't seem to solve this)\n>> @@ -683,15 +702,20 @@ cmd_split()\n>>\n>>  cmd_merge()\n>>  {\n>> -\trevs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n>> +\trevs=$(git rev-parse $default --revs-only \"$1\") || exit $?\n>>  \tensure_clean\n>> -\t\n>>  \tset -- $revs\n>>  \tif [ $# -ne 1 ]; then\n>>  \t\tdie \"You must provide exactly one revision.  Got: '$revs'\"\n>>  \tfi\n>> +\tdo_merge \"$@\"\n>> +}\n>> +\n>> +do_merge()\n>> +{\n>>  \trev=\"$1\"\n>> -\t\n>> +\trepo=\"$2\" # optional\n>> +\n>>  \tif [ -n \"$squash\" ]; then\n>>  \t\tfirst_split=\"$(find_latest_squash \"$dir\")\"\n>>  \t\tif [ -z \"$first_split\" ]; then\n>> @@ -704,7 +728,7 @@ cmd_merge()\n>>  \t\t\tsay \"Subtree is already at commit $rev.\"\n>>  \t\t\texit 0\n>>  \t\tfi\n>> -\t\tnew=$(new_squash_commit \"$old\" \"$sub\" \"$rev\") || exit $?\n>> +\t\tnew=$(new_squash_commit \"$old\" \"$sub\" \"$rev\" \"$repo\") || exit $?\n>>  \t\tdebug \"New squash commit: $new\"\n>>  \t\trev=\"$new\"\n>>  \tfi\n>> @@ -730,12 +754,13 @@ cmd_pull()\n>>  \tif [ $# -ne 2 ]; then\n>>  \t    die \"You must provide <repository> <ref>\"\n>>  \tfi\n>> +\trepo=$1\n>>  \tensure_clean\n>>  \tensure_valid_ref_format \"$2\"\n>>  \tgit fetch \"$@\" || exit $?\n>>  \trevs=FETCH_HEAD\n>> -\tset -- $revs\n>> -\tcmd_merge \"$@\"\n>> +\tset -- $revs $repo\n>> +\tdo_merge \"$@\"\n>>  }\n>>\n>>  cmd_push()\n"}]}