branch_get() can return NULL (so far on detached HEAD only) but some
code paths in builtin/branch.c cannot deal with that and cause
segfaults.
While at there, make sure to bail out when the user gives 2 or more
branches with --set-upstream-to or --unset-upstream, where only the
first branch is processed and the rest silently dropped.
Reported-by: Per Cederqvist ced...@opera.com
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
On Sat, Feb 23, 2013 at 3:27 AM, Junio C Hamano gits...@pobox.com wrote:
Instead of asking Is it detached?, perhaps we can say something
like You told me to set the upstream of HEAD to branch X, but in
front? At least, that will be a better explanation for the reason
why the operation is failing.
Fixed.
What you can do is to have a single helper function that can explain
why branch_get() returned NULL (or extend branch_get() to serve that
purpose as well); then you do not have to duplicate the logic twice
on the caller's side (and there may be other callers that want to do
the same).
The explanation mentions about the failed operation, which makes a
helper less useful. We could still do the helper, but it may lead to
i18n legos. So no helper in this version.
The existing test might be wrong, by the way. Your HEAD may point
at a branch Y but you may not have any commit on it yet, and you may
want to allow setting the upstream of that to-be-born branch to
another branch X with branch --set-upstream-to=X [Y|HEAD].
It sounds complicated. I think we can revisit it when a user actually
complains about it.
builtin/branch.c | 27 +++
t/t3200-branch.sh | 21 +
2 files changed, 48 insertions(+)
diff --git a/builtin/branch.c b/builtin/branch.c
index 6371bf9..00d17d2 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -889,6 +889,17 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
} else if (new_upstream) {
struct branch *branch = branch_get(argv[0]);
+ if (argc 1)
+ die(_(too many branches to set new upstream));
+
+ if (!branch) {
+ if (!argc || !strcmp(argv[0], HEAD))
+ die(_(could not set upstream of HEAD to %s
when
+ it does not point to any branch.),
+ new_upstream);
+ die(_(no such branch '%s'), argv[0]);
+ }
+
if (!ref_exists(branch-refname))
die(_(branch '%s' does not exist), branch-name);
@@ -901,6 +912,16 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
struct branch *branch = branch_get(argv[0]);
struct strbuf buf = STRBUF_INIT;
+ if (argc 1)
+ die(_(too many branches to unset upstream));
+
+ if (!branch) {
+ if (!argc || !strcmp(argv[0], HEAD))
+ die(_(could not unset upstream of HEAD when
+ it does not point to any branch.));
+ die(_(no such branch '%s'), argv[0]);
+ }
+
if (!branch_has_merge_config(branch)) {
die(_(Branch '%s' has no upstream information),
branch-name);
}
@@ -916,6 +937,12 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
int branch_existed = 0, remote_tracking = 0;
struct strbuf buf = STRBUF_INIT;
+ if (!strcmp(argv[0], HEAD))
+ die(_(it does not make sense to create 'HEAD'
manually));
+
+ if (!branch)
+ die(_(no such branch '%s'), argv[0]);
+
if (kinds != REF_LOCAL_BRANCH)
die(_(-a and -r options to 'git branch' do not make
sense with a branch name));
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index f3e0e4a..12f1e4a 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -42,6 +42,10 @@ test_expect_success \
'git branch a/b/c should create a branch' \
'git branch a/b/c test_path_is_file .git/refs/heads/a/b/c'
+test_expect_success \
+'git branch HEAD should fail' \
+'test_must_fail git branch HEAD'
+
cat expect EOF
$_z40 $HEAD $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL 1117150200 +
branch: Created from master
EOF
@@ -388,6 +392,14 @@ test_expect_success \
'git tag foobar
test_must_fail git branch --track my11 foobar'
+test_expect_success '--set-upstream-to fails on multiple branches' \
+'test_must_fail git branch --set-upstream-to master a b c'
+
+test_expect_success '--set-upstream-to fails on detached HEAD' \
+'git checkout HEAD^{}
+ test_must_fail git branch --set-upstream-to master
+ git checkout -'
+
test_expect_success 'use --set-upstream-to
