[PATCH] Teach git-branch howto rename a branch

[PATCH] Teach git-branch howto rename a branch

[Lars Hjemli]

This adds a '--rename' option to git branch. If specified, branch
creation becomes branch renaming.

With a single branchname, the current branch is renamed and .git/HEAD is
updated.

With two branchnames, the second name is renamed to the first.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---

This seems to do the right thing for both refs and reflogs, but 'make test' 
probably should be expanded with some evil test-cases to confirm my manual
testing.


 builtin-branch.c |   30 ++++++++++-
 refs.c           |  162 ++++++++++++++++++++++++++++++++++++++++++++++--------
 refs.h           |    3 +
 3 files changed, 171 insertions(+), 24 deletions(-)

diff --git a/builtin-branch.c b/builtin-branch.c
index 3d5cb0e..2a21263 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -245,9 +245,29 @@ static void create_branch(const char *name, const char *start,
 		die("Failed to write ref: %s.", strerror(errno));
 }
 
+static void rename_branch(const char *oldname, const char *newname, int force)
+{
+	char oldref[PATH_MAX], newref[PATH_MAX];
+	unsigned char sha1[20];
+
+	snprintf(oldref, sizeof oldref, "refs/heads/%s", oldname);
+	if (check_ref_format(oldref))
+		die("Invalid branch name: %s", oldref);
+
+	snprintf(newref, sizeof newref, "refs/heads/%s", newname);
+	if (check_ref_format(newref))
+		die("Invalid branch name: %s", newref);
+
+	if (resolve_ref(newref, sha1, 1, NULL) && !force)
+		die("A branch named '%s' already exists.\n", newname);
+
+	if (!rename_ref(oldref, newref) && !strcmp(oldname, head))
+		create_symref("HEAD", newref);
+}
+
 int cmd_branch(int argc, const char **argv, const char *prefix)
 {
-	int delete = 0, force_delete = 0, force_create = 0;
+	int delete = 0, rename = 0, force_delete = 0, force_create = 0;
 	int verbose = 0, abbrev = DEFAULT_ABBREV;
 	int reflog = 0;
 	int kinds = REF_LOCAL_BRANCH;
@@ -277,6 +297,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			force_create = 1;
 			continue;
 		}
+		if (!strcmp(arg, "--rename")) {
+			rename = 1;
+			continue;
+		}
 		if (!strcmp(arg, "-r")) {
 			kinds = REF_REMOTE_BRANCH;
 			continue;
@@ -311,6 +335,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		delete_branches(argc - i, argv + i, force_delete);
 	else if (i == argc)
 		print_ref_list(kinds, verbose, abbrev);
+	else if (rename && (i == argc - 1))
+		rename_branch(head, argv[i], force_create);
+	else if (rename && (i == argc - 2))
+		rename_branch(argv[i + 1], argv[i], force_create);
 	else if (i == argc - 1)
 		create_branch(argv[i], head, force_create, reflog);
 	else if (i == argc - 2)
diff --git a/refs.c b/refs.c
index 0e156c5..1cb610f 100644
--- a/refs.c
+++ b/refs.c
@@ -534,6 +534,29 @@ static int remove_empty_directories(char *file)
 	return remove_empty_dir_recursive(path, len);
 }
 
+static int is_refname_available(const char *ref, const char *oldref, 
+				struct ref_list *list, int quiet)
+{
+	int namlen = strlen(ref); /* e.g. 'foo/bar' */
+	while (list) {
+		/* list->name could be 'foo' or 'foo/bar/baz' */
+		if (!oldref || strcmp(oldref, list->name)) {
+			int len = strlen(list->name);
+			int cmplen = (namlen < len) ? namlen : len;
+			const char *lead = (namlen < len) ? list->name : ref;
+			if (!strncmp(ref, list->name, cmplen) && 
+			    lead[cmplen] == '/') { 
+				if (!quiet)
+					error("'%s' exists; cannot create '%s'", 
+					      list->name, ref);
+				return 0;
+			}
+		}
+		list = list->next;
+	}
+	return 1;
+}
+
 static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char *old_sha1, int *flag)
 {
 	char *ref_file;
@@ -567,29 +590,14 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
 			orig_ref, strerror(errno));
 		goto error_return;
 	}
