All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH, final version] git-branch, git-checkout: autosetup for remote branch tracking
@ 2007-03-08  6:49 Paolo Bonzini
  2007-03-08  9:47 ` Junio C Hamano
  0 siblings, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2007-03-08  6:49 UTC (permalink / raw)
  To: git, Junio C Hamano, Johannes Schindelin

In order to track and build on top of a branch 'topic' you track from
your upstream repository, you often would end up doing this sequence:

  git checkout -b mytopic origin/topic
  git config --add branch.mytopic.remote origin
  git config --add branch.mytopic.merge refs/heads/topic

This would first fork your own 'mytopic' branch from the 'topic'
branch you track from the 'origin' repository; then it would set up two
configuration variables so that 'git pull' without parameters does the
right thing while you are on your own 'mytopic' branch.

This commit adds a --track option to git-branch, so that "git
branch --track mytopic origin/topic" performs the latter two actions
when creating your 'mytopic' branch.

If the configuration variable branch.autosetupmerge is set to true, you
do not have to pass the --track option explicitly; further patches in
this series allow setting the variable with a "git remote add" option.
The configuration variable is off by default, and there is a --no-track
option to countermand it even if the variable is set.

Signed-off-by: Paolo Bonzini  <bonzini@gnu.org>
---
 Documentation/git-branch.txt   |    9 ++
 Documentation/git-checkout.txt |   15 ++++
 builtin-branch.c               |  128 ++++++++++++++++++++++++++++++++++++++---
 cache.h                        |    1 
 git-checkout.sh                |   17 +++--
 t/t3200-branch.sh              |   65 +++++++++++++++++---
 trace.c                        |   14 +---
 7 files changed, 214 insertions(+), 35 deletions(-)

	This is the hopefully final version.  I am now looking at all
	remote configs, not restricting the search to those in refs/remotes/
	(and this made the parsing simpler, so no sscanf anymore).
	I also gave up the per-remote config and I am using instead a
	global variable.

	This version also includes tests and changes to git-checkout.
	git-remote is no longer affected by the change.

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 3ea3b80..603f87f 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 [verse]
 'git-branch' [--color | --no-color] [-r | -a]
 	   [-v [--abbrev=<length> | --no-abbrev]]
-'git-branch' [-l] [-f] <branchname> [<start-point>]
+'git-branch' [--track | --no-track] [-l] [-f] <branchname> [<start-point>]
 'git-branch' (-m | -M) [<oldbranch>] <newbranch>
 'git-branch' (-d | -D) [-r] <branchname>...
 
@@ -26,6 +26,13 @@ It will start out with a head equal to the one given as <start-point>.
 If no <start-point> is given, the branch will be created with a head
 equal to that of the currently checked out branch.
 
+When a local branch is started off a remote branch, git can setup the
+branch so that gitlink:git-pull[1] will appropriately merge from that
+remote branch.  If this behavior is desired, it is possible to make it
+the default using the global `branch.autosetupmerge` configuration
+flag.  Otherwise, it can be chosen per-branch using the `--track`
+and `--no-track` options.
+
 With a '-m' or '-M' option, <oldbranch> will be renamed to <newbranch>.
 If <oldbranch> had a corresponding reflog, it is renamed to match
 <newbranch>, and a reflog entry is created to remember the branch
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 1ae77be..f5b2d50 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -8,7 +8,7 @@ git-checkout - Checkout and switch to a branch
 SYNOPSIS
 --------
 [verse]
-'git-checkout' [-q] [-f] [-b <new_branch> [-l]] [-m] [<branch>]
+'git-checkout' [-q] [-f] [-b [--track | --no-track] <new_branch> [-l]] [-m] [<branch>]
 'git-checkout' [<tree-ish>] <paths>...
 
 DESCRIPTION
@@ -18,7 +18,8 @@ When <paths> are not given, this command switches branches by
 updating the index and working tree to reflect the specified
 branch, <branch>, and updating HEAD to be <branch> or, if
 specified, <new_branch>.  Using -b will cause <new_branch> to
-be created.
+be created; in this case you can use the --track or --no-track
+options, which will be passed to `git branch`.
 
 When <paths> are given, this command does *not* switch
 branches.  It updates the named paths in the working tree from
@@ -45,6 +46,16 @@ OPTIONS
 	by gitlink:git-check-ref-format[1].  Some of these checks
 	may restrict the characters allowed in a branch name.
 
+--track::
+	When -b is given and a branch is created off a remote branch,
+	setup so that git-pull will automatically retrieve data from
+	the remote branch.
+
+--no-track::
+	When -b is given and a branch is created off a remote branch,
+	force that git-pull will automatically retrieve data from
+	the remote branch independent of the configuration settings.
+
 -l::
 	Create the new branch's ref log.  This activates recording of
 	all changes to made the branch ref, enabling use of date
diff --git a/builtin-branch.c b/builtin-branch.c
index d371849..34b1bbf 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -12,7 +12,7 @@
 #include "builtin.h"
 
 static const char builtin_branch_usage[] =
-  "git-branch [-r] (-d | -D) <branchname> | [-l] [-f] <branchname> [<start-point>] | (-m | -M) [<oldbranch>] <newbranch> | [--color | --no-color] [-r | -a] [-v [--abbrev=<length> | --no-abbrev]]";
+  "git-branch [-r] (-d | -D) <branchname> | [--track | --no-track] [-l] [-f] <branchname> [<start-point>] | (-m | -M) [<oldbranch>] <newbranch> | [--color | --no-color] [-r | -a] [-v [--abbrev=<length> | --no-abbrev]]";
 
 #define REF_UNKNOWN_TYPE    0x00
 #define REF_LOCAL_BRANCH    0x01
@@ -22,6 +22,8 @@ static const char builtin_branch_usage[] =
 static const char *head;
 static unsigned char head_sha1[20];
 
+static int branch_track_remotes;
+
 static int branch_use_color;
 static char branch_colors[][COLOR_MAXLEN] = {
 	"\033[m",	/* reset */
@@ -64,6 +66,9 @@ int git_branch_config(const char *var, const char *value)
 		color_parse(value, var, branch_colors[slot]);
 		return 0;
 	}
+	if (!strcmp(var, "branch.autosetupmerge"))
+		branch_track_remotes = git_config_bool(var, value);
+
 	return git_default_config(var, value);
 }
 
@@ -308,14 +313,102 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev)
 	free_ref_list(&ref_list);
 }
 
+static char *config_repo;
+static char *config_remote;
+static const char *start_ref;
+static int start_len;
+static int base_len;
+
+static int get_remote_branch_name(const char* value)
+{
+	const char *colon;
+	const char *end;
+
+	if (*value == '+')
+		value++;
+
+	colon = strchr(value, ':');
+	end = value + strlen(value);
+
+	/* Try an exact match first.  */
+	if (!strcmp(colon + 1, start_ref)) {
+		/* Truncate the value before the colon.  */
+		nfasprintf(&config_repo, "%.*s", colon - value, value);
+		return 1;
+	}
+
+	/* Try with a wildcard match now.  */
+	if (end - value > 2 && end[-2] == '/' && end[-1] == '*' &&
+	    colon - value > 2 && colon[-2] == '/' && colon[-1] == '*' &&
+	    (end - 2) - (colon + 1) == base_len &&
+	    !strncmp(colon + 1, start_ref, base_len)) {
+		/* Replace the star with the remote branch name.  */
+		nfasprintf(&config_repo, "%.*s%s",
+			   (colon - 2) - value, value,
+			   start_ref + base_len);
+		return 1;
+	}
+
+	return 0;
+}
+
+static int get_remote_config(const char* key, const char* value)
+{
+	const char *var;
+        if (prefixcmp(key, "remote."))
+		return 0;
+
+	var = strrchr(key, '.');
+	if (var == key + 6)
+		return 0;
+
+	if (!strcmp(var, ".fetch") && get_remote_branch_name(value))
+		nfasprintf(&config_remote, "%.*s", var - (key + 7), key + 7);
+
+        return 0;
+}
+
+static void set_branch_defaults(const char *name, const char *real_ref)
+{
+	char key[1024];
+	const char *slash = strrchr(real_ref, '/');
+
+	if (!slash)
+		return;
+
+	start_ref = real_ref;
+	start_len = strlen(real_ref);
+	base_len = slash - real_ref;
+	git_config(get_remote_config);
+
+	/* Change to != 0 to enable this feature by default.  */
+	if (config_repo && config_remote) {
+		if (snprintf(key, sizeof(key), "branch.%s.remote", name)
+		    > sizeof(key))
+			die("what a long branch name you have!");
+		git_config_set(key, config_remote);
+
+		snprintf(key, sizeof(key), "branch.%s.merge", name);
+		git_config_set(key, config_repo);
+
+		printf("Branch %s set up to track remote branch %s.\n",
+		       name, real_ref);
+	}
+
+	if (config_repo)
+		free(config_repo);
+	if (config_remote)
+		free(config_remote);
+}
+
 static void create_branch(const char *name, const char *start_name,
 			  unsigned char *start_sha1,
-			  int force, int reflog)
+			  int force, int reflog, int track)
 {
 	struct ref_lock *lock;
 	struct commit *commit;
 	unsigned char sha1[20];
-	char ref[PATH_MAX], msg[PATH_MAX + 20];
+	char *real_ref, ref[PATH_MAX], msg[PATH_MAX + 20];
 	int forcing = 0;
 
 	snprintf(ref, sizeof ref, "refs/heads/%s", name);
@@ -333,7 +426,9 @@ static void create_branch(const char *name, const char *start_name,
 	if (start_sha1)
 		/* detached HEAD */
 		hashcpy(sha1, start_sha1);
-	else if (get_sha1(start_name, sha1))
+	else if (dwim_ref(start_name, strlen(start_name), sha1, &real_ref) > 1)
+		die("Ambiguous object name: '%s'.", start_name);
+	else if (real_ref == NULL)
 		die("Not a valid object name: '%s'.", start_name);
 
 	if ((commit = lookup_commit_reference(sha1)) == NULL)
@@ -354,8 +449,17 @@ static void create_branch(const char *name, const char *start_name,
 		snprintf(msg, sizeof msg, "branch: Created from %s",
 			 start_name);
 
+	/* When branching off a remote branch, set up so that git-pull
+	   automatically merges from there.  So far, this is only done for
+	   remotes registered via .git/config.  */
+	if (real_ref && track)
+		set_branch_defaults(name, real_ref);
+
 	if (write_ref_sha1(lock, sha1, msg) < 0)
 		die("Failed to write ref: %s.", strerror(errno));
+
+	if (real_ref)
+		free(real_ref);
 }
 
 static void rename_branch(const char *oldname, const char *newname, int force)
@@ -397,11 +501,12 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	int delete = 0, force_delete = 0, force_create = 0;
 	int rename = 0, force_rename = 0;
 	int verbose = 0, abbrev = DEFAULT_ABBREV, detached = 0;
-	int reflog = 0;
+	int reflog = 0, track;
 	int kinds = REF_LOCAL_BRANCH;
 	int i;
 
 	git_config(git_branch_config);
+	track = branch_track_remotes;
 
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
@@ -412,6 +517,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			i++;
 			break;
 		}
+		if (!strcmp(arg, "--track")) {
+			track = 1;
+			continue;
+		}
+		if (!strcmp(arg, "--no-track")) {
+			track = 0;
+			continue;
+		}
 		if (!strcmp(arg, "-d")) {
 			delete = 1;
 			continue;
@@ -498,9 +611,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	else if (rename && (i == argc - 2))
 		rename_branch(argv[i], argv[i + 1], force_rename);
 	else if (i == argc - 1)
-		create_branch(argv[i], head, head_sha1, force_create, reflog);
+		create_branch(argv[i], head, head_sha1, force_create, reflog,
+			      track);
 	else if (i == argc - 2)
-		create_branch(argv[i], argv[i+1], NULL, force_create, reflog);
+		create_branch(argv[i], argv[i+1], NULL, force_create, reflog,
+			      track);
 	else
 		usage(builtin_branch_usage);
 
diff --git a/cache.h b/cache.h
index 4b5a754..cb4fbfd 100644
--- a/cache.h
+++ b/cache.h
@@ -480,6 +480,7 @@ extern struct tag *alloc_tag_node(void);
 extern void alloc_report(void);
 
 /* trace.c */
+extern int nfasprintf(char **str, const char *fmt, ...);
 extern int nfvasprintf(char **str, const char *fmt, va_list va);
 extern void trace_printf(const char *format, ...);
 extern void trace_argv_printf(const char **argv, int count, const char *format, ...);
diff --git a/git-checkout.sh b/git-checkout.sh
index 14835a4..6caa9fd 100755
--- a/git-checkout.sh
+++ b/git-checkout.sh
@@ -12,6 +12,7 @@ new=
 new_name=
 force=
 branch=
+track=
 newbranch=
 newbranch_log=
 merge=
@@ -33,7 +34,10 @@ while [ "$#" != "0" ]; do
 			die "git checkout: we do not like '$newbranch' as a branch name."
 		;;
 	"-l")
-		newbranch_log=1
+		newbranch_log=-l
+		;;
+	"--track"|"--no-track")
+		track="$arg"
 		;;
 	"-f")
 		force=1
@@ -85,6 +89,11 @@ while [ "$#" != "0" ]; do
     esac
 done
 
+case "$new_branch,$track" in
+,--*)
+	die "git checkout: --track and --no-track require -b"
+esac
+
 case "$force$merge" in
 11)
 	die "git checkout: -f and -m are incompatible"
@@ -235,11 +244,7 @@ fi
 #
 if [ "$?" -eq 0 ]; then
 	if [ "$newbranch" ]; then
-		if [ "$newbranch_log" ]; then
-			mkdir -p $(dirname "$GIT_DIR/logs/refs/heads/$newbranch")
-			touch "$GIT_DIR/logs/refs/heads/$newbranch"
-		fi
-		git-update-ref -m "checkout: Created from $new_name" "refs/heads/$newbranch" $new || exit
+		git-branch $track $newbranch_log "$newbranch" "$new_name" || exit
 		branch="$newbranch"
 	fi
 	if test -n "$branch"
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 5565c27..75c000a 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -47,17 +47,6 @@ test_expect_success \
 	 test ! -f .git/refs/heads/d/e/f &&
 	 test ! -f .git/logs/refs/heads/d/e/f'
 
-cat >expect <<EOF
-0000000000000000000000000000000000000000 $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	checkout: Created from master
-EOF
-test_expect_success \
-    'git checkout -b g/h/i -l should create a branch and a log' \
-	'GIT_COMMITTER_DATE="2005-05-26 23:30" \
-     git-checkout -b g/h/i -l master &&
-	 test -f .git/refs/heads/g/h/i &&
-	 test -f .git/logs/refs/heads/g/h/i &&
-	 diff expect .git/logs/refs/heads/g/h/i'
-
 test_expect_success \
     'git branch j/k should work after branch j has been deleted' \
        'git-branch j &&
@@ -117,4 +106,58 @@ test_expect_failure \
      ln -s real-u .git/logs/refs/heads/u &&
      git-branch -m u v'
 
+test_expect_success 'test tracking setup via --track' \
+    'git-config remote.local.url . &&
+     git-config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
+     (git-show-ref -q refs/remotes/local/master || git-fetch local) &&
+     git-branch --track my1 local/master &&
+     test $(git-config branch.my1.remote) = local &&
+     test $(git-config branch.my1.merge) = refs/heads/master'
+
+test_expect_success 'test tracking setup (non-wildcard, matching)' \
+    'git-config remote.local.url . &&
+     git-config remote.local.fetch refs/heads/master:refs/remotes/local/master &&
+     (git-show-ref -q refs/remotes/local/master || git-fetch local) &&
+     git-branch --track my4 local/master &&
+     test $(git-config branch.my4.remote) = local &&
+     test $(git-config branch.my4.merge) = refs/heads/master'
+
+test_expect_success 'test tracking setup (non-wildcard, not matching)' \
+    'git-config remote.local.url . &&
+     git-config remote.local.fetch refs/heads/s:refs/remotes/local/s &&
+     (git-show-ref -q refs/remotes/local/master || git-fetch local) &&
+     git-branch --track my5 local/master &&
+     ! test $(git-config branch.my5.remote) = local &&
+     ! test $(git-config branch.my5.merge) = refs/heads/master'
+
+test_expect_success 'test tracking setup via config' \
+    'git-config branch.autosetupmerge true &&
+     git-config remote.local.url . &&
+     git-config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
+     (git-show-ref -q refs/remotes/local/master || git-fetch local) &&
+     git-branch my3 local/master &&
+     test $(git-config branch.my3.remote) = local &&
+     test $(git-config branch.my3.merge) = refs/heads/master'
+
+test_expect_success 'test overriding tracking setup via --no-track' \
+    'git-config branch.autosetupmerge true &&
+     git-config remote.local.url . &&
+     git-config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
+     (git-show-ref -q refs/remotes/local/master || git-fetch local) &&
+     git-branch --no-track my2 local/master &&
+     ! test $(git-config branch.my2.remote) = local &&
+     ! test $(git-config branch.my2.merge) = refs/heads/master'
+
+# Keep this test last, as it changes the current branch
+cat >expect <<EOF
+0000000000000000000000000000000000000000 $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	branch: Created from master
+EOF
+test_expect_success \
+    'git checkout -b g/h/i -l should create a branch and a log' \
+	'GIT_COMMITTER_DATE="2005-05-26 23:30" \
+     git-checkout -b g/h/i -l master &&
+	 test -f .git/refs/heads/g/h/i &&
+	 test -f .git/logs/refs/heads/g/h/i &&
+	 diff expect .git/logs/refs/heads/g/h/i'
+
 test_done
diff --git a/trace.c b/trace.c
index 27fef86..1d4179d 100644
--- a/trace.c
+++ b/trace.c
@@ -26,14 +26,14 @@
 #include "quote.h"
 
 /* Stolen from "imap-send.c". */
-static int git_vasprintf(char **strp, const char *fmt, va_list ap)
+int nfvasprintf(char **strp, const char *fmt, va_list ap)
 {
 	int len;
 	char tmp[1024];
 
 	if ((len = vsnprintf(tmp, sizeof(tmp), fmt, ap)) < 0 ||
 	    !(*strp = xmalloc(len + 1)))
-		return -1;
+		die("Fatal: Out of memory\n");
 	if (len >= (int)sizeof(tmp))
 		vsprintf(*strp, fmt, ap);
 	else
@@ -41,13 +41,11 @@ static int git_vasprintf(char **strp, const char *fmt, va_list ap)
 	return len;
 }
 
-/* Stolen from "imap-send.c". */
-int nfvasprintf(char **str, const char *fmt, va_list va)
+int nfasprintf(char **str, const char *fmt, ...)
 {
-	int ret = git_vasprintf(str, fmt, va);
-	if (ret < 0)
-		die("Fatal: Out of memory\n");
-	return ret;
+	va_list args;
+	va_start(args, fmt);
+	return nfvasprintf(str, fmt, args);
 }
 
 /* Get a trace file descriptor from GIT_TRACE env variable. */

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH, final version] git-branch, git-checkout: autosetup for remote branch tracking
  2007-03-08  6:49 [PATCH, final version] git-branch, git-checkout: autosetup for remote branch tracking Paolo Bonzini
@ 2007-03-08  9:47 ` Junio C Hamano
  2007-03-08  9:58   ` [PATCH, nitpickingly " Paolo Bonzini
  0 siblings, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2007-03-08  9:47 UTC (permalink / raw)
  To: bonzini; +Cc: git, Johannes Schindelin

Paolo Bonzini <paolo.bonzini@lu.unisi.ch> writes:

> ...
> Signed-off-by: Paolo Bonzini  <bonzini@gnu.org>

Beautifully done, with even a sign-off ;-).

> @@ -45,6 +46,16 @@ OPTIONS
>  	by gitlink:git-check-ref-format[1].  Some of these checks
>  	may restrict the characters allowed in a branch name.
>  
> +--track::
> +	When -b is given and a branch is created off a remote branch,
> +	setup so that git-pull will automatically retrieve data from
> +	the remote branch.
> +
> +--no-track::
> +	When -b is given and a branch is created off a remote branch,
> +	force that git-pull will automatically retrieve data from
> +	the remote branch independent of the configuration settings.
> +
>  -l::
>  	Create the new branch's ref log.  This activates recording of
>  	all changes to made the branch ref, enabling use of date

Earlier I said "checkout -b" vs "checkout -B", but deciding the
tracking default with congiguration mechanism and overriding
with an additional --track/--no-track as you did here is way
superiour.  Very nice.

> diff --git a/builtin-branch.c b/builtin-branch.c
> index d371849..34b1bbf 100644
> --- a/builtin-branch.c
> +++ b/builtin-branch.c
> @@ -308,14 +313,102 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev)
> +static int get_remote_branch_name(const char* value)
> +{
> +	const char *colon;
> +	const char *end;
> +
> +	if (*value == '+')
> +		value++;
> +
> +	colon = strchr(value, ':');
> +	end = value + strlen(value);
> +
> +	/* Try an exact match first.  */
> +	if (!strcmp(colon + 1, start_ref)) {

Careful.  colon may be NULL here, in which case you would want
to return with 0 from this function before this strcmp.

You seem to declare variables with "const char *colon" but
function args with "const char* value".  Are they deliberately
done differently?  The existing code seems to favor asterisk
next to the variable name, not type name.

Other than that, I find this function and get_remote_config()
very nicely done.

> +static void set_branch_defaults(const char *name, const char *real_ref)
> +{
> +	char key[1024];
> +	const char *slash = strrchr(real_ref, '/');
> +
> +	if (!slash)
> +		return;
> +
> +	start_ref = real_ref;
> +	start_len = strlen(real_ref);
> +	base_len = slash - real_ref;
> +	git_config(get_remote_config);
> +
> +	/* Change to != 0 to enable this feature by default.  */
> +	if (config_repo && config_remote) {
> +		if (snprintf(key, sizeof(key), "branch.%s.remote", name)
> +		    > sizeof(key))
> +			die("what a long branch name you have!");
> +		git_config_set(key, config_remote);
> +
> +		snprintf(key, sizeof(key), "branch.%s.merge", name);
> +		git_config_set(key, config_repo);

This is nice, but you might want to have a comment that says why
you do not have to check the return value from the second
snprintf().  Otherwise, somebody less careful may update this
part of the code and swap the order of config_remote and
config_repo are set up without thinking in the future.

> diff --git a/trace.c b/trace.c
> index 27fef86..1d4179d 100644
> --- a/trace.c
> +++ b/trace.c
> @@ -41,13 +41,11 @@ static int git_vasprintf(char **strp, const char *fmt, va_list ap)
>  	return len;
>  }
>  
> +int nfasprintf(char **str, const char *fmt, ...)
>  {
> +	va_list args;
> +	va_start(args, fmt);
> +	return nfvasprintf(str, fmt, args);
>  }
>  
>  /* Get a trace file descriptor from GIT_TRACE env variable. */

It might not matter on common architectures in practice, but
va_start() must be matched by a corresponding va_end() in the
same function.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH, nitpickingly final version] git-branch, git-checkout: autosetup for remote branch tracking
  2007-03-08  9:47 ` Junio C Hamano
