threads / patch / 6019

patchMake git-branch a builtin

Subject: [PATCH] Make git-branch a builtin

## tl;dr

11 messages between Oct 23, 2006 and Oct 24, 2006. Diffs are folded; open one to read it.

replies: 10people: 4as markdown or json

Lars Hjemli· Oct 23, 2006, 21:40 UTC · lore
This replaces git-branch.sh with builtin-branch.c

The changes is basically a patch from Kristian Høgsberg, updated to apply onto current 'next'

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
 Makefile         |    3 +-
 builtin-branch.c |  217 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 builtin.h        |    1 +
 git-branch.sh    |  131 --------------------------------
 git.c            |    1 +
 5 files changed, 221 insertions(+), 132 deletions(-)
Show changes to 5 files +221 −132

Makefile, builtin-branch.c, builtin.h, git-branch.sh, git.c

diff --git a/Makefile b/Makefile
index 36b9e06..e249d34 100644
--- a/Makefile
+++ b/Makefile
@@ -158,7 +158,7 @@ BASIC_CFLAGS =
 BASIC_LDFLAGS =
 
 SCRIPT_SH = \
-	git-bisect.sh git-branch.sh git-checkout.sh \
+	git-bisect.sh git-checkout.sh \
 	git-cherry.sh git-clean.sh git-clone.sh git-commit.sh \
 	git-fetch.sh \
 	git-ls-remote.sh \
@@ -270,6 +270,7 @@ BUILTIN_OBJS = \
 	builtin-annotate.o \
 	builtin-apply.o \
 	builtin-archive.o \
+	builtin-branch.o \
 	builtin-cat-file.o \
 	builtin-checkout-index.o \
 	builtin-check-ref-format.o \