-	if (is_null_sha1(lock->old_sha1)) {
-		/* The ref did not exist and we are creating it.
-		 * Make sure there is no existing ref that is packed
-		 * whose name begins with our refname, nor a ref whose
-		 * name is a proper prefix of our refname.
-		 */
-		int namlen = strlen(ref); /* e.g. 'foo/bar' */
-		struct ref_list *list = get_packed_refs();
-		while (list) {
-			/* list->name could be 'foo' or 'foo/bar/baz' */
-			int len = strlen(list->name);
-			int cmplen = (namlen < len) ? namlen : len;
-			const char *lead = (namlen < len) ? list->name : ref;
-
-			if (!strncmp(ref, list->name, cmplen) &&
-			    lead[cmplen] == '/') {
-				error("'%s' exists; cannot create '%s'",
-				      list->name, ref);
-				goto error_return;
-			}
-			list = list->next;
-		}
-	}
+	/* When the ref did not exist and we are creating it,
+	 * make sure there is no existing ref that is packed
+	 * whose name begins with our refname, nor a ref whose
+	 * name is a proper prefix of our refname.
+	 */
+	if (is_null_sha1(lock->old_sha1) && 
+            !is_refname_available(ref, NULL, get_packed_refs(), 0))
+		goto error_return;
 
 	lock->lk = xcalloc(1, sizeof(struct lock_file));
 
@@ -700,6 +708,114 @@ int delete_ref(const char *refname, unsigned char *sha1)
 	return ret;
 }
 