@ 2007-03-08  9:58   ` Paolo Bonzini
  2007-03-08 10:22     ` Junio C Hamano
  0 siblings, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2007-03-08  9:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: bonzini, git, Johannes Schindelin

In order to track and build on top of a branch 'topic' you track from
your upstream repository, you often would end up doing this sequence:

  git checkout -b mytopic origin/topic
  git config --add branch.mytopic.remote origin
  git config --add branch.mytopic.merge refs/heads/topic

This would first fork your own 'mytopic' branch from the 'topic'
branch you track from the 'origin' repository; then it would set up two
configuration variables so that 'git pull' without parameters does the
right thing while you are on your own 'mytopic' branch.

This commit adds a --track option to git-branch, so that "git
branch --track mytopic origin/topic" performs the latter two actions
when creating your 'mytopic' branch.

If the configuration variable branch.autosetupmerge is set to true, you
do not have to pass the --track option explicitly; further patches in
this series allow setting the variable with a "git remote add" option.
The configuration variable is off by default, and there is a --no-track
option to countermand it even if the variable is set.

Signed-off-by: Paolo Bonzini  <bonzini@gnu.org>
---
 Documentation/git-branch.txt   |    9 ++
 Documentation/git-checkout.txt |   15 +++-
 builtin-branch.c               |  137 ++++++++++++++++++++++++++++++++++++++---
 cache.h                        |    1
 git-checkout.sh                |   17 +++--
 t/t3200-branch.sh              |   65 ++++++++++++++++---
 trace.c                        |   18 ++---
 7 files changed, 226 insertions(+), 36 deletions(-)

	This fixes all the nits you pointed out. :-D

	Now, this was an experience to make...


diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 3ea3b80..603f87f 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 [verse]
 'git-branch' [--color | --no-color] [-r | -a]
 	   [-v [--abbrev=<length> | --no-abbrev]]
-'git-branch' [-l] [-f] <branchname> [<start-point>]
+'git-branch' [--track | --no-track] [-l] [-f] <branchname> [<start-point>]
 'git-branch' (-m | -M) [<oldbranch>] <newbranch>
 'git-branch' (-d | -D) [-r] <branchname>...
 
@@ -26,6 +26,13 @@ It will start out with a head equal to the one given as <start-point>.
 If no <start-point> is given, the branch will be created with a head
 equal to that of the currently checked out branch.
 
+When a local branch is started off a remote branch, git can setup the
+branch so that gitlink:git-pull[1] will appropriately merge from that
+remote branch.  If this behavior is desired, it is possible to make it
+the default using the global `branch.autosetupmerge` configuration
+flag.  Otherwise, it can be chosen per-branch using the `--track`
+and `--no-track` options.
+
 With a '-m' or '-M' option, <oldbranch> will be renamed to <newbranch>.
 If <oldbranch> had a corresponding reflog, it is renamed to match
 <newbranch>, and a reflog entry is created to remember the branch
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 1ae77be..f5b2d50 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -8,7 +8,7 @@ git-checkout - Checkout and switch to a branch
 SYNOPSIS
 --------
 [verse]
