{"thread":{"id":"43309","subject":"[PATCH] Teach git-branch howto rename a branch","startedAt":"2006-11-28T02:01:27Z","lastAt":"2006-11-28T09:55:47Z","messageCount":4,"participants":["Lars Hjemli","Jakub Narebski","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"294297","messageId":"1164679287192-git-send-email-hjemli@gmail.com","threadId":"43309","inReplyTo":null,"subject":"[PATCH] Teach git-branch howto rename a branch","fromName":"Lars Hjemli","fromEmail":"hjemli@gmail.com","sentAt":"2006-11-28T02:01:27Z","receivedAt":"2006-11-28T02:01:27Z","isPatch":true,"sender":{"key":"hjemli@gmail.com","avatar":null},"body":"This adds a '--rename' option to git branch. If specified, branch\ncreation becomes branch renaming.\n\nWith a single branchname, the current branch is renamed and .git/HEAD is\nupdated.\n\nWith two branchnames, the second name is renamed to the first.\n\nSigned-off-by: Lars Hjemli <hjemli@gmail.com>\n---\n\nThis seems to do the right thing for both refs and reflogs, but 'make test' \nprobably should be expanded with some evil test-cases to confirm my manual\ntesting.\n\n\n builtin-branch.c |   30 ++++++++++-\n refs.c           |  162 ++++++++++++++++++++++++++++++++++++++++++++++--------\n refs.h           |    3 +\n 3 files changed, 171 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin-branch.c b/builtin-branch.c\nindex 3d5cb0e..2a21263 100644\n--- a/builtin-branch.c\n+++ b/builtin-branch.c\n@@ -245,9 +245,29 @@ static void create_branch(const char *name, const char *start,\n \t\tdie(\"Failed to write ref: %s.\", strerror(errno));\n }\n \n+static void rename_branch(const char *oldname, const char *newname, int force)\n+{\n+\tchar oldref[PATH_MAX], newref[PATH_MAX];\n+\tunsigned char sha1[20];\n+\n+\tsnprintf(oldref, sizeof oldref, \"refs/heads/%s\", oldname);\n+\tif (check_ref_format(oldref))\n+\t\tdie(\"Invalid branch name: %s\", oldref);\n+\n+\tsnprintf(newref, sizeof newref, \"refs/heads/%s\", newname);\n+\tif (check_ref_format(newref))\n+\t\tdie(\"Invalid branch name: %s\", newref);\n+\n+\tif (resolve_ref(newref, sha1, 1, NULL) && !force)\n+\t\tdie(\"A branch named '%s' already exists.\\n\", newname);\n+\n+\tif (!rename_ref(oldref, newref) && !strcmp(oldname, head))\n+\t\tcreate_symref(\"HEAD\", newref);\n+}\n+\n int cmd_branch(int argc, const char **argv, const char *prefix)\n {\n-\tint delete = 0, force_delete = 0, force_create = 0;\n+\tint delete = 0, rename = 0, force_delete = 0, force_create = 0;\n \tint verbose = 0, abbrev = DEFAULT_ABBREV;\n \tint reflog = 0;\n \tint kinds = REF_LOCAL_BRANCH;\n@@ -277,6 +297,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\t\tforce_create = 1;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!strcmp(arg, \"--rename\")) {\n+\t\t\trename = 1;\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!strcmp(arg, \"-r\")) {\n \t\t\tkinds = REF_REMOTE_BRANCH;\n \t\t\tcontinue;\n@@ -311,6 +335,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tdelete_branches(argc - i, argv + i, force_delete);\n \telse if (i == argc)\n \t\tprint_ref_list(kinds, verbose, abbrev);\n+\telse if (rename && (i == argc - 1))\n+\t\trename_branch(head, argv[i], force_create);\n+\telse if (rename && (i == argc - 2))\n+\t\trename_branch(argv[i + 1], argv[i], force_create);\n \telse if (i == argc - 1)\n \t\tcreate_branch(argv[i], head, force_create, reflog);\n \telse if (i == argc - 2)\ndiff --git a/refs.c b/refs.c\nindex 0e156c5..1cb610f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -534,6 +534,29 @@ static int remove_empty_directories(char *file)\n \treturn remove_empty_dir_recursive(path, len);\n }\n \n+static int is_refname_available(const char *ref, const char *oldref, \n+\t\t\t\tstruct ref_list *list, int quiet)\n+{\n+\tint namlen = strlen(ref); /* e.g. 'foo/bar' */\n+\twhile (list) {\n+\t\t/* list->name could be 'foo' or 'foo/bar/baz' */\n+\t\tif (!oldref || strcmp(oldref, list->name)) {\n+\t\t\tint len = strlen(list->name);\n+\t\t\tint cmplen = (namlen < len) ? namlen : len;\n+\t\t\tconst char *lead = (namlen < len) ? list->name : ref;\n+\t\t\tif (!strncmp(ref, list->name, cmplen) && \n+\t\t\t    lead[cmplen] == '/') { \n+\t\t\t\tif (!quiet)\n+\t\t\t\t\terror(\"'%s' exists; cannot create '%s'\", \n+\t\t\t\t\t      list->name, ref);\n+\t\t\t\treturn 0;\n+\t\t\t}\n+\t\t}\n+\t\tlist = list->next;\n+\t}\n+\treturn 1;\n+}\n+\n static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char *old_sha1, int *flag)\n {\n \tchar *ref_file;\n@@ -567,29 +590,14 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char\n \t\t\torig_ref, strerror(errno));\n \t\tgoto error_return;\n \t}\n-\tif (is_null_sha1(lock->old_sha1)) {\n-\t\t/* The ref did not exist and we are creating it.\n-\t\t * Make sure there is no existing ref that is packed\n-\t\t * whose name begins with our refname, nor a ref whose\n-\t\t * name is a proper prefix of our refname.\n-\t\t */\n-\t\tint namlen = strlen(ref); /* e.g. 'foo/bar' */\n-\t\tstruct ref_list *list = get_packed_refs();\n-\t\twhile (list) {\n-\t\t\t/* list->name could be 'foo' or 'foo/bar/baz' */\n-\t\t\tint len = strlen(list->name);\n-\t\t\tint cmplen = (namlen < len) ? namlen : len;\n-\t\t\tconst char *lead = (namlen < len) ? list->name : ref;\n-\n-\t\t\tif (!strncmp(ref, list->name, cmplen) &&\n-\t\t\t    lead[cmplen] == '/') {\n-\t\t\t\terror(\"'%s' exists; cannot create '%s'\",\n-\t\t\t\t      list->name, ref);\n-\t\t\t\tgoto error_return;\n-\t\t\t}\n-\t\t\tlist = list->next;\n-\t\t}\n-\t}\n+\t/* When the ref did not exist and we are creating it,\n+\t * make sure there is no existing ref that is packed\n+\t * whose name begins with our refname, nor a ref whose\n+\t * name is a proper prefix of our refname.\n+\t */\n+\tif (is_null_sha1(lock->old_sha1) && \n+            !is_refname_available(ref, NULL, get_packed_refs(), 0))\n+\t\tgoto error_return;\n \n \tlock->lk = xcalloc(1, sizeof(struct lock_file));\n \n@@ -700,6 +708,114 @@ int delete_ref(const char *refname, unsigned char *sha1)\n \treturn ret;\n }\n \n+int rename_ref(const char *oldref, const char *newref)\n+{\n+\tunsigned char sha1[20], orig_sha1[20];\n+\tint flag = 0, logmoved = 0;\n+\tstruct ref_lock *lock;\n+\tchar msg[PATH_MAX*2 + 100];\n+\tstruct stat stat;\n+\tint log = !lstat(git_path(\"logs/%s\", oldref), &stat);\n+\n+\tif (!resolve_ref(oldref, orig_sha1, 1, &flag))\n+\t\treturn error(\"refname %s not found\", oldref);\n+\n+\tif (!is_refname_available(newref, oldref, get_packed_refs(), 0))\n+\t\treturn 1;\n+\n+\tif (!is_refname_available(newref, oldref, get_loose_refs(), 0))\n+\t\treturn 1;\n+\n+\tlock = lock_ref_sha1_basic(\"tmp-renamed-ref\", NULL, NULL);\n+\tif (!lock)\n+\t\treturn error(\"unable to lock tmp-renamed-ref\");\n+\tlock->force_write = 1;\n+\tif (write_ref_sha1(lock, orig_sha1, msg))\n+\t\treturn error(\"unable to save current sha1 in tmp-renamed-ref\");\n+\tif (log && rename(git_path(\"logs/%s\", oldref), git_path(\"tmp-renamed-log\")))\n+\t\treturn error(\"unable to move logfile logs/%s to tmp-renamed-log: %s\",\n+\t\t\toldref, strerror(errno));\n+\t\n+\tif (delete_ref(oldref, orig_sha1)) {\n+\t\terror(\"unable to delete old %s\", oldref);\n+\t\tgoto rollback;\n+\t}\n+\n+\tif (resolve_ref(newref, sha1, 1, &flag) && delete_ref(newref, sha1)) {\n+\t\tif (errno==EISDIR) {\n+\t\t\tif (remove_empty_directories(git_path(\"%s\", newref))) {\n+\t\t\t\terror(\"Directory not empty: %s\", newref);\n+\t\t\t\tgoto rollback;\n+\t\t\t}\n+\t\t} else {\n+\t\t\terror(\"unable to delete existing %s\", newref);\n+\t\t\tgoto rollback;\n+\t\t}\n+\t} \n+\n+\tif (log && safe_create_leading_directories(git_path(\"logs/%s\", newref))) {\n+\t\terror(\"unable to create directory for %s\", newref);\n+\t\tgoto rollback;\n+\t}\n+\n+ retry:\n+\tif (log && rename(git_path(\"tmp-renamed-log\"), git_path(\"logs/%s\", newref))) {\n+\t\tif (errno==EISDIR) {\n+\t\t\tif (remove_empty_directories(git_path(\"logs/%s\", newref))) {\n+\t\t\t\terror(\"Directory not empty: logs/%s\", newref);\n+\t\t\t\tgoto rollback;\n+\t\t\t}\n+\t\t\tgoto retry;\n+\t\t} else {\n+\t\t\terror(\"unable to move logfile tmp-renamed-log to logs/%s: %s\",\n+\t\t\t\tnewref, strerror(errno));\n+\t\t\tgoto rollback;\n+\t\t}\n+\t}\n+\tlogmoved = log;\n+\n+\tlock = lock_ref_sha1_basic(newref, NULL, NULL);\t\n+\tif (!lock) {\n+\t\terror(\"unable to lock %s for update\", newref);\n+\t\tgoto rollback;\n+\t}\n+\n+\tsnprintf(msg, sizeof msg, \"renamed from %s to %s\", oldref, newref);\n+\tlock->force_write = 1;\n+\thashcpy(lock->old_sha1, orig_sha1);\n+\tif (write_ref_sha1(lock, orig_sha1, msg)) {\n+\t\terror(\"unable to write current sha1 into %s\", newref);\n+\t\tgoto rollback;\n+\t}\n+\n+\treturn 0;\n+\n+ rollback:\n+\tlock = lock_ref_sha1_basic(oldref, NULL, NULL);\t\n+\tif (!lock) {\n+\t\terror(\"unable to lock %s for rollback\", oldref);\n+\t\tgoto rollbacklog;\n+\t}\n+\n+\tlock->force_write = 1;\n+\tflag = log_all_ref_updates;\n+\tlog_all_ref_updates = 0;\n+\tif (write_ref_sha1(lock, orig_sha1, NULL))\n+\t\terror(\"unable to write current sha1 into %s\", oldref);\n+\tlog_all_ref_updates = flag;\n+\n+ rollbacklog:\n+\tif (logmoved && rename(git_path(\"logs/%s\", newref), git_path(\"logs/%s\", oldref)))\n+\t\terror(\"unable to restore logfile %s from %s: %s\",\n+\t\t\toldref, newref, strerror(errno));\n+\tif (!logmoved && log && \n+\t    rename(git_path(\"tmp-renamed-log\"), git_path(\"logs/%s\", oldref)))\n+\t\terror(\"unable to restore logfile %s from tmp-renamed-log: %s\",\n+\t\t\toldref, strerror(errno));\n+\n+\treturn 1;\n+}\n+\n void unlock_ref(struct ref_lock *lock)\n {\n \tif (lock->lock_fd >= 0) {\ndiff --git a/refs.h b/refs.h\nindex a57d437..61419db 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -44,4 +44,7 @@ extern int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned\n /** Returns 0 if target has the right format for a ref. **/\n extern int check_ref_format(const char *target);\n \n+/** rename ref, return 0 on success **/\n+extern int rename_ref(const char *oldref, const char *newref);\n+\n #endif /* REFS_H */\n-- \n1.4.4.1.gf64d\n"},{"id":"295870","messageId":"ekgqku$tqt$1@sea.gmane.org","threadId":"43309","inReplyTo":"1164679287192-git-send-email-hjemli@gmail.com","subject":"Re: [PATCH] Teach git-branch howto rename a branch","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-11-28T08:07:37Z","receivedAt":"2006-11-28T08:07:37Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Lars Hjemli wrote:\n\n> This adds a '--rename' option to git branch. If specified, branch\n> creation becomes branch renaming.\n> \n> With a single branchname, the current branch is renamed and .git/HEAD is\n> updated.\n> \n> With two branchnames, the second name is renamed to the first.\n\nCould you please document this new feature?\n\nAnd wouldn't it be better to use <source> <target>, aka <branch> <newname>\nlike in git-mv and other rename/move commands? \"Second name is renamed to\nthe first\" although fits with \"<branchname> [<start-point>]\" is not natural\nfor rename.\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n\n"},{"id":"296351","messageId":"7vzmac3qig.fsf@assigned-by-dhcp.cox.net","threadId":"43309","inReplyTo":"1164679287192-git-send-email-hjemli@gmail.com","subject":"Re: [PATCH] Teach git-branch howto rename a branch","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-11-28T08:49:11Z","receivedAt":"2006-11-28T08:49:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Hjemli <hjemli@gmail.com> writes:\n\n> This adds a '--rename' option to git branch. If specified, branch\n> creation becomes branch renaming.\n>\n> With a single branchname, the current branch is renamed and .git/HEAD is\n> updated.\n>\n> With two branchnames, the second name is renamed to the first.\n\nThanks.\n\n\"--rename newname oldname\" feels funny, as already mentioned a\nfew times on the list.  rename(2) is \"rename(old, new)\" and\nmv(1) is \"mv old new\".\n\n> This seems to do the right thing for both refs and reflogs, but 'make test' \n> probably should be expanded with some evil test-cases to confirm my manual\n> testing.\n\nCertainly.  Let's keep discipline to have tests and docs for new\nfeatures.\n \n+static void rename_branch(const char *oldname, const char *newname, int force)\n+{\n+\tchar oldref[PATH_MAX], newref[PATH_MAX];\n+\tunsigned char sha1[20];\n+\n+\tsnprintf(oldref, sizeof oldref, \"refs/heads/%s\", oldname);\n+\tif (check_ref_format(oldref))\n+\t\tdie(\"Invalid branch name: %s\", oldref);\n\nAlthough \"sizeof type\" is valid C, we tend to prefer\n\"sizeof(type)\".  If you use snprintf(), it would make sense to\ncheck its return value.\n\n+\tif (resolve_ref(newref, sha1, 1, NULL) && !force)\n+\t\tdie(\"A branch named '%s' already exists.\\n\", newname);\n\nNo trailing '\\n' is necessary for die().\n\n+\tif (!rename_ref(oldref, newref) && !strcmp(oldname, head))\n+\t\tcreate_symref(\"HEAD\", newref);\n+}\n\nCan create_symref() fail?\n\n+int rename_ref(const char *oldref, const char *newref)\n+{\n+\tunsigned char sha1[20], orig_sha1[20];\n+\tint flag = 0, logmoved = 0;\n+\tstruct ref_lock *lock;\n+\tchar msg[PATH_MAX*2 + 100];\n+\tstruct stat stat;\n+\tint log = !lstat(git_path(\"logs/%s\", oldref), &stat);\n\nThis is not wrong per-se, but it made me stop and wonder if we\nwant to error out when we find out \"logs/oldref\" is a symlink; I\ndo not think we care about it that much, but in that case we may\nwant to say stat() here instead...  Just a minor detail.\n\n+\tlock = lock_ref_sha1_basic(\"tmp-renamed-ref\", NULL, NULL);\n+\tif (!lock)\n+\t\treturn error(\"unable to lock tmp-renamed-ref\");\n+\tlock->force_write = 1;\n+\tif (write_ref_sha1(lock, orig_sha1, msg))\n+\t\treturn error(\"unable to save current sha1 in tmp-renamed-ref\");\n+\tif (log && rename(git_path(\"logs/%s\", oldref), git_path(\"tmp-renamed-log\")))\n+\t\treturn error(\"unable to move logfile logs/%s to tmp-renamed-log: %s\",\n+\t\t\toldref, strerror(errno));\n\nI am confused with this code.  tmp-renamed-ref is not even a\nref, you lock $GIT_DIR/tmp-renamed-ref and call write-ref_sha1()\nwith an uninitialized msg[] buffer to write into a logfile. What\nis the name of that logfile?  $GIT_DIR/log/tmp-renamed-ref???\n"},{"id":"295581","messageId":"8c5c35580611280155v3b21cbe1m4b580ca9838eac15@mail.gmail.com","threadId":"43309","inReplyTo":"7vzmac3qig.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Teach git-branch howto rename a branch","fromName":"Lars Hjemli","fromEmail":"hjemli@gmail.com","sentAt":"2006-11-28T09:55:47Z","receivedAt":"2006-11-28T09:55:47Z","isPatch":true,"sender":{"key":"hjemli@gmail.com","avatar":null},"body":"On 11/28/06, Junio C Hamano <junkio@cox.net> wrote:\n> Lars Hjemli <hjemli@gmail.com> writes:\n> >\n> > With two branchnames, the second name is renamed to the first.\n>\n> Thanks.\n>\n> \"--rename newname oldname\" feels funny, as already mentioned a\n> few times on the list.  rename(2) is \"rename(old, new)\" and\n> mv(1) is \"mv old new\".\n\nOk, how about\n\n  git branch [-m|-M] [oldbranch] newbranch\n\nwhere -m is 'move' and -M is 'force move'?\n\n\n>\n> +       if (!rename_ref(oldref, newref) && !strcmp(oldname, head))\n> +               create_symref(\"HEAD\", newref);\n> +}\n>\n> Can create_symref() fail?\n\nYes... But what can been done if/when it fails? create_symref()\nalready seems to be pretty verbose about errors, so the only thing I\ncan think of is to return the errorcode to the caller (which I should\nhave done in the first place, git-branch ought to have a usable\nexitcode)\n\n>\n> +int rename_ref(const char *oldref, const char *newref)\n> +{\n> +       unsigned char sha1[20], orig_sha1[20];\n> +       int flag = 0, logmoved = 0;\n> +       struct ref_lock *lock;\n> +       char msg[PATH_MAX*2 + 100];\n> +       struct stat stat;\n> +       int log = !lstat(git_path(\"logs/%s\", oldref), &stat);\n>\n> This is not wrong per-se, but it made me stop and wonder if we\n> want to error out when we find out \"logs/oldref\" is a symlink; I\n> do not think we care about it that much, but in that case we may\n> want to say stat() here instead...  Just a minor detail.\n\nWell, it's a good point. If it's a symlink that's a pretty strong\nindication that someone has been messing with the log for some reason,\nso to error out is probably the right thing to do.\n\n\n>\n> +       lock = lock_ref_sha1_basic(\"tmp-renamed-ref\", NULL, NULL);\n> +       if (!lock)\n> +               return error(\"unable to lock tmp-renamed-ref\");\n> +       lock->force_write = 1;\n> +       if (write_ref_sha1(lock, orig_sha1, msg))\n> +               return error(\"unable to save current sha1 in tmp-renamed-ref\");\n> +       if (log && rename(git_path(\"logs/%s\", oldref), git_path(\"tmp-renamed-log\")))\n> +               return error(\"unable to move logfile logs/%s to tmp-renamed-log: %s\",\n> +                       oldref, strerror(errno));\n>\n> I am confused with this code.  tmp-renamed-ref is not even a\n> ref, you lock $GIT_DIR/tmp-renamed-ref and call write-ref_sha1()\n> with an uninitialized msg[] buffer to write into a logfile. What\n> is the name of that logfile?  $GIT_DIR/log/tmp-renamed-ref???\n\nMy goal was to save the ref in a tmp-file before deleting the old ref,\nnot to log the event. I think of it as a way to get out of trouble if\nrename_ref should fail badly.\n\nBtw: I't _might_ be interesting to have $GIT_DIR/logs/tmp-renamed-ref\n(or something similar) as a branch-independent log of branch renames\n\n\nAnyway, I'l fix up the mentioned issues in a new patch\n\n--\n"}]}