+int rename_ref(const char *oldref, const char *newref)
+{
+	unsigned char sha1[20], orig_sha1[20];
+	int flag = 0, logmoved = 0;
+	struct ref_lock *lock;
+	char msg[PATH_MAX*2 + 100];
+	struct stat stat;
+	int log = !lstat(git_path("logs/%s", oldref), &stat);
+
+	if (!resolve_ref(oldref, orig_sha1, 1, &flag))
+		return error("refname %s not found", oldref);
+
+	if (!is_refname_available(newref, oldref, get_packed_refs(), 0))
+		return 1;
+
+	if (!is_refname_available(newref, oldref, get_loose_refs(), 0))
+		return 1;
+
+	lock = lock_ref_sha1_basic("tmp-renamed-ref", NULL, NULL);
+	if (!lock)
+		return error("unable to lock tmp-renamed-ref");
+	lock->force_write = 1;
+	if (write_ref_sha1(lock, orig_sha1, msg))
+		return error("unable to save current sha1 in tmp-renamed-ref");
+	if (log && rename(git_path("logs/%s", oldref), git_path("tmp-renamed-log")))
+		return error("unable to move logfile logs/%s to tmp-renamed-log: %s",
+			oldref, strerror(errno));
+	
+	if (delete_ref(oldref, orig_sha1)) {
+		error("unable to delete old %s", oldref);
+		goto rollback;
+	}
+
+	if (resolve_ref(newref, sha1, 1, &flag) && delete_ref(newref, sha1)) {
+		if (errno==EISDIR) {
+			if (remove_empty_directories(git_path("%s", newref))) {
+				error("Directory not empty: %s", newref);
+				goto rollback;
+			}
+		} else {
+			error("unable to delete existing %s", newref);
+			goto rollback;
+		}
+	} 
+
+	if (log && safe_create_leading_directories(git_path("logs/%s", newref))) {
+		error("unable to create directory for %s", newref);
+		goto rollback;
+	}
+
+ retry:
+	if (log && rename(git_path("tmp-renamed-log"), git_path("logs/%s", newref))) {
+		if (errno==EISDIR) {
+			if (remove_empty_directories(git_path("logs/%s", newref))) {
+				error("Directory not empty: logs/%s", newref);
+				goto rollback;
+			}
+			goto retry;
+		} else {
+			error("unable to move logfile tmp-renamed-log to logs/%s: %s",
+				newref, strerror(errno));
+			goto rollback;
+		}
+	}
+	logmoved = log;
+
+	lock = lock_ref_sha1_basic(newref, NULL, NULL);	
+	if (!lock) {
+		error("unable to lock %s for update", newref);
+		goto rollback;
+	}
+
+	snprintf(msg, sizeof msg, "renamed from %s to %s", oldref, newref);
+	lock->force_write = 1;
+	hashcpy(lock->old_sha1, orig_sha1);
+	if (write_ref_sha1(lock, orig_sha1, msg)) {
+		error("unable to write current sha1 into %s", newref);
+		goto rollback;
+	}
+
+	return 0;
+
+ rollback:
+	lock = lock_ref_sha1_basic(oldref, NULL, NULL);	
+	if (!lock) {
+		error("unable to lock %s for rollback", oldref);
+		goto rollbacklog;
+	}
+
+	lock->force_write = 1;
+	flag = log_all_ref_updates;
+	log_all_ref_updates = 0;
+	if (write_ref_sha1(lock, orig_sha1, NULL))
+		error("unable to write current sha1 into %s", oldref);
+	log_all_ref_updates = flag;
+
+ rollbacklog:
+	if (logmoved && rename(git_path("logs/%s", newref), git_path("logs/%s", oldref)))
+		error("unable to restore logfile %s from %s: %s",
+			oldref, newref, strerror(errno));
+	if (!logmoved && log && 
+	    rename(git_path("tmp-renamed-log"), git_path("logs/%s", oldref)))
+		error("unable to restore logfile %s from tmp-renamed-log: %s",
+			oldref, strerror(errno));
+
+	return 1;
+}
+
 void unlock_ref(struct ref_lock *lock)
 {
 	if (lock->lock_fd >= 0) {
diff --git a/refs.h b/refs.h
index a57d437..61419db 100644
--- a/refs.h
+++ b/refs.h
@@ -44,4 +44,7 @@ extern int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned
 /** Returns 0 if target has the right format for a ref. **/
 extern int check_ref_format(const char *target);
 
+/** rename ref, return 0 on success **/
+extern int rename_ref(const char *oldref, const char *newref);
+
 #endif /* REFS_H */
-- 
1.4.4.1.gf64d

Re: [PATCH] Teach git-branch howto rename a branch

[Jakub Narebski]

Lars Hjemli wrote:

> This adds a '--rename' option to git branch. If specified, branch
> creation becomes branch renaming.
> 
> With a single branchname, the current branch is renamed and .git/HEAD is
> updated.
> 
> With two branchnames, the second name is renamed to the first.

Could you please document this new feature?

And wouldn't it be better to use <source> <target>, aka <branch> <newname>
like in git-mv and other rename/move commands? "Second name is renamed to
the first" although fits with "<branchname> [<start-point>]" is not natural
for rename.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] Teach git-branch howto rename a branch

[Junio C Hamano]

Lars Hjemli <hjemli@gmail.com> writes:

> This adds a '--rename' option to git branch. If specified, branch
> creation becomes branch renaming.
>
> With a single branchname, the current branch is renamed and .git/HEAD is
> updated.
>
> With two branchnames, the second name is renamed to the first.

Thanks.

"--rename newname oldname" feels funny, as already mentioned a
few times on the list.  rename(2) is "rename(old, new)" and
mv(1) is "mv old new".

> This seems to do the right thing for both refs and reflogs, but 'make test' 
> probably should be expanded with some evil test-cases to confirm my manual
> testing.

Certainly.  Let's keep discipline to have tests and docs for new
features.
 
+static void rename_branch(const char *oldname, const char *newname, int force)
+{
+	char oldref[PATH_MAX], newref[PATH_MAX];
+	unsigned char sha1[20];
+
+	snprintf(oldref, sizeof oldref, "refs/heads/%s", oldname);
+	if (check_ref_format(oldref))
+		die("Invalid branch name: %s", oldref);

Although "sizeof type" is valid C, we tend to prefer
"sizeof(type)".  If you use snprintf(), it would make sense to
check its return value.

+	if (resolve_ref(newref, sha1, 1, NULL) && !force)
+		die("A branch named '%s' already exists.\n", newname);

No trailing '\n' is necessary for die().

+	if (!rename_ref(oldref, newref) && !strcmp(oldname, head))
+		create_symref("HEAD", newref);
+}

Can create_symref() fail?

+int rename_ref(const char *oldref, const char *newref)
+{
+	unsigned char sha1[20], orig_sha1[20];
+	int flag = 0, logmoved = 0;
+	struct ref_lock *lock;
+	char msg[PATH_MAX*2 + 100];
+	struct stat stat;
+	int log = !lstat(git_path("logs/%s", oldref), &stat);

This is not wrong per-se, but it made me stop and wonder if we
want to error out when we find out "logs/oldref" is a symlink; I
do not think we care about it that much, but in that case we may
want to say stat() here instead...  Just a minor detail.

+	lock = lock_ref_sha1_basic("tmp-renamed-ref", NULL, NULL);
+	if (!lock)
+		return error("unable to lock tmp-renamed-ref");
+	lock->force_write = 1;
+	if (write_ref_sha1(lock, orig_sha1, msg))
+		return error("unable to save current sha1 in tmp-renamed-ref");
+	if (log && rename(git_path("logs/%s", oldref), git_path("tmp-renamed-log")))
+		return error("unable to move logfile logs/%s to tmp-renamed-log: %s",
+			oldref, strerror(errno));

I am confused with this code.  tmp-renamed-ref is not even a
ref, you lock $GIT_DIR/tmp-renamed-ref and call write-ref_sha1()
with an uninitialized msg[] buffer to write into a logfile. What
is the name of that logfile?  $GIT_DIR/log/tmp-renamed-ref???

Re: [PATCH] Teach git-branch howto rename a branch

[Lars Hjemli]

On 11/28/06, Junio C Hamano <junkio@cox.net> wrote:
> Lars Hjemli <hjemli@gmail.com> writes:
> >
> > With two branchnames, the second name is renamed to the first.
>
> Thanks.
>
> "--rename newname oldname" feels funny, as already mentioned a
> few times on the list.  rename(2) is "rename(old, new)" and
> mv(1) is "mv old new".

Ok, how about

  git branch [-m|-M] [oldbranch] newbranch

where -m is 'move' and -M is 'force move'?


>
> +       if (!rename_ref(oldref, newref) && !strcmp(oldname, head))
> +               create_symref("HEAD", newref);
> +}
>
> Can create_symref() fail?

Yes... But what can been done if/when it fails? create_symref()
already seems to be pretty verbose about errors, so the only thing I
can think of is to return the errorcode to the caller (which I should
have done in the first place, git-branch ought to have a usable
exitcode)

>
> +int rename_ref(const char *oldref, const char *newref)
> +{
> +       unsigned char sha1[20], orig_sha1[20];
> +       int flag = 0, logmoved = 0;
> +       struct ref_lock *lock;
> +       char msg[PATH_MAX*2 + 100];
> +       struct stat stat;
> +       int log = !lstat(git_path("logs/%s", oldref), &stat);
>
> This is not wrong per-se, but it made me stop and wonder if we
> want to error out when we find out "logs/oldref" is a symlink; I
> do not think we care about it that much, but in that case we may
> want to say stat() here instead...  Just a minor detail.

Well, it's a good point. If it's a symlink that's a pretty strong
indication that someone has been messing with the log for some reason,
so to error out is probably the right thing to do.


>
> +       lock = lock_ref_sha1_basic("tmp-renamed-ref", NULL, NULL);
> +       if (!lock)
> +               return error("unable to lock tmp-renamed-ref");
> +       lock->force_write = 1;
> +       if (write_ref_sha1(lock, orig_sha1, msg))
> +               return error("unable to save current sha1 in tmp-renamed-ref");
> +       if (log && rename(git_path("logs/%s", oldref), git_path("tmp-renamed-log")))
> +               return error("unable to move logfile logs/%s to tmp-renamed-log: %s",
> +                       oldref, strerror(errno));
>
> I am confused with this code.  tmp-renamed-ref is not even a
> ref, you lock $GIT_DIR/tmp-renamed-ref and call write-ref_sha1()
> with an uninitialized msg[] buffer to write into a logfile. What
> is the name of that logfile?  $GIT_DIR/log/tmp-renamed-ref???

My goal was to save the ref in a tmp-file before deleting the old ref,
not to log the event. I think of it as a way to get out of trouble if
rename_ref should fail badly.

Btw: I't _might_ be interesting to have $GIT_DIR/logs/tmp-renamed-ref
(or something similar) as a branch-independent log of branch renames


Anyway, I'l fix up the mentioned issues in a new patch

--