diff --git a/builtin-branch.c b/builtin-branch.c
new file mode 100755
index 0000000..3105efd
--- /dev/null
+++ b/builtin-branch.c
@@ -0,0 +1,217 @@
+/*
+ * Builtin "git branch"
+ *
+ * Copyright (c) 2006 Kristian Høgsberg <krh@redhat.com>
+ * Based on git-branch.sh by Junio C Hamano.
+ */
+
+#include "cache.h"
+#include "refs.h"
+#include "commit.h"
+#include "builtin.h"
+
+static const char builtin_branch_usage[] =
+"git-branch [(-d | -D) <branchname>] | [[-f] <branchname> [<start-point>]] | -r";
+
+
+static const char *head;
+static unsigned char head_sha1[20];
+
+static int in_merge_bases(const unsigned char *sha1,
+			  struct commit *rev1,
+			  struct commit *rev2)
+{
+	struct commit_list *bases, *b;
+	int ret = 0;
+
+	bases = get_merge_bases(rev1, rev2, 1);
+	for (b = bases; b; b = b->next) {
+		if (!hashcmp(sha1, b->item->object.sha1)) {
+			ret = 1;
+			break;
+		}
+	}
+
+	free_commit_list(bases);
+	return ret;
+}
+
+static void delete_branches(int argc, const char **argv, int force)
+{
+	struct commit *rev, *head_rev;
+	unsigned char sha1[20];
+	const char *name;
+	int i;
+
+	head_rev = lookup_commit_reference(head_sha1);
+	for (i = 0; i < argc; i++) {
+		if (!strcmp(head, argv[i]))
+			die("Cannot delete the branch you are currently on.");
+
+		name = xstrdup(mkpath("refs/heads/%s", argv[i]));
+		if (!resolve_ref(name, sha1, 1, NULL))
+			die("Branch '%s' not found.", argv[i]);
+
+		rev = lookup_commit_reference(sha1);
+		if (!rev || !head_rev)
+			die("Couldn't look up commit objects.");
+
+		/* This checks whether the merge bases of branch and
+		 * HEAD contains branch -- which means that the HEAD
+		 * contains everything in both.
+		 */
+
+		if (!force &&
+		    !in_merge_bases(sha1, rev, head_rev)) {
+			fprintf(stderr,
+				"The branch '%s' is not a strict subset of your current HEAD.\n"
+				"If you are sure you want to delete it, run 'git branch -D %s'.\n",
+				argv[i], argv[i]);
+			exit(1);
+		}
+
+		if (delete_ref(name, sha1))
+			printf("Error deleting branch '%s'\n", argv[i]);
+		else
+			printf("Deleted branch %s.\n", argv[i]);
+			
+		free(name);
+	}
+}
+
+static int ref_index, ref_alloc;
+static char **ref_list;
+
+static int append_ref(const char *refname, const unsigned char *sha1, int flags, 
+		void *cb_data)
+{
+	if (ref_index >= ref_alloc) {
+		ref_alloc = ref_alloc > 0 ? ref_alloc * 2 : 16;
+		ref_list = xrealloc(ref_list, ref_alloc * sizeof(char *));
+	}
+
+	ref_list[ref_index++] = xstrdup(refname);
+
+	return 0;
+}
+
+static int ref_cmp(const void *r1, const void *r2)
+{
+	return strcmp(*(char **)r1, *(char **)r2);
+}
+
+static void print_ref_list(int remote_only)
+{
+	int i;
+
+	if (remote_only)
+		for_each_remote_ref(append_ref, NULL);
+	else
+		for_each_branch_ref(append_ref, NULL);
+
+	qsort(ref_list, ref_index, sizeof(char *), ref_cmp);
+
+	for (i = 0; i < ref_index; i++) {
+		if (!strcmp(ref_list[i], head))
+			printf("* %s\n", ref_list[i]);
+		else
+			printf("  %s\n", ref_list[i]);
+	}
+}
+
+static void create_branch(const char *name, const char *start,
+			  int force, int reflog)
+{
+	struct ref_lock *lock;
+	unsigned char sha1[20];
+	char ref[PATH_MAX], msg[PATH_MAX + 20];
+
+	snprintf(ref, sizeof ref, "refs/heads/%s", name);
+	if (check_ref_format(ref))
+		die("'%s' is not a valid branch name.", name);
+
+	if (resolve_ref(ref, sha1, 1, NULL)) {
+		if (!force)
+			die("A branch named '%s' already exists.", name);
+		else if (!strcmp(head, name))
+			die("Cannot force update the current branch.");
+	}
+
+	if (get_sha1(start, sha1))
+		die("Not a valid branch point: '%s'.", start);
+
+	lock = lock_any_ref_for_update(ref, NULL);
+	if (!lock)
+		die("Failed to lock ref for update: %s.", strerror(errno));
+		
+	if (reflog){
+		log_all_ref_updates = 1;
+		snprintf(msg, sizeof msg, "branch: Created from %s", start);
+	}
+
+	if (write_ref_sha1(lock, sha1, msg) < 0)
+		die("Failed to write ref: %s.", strerror(errno));
+}
+
+int cmd_branch(int argc, const char **argv, const char *prefix)
+{
+	int delete = 0, force_delete = 0, force_create = 0, remote_only = 0;
+	int reflog = 0;
+	int i, prefix_length;
+
+	git_config(git_default_config);
+
+	for (i = 1; i < argc; i++) {
+		const char *arg = argv[i];
+
+		if (arg[0] != '-')
+			break;
+		if (!strcmp(arg, "--")) {
+			i++;
+			break;
+		}
+		if (!strcmp(arg, "-d")) {
+			delete = 1;
+			continue;
+		}
+		if (!strcmp(arg, "-D")) {
+			delete = 1;
+			force_delete = 1;
+			continue;
+		}
+		if (!strcmp(arg, "-f")) {
+			force_create = 1;
+			continue;
+		}
+		if (!strcmp(arg, "-r")) {
+			remote_only = 1;
+			continue;
+		}
+		if (!strcmp(arg, "-l")) {
+			reflog = 1;
+			continue;
+		}
+		usage(builtin_branch_usage);
+	}
+
+	head = xstrdup(resolve_ref("HEAD", head_sha1, 0, NULL));
+	if (!head)
+		die("Failed to resolve HEAD as a valid ref.");
+	if (strncmp(head, "refs/heads/", 11))
+		die("HEAD not found below refs/heads!");
+		
+	head += 11;
+	
+	if (delete)
+		delete_branches(argc - i, argv + i, force_delete);
+	else if (i == argc)
+		print_ref_list(remote_only);
+	else if (i == argc - 1)
+		create_branch(argv[i], head, force_create, reflog);
+	else if (i == argc - 2)
+		create_branch(argv[i], argv[i + 1], force_create, reflog);
+	else
+		usage(builtin_branch_usage);
+
+	return 0;
+}
diff --git a/builtin.h b/builtin.h
index 9683a7c..144d299 100644
--- a/builtin.h
+++ b/builtin.h
@@ -15,6 +15,7 @@ extern int write_tree(unsigned char *sha
 extern int cmd_add(int argc, const char **argv, const char *prefix);
 extern int cmd_annotate(int argc, const char **argv, const char *prefix);
 extern int cmd_apply(int argc, const char **argv, const char *prefix);
+extern int cmd_branch(int argc, const char **argv, const char *prefix);
 extern int cmd_archive(int argc, const char **argv, const char *prefix);
 extern int cmd_cat_file(int argc, const char **argv, const char *prefix);
 extern int cmd_checkout_index(int argc, const char **argv, const char *prefix);
diff --git a/git-branch.sh b/git-branch.sh
deleted file mode 100755
index 4379a07..0000000
--- a/git-branch.sh
+++ /dev/null
@@ -1,131 +0,0 @@
-#!/bin/sh
-
-USAGE='[-l] [(-d | -D) <branchname>] | [[-f] <branchname> [<start-point>]] | -r'
-LONG_USAGE='If no arguments, show available branches and mark current branch with a star.
-If one argument, create a new branch <branchname> based off of current HEAD.
-If two arguments, create a new branch <branchname> based off of <start-point>.'
-
-SUBDIRECTORY_OK='Yes'
-. git-sh-setup
-
-headref=$(git-symbolic-ref HEAD | sed -e 's|^refs/heads/||')
-
-delete_branch () {
-    option="$1"
-    shift
-    for branch_name
-    do
-	case ",$headref," in
-	",$branch_name,")
-	    die "Cannot delete the branch you are on." ;;
-	,,)
-	    die "What branch are you on anyway?" ;;
-	esac
-	branch=$(git-show-ref --verify --hash -- "refs/heads/$branch_name") &&
-	    branch=$(git-rev-parse --verify "$branch^0") ||
-		die "Seriously, what branch are you talking about?"
-	case "$option" in
-	-D)
-	    ;;
-	*)
-	    mbs=$(git-merge-base -a "$branch" HEAD | tr '\012' ' ')
-	    case " $mbs " in
-	    *' '$branch' '*)
-		# the merge base of branch and HEAD contains branch --
-		# which means that the HEAD contains everything in both.
-		;;
-	    *)
-		echo >&2 "The branch '$branch_name' is not a strict subset of your current HEAD.
-If you are sure you want to delete it, run 'git branch -D $branch_name'."
-		exit 1
-		;;
-	    esac
-	    ;;
-	esac
-	git update-ref -d "refs/heads/$branch_name" "$branch"
-	echo "Deleted branch $branch_name."
-    done
-    exit 0
-}
-
-ls_remote_branches () {
-    git-rev-parse --symbolic --all |
-    sed -ne 's|^refs/\(remotes/\)|\1|p' |
-    sort
-}
-
-force=
-create_log=
-while case "$#,$1" in 0,*) break ;; *,-*) ;; *) break ;; esac
-do
-	case "$1" in
-	-d | -D)
-		delete_branch "$@"
-		exit
-		;;
-	-r)
-		ls_remote_branches
-		exit
-		;;
-	-f)
-		force="$1"
-		;;
-	-l)
-		create_log="yes"
-		;;
-	--)
-		shift
-		break
-		;;
-	-*)
-		usage
-		;;
-	esac
-	shift
-done
-
-case "$#" in
-0)
-	git-rev-parse --symbolic --branches |
-	sort |
-	while read ref
-	do
-		if test "$headref" = "$ref"
-		then
-			pfx='*'
-		else
-			pfx=' '
-		fi
-		echo "$pfx $ref"
-	done
-	exit 0 ;;
-1)
-	head=HEAD ;;
-2)
-	head="$2^0" ;;
-esac
-branchname="$1"
-
-rev=$(git-rev-parse --verify "$head") || exit
-
-git-check-ref-format "heads/$branchname" ||
-	die "we do not like '$branchname' as a branch name."
-
-prev=''
-if git-show-ref --verify --quiet -- "refs/heads/$branchname"
-then
-	if test '' = "$force"
-	then
-		die "$branchname already exists."
-	elif test "$branchname" = "$headref"
-	then
-		die "cannot force-update the current branch."
-	fi
-	prev=`git rev-parse --verify "refs/heads/$branchname"`
-fi
-if test "$create_log" = 'yes'
-then
-	mkdir -p $(dirname "$GIT_DIR/logs/refs/heads/$branchname")
-	touch "$GIT_DIR/logs/refs/heads/$branchname"
-fi
-git update-ref -m "branch: Created from $head" "refs/heads/$branchname" "$rev" "$prev"
diff --git a/git.c b/git.c
index 308252a..671b212 100644
--- a/git.c
+++ b/git.c
@@ -222,6 +222,7 @@ static void handle_internal_command(int 
 		{ "annotate", cmd_annotate, },
 		{ "apply", cmd_apply },
 		{ "archive", cmd_archive },
+		{ "branch", cmd_branch },
 		{ "cat-file", cmd_cat_file, RUN_SETUP },
 		{ "checkout-index", cmd_checkout_index, RUN_SETUP },
 		{ "check-ref-format", cmd_check_ref_format },
-- 
1.4.3.1.g9164-dirty
Shawn Pearce· Oct 23, 2006, 21:55 UTC · re: Lars Hjemli · lore

Re: [PATCH] Make git-branch a builtin

Lars Hjemli <hjemli@gmail.com> wrote: [snip]

Show 7 quoted lines
> ---
>  Makefile         |    3 +-
>  builtin-branch.c |  217 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  builtin.h        |    1 +
>  git-branch.sh    |  131 --------------------------------
>  git.c            |    1 +
>  5 files changed, 221 insertions(+), 132 deletions(-)
[snip]
Show 6 quoted lines
> diff --git a/git-branch.sh b/git-branch.sh
> deleted file mode 100755
> index 4379a07..0000000
> --- a/git-branch.sh
> +++ /dev/null
> @@ -1,131 +0,0 @@
[snip 131 lines]
Aside from compatability with non-Git tools...

Wouldn't it make more sense to just include the full SHA1 of the file we are deleting rather than the entire 131 line negative diff?

-- 
Shawn.
Petr Baudis· Oct 23, 2006, 23:19 UTC · re: Shawn Pearce · lore

Re: [PATCH] Make git-branch a builtin

Dear diary, on Mon, Oct 23, 2006 at 11:55:06PM CEST, I got a letter where Shawn Pearce <spearce@spearce.org> said that...

Show 22 quoted lines
> Lars Hjemli <hjemli@gmail.com> wrote:
> [snip]
> > ---
> >  Makefile         |    3 +-
> >  builtin-branch.c |  217 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  builtin.h        |    1 +
> >  git-branch.sh    |  131 --------------------------------
> >  git.c            |    1 +
> >  5 files changed, 221 insertions(+), 132 deletions(-)
> [snip]
> > diff --git a/git-branch.sh b/git-branch.sh
> > deleted file mode 100755
> > index 4379a07..0000000
> > --- a/git-branch.sh
> > +++ /dev/null
> > @@ -1,131 +0,0 @@
> [snip 131 lines]
> 
> Aside from compatability with non-Git tools...
> 
> Wouldn't it make more sense to just include the full SHA1 of the
> file we are deleting rather than the entire 131 line negative diff?

I think this way it's easier to review since you can check what is going away with the new stuff you introduce instead.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)
Junio C Hamano· Oct 23, 2006, 23:44 UTC · re: Shawn Pearce · lore

Re: [PATCH] Make git-branch a builtin

Shawn Pearce <spearce@spearce.org> writes:
Show 22 quoted lines
> Lars Hjemli <hjemli@gmail.com> wrote:
> [snip]
>> ---
>>  Makefile         |    3 +-
>>  builtin-branch.c |  217 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  builtin.h        |    1 +
>>  git-branch.sh    |  131 --------------------------------
>>  git.c            |    1 +
>>  5 files changed, 221 insertions(+), 132 deletions(-)
> [snip]
>> diff --git a/git-branch.sh b/git-branch.sh
>> deleted file mode 100755
>> index 4379a07..0000000
>> --- a/git-branch.sh
>> +++ /dev/null
>> @@ -1,131 +0,0 @@
> [snip 131 lines]
>
> Aside from compatability with non-Git tools...
>
> Wouldn't it make more sense to just include the full SHA1 of the
> file we are deleting rather than the entire 131 line negative diff?
How would you do "git apply -R" on something like that?
Shawn Pearce· Oct 24, 2006, 05:44 UTC · re: Junio C Hamano · lore

Re: [PATCH] Make git-branch a builtin

Junio C Hamano <junkio@cox.net> wrote:
Show 26 quoted lines
> Shawn Pearce <spearce@spearce.org> writes:
> 
> > Lars Hjemli <hjemli@gmail.com> wrote:
> > [snip]
> >> ---
> >>  Makefile         |    3 +-
> >>  builtin-branch.c |  217 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  builtin.h        |    1 +
> >>  git-branch.sh    |  131 --------------------------------
> >>  git.c            |    1 +
> >>  5 files changed, 221 insertions(+), 132 deletions(-)
> > [snip]
> >> diff --git a/git-branch.sh b/git-branch.sh
> >> deleted file mode 100755
> >> index 4379a07..0000000
> >> --- a/git-branch.sh
> >> +++ /dev/null
> >> @@ -1,131 +0,0 @@
> > [snip 131 lines]
> >
> > Aside from compatability with non-Git tools...
> >
> > Wouldn't it make more sense to just include the full SHA1 of the
> > file we are deleting rather than the entire 131 line negative diff?
> 
> How would you do "git apply -R" on something like that?

Uh, you have the full SHA1 in the index line. So you just have to reattach that blob to the named path... pretty simple actually.

Of course this is yet another code path in apply and as Petr pointed out on this same thread its nice to know what you are deleting so you can compare it to what else is changing in the same patch... so this suggestion of mine is probably just a waste of bytes.

-- 
Shawn.
Junio C Hamano· Oct 24, 2006, 06:43 UTC · re: Shawn Pearce · lore

Re: [PATCH] Make git-branch a builtin

Shawn Pearce <spearce@spearce.org> writes:
Show 7 quoted lines
>> > Wouldn't it make more sense to just include the full SHA1 of the
>> > file we are deleting rather than the entire 131 line negative diff?
>> 
>> How would you do "git apply -R" on something like that?
>
> Uh, you have the full SHA1 in the index line.  So you just have to
> reattach that blob to the named path...  pretty simple actually.
Bzzzt; wrong answer.

Think of a future when you can shallowly clone near the tip of git repository that does not have shell-script git-branch.sh anymore. You cannot expect to already have the preimage of the patch in such a case. You would still want to be able to revert the change with "git apply -R".

Shawn Pearce· Oct 24, 2006, 06:55 UTC · re: Junio C Hamano · lore

Re: [PATCH] Make git-branch a builtin

Junio C Hamano <junkio@cox.net> wrote:
Show 17 quoted lines
> Shawn Pearce <spearce@spearce.org> writes:
> 
> >> > Wouldn't it make more sense to just include the full SHA1 of the
> >> > file we are deleting rather than the entire 131 line negative diff?
> >> 
> >> How would you do "git apply -R" on something like that?
> >
> > Uh, you have the full SHA1 in the index line.  So you just have to
> > reattach that blob to the named path...  pretty simple actually.
> 
> Bzzzt; wrong answer.
> 
> Think of a future when you can shallowly clone near the tip of
> git repository that does not have shell-script git-branch.sh
> anymore.  You cannot expect to already have the preimage of the
> patch in such a case.  You would still want to be able to revert
> the change with "git apply -R".

Yet another valid argument for having the data needed for the preimage available.

I've already agreed with Petr that having the data there is probably better than not. Now I'm also agreeing with you. There is little need to beat a dead horse. :-)

