[PATCH] Make git-branch a builtin

[PATCH] Make git-branch a builtin

[Lars Hjemli]

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(-)

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

Re: [PATCH] Make git-branch a builtin

[Shawn Pearce]

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?

-- 
Shawn.

Re: [PATCH] Make git-branch a builtin

[Junio C Hamano]

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.

> +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.

> +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.

> 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...

> 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.

Re: [PATCH] Make git-branch a builtin

[Petr Baudis]

Dear diary, on Mon, Oct 23, 2006 at 11:55:06PM CEST, I got a letter
where Shawn Pearce <spearce@spearce.org> said that...
> 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*',/((..)*)$/)

Re: [PATCH] Make git-branch a builtin

[Junio C Hamano]

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?

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

[Lars Hjemli]

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(-)

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

Re: [PATCH] Make git-branch a builtin

[Shawn Pearce]

Junio C Hamano <junkio@cox.net> wrote:
> 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.

Re: [PATCH] Make git-branch a builtin

[Junio C Hamano]

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".

Re: [PATCH] Make git-branch a builtin

[Shawn Pearce]

Junio C Hamano <junkio@cox.net> wrote:
> 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.

Re: [PATCH] Make git-branch a builtin

[Petr Baudis]

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...
> 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*',/((..)*)$/)

Re: [PATCH] Make git-branch a builtin

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

>> 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? ;-)