-'git-checkout' [-q] [-f] [-b <new_branch> [-l]] [-m] [<branch>]
+'git-checkout' [-q] [-f] [-b [--track | --no-track] <new_branch> [-l]] [-m] [<branch>]
 'git-checkout' [<tree-ish>] <paths>...
 
 DESCRIPTION
@@ -18,7 +18,8 @@ When <paths> are not given, this command switches branches by
 updating the index and working tree to reflect the specified
 branch, <branch>, and updating HEAD to be <branch> or, if
 specified, <new_branch>.  Using -b will cause <new_branch> to
-be created.
+be created; in this case you can use the --track or --no-track
+options, which will be passed to `git branch`.
 
 When <paths> are given, this command does *not* switch
 branches.  It updates the named paths in the working tree from
@@ -45,6 +46,16 @@ OPTIONS
 	by gitlink:git-check-ref-format[1].  Some of these checks
 	may restrict the characters allowed in a branch name.
 
+--track::
+	When -b is given and a branch is created off a remote branch,
+	setup so that git-pull will automatically retrieve data from
+	the remote branch.
+
+--no-track::
+	When -b is given and a branch is created off a remote branch,
+	force that git-pull will automatically retrieve data from
+	the remote branch independent of the configuration settings.
+
 -l::
 	Create the new branch's ref log.  This activates recording of
 	all changes to made the branch ref, enabling use of date