-- 
Shawn.
Petr Baudis· Oct 24, 2006, 11:39 UTC · re: Junio C Hamano · lore

Re: [PATCH] Make git-branch a builtin

Dear diary, on Tue, Oct 24, 2006 at 08:43:35AM CEST, I got a letter where Junio C Hamano <junkio@cox.net> said that...

Show 17 quoted lines
> Shawn Pearce <spearce@spearce.org> writes:
> 
> >> > Wouldn't it make more sense to just include the full SHA1 of the
> >> > file we are deleting rather than the entire 131 line negative diff?
> >> 
> >> How would you do "git apply -R" on something like that?
> >
> > Uh, you have the full SHA1 in the index line.  So you just have to
> > reattach that blob to the named path...  pretty simple actually.
> 
> Bzzzt; wrong answer.
> 
> Think of a future when you can shallowly clone near the tip of
> git repository that does not have shell-script git-branch.sh
> anymore.  You cannot expect to already have the preimage of the
> patch in such a case.  You would still want to be able to revert
> the change with "git apply -R".

Hmm, how is this argument not applying to binary diffs you can't revert either?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)
Junio C Hamano· Oct 24, 2006, 17:00 UTC · re: Petr Baudis · lore

Re: [PATCH] Make git-branch a builtin

Petr Baudis <pasky@suse.cz> writes:
Show 8 quoted lines
>> Think of a future when you can shallowly clone near the tip of
>> git repository that does not have shell-script git-branch.sh
>> anymore.  You cannot expect to already have the preimage of the
>> patch in such a case.  You would still want to be able to revert
>> the change with "git apply -R".
>
> Hmm, how is this argument not applying to binary diffs you can't revert
> either?
Earlier you cannot, but because now you can, perhaps? ;-)
Junio C Hamano· Oct 23, 2006, 22:34 UTC · re: Lars Hjemli · lore

