{"thread":{"id":"22259","subject":"[PATCH 0/2] Make it easy to use branch --track on existing branch","startedAt":"2010-01-17T14:06:50Z","lastAt":"2010-01-17T19:38:33Z","messageCount":7,"participants":["Matthieu Moy","Ilari Liusvaara","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"131944","messageId":"1263737212-8101-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"22259","inReplyTo":null,"subject":"[PATCH 0/2] Make it easy to use branch --track on existing branch","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-01-17T14:06:50Z","receivedAt":"2010-01-17T14:06:50Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"I'm starting a new thread to avoid hiding the message in another one,\nbut this is a followup to a message in the \"git push --track\" thread:\n\nhttp://article.gmane.org/gmane.comp.version-control.git/137066\n\nI wrote:\n> Junio C Hamano pobox.com> writes:\n> \n> > The small nit is that \"branch -f --track me origin/me\" will happily\n> > overwrite \"me\", even when your \"me\" is not up to date with \"origin/me\",\n> > losing commits.\n> \n> And another issue is:\n> \n> $ git branch -f --track my-branch origin/my-branch\n> fatal: Cannot force update the current branch.\n> $ git branch --track my-branch origin/my-branch\n> fatal: A branch named 'my-branch' already exists.\n> \n> Actually, I just can't find a natural set of commands doing:\n> \n> 1. create a branch (git checkout -b)\n> 2. work on it\n> 3. send it upstream (git push)\n> 4. set the upstream as tracking (???)\n> \n> with the current version of Git. I just do 4. with $EDITOR\n> .git/config ...\n\nThe first patch makes it possible to use branch --track on an existing\nbranch (checked-out or not, regardless of -f), and the second warns on\na newly introduced irrelevant case.\n\nThis should be a nice complement to \"push --set-upstream\". I think\n\"push --set-upstream\" is the most natural in 99% of cases, but using\n\"git branch\" should work too.\n\nMatthieu Moy (2):\n branch: allow creating a branch with same name and same starting\n point.\n branch: warn and refuse to set a branch as a tracking branch of\n itself.\n\n branch.c | 59 ++++++++++++++++++++++++++++++++-------------\n t/t6040-tracking-info.sh | 8 ++++++\n 2 files changed, 50 insertions(+), 17 deletions(-)\n"},{"id":"131945","messageId":"1263737212-8101-2-git-send-email-Matthieu.Moy@imag.fr","threadId":"22259","inReplyTo":"1263737212-8101-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 1/2] branch: allow creating a branch with same name and same starting point.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-01-17T14:06:51Z","receivedAt":"2010-01-17T14:06:51Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Previously, \"git branch --track newname old\" was rejected if newname\nexisted, even if it already had the same value. This patch allows it,\neven without --force. This has two advantages:\n\n* Not requiring --force for a safe operation, hence allowing the user to\n benefit from the other safety checks.\n\n* Allow changing the configuration of the checked-out branch.\n\nAs a result, \"git branch --track\" becomes a conveinient and safe way to\nset up upstream information for an existing branch.\n\nThe error message \"A branch named '%s' already exists\" is no longer\nprecise enough to refuse to act, since part of the reason of the failure\nis the old branch value, so we include this value in the message.\n\nSigned-off-by: Matthieu Moy \n---\n branch.c | 45 ++++++++++++++++++++++++++++++---------------\n t/t6040-tracking-info.sh | 8 ++++++++\n 2 files changed, 38 insertions(+), 15 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 05ef3f5..d187891 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -128,27 +128,39 @@ void create_branch(const char *head,\n \t\t const char *name, const char *start_name,\n \t\t int force, int reflog, enum branch_track track)\n {\n-\tstruct ref_lock *lock;\n+\tstruct ref_lock *lock = NULL;\n \tstruct commit *commit;\n-\tunsigned char sha1[20];\n+\tunsigned char old_sha1[20], sha1[20];\n \tchar *real_ref, msg[PATH_MAX + 20];\n \tstruct strbuf ref = STRBUF_INIT;\n \tint forcing = 0;\n+\tint dont_change_ref = 0;\n \n \tif (strbuf_check_branch_ref(&ref, name))\n \t\tdie(\"'%s' is not a valid branch name.\", name);\n \n-\tif (resolve_ref(ref.buf, sha1, 1, NULL)) {\n-\t\tif (!force)\n-\t\t\tdie(\"A branch named '%s' already exists.\", name);\n-\t\telse if (!is_bare_repository() && !strcmp(head, name))\n-\t\t\tdie(\"Cannot force update the current branch.\");\n+\tif (get_sha1(start_name, sha1))\n+\t\tdie(\"Not a valid object name: '%s'.\", start_name);\n+\n+\tif (resolve_ref(ref.buf, old_sha1, 1, NULL)) {\n+\t\tif (hashcmp(old_sha1, sha1)) {\n+\t\t\tif (!force)\n+\t\t\t\tdie(\"A branch named '%s' already exists pointing to %.*s.\",\n+\t\t\t\t name, 8, sha1_to_hex(old_sha1));\n+\t\t\telse if (!is_bare_repository() && !strcmp(head, name))\n+\t\t\t\tdie(\"Cannot force update the current branch.\");\n+\t\t} else {\n+\t\t\t/*\n+\t\t\t * Re-creating a branch with the same value.\n+\t\t\t * Safe, and usefull to set up the upstream\n+\t\t\t * branch without touching the ref.\n+\t\t\t */\n+\t\t\tdont_change_ref = 1;\n+\t\t}\n \t\tforcing = 1;\n \t}\n \n \treal_ref = NULL;\n-\tif (get_sha1(start_name, sha1))\n-\t\tdie(\"Not a valid object name: '%s'.\", start_name);\n \n \tswitch (dwim_ref(start_name, strlen(start_name), sha1, &real_ref)) {\n \tcase 0:\n@@ -170,9 +182,11 @@ void create_branch(const char *head,\n \t\tdie(\"Not a valid branch point: '%s'.\", start_name);\n \thashcpy(sha1, commit->object.sha1);\n \n-\tlock = lock_any_ref_for_update(ref.buf, NULL, 0);\n-\tif (!lock)\n-\t\tdie_errno(\"Failed to lock ref for update\");\n+\tif (!dont_change_ref) {\n+\t\tlock = lock_any_ref_for_update(ref.buf, NULL, 0);\n+\t\tif (!lock)\n+\t\t\tdie_errno(\"Failed to lock ref for update\");\n+\t}\n \n \tif (reflog)\n \t\tlog_all_ref_updates = 1;\n@@ -180,15 +194,16 @@ void create_branch(const char *head,\n \tif (forcing)\n \t\tsnprintf(msg, sizeof msg, \"branch: Reset from %s\",\n \t\t\t start_name);\n-\telse\n+\telse if (!dont_change_ref)\n \t\tsnprintf(msg, sizeof msg, \"branch: Created from %s\",\n \t\t\t start_name);\n \n \tif (real_ref && track)\n \t\tsetup_tracking(name, real_ref, track);\n \n-\tif (write_ref_sha1(lock, sha1, msg) < 0)\n-\t\tdie_errno(\"Failed to write ref\");\n+\tif (!dont_change_ref)\n+\t\tif (write_ref_sha1(lock, sha1, msg) < 0)\n+\t\t\tdie_errno(\"Failed to write ref\");\n \n \tstrbuf_release(&ref);\n \tfree(real_ref);\ndiff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh\nindex 664b0f8..3c200b6 100755\n--- a/t/t6040-tracking-info.sh\n+++ b/t/t6040-tracking-info.sh\n@@ -89,4 +89,12 @@ test_expect_success 'status when tracking annotated tags' '\n \tgrep \"set up to track\" actual &&\n \tgit checkout heavytrack\n '\n+\n+test_expect_success 'setup tracking with branch --track on existing branch' '\n+\tgit branch from-master master &&\n+\ttest_must_fail git config branch.from-master.merge > actual &&\n+\tgit branch from-master master --track &&\n+\tgit config branch.from-master.merge > actual &&\n+\tgrep -q \"^refs/heads/master$\" actual\n+'\n test_done\n-- \n1.6.6.198.gbaea2\n"},{"id":"131946","messageId":"1263737212-8101-3-git-send-email-Matthieu.Moy@imag.fr","threadId":"22259","inReplyTo":"1263737212-8101-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 2/2] branch: warn and refuse to set a branch as a tracking branch of itself.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-01-17T14:06:52Z","receivedAt":"2010-01-17T14:06:52Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Previous patch allows commands like \"git branch foo foo --track\", which\ndoesn't make much sense. Warn the user and don't change the configuration\nin this case. Don't die to let the caller finish its job in such case.\n\nSigned-off-by: Matthieu Moy \n---\n branch.c | 14 ++++++++++++--\n 1 files changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex d187891..d769c8a 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -49,9 +49,19 @@ static int should_setup_rebase(const char *origin)\n \n void install_branch_config(int flag, const char *local, const char *origin, const char *remote)\n {\n+\tconst char *shortname = remote + 11;\n+\tint remote_is_branch = !prefixcmp(remote, \"refs/heads/\");\n \tstruct strbuf key = STRBUF_INIT;\n \tint rebasing = should_setup_rebase(origin);\n \n+\tif (remote_is_branch\n+\t && !strcmp(local, shortname)\n+\t && !origin) {\n+\t\twarning(\"Not setting branch %s as its own upstream.\",\n+\t\t\tlocal);\n+\t\treturn;\n+\t}\n+\n \tstrbuf_addf(&key, \"branch.%s.remote\", local);\n \tgit_config_set(key.buf, origin ? origin : \".\");\n \n@@ -71,8 +81,8 @@ void install_branch_config(int flag, const char *local, const char *origin, cons\n \t\tstrbuf_addstr(&key, origin ? \"remote\" : \"local\");\n \n \t\t/* Are we tracking a proper \"branch\"? */\n-\t\tif (!prefixcmp(remote, \"refs/heads/\")) {\n-\t\t\tstrbuf_addf(&key, \" branch %s\", remote + 11);\n+\t\tif (remote_is_branch) {\n+\t\t\tstrbuf_addf(&key, \" branch %s\", shortname);\n \t\t\tif (origin)\n \t\t\t\tstrbuf_addf(&key, \" from %s\", origin);\n \t\t}\n-- \n1.6.6.198.gbaea2\n"},{"id":"131947","messageId":"20100117144031.GA20335@Knoppix","threadId":"22259","inReplyTo":"1263737212-8101-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 0/2] Make it easy to use branch --track on existing branch","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2010-01-17T14:40:31Z","receivedAt":"2010-01-17T14:40:31Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Sun, Jan 17, 2010 at 03:06:50PM +0100, Matthieu Moy wrote:\n> \n> The first patch makes it possible to use branch --track on an existing\n> branch (checked-out or not, regardless of -f), and the second warns on\n> a newly introduced irrelevant case.\n\nYay. This is one of smaller entries from my todo list (the remaining\nentry that's small enough to still make it into 1.7 is command to\nset URL remote points to[*]).\n\nBut If I read the patch correctly, you can't just arbitrarily set the\ntracking branch since the IDs must match. So if somebody screws the\nref creation so that it lacks upstream data, then both local and upstream\nchange. Push will fail. Pull will fail. And now neither checkout --track\n(due to same-hash check) nor push --set-upstream (due to pushed or up to\ndate) help.\n\n[*] Yes, I know you can edit .git/config, but I would want \"official sounding\"\n(read: git remote subcommand) command to edit it (and no, delete & recreate\ndoesn't do the right thing).\n\n-Ilari\n"},{"id":"131948","messageId":"vpqska4n5pr.fsf@bauges.imag.fr","threadId":"22259","inReplyTo":"20100117144031.GA20335@Knoppix","subject":"Re: [PATCH 0/2] Make it easy to use branch --track on existing branch","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-01-17T14:53:04Z","receivedAt":"2010-01-17T14:53:04Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Ilari Liusvaara writes:\n\n> But If I read the patch correctly, you can't just arbitrarily set the\n> tracking branch since the IDs must match.\n\nYes. The patch really helps when you already have synchronized, or\nwhen you haven't already desynchronized your local branch and its\nupstream.\n\nWe could go one step further, and allow the new branch point to be\ndifferent as long as it has the old point as an ancestor, but that\nwould still be a ref update.\n\n> [*] Yes, I know you can edit .git/config, but I would want \"official sounding\"\n> (read: git remote subcommand) command to edit it (and no, delete & recreate\n> doesn't do the right thing).\n\nHaving a \"git remote subcommand\" to do the same thing could help too,\nbut it could just come in addition to my patch.\n\nMy patch doesn't introduce new complexity: the command is already\nthere, and the use-case I'm allowing are legitimate. So, it can help\nto let users run in, and it doesn't harm.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"131949","messageId":"20100117152912.GA20465@Knoppix","threadId":"22259","inReplyTo":"vpqska4n5pr.fsf@bauges.imag.fr","subject":"Re: [PATCH 0/2] Make it easy to use branch --track on existing branch","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2010-01-17T15:29:12Z","receivedAt":"2010-01-17T15:29:12Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Sun, Jan 17, 2010 at 03:53:04PM +0100, Matthieu Moy wrote:\n> Ilari Liusvaara writes:\n> \n> > [*] Yes, I know you can edit .git/config, but I would want \"official sounding\"\n> > (read: git remote subcommand) command to edit it (and no, delete & recreate\n> > doesn't do the right thing).\n> \n> Having a \"git remote subcommand\" to do the same thing could help too,\n> but it could just come in addition to my patch.\n\nErr... That was about changing URL, not about changing tracking branch.\n\n-Ilari\n"},{"id":"131960","messageId":"7vzl4cy11i.fsf@alter.siamese.dyndns.org","threadId":"22259","inReplyTo":"1263737212-8101-2-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 1/2] branch: allow creating a branch with same name and same starting point.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-17T19:38:33Z","receivedAt":"2010-01-17T19:38:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Previously, \"git branch --track newname old\" was rejected if newname\n> existed, even if it already had the same value. This patch allows it,\n> even without --force. This has two advantages:\n>\n> * Not requiring --force for a safe operation, hence allowing the user to\n> benefit from the other safety checks.\n>\n> * Allow changing the configuration of the checked-out branch.\n\nTwo issues I have to ask because you didn't cover these cases in your\ntests [*1*]:\n\n - What should \"git branch new old\" do when (0) local branch new already\n exists and points at old, (1) branch.new.{remote,rebase,merge} do not\n currently exist, and (2) branch.autosetup{merge,rebase} is set?\n\n - What should \"git branch --no-track new old\" do when (0) local branch\n new already exists and points at old, (1)\n branch.new.{remote,rebase,merge} do currently exist?\n\nNo matter what they do, should there be some extra message when the new\ncode does something that the old code didn't do?\n\n> +test_expect_success 'setup tracking with branch --track on existing branch' '\n> ...\n> +\tgit branch from-master master --track &&\n\nThe parser may be too loosely implemented and might allow this today, but\nplease stick to \"command names then dashed options and then arguments\".\n\nWe have to fix the parser someday to be more strict and broken order in\ntests like this will get in the way.\n\n> +\tgit config branch.from-master.merge > actual &&\n> +\tgrep -q \"^refs/heads/master$\" actual\n> +'\n> test_done\n\n[Footnote]\n\n*1* it is an easy trap to fall into to test only what it newly does while\nshowing your shiny new toy and forget to test conditions that the shiny\nnew toy should not kick in.\n"}]}