diff --git a/builtin-branch.c b/builtin-branch.c
index d371849..43140c4 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -12,7 +12,7 @@
 #include "builtin.h"
 
 static const char builtin_branch_usage[] =
-  "git-branch [-r] (-d | -D) <branchname> | [-l] [-f] <branchname> [<start-point>] | (-m | -M) [<oldbranch>] <newbranch> | [--color | --no-color] [-r | -a] [-v [--abbrev=<length> | --no-abbrev]]";
+  "git-branch [-r] (-d | -D) <branchname> | [--track | --no-track] [-l] [-f] <branchname> [<start-point>] | (-m | -M) [<oldbranch>] <newbranch> | [--color | --no-color] [-r | -a] [-v [--abbrev=<length> | --no-abbrev]]";
 
 #define REF_UNKNOWN_TYPE    0x00
 #define REF_LOCAL_BRANCH    0x01
@@ -22,6 +22,8 @@ static const char builtin_branch_usage[] =
 static const char *head;
 static unsigned char head_sha1[20];
 
+static int branch_track_remotes;
+
 static int branch_use_color;
 static char branch_colors[][COLOR_MAXLEN] = {
 	"\033[m",	/* reset */
@@ -64,6 +66,9 @@ int git_branch_config(const char *var, const char *value)
 		color_parse(value, var, branch_colors[slot]);
 		return 0;
 	}
+	if (!strcmp(var, "branch.autosetupmerge"))
+		branch_track_remotes = git_config_bool(var, value);
+
 	return git_default_config(var, value);
 }
 
@@ -308,14 +313,107 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev)
 	free_ref_list(&ref_list);
 }
 
