"John Cai via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> diff --git a/reset.c b/reset.c
> index e3383a93343..f8e32fcc240 100644
> --- a/reset.c
> +++ b/reset.c
> @@ -101,6 +101,9 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
> if (opts->branch_msg && !opts->branch)
> BUG("branch reflog message given without a branch");
>
> + if (switch_to_branch && opts->flags & RESET_HEAD_DETACH)
It's just style thing but it probably is easier to read to have
an extra () around the bitwise-&.
> + BUG("attempting to detach HEAD when branch is given");
I wonder if there is a valid use case NOT to use RESET_HEAD_DETACH
when switch_to_branch == NULL. If there isn't, it could be that
we can get rid of RESET_HEAD_DETACH bit and base this decision
solely on switch_to_branch'es NULLness.
But that is totally outside the scope of this fix. I'd prefer to
see a narrowly and sharply focused fix, and to be quite honest, I
would be happier if this assertion weren't in this topic.
> if (!refs_only && repo_hold_locked_index(r, &lock, LOCK_REPORT_ON_ERROR) < 0) {
> ret = -1;
> goto leave_reset_head;
> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
> index 0643d015255..d5a8ee39fc4 100755
> --- a/t/t3400-rebase.sh
> +++ b/t/t3400-rebase.sh
> @@ -394,6 +394,15 @@ test_expect_success 'switch to branch not checked out' '
> git rebase main other
> '
>
> +test_expect_success 'switch to non-branch detaches HEAD' '
> + git checkout main &&
> + old_main=$(git rev-parse HEAD) &&
> + git rebase First Second^0 &&
Good. For reproduction, the fork-point "First" does not have to be
a raw object name. "Second^0" not being a branch name does matter.
> + test_cmp_rev HEAD Second &&
> + test_cmp_rev main $old_main &&
> + test_must_fail git symbolic-ref HEAD
All correct and to the point. Good.
Will queue. Thanks.
> +'
> +
> test_expect_success 'refuse to switch to branch checked out elsewhere' '
> git checkout main &&
> git worktree add wt &&
