Re: [PATCH 3/4] worktree add: trim slashes when deriving branch name from path
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 25, 2026, 19:47 UTC
- Message-ID
- <xmqq33w2m186.fsf@gitster.g>
- In-Reply-To
- <20260825180350.2099-4-l.s.r@web.de>
René Scharfe <l.s.r@web.de> writes:
Show 20 quoted lines
> worktree_basename() sets `n` to the length of `path` without trailing > path separators, not to the length of the basename. This matters when > deriving a branch name from a path with more than one component. E.g.: > > path: /new/worktree/ > s: ^ > n: |-----------| > > So here xstrndup(s, n) copies up to 13 characters from "worktree/", > effectively to the end of the string, including the trailing dash. > > Path separators are not allowed at the end of branch names, so strip > them off by calculating the basename length and extracting just that > part. > > Signed-off-by: René Scharfe <l.s.r@web.de> > --- > builtin/worktree.c | 4 ++-- > t/t2400-worktree-add.sh | 13 +++++++++++++ > 2 files changed, 15 insertions(+), 2 deletions(-)
Hmph, so am I correct to understand that the symptom observable by end-users of this is that we used to attempt creating "bit/" branch when they request
$ git worktree add waffle/bit/
and it wouldn't have worked until they said
$ git worktree add waffle/bit
instead? Not allowing a trailing slash when naming a directory is nasty (it is a good practice to explicitly give a trailing slash when naming a directory to avoid confusion), and it is a good fix.
It also should work fine with waffle/bit/// even though there is no strong reason to allow it ;-).
Show 53 quoted lines
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index a53e815cc9..01c245778e 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -769,7 +769,7 @@ static char *dwim_branch(const char *path, char **new_branch)
> int n;
> int branch_exists;
> const char *s = worktree_basename(path, &n);
> - char *branchname = xstrndup(s, n);
> + char *branchname = xmemdupz(s, path + n - s);
> struct strbuf ref = STRBUF_INIT;
>
> branch_exists = !check_branch_ref(the_repository, &ref, branchname) &&
> @@ -878,7 +878,7 @@ static int add(int ac, const char **av, const char *prefix,
> if (opts.orphan && !new_branch) {
> int n;
> const char *s = worktree_basename(path, &n);
> - new_branch = new_branch_to_free = xstrndup(s, n);
> + new_branch = new_branch_to_free = xmemdupz(s, path + n - s);
> } else if (opts.orphan) {
> ; /* no-op */
> } else if (opts.detach) {
> diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
> index 280d2e2c07..7e2811fa77 100755
> --- a/t/t2400-worktree-add.sh
> +++ b/t/t2400-worktree-add.sh
> @@ -298,6 +298,11 @@ test_expect_success '"add" with <branch> omitted' '
> test_cmp_rev HEAD bat
> '
>
> +test_expect_success '"add" with trailing slash and <branch> omitted' '
> + git worktree add waffle/bit/ &&
> + test_cmp_rev HEAD bit
> +'
> +
> test_expect_success '"add" checks out existing branch of dwimd name' '
> git branch dwim HEAD~1 &&
> git worktree add dwim &&
> @@ -388,6 +393,14 @@ test_expect_success '"add --orphan (no -b)"' '
> test_cmp expected actual
> '
>
> +test_expect_success '"add --orphan with trailing slash (no -b)"' '
> + test_when_finished "git worktree remove -f -f neworphan" &&
> + git worktree add --orphan ./neworphan/ &&
> + echo refs/heads/neworphan >expected &&
> + git -C neworphan symbolic-ref HEAD >actual &&
> + test_cmp expected actual
> +'
> +
> test_expect_success '"add --orphan --quiet"' '
> test_when_finished "git worktree remove -f -f orphandir" &&
> git worktree add --quiet --orphan -b neworphan orphandir 2>log.actual &&