+static char *config_repo;
+static char *config_remote;
+static const char *start_ref;
+static int start_len;
+static int base_len;
+
+static int get_remote_branch_name(const char *value)
+{
+	const char *colon;
+	const char *end;
+
+	if (*value == '+')
+		value++;
+
+	colon = strchr(value, ':');
+        if (!colon)
+		return 0;
+
+	end = value + strlen(value);
+
+	/* Try an exact match first.  */
+	if (!strcmp(colon + 1, start_ref)) {
+		/* Truncate the value before the colon.  */
+		nfasprintf(&config_repo, "%.*s", colon - value, value);
+		return 1;
+	}
+
+	/* Try with a wildcard match now.  */
+	if (end - value > 2 && end[-2] == '/' && end[-1] == '*' &&
+	    colon - value > 2 && colon[-2] == '/' && colon[-1] == '*' &&
+	    (end - 2) - (colon + 1) == base_len &&
+	    !strncmp(colon + 1, start_ref, base_len)) {
+		/* Replace the star with the remote branch name.  */
+		nfasprintf(&config_repo, "%.*s%s",
+			   (colon - 2) - value, value,
+			   start_ref + base_len);
+		return 1;
+	}
+
+	return 0;
+}
+
+static int get_remote_config(const char *key, const char *value)
+{
+	const char *var;
+        if (prefixcmp(key, "remote."))
+		return 0;
+
+	var = strrchr(key, '.');
+	if (var == key + 6)
+		return 0;
+
+	if (!strcmp(var, ".fetch") && get_remote_branch_name(value))
+		nfasprintf(&config_remote, "%.*s", var - (key + 7), key + 7);
+
+        return 0;
+}
+
+static void set_branch_defaults(const char *name, const char *real_ref)
+{
+	char key[1024];
+	const char *slash = strrchr(real_ref, '/');
+
+	if (!slash)
+		return;
+
+	start_ref = real_ref;
+	start_len = strlen(real_ref);
+	base_len = slash - real_ref;
+	git_config(get_remote_config);
+
+	/* Change to != 0 to enable this feature by default.  */
+	if (config_repo && config_remote) {
+		if (snprintf(key, sizeof(key), "branch.%s.remote", name)
+		    > sizeof(key))
+			die("what a long branch name you have!");
+		git_config_set(key, config_remote);
+
+		/* We must have space for the key, since it's shorter than
+		   the previous one.  */
+		snprintf(key, sizeof(key), "branch.%s.merge", name);
+		git_config_set(key, config_repo);
+
+		printf("Branch %s set up to track remote branch %s.\n",
+		       name, real_ref);
+	}
+
+	if (config_repo)
+		free(config_repo);
+	if (config_remote)
+		free(config_remote);
+}
+
 static void create_branch(const char *name, const char *start_name,
 			  unsigned char *start_sha1,
-			  int force, int reflog)
+			  int force, int reflog, int track)
 {
 	struct ref_lock *lock;
 	struct commit *commit;
 	unsigned char sha1[20];
-	char ref[PATH_MAX], msg[PATH_MAX + 20];
+	char *real_ref, ref[PATH_MAX], msg[PATH_MAX + 20];
 	int forcing = 0;
 
 	snprintf(ref, sizeof ref, "refs/heads/%s", name);
@@ -330,10 +428,13 @@ static void create_branch(const char *name, const char *start_name,
 		forcing = 1;
 	}
 
-	if (start_sha1)
+	if (start_sha1) {
 		/* detached HEAD */
 		hashcpy(sha1, start_sha1);
-	else if (get_sha1(start_name, sha1))
+		real_ref = NULL;
+	} else if (dwim_ref(start_name, strlen(start_name), sha1, &real_ref) > 1)
+		die("Ambiguous object name: '%s'.", start_name);
+	else if (real_ref == NULL)
 		die("Not a valid object name: '%s'.", start_name);
 
 	if ((commit = lookup_commit_reference(sha1)) == NULL)
@@ -354,8 +455,17 @@ static void create_branch(const char *name, const char *start_name,
 		snprintf(msg, sizeof msg, "branch: Created from %s",
 			 start_name);
 
+	/* When branching off a remote branch, set up so that git-pull
+	   automatically merges from there.  So far, this is only done for
+	   remotes registered via .git/config.  */
+	if (real_ref && track)
+		set_branch_defaults(name, real_ref);
+
 	if (write_ref_sha1(lock, sha1, msg) < 0)
 		die("Failed to write ref: %s.", strerror(errno));
+
+	if (real_ref)
+		free(real_ref);
 }
 
 static void rename_branch(const char *oldname, const char *newname, int force)
@@ -397,11 +507,12 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	int delete = 0, force_delete = 0, force_create = 0;
 	int rename = 0, force_rename = 0;
 	int verbose = 0, abbrev = DEFAULT_ABBREV, detached = 0;
-	int reflog = 0;
+	int reflog = 0, track;
 	int kinds = REF_LOCAL_BRANCH;
 	int i;
 
 	git_config(git_branch_config);
+	track = branch_track_remotes;
 
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
@@ -412,6 +523,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			i++;
 			break;
 		}
+		if (!strcmp(arg, "--track")) {
+			track = 1;
+			continue;
+		}
+		if (!strcmp(arg, "--no-track")) {
+			track = 0;
+			continue;
+		}
 		if (!strcmp(arg, "-d")) {
 			delete = 1;
 			continue;
@@ -498,9 +617,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	else if (rename && (i == argc - 2))
 		rename_branch(argv[i], argv[i + 1], force_rename);
 	else if (i == argc - 1)
-		create_branch(argv[i], head, head_sha1, force_create, reflog);
+		create_branch(argv[i], head, head_sha1, force_create, reflog,
+			      track);
 	else if (i == argc - 2)
-		create_branch(argv[i], argv[i+1], NULL, force_create, reflog);
+		create_branch(argv[i], argv[i+1], NULL, force_create, reflog,
+			      track);
 	else
 		usage(builtin_branch_usage);
 
diff --git a/cache.h b/cache.h
index 4b5a754..cb4fbfd 100644
--- a/cache.h
+++ b/cache.h
@@ -480,6 +480,7 @@ extern struct tag *alloc_tag_node(void);
 extern void alloc_report(void);
 
 /* trace.c */
+extern int nfasprintf(char **str, const char *fmt, ...);
 extern int nfvasprintf(char **str, const char *fmt, va_list va);
 extern void trace_printf(const char *format, ...);
 extern void trace_argv_printf(const char **argv, int count, const char *format, ...);
diff --git a/git-checkout.sh b/git-checkout.sh
index 14835a4..6caa9fd 100755
--- a/git-checkout.sh
+++ b/git-checkout.sh
@@ -12,6 +12,7 @@ new=
 new_name=
 force=
 branch=
+track=
 newbranch=
 newbranch_log=
 merge=
@@ -33,7 +34,10 @@ while [ "$#" != "0" ]; do
 			die "git checkout: we do not like '$newbranch' as a branch name."
 		;;
 	"-l")
-		newbranch_log=1
+		newbranch_log=-l
+		;;
+	"--track"|"--no-track")
+		track="$arg"
 		;;
 	"-f")
 		force=1
@@ -85,6 +89,11 @@ while [ "$#" != "0" ]; do
     esac
 done
 
+case "$new_branch,$track" in
+,--*)
+	die "git checkout: --track and --no-track require -b"
+esac
+
 case "$force$merge" in
 11)
 	die "git checkout: -f and -m are incompatible"
@@ -235,11 +244,7 @@ fi
 #
 if [ "$?" -eq 0 ]; then
 	if [ "$newbranch" ]; then
-		if [ "$newbranch_log" ]; then
-			mkdir -p $(dirname "$GIT_DIR/logs/refs/heads/$newbranch")
-			touch "$GIT_DIR/logs/refs/heads/$newbranch"
-		fi
-		git-update-ref -m "checkout: Created from $new_name" "refs/heads/$newbranch" $new || exit
+		git-branch $track $newbranch_log "$newbranch" "$new_name" || exit
 		branch="$newbranch"
 	fi
 	if test -n "$branch"
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 5565c27..75c000a 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -47,17 +47,6 @@ test_expect_success \
 	 test ! -f .git/refs/heads/d/e/f &&
 	 test ! -f .git/logs/refs/heads/d/e/f'
 
-cat >expect <<EOF
-0000000000000000000000000000000000000000 $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	checkout: Created from master
-EOF
-test_expect_success \
-    'git checkout -b g/h/i -l should create a branch and a log' \
-	'GIT_COMMITTER_DATE="2005-05-26 23:30" \
-     git-checkout -b g/h/i -l master &&
-	 test -f .git/refs/heads/g/h/i &&
-	 test -f .git/logs/refs/heads/g/h/i &&
-	 diff expect .git/logs/refs/heads/g/h/i'
-
 test_expect_success \
     'git branch j/k should work after branch j has been deleted' \
        'git-branch j &&
