* [rebase] `fatal: cannot force update the branch ... checkout out at ...` is confusing when it isn't active
@ 2023-07-12 0:45 Josh Soref
2023-07-12 15:04 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Josh Soref @ 2023-07-12 0:45 UTC (permalink / raw)
To: git
Consider this series of unfortunate commands:
```sh
% (cd $(mktemp -d); git init; (touch a; git add a; git commit -m a;
touch b; git add b; git commit -m b; git bisect start; git branch next
HEAD~; git checkout next ) 2>/dev/null >/dev/null; git log --oneline
-1 HEAD; git branch -f main HEAD;)
Initialized empty Git repository in
/private/var/folders/r3/n29fz25x72x191fdv6mhhr3m0000gp/T/tmp.fGD64HAf/.git/
accc238 (HEAD -> next) a
fatal: cannot force update the branch 'main' checked out at
'/private/var/folders/r3/n29fz25x72x191fdv6mhhr3m0000gp/T/tmp.fGD64HAf'
```
It's true git in the repository does have a pin of sorts for the
`main` branch and that `git-rebase` would be very upset if the
branch's location were changed, but as an end user, the `main` branch
is not checked out, the `next` branch is checked out.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [rebase] `fatal: cannot force update the branch ... checkout out at ...` is confusing when it isn't active
2023-07-12 0:45 [rebase] `fatal: cannot force update the branch ... checkout out at ...` is confusing when it isn't active Josh Soref
@ 2023-07-12 15:04 ` Junio C Hamano
2023-07-12 20:33 ` Josh Soref
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2023-07-12 15:04 UTC (permalink / raw)
To: Josh Soref; +Cc: git
Josh Soref <jsoref@gmail.com> writes:
> Consider this series of unfortunate commands:
> ```sh
> % (cd $(mktemp -d); git init; (touch a; git add a; git commit -m a;
> touch b; git add b; git commit -m b; git bisect start; git branch next
> HEAD~; git checkout next ) 2>/dev/null >/dev/null; git log --oneline
> -1 HEAD; git branch -f main HEAD;)
> Initialized empty Git repository in
> /private/var/folders/r3/n29fz25x72x191fdv6mhhr3m0000gp/T/tmp.fGD64HAf/.git/
> accc238 (HEAD -> next) a
> fatal: cannot force update the branch 'main' checked out at
> '/private/var/folders/r3/n29fz25x72x191fdv6mhhr3m0000gp/T/tmp.fGD64HAf'
> ```
>
> It's true git in the repository does have a pin of sorts for the
> `main` branch and that `git-rebase` would be very upset if the
> branch's location were changed, but as an end user, the `main` branch
> is not checked out, the `next` branch is checked out.
Fair enough. Perhaps "cannot force update the branch 'main' in use"
would alleviate the confusion?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [rebase] `fatal: cannot force update the branch ... checkout out at ...` is confusing when it isn't active
2023-07-12 15:04 ` Junio C Hamano
@ 2023-07-12 20:33 ` Josh Soref
2023-07-12 20:49 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Josh Soref @ 2023-07-12 20:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Josh Soref writes:
> fatal: cannot force update the branch 'main' checked out at
> '/private/var/folders/r3/n29fz25x72x191fdv6mhhr3m0000gp/T/tmp.fGD64HAf'
> It's true git in the repository does have a pin of sorts for the
> `main` branch and that `git-rebase` would be very upset if the
> branch's location were changed, but as an end user, the `main` branch
> is not checked out, the `next` branch is checked out.
Junio C Hamano wrote:
> Fair enough. Perhaps "cannot force update the branch 'main' in use"
> would alleviate the confusion?
That would be better.
Even better would be to tell me what's using it (rebase). I'm not sure
how possible that is.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [rebase] `fatal: cannot force update the branch ... checkout out at ...` is confusing when it isn't active
2023-07-12 20:33 ` Josh Soref
@ 2023-07-12 20:49 ` Junio C Hamano
2023-07-12 22:25 ` Re* " Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2023-07-12 20:49 UTC (permalink / raw)
To: Josh Soref; +Cc: git
Josh Soref <jsoref@gmail.com> writes:
> Josh Soref writes:
>> fatal: cannot force update the branch 'main' checked out at
>> '/private/var/folders/r3/n29fz25x72x191fdv6mhhr3m0000gp/T/tmp.fGD64HAf'
>
>> It's true git in the repository does have a pin of sorts for the
>> `main` branch and that `git-rebase` would be very upset if the
>> branch's location were changed, but as an end user, the `main` branch
>> is not checked out, the `next` branch is checked out.
>
> Junio C Hamano wrote:
>> Fair enough. Perhaps "cannot force update the branch 'main' in use"
>> would alleviate the confusion?
>
> That would be better.
>
> Even better would be to tell me what's using it (rebase). I'm not sure
> how possible that is.
When branch.c:prepare_checked_out() notices which branch is in use
in what way in which worktree, it does check many conditions (e.g.
"in this worktree, is a rebase in progress and if so which branch is
being affected?") but the information is lost there to only record a
mapping from branch names to worktree location. So "it is in use
over there" with exact location is given, but "how it is used" is
not recorded.
It shouldn't be a rocket surgery to teach the function to leave that
information, but I am not offhand sure how valuable it would be to
do so. After all, once you learn which worktree of yours is using
the branch you wanted to touch, you'll either know already or it
would be easy for you to find out what you have been doing in that
other worktree anyway, no?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re* [rebase] `fatal: cannot force update the branch ... checkout out at ...` is confusing when it isn't active
2023-07-12 20:49 ` Junio C Hamano
@ 2023-07-12 22:25 ` Junio C Hamano
2023-07-21 21:53 ` [PATCH v2] branch: update the message to refuse touching a branch in-use Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2023-07-12 22:25 UTC (permalink / raw)
To: git; +Cc: Josh Soref
Junio C Hamano <gitster@pobox.com> writes:
> It shouldn't be a rocket surgery to teach the function to leave that
> information, but I am not offhand sure how valuable it would be to
> do so. After all, once you learn which worktree of yours is using
> the branch you wanted to touch, you'll either know already or it
> would be easy for you to find out what you have been doing in that
> other worktree anyway, no?
So, before we forget, let's make a minimum patch and let the list
archive it.
----- >8 --------- >8 --------- >8 --------- >8 --------- >8 -----
Subject: [PATCH] branch: update the message to refuse touching a branch in-use
The "git branch -f" command can refuse to force-update a branch that
is in use in another worktree. The original rationale for this
behaviour was that updating a branch that is checked out in another
worktree, without making a matching change to the index and the
working tree files in that worktree, will lead to a very confused
user. "git diff HEAD" will no longer give a useful patch, because
HEAD is a commit unrelated to what the index and the working tree in
the worktree were based on, for example.
These days, however, the same mechanism also protects branches that
are being rebased or bisected, and the same machanism is expected to
be the right place to add more checks, when we decide to protect
branches undergoing other kinds of operations. We forgot to rethink
the messaging, which originally said that we are refusing to touch
the branch because it is "checked out" elsewhere, when d2ba271a
(branch: check for bisects and rebases, 2022-06-14) started to
protect branches that are being rebased or bisected.
The spirit of the check is that we do not want to disrupt the use of
the same branch in other worktrees. Let's reword the message slightly
to say that the branch is "in use", instead of "checked out".
We could teach the branch.c:prepare_checked_out_branches() function
to remember why it decided that a particular branch needs protecting
(i.e. was it because it was checked out? being bisected? something
else?) in addition to which worktree the branch was in use, and use
that in the error message to say "you cannot force update this
branch because it is being bisected in the worktree X", etc., but it
is dubious that such extra complexity is worth it. The message
already tells which directory the worktree in question is, and it
should be just a "chdir" away for the user to find out what state it
is in, if the user felt curious enough. So let's not go there yet.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
branch.c | 2 +-
t/t2407-worktree-heads.sh | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/branch.c b/branch.c
index cdf70b0ef0..6cb3b4c56d 100644
--- a/branch.c
+++ b/branch.c
@@ -471,7 +471,7 @@ int validate_new_branchname(const char *name, struct strbuf *ref, int force)
if ((path = branch_checked_out(ref->buf)))
die(_("cannot force update the branch '%s' "
- "checked out at '%s'"),
+ "in use at '%s'"),
ref->buf + strlen("refs/heads/"), path);
return 1;
diff --git a/t/t2407-worktree-heads.sh b/t/t2407-worktree-heads.sh
index 019a40df2c..d005ce2f20 100755
--- a/t/t2407-worktree-heads.sh
+++ b/t/t2407-worktree-heads.sh
@@ -58,7 +58,7 @@ test_expect_success !SANITIZE_LEAK 'refuse to overwrite: worktree in bisect' '
git -C wt-4 bisect good wt-1 &&
test_must_fail git branch -f wt-4 HEAD 2>err &&
- grep "cannot force update the branch '\''wt-4'\'' checked out at.*wt-4" err
+ grep "cannot force update the branch '\''wt-4'\'' in use at.*wt-4" err
'
test_expect_success !SANITIZE_LEAK 'refuse to overwrite: worktree in rebase (apply)' '
@@ -68,7 +68,7 @@ test_expect_success !SANITIZE_LEAK 'refuse to overwrite: worktree in rebase (app
test_must_fail git -C wt-2 rebase --apply conflict-2 &&
test_must_fail git branch -f wt-2 HEAD 2>err &&
- grep "cannot force update the branch '\''wt-2'\'' checked out at.*wt-2" err
+ grep "cannot force update the branch '\''wt-2'\'' in use at.*wt-2" err
'
test_expect_success !SANITIZE_LEAK 'refuse to overwrite: worktree in rebase (merge)' '
@@ -78,7 +78,7 @@ test_expect_success !SANITIZE_LEAK 'refuse to overwrite: worktree in rebase (mer
test_must_fail git -C wt-2 rebase conflict-2 &&
test_must_fail git branch -f wt-2 HEAD 2>err &&
- grep "cannot force update the branch '\''wt-2'\'' checked out at.*wt-2" err
+ grep "cannot force update the branch '\''wt-2'\'' in use at.*wt-2" err
'
test_expect_success !SANITIZE_LEAK 'refuse to overwrite: worktree in rebase with --update-refs' '
@@ -90,7 +90,7 @@ test_expect_success !SANITIZE_LEAK 'refuse to overwrite: worktree in rebase with
for i in 3 4
do
test_must_fail git branch -f can-be-updated HEAD 2>err &&
- grep "cannot force update the branch '\''can-be-updated'\'' checked out at.*wt-3" err ||
+ grep "cannot force update the branch '\''can-be-updated'\'' in use at.*wt-3" err ||
return 1
done
'
@@ -150,7 +150,7 @@ test_expect_success 'refuse to overwrite when in error states' '
for i in 1 2
do
test_must_fail git branch -f fake-$i HEAD 2>err &&
- grep "cannot force update the branch '\''fake-$i'\'' checked out at" err ||
+ grep "cannot force update the branch '\''fake-$i'\'' in use at" err ||
return 1
done
'
--
2.41.0-327-gaa9166bcc0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2] branch: update the message to refuse touching a branch in-use
2023-07-12 22:25 ` Re* " Junio C Hamano
@ 2023-07-21 21:53 ` Junio C Hamano
0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2023-07-21 21:53 UTC (permalink / raw)
To: git; +Cc: Josh Soref
The "git branch -f" command can refuse to force-update a branch that
is used by another worktree. The original rationale for this
behaviour was that updating a branch that is checked out in another
worktree, without making a matching change to the index and the
working tree files in that worktree, will lead to a very confused
user. "git diff HEAD" will no longer give a useful patch, because
HEAD is a commit unrelated to what the index and the working tree in
the worktree were based on, for example.
These days, the same mechanism also protects branches that are being
rebased or bisected, and the same machanism is expected to be the
right place to add more checks, when we decide to protect branches
undergoing other kinds of operations. We however forgot to rethink
the messaging, which originally said that we are refusing to touch
the branch because it is "checked out" elsewhere, when d2ba271a
(branch: check for bisects and rebases, 2022-06-14) started to
protect branches that are being rebased or bisected.
The spirit of the check has always been that we do not want to
disrupt the use of the same branch in other worktrees. Let's reword
the message slightly to say that the branch is "used by" another
worktree, instead of "checked out".
We could teach the branch.c:prepare_checked_out_branches() function
to remember why it decided that a particular branch needs protecting
(i.e. was it because it was checked out? being bisected? something
else?) in addition to which worktree the branch was in use, and use
that in the error message to say "you cannot force update this
branch because it is being bisected in the worktree X", etc., but it
is dubious that such extra complexity is worth it. The message
already tells which directory the worktree in question is, and it
should be just a "chdir" away for the user to find out what state it
is in, if the user felt curious enough. So let's not go there yet.
Helped-by: Josh Sref <jsoref@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* Instead of saying "branch in use", use a bit more natural "branch
is used by ...".
branch.c | 2 +-
t/t2407-worktree-heads.sh | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/branch.c b/branch.c
index cdf70b0ef0..3e4684f79f 100644
--- a/branch.c
+++ b/branch.c
@@ -471,7 +471,7 @@ int validate_new_branchname(const char *name, struct strbuf *ref, int force)
if ((path = branch_checked_out(ref->buf)))
die(_("cannot force update the branch '%s' "
- "checked out at '%s'"),
+ "used by worktree at '%s'"),
ref->buf + strlen("refs/heads/"), path);
return 1;
diff --git a/t/t2407-worktree-heads.sh b/t/t2407-worktree-heads.sh
index 019a40df2c..469443d8ae 100755
--- a/t/t2407-worktree-heads.sh
+++ b/t/t2407-worktree-heads.sh
@@ -58,7 +58,7 @@ test_expect_success !SANITIZE_LEAK 'refuse to overwrite: worktree in bisect' '
git -C wt-4 bisect good wt-1 &&
test_must_fail git branch -f wt-4 HEAD 2>err &&
- grep "cannot force update the branch '\''wt-4'\'' checked out at.*wt-4" err
+ grep "cannot force update the branch '\''wt-4'\'' used by worktree at.*wt-4" err
'
test_expect_success !SANITIZE_LEAK 'refuse to overwrite: worktree in rebase (apply)' '
@@ -68,7 +68,7 @@ test_expect_success !SANITIZE_LEAK 'refuse to overwrite: worktree in rebase (app
test_must_fail git -C wt-2 rebase --apply conflict-2 &&
test_must_fail git branch -f wt-2 HEAD 2>err &&
- grep "cannot force update the branch '\''wt-2'\'' checked out at.*wt-2" err
+ grep "cannot force update the branch '\''wt-2'\'' used by worktree at.*wt-2" err
'
test_expect_success !SANITIZE_LEAK 'refuse to overwrite: worktree in rebase (merge)' '
@@ -78,7 +78,7 @@ test_expect_success !SANITIZE_LEAK 'refuse to overwrite: worktree in rebase (mer
test_must_fail git -C wt-2 rebase conflict-2 &&
test_must_fail git branch -f wt-2 HEAD 2>err &&
- grep "cannot force update the branch '\''wt-2'\'' checked out at.*wt-2" err
+ grep "cannot force update the branch '\''wt-2'\'' used by worktree at.*wt-2" err
'
test_expect_success !SANITIZE_LEAK 'refuse to overwrite: worktree in rebase with --update-refs' '
@@ -90,7 +90,7 @@ test_expect_success !SANITIZE_LEAK 'refuse to overwrite: worktree in rebase with
for i in 3 4
do
test_must_fail git branch -f can-be-updated HEAD 2>err &&
- grep "cannot force update the branch '\''can-be-updated'\'' checked out at.*wt-3" err ||
+ grep "cannot force update the branch '\''can-be-updated'\'' used by worktree at.*wt-3" err ||
return 1
done
'
@@ -150,7 +150,7 @@ test_expect_success 'refuse to overwrite when in error states' '
for i in 1 2
do
test_must_fail git branch -f fake-$i HEAD 2>err &&
- grep "cannot force update the branch '\''fake-$i'\'' checked out at" err ||
+ grep "cannot force update the branch '\''fake-$i'\'' used by worktree at" err ||
return 1
done
'
--
2.41.0-376-gcba07a324d
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-07-21 21:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-12 0:45 [rebase] `fatal: cannot force update the branch ... checkout out at ...` is confusing when it isn't active Josh Soref
2023-07-12 15:04 ` Junio C Hamano
2023-07-12 20:33 ` Josh Soref
2023-07-12 20:49 ` Junio C Hamano
2023-07-12 22:25 ` Re* " Junio C Hamano
2023-07-21 21:53 ` [PATCH v2] branch: update the message to refuse touching a branch in-use Junio C Hamano
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).