Re: [PATCH] Make git-branch a builtin

Lars Hjemli <hjemli@gmail.com> writes:
> This replaces git-branch.sh with builtin-branch.c
> +static const char builtin_branch_usage[] =
> +"git-branch [(-d | -D) <branchname>] | [[-f] <branchname> [<start-point>]] | -r";
Probably needs a matching change to your recent clean-up here.
Show 7 quoted lines
> +static int append_ref(const char *refname, const unsigned char *sha1, int flags, 
> +		void *cb_data)
> +{
> +	if (ref_index >= ref_alloc) {
> +		ref_alloc = ref_alloc > 0 ? ref_alloc * 2 : 16;
> +		ref_list = xrealloc(ref_list, ref_alloc * sizeof(char *));
> +	}

It would be more consistent to use alloc_nr() from cache.h here unless you have a specific need for allocation growing pattern, which you don't. Other parts of the system even just grows by one and let xrealloc() (and in turn underlying realloc()) take care of the optimization of repeated reallocations, which may also be fine.

Show 24 quoted lines
> +static void create_branch(const char *name, const char *start,
> +			  int force, int reflog)
> +{
> +	struct ref_lock *lock;
> +	unsigned char sha1[20];
> +	char ref[PATH_MAX], msg[PATH_MAX + 20];
> +
> +	snprintf(ref, sizeof ref, "refs/heads/%s", name);
> +	if (check_ref_format(ref))
> +		die("'%s' is not a valid branch name.", name);
> +
> +	if (resolve_ref(ref, sha1, 1, NULL)) {
> +		if (!force)
> +			die("A branch named '%s' already exists.", name);
> +		else if (!strcmp(head, name))
> +			die("Cannot force update the current branch.");
> +	}
> +
> +	if (get_sha1(start, sha1))
> +		die("Not a valid branch point: '%s'.", start);
> +
> +	lock = lock_any_ref_for_update(ref, NULL);
> +	if (!lock)
> +		die("Failed to lock ref for update: %s.", strerror(errno));

This is not enough. "$2^0" in git-branch.sh makes sure the "start" specified dereferences to a commit object, and object name we use to initialize the new branch is of the commit object (IOW, test your code with "git branch new v1.0.0" and make sure "git rev-parse new" returns the commit object not the tag that points at it).

> +		
> +	if (reflog){

Style; (1) remove trailing TAB on the empty line; (2) one SP between ')' and '{' please.

Show 12 quoted lines
> diff --git a/builtin.h b/builtin.h
> index 9683a7c..144d299 100644
> --- a/builtin.h
> +++ b/builtin.h
> @@ -15,6 +15,7 @@ extern int write_tree(unsigned char *sha
>  extern int cmd_add(int argc, const char **argv, const char *prefix);
>  extern int cmd_annotate(int argc, const char **argv, const char *prefix);
>  extern int cmd_apply(int argc, const char **argv, const char *prefix);
> +extern int cmd_branch(int argc, const char **argv, const char *prefix);
>  extern int cmd_archive(int argc, const char **argv, const char *prefix);
>  extern int cmd_cat_file(int argc, const char **argv, const char *prefix);
>  extern int cmd_checkout_index(int argc, const char **argv, const char *prefix);
Style; order them alphabetically please...
Show 12 quoted lines
> diff --git a/git.c b/git.c
> index 308252a..671b212 100644
> --- a/git.c
> +++ b/git.c
> @@ -222,6 +222,7 @@ static void handle_internal_command(int 
>  		{ "annotate", cmd_annotate, },
>  		{ "apply", cmd_apply },
>  		{ "archive", cmd_archive },
> +		{ "branch", cmd_branch },
>  		{ "cat-file", cmd_cat_file, RUN_SETUP },
>  		{ "checkout-index", cmd_checkout_index, RUN_SETUP },
>  		{ "check-ref-format", cmd_check_ref_format },
In other words, match this.

We may someday want to do binary search into the command list, and before that happens we should make sure this list is really ordered.

Lars Hjemli· Oct 23, 2006, 23:51 UTC · re: Junio C Hamano · lore

[PATCH] Git-branch: fix regression and style issues

This is on top of my previuos patch to builtin-branch.c
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
 builtin-branch.c |   17 ++++++++---------
 builtin.h        |    2 +-
 2 files changed, 9 insertions(+), 10 deletions(-)
Show changes to 2 files +9 −10

builtin-branch.c, builtin.h

diff --git a/builtin-branch.c b/builtin-branch.c
index 3105efd..ffc2db0 100755
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -11,7 +11,7 @@ #include "commit.h"
 #include "builtin.h"
 
 static const char builtin_branch_usage[] =
-"git-branch [(-d | -D) <branchname>] | [[-f] <branchname> [<start-point>]] | -r";
+"git-branch (-d | -D) <branchname> | [-l] [-f] <branchname> [<start-point>] | [-r]";
 
 
 static const char *head;
@@ -74,7 +74,7 @@ static void delete_branches(int argc, co
 			printf("Error deleting branch '%s'\n", argv[i]);
 		else
 			printf("Deleted branch %s.\n", argv[i]);
-			
+
 		free(name);
 	}
 }
@@ -82,11 +82,11 @@ static void delete_branches(int argc, co
 static int ref_index, ref_alloc;
 static char **ref_list;
 
-static int append_ref(const char *refname, const unsigned char *sha1, int flags, 
+static int append_ref(const char *refname, const unsigned char *sha1, int flags,
 		void *cb_data)
 {
 	if (ref_index >= ref_alloc) {
-		ref_alloc = ref_alloc > 0 ? ref_alloc * 2 : 16;
+		ref_alloc = alloc_nr(ref_alloc);
 		ref_list = xrealloc(ref_list, ref_alloc * sizeof(char *));
 	}
 
@@ -137,14 +137,14 @@ static void create_branch(const char *na
 			die("Cannot force update the current branch.");
 	}
 
-	if (get_sha1(start, sha1))
+	if (get_sha1(mkpath("%s^0", start), sha1))
 		die("Not a valid branch point: '%s'.", start);
 
 	lock = lock_any_ref_for_update(ref, NULL);
 	if (!lock)
 		die("Failed to lock ref for update: %s.", strerror(errno));
-		
-	if (reflog){
+
+	if (reflog) {
 		log_all_ref_updates = 1;
 		snprintf(msg, sizeof msg, "branch: Created from %s", start);
 	}
@@ -199,9 +199,8 @@ int cmd_branch(int argc, const char **ar
 		die("Failed to resolve HEAD as a valid ref.");
 	if (strncmp(head, "refs/heads/", 11))
 		die("HEAD not found below refs/heads!");
-		
 	head += 11;
-	
+
 	if (delete)
 		delete_branches(argc - i, argv + i, force_delete);
 	else if (i == argc)
diff --git a/builtin.h b/builtin.h
index 144d299..9b4749f 100644
--- a/builtin.h
+++ b/builtin.h
@@ -15,8 +15,8 @@ extern int write_tree(unsigned char *sha
 extern int cmd_add(int argc, const char **argv, const char *prefix);
 extern int cmd_annotate(int argc, const char **argv, const char *prefix);
 extern int cmd_apply(int argc, const char **argv, const char *prefix);
-extern int cmd_branch(int argc, const char **argv, const char *prefix);
 extern int cmd_archive(int argc, const char **argv, const char *prefix);
+extern int cmd_branch(int argc, const char **argv, const char *prefix);
 extern int cmd_cat_file(int argc, const char **argv, const char *prefix);
 extern int cmd_checkout_index(int argc, const char **argv, const char *prefix);
 extern int cmd_check_ref_format(int argc, const char **argv, const char *prefix);
-- 
1.4.3.1.g4604-dirty

← back to recent threads