@@ -117,4 +106,58 @@ test_expect_failure \
      ln -s real-u .git/logs/refs/heads/u &&
      git-branch -m u v'
 
+test_expect_success 'test tracking setup via --track' \
+    'git-config remote.local.url . &&
+     git-config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
+     (git-show-ref -q refs/remotes/local/master || git-fetch local) &&
+     git-branch --track my1 local/master &&
+     test $(git-config branch.my1.remote) = local &&
+     test $(git-config branch.my1.merge) = refs/heads/master'
+
+test_expect_success 'test tracking setup (non-wildcard, matching)' \
+    'git-config remote.local.url . &&
+     git-config remote.local.fetch refs/heads/master:refs/remotes/local/master &&
+     (git-show-ref -q refs/remotes/local/master || git-fetch local) &&
+     git-branch --track my4 local/master &&
+     test $(git-config branch.my4.remote) = local &&
+     test $(git-config branch.my4.merge) = refs/heads/master'
+
+test_expect_success 'test tracking setup (non-wildcard, not matching)' \
+    'git-config remote.local.url . &&
+     git-config remote.local.fetch refs/heads/s:refs/remotes/local/s &&
+     (git-show-ref -q refs/remotes/local/master || git-fetch local) &&
+     git-branch --track my5 local/master &&
+     ! test $(git-config branch.my5.remote) = local &&
+     ! test $(git-config branch.my5.merge) = refs/heads/master'
+
+test_expect_success 'test tracking setup via config' \
+    'git-config branch.autosetupmerge true &&
+     git-config remote.local.url . &&
+     git-config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
+     (git-show-ref -q refs/remotes/local/master || git-fetch local) &&
+     git-branch my3 local/master &&
+     test $(git-config branch.my3.remote) = local &&
+     test $(git-config branch.my3.merge) = refs/heads/master'
+
+test_expect_success 'test overriding tracking setup via --no-track' \
+    'git-config branch.autosetupmerge true &&
+     git-config remote.local.url . &&
+     git-config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
+     (git-show-ref -q refs/remotes/local/master || git-fetch local) &&
+     git-branch --no-track my2 local/master &&
+     ! test $(git-config branch.my2.remote) = local &&
+     ! test $(git-config branch.my2.merge) = refs/heads/master'
+
+# Keep this test last, as it changes the current branch
+cat >expect <<EOF
+0000000000000000000000000000000000000000 $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	branch: Created from master
+EOF
+test_expect_success \
+    'git checkout -b g/h/i -l should create a branch and a log' \
+	'GIT_COMMITTER_DATE="2005-05-26 23:30" \
+     git-checkout -b g/h/i -l master &&
+	 test -f .git/refs/heads/g/h/i &&
+	 test -f .git/logs/refs/heads/g/h/i &&
+	 diff expect .git/logs/refs/heads/g/h/i'
+
 test_done
diff --git a/trace.c b/trace.c
index 27fef86..27b2e85 100644
--- a/trace.c
+++ b/trace.c
@@ -26,14 +26,14 @@
 #include "quote.h"
 
 /* Stolen from "imap-send.c". */
-static int git_vasprintf(char **strp, const char *fmt, va_list ap)
+int nfvasprintf(char **strp, const char *fmt, va_list ap)
 {
 	int len;
 	char tmp[1024];
 
 	if ((len = vsnprintf(tmp, sizeof(tmp), fmt, ap)) < 0 ||
 	    !(*strp = xmalloc(len + 1)))
-		return -1;
+		die("Fatal: Out of memory\n");
 	if (len >= (int)sizeof(tmp))
 		vsprintf(*strp, fmt, ap);
 	else
@@ -41,13 +41,15 @@ static int git_vasprintf(char **strp, const char *fmt, va_list ap)
 	return len;
 }
 
-/* Stolen from "imap-send.c". */
-int nfvasprintf(char **str, const char *fmt, va_list va)
+int nfasprintf(char **str, const char *fmt, ...)
 {
-	int ret = git_vasprintf(str, fmt, va);
-	if (ret < 0)
-		die("Fatal: Out of memory\n");
-	return ret;
+	int rc;
+	va_list args;
+
+	va_start(args, fmt);
+	rc = nfvasprintf(str, fmt, args);
+	va_end(args);
+	return args;
 }
 
 /* Get a trace file descriptor from GIT_TRACE env variable. */

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH, nitpickingly final version] git-branch, git-checkout: autosetup for remote branch tracking
  2007-03-08  9:58   ` [PATCH, nitpickingly " Paolo Bonzini
@ 2007-03-08 10:22     ` Junio C Hamano
  2007-03-08 21:59       ` Junio C Hamano
  0 siblings, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2007-03-08 10:22 UTC (permalink / raw)
  To: bonzini; +Cc: git, Johannes Schindelin

Paolo Bonzini <paolo.bonzini@lu.unisi.ch> writes:

> 	This fixes all the nits you pointed out. :-D
> 	Now, this was an experience to make...

Thanks.

Will apply after reviewing once more tomorrow, with fix-ups
locally if needed.  No need to resend.

> +
> +	colon = strchr(value, ':');
> +        if (!colon)

s/^\040{8}/\011/;

> diff --git a/trace.c b/trace.c
> index 27fef86..27b2e85 100644
> --- a/trace.c
> +++ b/trace.c
> @@ -41,13 +41,15 @@ static in...
> +int nfasprintf(char **str, const char *fmt, ...)
>  {
> +	int rc;
> +	va_list args;
> +
> +	va_start(args, fmt);
> +	rc = nfvasprintf(str, fmt, args);
> +	va_end(args);
> +	return args;
>  }

s/return args/return rc/;

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH, nitpickingly final version] git-branch, git-checkout: autosetup for remote branch tracking
  2007-03-08 10:22     ` Junio C Hamano
