* [PATCH] branch: error copying or renaming a detached HEAD
@ 2022-10-25 23:01 Rubén Justo
2022-10-25 23:50 ` Junio C Hamano
0 siblings, 1 reply; 2+ messages in thread
From: Rubén Justo @ 2022-10-25 23:01 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano
In c847f53712 (Detached HEAD (experimental), 2007-01-01) an error
condition was introduced in rename_branch() to prevent renaming, later
also copying, a detached HEAD.
The condition used was checking for NULL in oldname, the source branch
to rename/copy. That condition cannot be satisfied because if no source
branch is specified, HEAD is going to be used in the call.
The error issued instead is:
fatal: Invalid branch name: 'HEAD'
Let's remove the condition in copy_or_rename_branch() (the current
function name) and check for HEAD before calling it, dying with the
original intended error if we're in a detached HEAD.
Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
builtin/branch.c | 28 ++++++++--------------------
t/t3200-branch.sh | 11 +++++++++++
2 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 407517ba68..c964ac7bb4 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -520,13 +520,6 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
const char *interpreted_newname = NULL;
int recovery = 0;
- if (!oldname) {
- if (copy)
- die(_("cannot copy the current branch while not on any."));
- else
- die(_("cannot rename the current branch while not on any."));
- }
-
if (strbuf_check_branch_ref(&oldref, oldname)) {
/*
* Bad name --- this could be an attempt to rename a
@@ -827,24 +820,19 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
strbuf_release(&buf);
return ret;
- } else if (copy) {
- if (!argc)
- die(_("branch name required"));
- else if (argc == 1)
- copy_or_rename_branch(head, argv[0], 1, copy > 1);
- else if (argc == 2)
- copy_or_rename_branch(argv[0], argv[1], 1, copy > 1);
- else
- die(_("too many branches for a copy operation"));
- } else if (rename) {
+ } else if (copy || rename) {
if (!argc)
die(_("branch name required"));
+ else if ((argc == 1) && filter.detached)
+ die(copy? _("cannot copy the current branch while not on any.")
+ : _("cannot rename the current branch while not on any."));
else if (argc == 1)
- copy_or_rename_branch(head, argv[0], 0, rename > 1);
+ copy_or_rename_branch(head, argv[0], copy, copy + rename > 1);
else if (argc == 2)
- copy_or_rename_branch(argv[0], argv[1], 0, rename > 1);
+ copy_or_rename_branch(argv[0], argv[1], copy, copy + rename > 1);
else
- die(_("too many arguments for a rename operation"));
+ die(copy? _("too many branches for a copy operation")
+ : _("too many arguments for a rename operation"));
} else if (new_upstream) {
struct branch *branch;
struct strbuf buf = STRBUF_INIT;
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 7d8edff9c3..38c57de71b 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -268,6 +268,17 @@ test_expect_success 'git branch -M topic topic should work when main is checked
git branch -M topic topic
'
+test_expect_success 'git branch -M and -C fail on detached HEAD' '
+ git checkout HEAD^{} &&
+ test_when_finished git checkout - &&
+ echo "fatal: cannot rename the current branch while not on any." >expect &&
+ test_must_fail git branch -M must-fail 2>err &&
+ test_cmp expect err &&
+ echo "fatal: cannot copy the current branch while not on any." >expect &&
+ test_must_fail git branch -C must-fail 2>err &&
+ test_cmp expect err
+'
+
test_expect_success 'git branch -v -d t should work' '
git branch t &&
git rev-parse --verify refs/heads/t &&
--
2.36.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] branch: error copying or renaming a detached HEAD
2022-10-25 23:01 [PATCH] branch: error copying or renaming a detached HEAD Rubén Justo
@ 2022-10-25 23:50 ` Junio C Hamano
0 siblings, 0 replies; 2+ messages in thread
From: Junio C Hamano @ 2022-10-25 23:50 UTC (permalink / raw)
To: Rubén Justo; +Cc: Git List
Rubén Justo <rjusto@gmail.com> writes:
> Let's remove the condition in copy_or_rename_branch() (the current
> function name) and check for HEAD before calling it, dying with the
> original intended error if we're in a detached HEAD.
Makes sense.
> @@ -827,24 +820,19 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> strbuf_release(&buf);
>
> return ret;
> - } else if (copy) {
> - if (!argc)
> - die(_("branch name required"));
> - else if (argc == 1)
> - copy_or_rename_branch(head, argv[0], 1, copy > 1);
> - else if (argc == 2)
> - copy_or_rename_branch(argv[0], argv[1], 1, copy > 1);
> - else
> - die(_("too many branches for a copy operation"));
> - } else if (rename) {
> + } else if (copy || rename) {
> if (!argc)
> die(_("branch name required"));
> + else if ((argc == 1) && filter.detached)
> + die(copy? _("cannot copy the current branch while not on any.")
> + : _("cannot rename the current branch while not on any."));
Missing " " before "?".
> else if (argc == 1)
> - copy_or_rename_branch(head, argv[0], 0, rename > 1);
> + copy_or_rename_branch(head, argv[0], copy, copy + rename > 1);
The third argument being 'copy' makes sense (as the original has 1
for copy and 0 for rename). As we reject if rename and copy are
both set, here we have either copy > 0 or rename > 0 but not both,
so the fourth argument makes sense, too.
> else if (argc == 2)
> - copy_or_rename_branch(argv[0], argv[1], 0, rename > 1);
> + copy_or_rename_branch(argv[0], argv[1], copy, copy + rename > 1);
> else
> - die(_("too many arguments for a rename operation"));
> + die(copy? _("too many branches for a copy operation")
> + : _("too many arguments for a rename operation"));
Ditto.
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index 7d8edff9c3..38c57de71b 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -268,6 +268,17 @@ test_expect_success 'git branch -M topic topic should work when main is checked
> git branch -M topic topic
> '
>
> +test_expect_success 'git branch -M and -C fail on detached HEAD' '
> + git checkout HEAD^{} &&
> + test_when_finished git checkout - &&
> + echo "fatal: cannot rename the current branch while not on any." >expect &&
> + test_must_fail git branch -M must-fail 2>err &&
> + test_cmp expect err &&
> + echo "fatal: cannot copy the current branch while not on any." >expect &&
> + test_must_fail git branch -C must-fail 2>err &&
> + test_cmp expect err
> +'
Excellent.
I'll do the whitespace tweak while queueing, so there is no need to
resend only to fix them.
Thanks.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-10-25 23:50 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-25 23:01 [PATCH] branch: error copying or renaming a detached HEAD Rubén Justo
2022-10-25 23:50 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).