@ 2007-03-08 21:59       ` Junio C Hamano
  2007-03-08 22:10         ` [PATCH] git-branch: rename nor delete should not work when HEAD is detached Junio C Hamano
  2007-03-10  0:59         ` [PATCH, nitpickingly final version] git-branch, git-checkout: autosetup for remote branch tracking Junio C Hamano
  0 siblings, 2 replies; 7+ messages in thread
From: Junio C Hamano @ 2007-03-08 21:59 UTC (permalink / raw)
  To: bonzini; +Cc: git, Johannes Schindelin

Junio C Hamano <junkio@cox.net> writes:

> Paolo Bonzini <paolo.bonzini@lu.unisi.ch> writes:
>
>> 	This fixes all the nits you pointed out. :-D
>> 	Now, this was an experience to make...
>
> Thanks.
>
> Will apply after reviewing once more tomorrow, with fix-ups
> locally if needed.  No need to resend.

Gaah.

The new create_branch() is totally borked, and I did not notice
it while exchanging review e-mails.

The problem is that we have:

	if (real_ref == NULL)
        	die();

which is completely bogus.  We do "checkout -b temp v2.6.20~15"
all the time, and in that case dwim_ref(v2.6.20~15) rightfully
would say "nope, that's not a branch tip".  We have no right to
die there.

I think your new tests should have caught this kind of breakage,
but now I notice that there is no such test that makes sure that
no auto set-up is made when the original branch is not a remote
tracking branch (your new tests are only interested in testing
to see if the new feature works, and it does not test that there
is no regression to old usage).  The breakage was caught by an
unrelated test, t3900.

The appended is on top of your "final".  This is what I am
considering to apply currently.


Grumble....

---

diff --git a/builtin-branch.c b/builtin-branch.c
index 011a2fc..1d6f0cb 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -409,7 +409,6 @@ static void set_branch_defaults(const char *name, const char *real_ref)
 }
 
 static void create_branch(const char *name, const char *start_name,
-			  unsigned char *start_sha1,
 			  int force, int reflog, int track)
 {
 	struct ref_lock *lock;
@@ -430,15 +429,22 @@ static void create_branch(const char *name, const char *start_name,
 		forcing = 1;
 	}
 
-	if (start_sha1) {
-		/* detached HEAD */
-		hashcpy(sha1, start_sha1);
+	real_ref = NULL;
+	if (get_sha1(start_name, sha1))
+		die("Not a valid object name: '%s'.", start_name);
+
+	switch (dwim_ref(start_name, strlen(start_name), sha1, &real_ref)) {
+	case 0:
+		/* Not branching from any existing branch */
 		real_ref = NULL;
-	}
-	else if (dwim_ref(start_name, strlen(start_name), sha1, &real_ref) > 1)
+		break;
+	case 1:
+		/* Unique completion -- good */
+		break;
+	default:
 		die("Ambiguous object name: '%s'.", start_name);
-	else if (real_ref == NULL)
-		die("Not a valid object name: '%s'.", start_name);
+		break;
+	}
 
 	if ((commit = lookup_commit_reference(sha1)) == NULL)
 		die("Not a valid branch point: '%s'.", start_name);
@@ -619,12 +625,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		rename_branch(head, argv[i], force_rename);
 	else if (rename && (i == argc - 2))
 		rename_branch(argv[i], argv[i + 1], force_rename);
-	else if (i == argc - 1)
-		create_branch(argv[i], head, head_sha1, force_create, reflog,
-			      track);
-	else if (i == argc - 2)
-		create_branch(argv[i], argv[i+1], NULL, force_create, reflog,
-			      track);
+	else if (i == argc - 1 || i == argc - 2)
+		create_branch(argv[i], (i == argc - 2) ? argv[i+1] : head,
+			      force_create, reflog, track);
 	else
 		usage(builtin_branch_usage);
 

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH] git-branch: rename nor delete should not work when HEAD is detached.
  2007-03-08 21:59       ` Junio C Hamano
@ 2007-03-08 22:10         ` Junio C Hamano
  2007-03-10  0:59         ` [PATCH, nitpickingly final version] git-branch, git-checkout: autosetup for remote branch tracking Junio C Hamano
  1 sibling, 0 replies; 7+ messages in thread
From: Junio C Hamano @ 2007-03-08 22:10 UTC (permalink / raw)
  To: git; +Cc: Lars Hjemli, Johannes Schindelin, bonzini

While reviewing the latest round of 'branch --track' changes, I
noticed that we do not do anything special when renaming or
deleting a detached HEAD.  We do not have anything to rename nor
delete in this case, so instead of attempting to rename/delete
refs/heads/HEAD, we should error out.

---
 * CC'ed are not guilty parties, but people who might know the
   code better than others.

diff --git a/builtin-branch.c b/builtin-branch.c
index 06d8a8c..28d4b71 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -483,6 +483,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		die("Failed to resolve HEAD as a valid ref.");
 	if (!strcmp(head, "HEAD")) {
 		detached = 1;
+		if (rename || delete)
+			die("Cannot rename nor delete a detached HEAD");
 	}
 	else {
 		if (prefixcmp(head, "refs/heads/"))

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH, nitpickingly final version] git-branch, git-checkout: autosetup for remote branch tracking
  2007-03-08 21:59       ` Junio C Hamano
  2007-03-08 22:10         ` [PATCH] git-branch: rename nor delete should not work when HEAD is detached Junio C Hamano
@ 2007-03-10  0:59         ` Junio C Hamano
  1 sibling, 0 replies; 7+ messages in thread
From: Junio C Hamano @ 2007-03-10  0:59 UTC (permalink / raw)
  To: git; +Cc: bonzini, Johannes Schindelin

Junio C Hamano <junkio@cox.net> writes:

> Gaah.
>
> The new create_branch() is totally borked, and I did not notice
> it while exchanging review e-mails.
> ...
> The appended is on top of your "final".  This is what I am
> considering to apply currently.
>
> Grumble....

I _like_ this feature.  I even think it _might_ make sense to
turn it on by default --- I would definitely argue for it if
this were before 1.5.0 which we have enough time to give advance
warning to users that the defaults and UIs would change, and the
only reason I am saying "it _might_" is because it _is_ a change
and it is post 1.5.0.

But because I had to say Gaah, I am currently in "discouraged"
state.  Extra eyeballing from the list, tests and Acks are
greatly appreciated on this series from Paolo.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2007-03-10  0:59 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-03-08  6:49 [PATCH, final version] git-branch, git-checkout: autosetup for remote branch tracking Paolo Bonzini
2007-03-08  9:47 ` Junio C Hamano
2007-03-08  9:58   ` [PATCH, nitpickingly " Paolo Bonzini
2007-03-08 10:22     ` Junio C Hamano
2007-03-08 21:59       ` Junio C Hamano
2007-03-08 22:10         ` [PATCH] git-branch: rename nor delete should not work when HEAD is detached Junio C Hamano
2007-03-10  0:59         ` [PATCH, nitpickingly final version] git-branch, git-checkout: autosetup for remote branch tracking Junio C Hamano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.