"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: fix duplicated tree item with prefix checkout
To:
Omar Polo <op@omarpolo.com>
Cc:
gameoftrees@openbsd.org
Date:
Sat, 18 Jul 2026 19:27:07 +0200

Download raw body.

Thread
On Sat, Jul 18, 2026 at 07:25:29PM +0200, Omar Polo wrote:
> "Omar Polo" <op@omarpolo.com> wrote:
> > +	got_path_strip_trailing_slashes(absprefix);
> 
> stuff like this was broken other tests, i was focussing on mine and
> haven't noticed, apologise.  the correct pattern seems to be
> 
> 	if (!got_path_is_root_dir(...))
> 		got_path_strip_trailing_slashes(...);
> 
> which probably doesn't work for pathological cases like "//", but it's a
> net improvement over what we have, and i'll take a look at making
> got_path_strip_trailing_slashes handier to use.
> 
> updated diff:

Nice.  ok stsp@

> diff /home/op/w/got
> path + /home/op/w/got
> commit - f90dbd6e6d9fa970e460f89a909e1b48f79c5692
> blob - 83b04b102ee7c7794010dc1e7a5e1aee931f63e9
> file + lib/worktree.c
> --- lib/worktree.c
> +++ lib/worktree.c
> @@ -180,7 +180,13 @@ got_worktree_init(const char *path, struct got_referen
>  	if (!got_path_is_absolute(prefix)) {
>  		if (asprintf(&absprefix, "/%s", prefix) == -1)
>  			return got_error_from_errno("asprintf");
> +	} else {
> +		absprefix = strdup(prefix);
> +		if (absprefix == NULL)
> +			return got_error_from_errno("strdup");
>  	}
> +	if (!got_path_is_root_dir(absprefix))
> +		got_path_strip_trailing_slashes(absprefix);
>  
>  	/* Create top-level directory (may already exist). */
>  	if (mkdir(path, GOT_DEFAULT_DIR_MODE) == -1 && errno != EEXIST) {
> @@ -229,8 +235,7 @@ got_worktree_init(const char *path, struct got_referen
>  		goto done;
>  
>  	/* Store in-repository path prefix. */
> -	err = create_meta_file(path_got, GOT_WORKTREE_PATH_PREFIX,
> -	    absprefix ? absprefix : prefix);
> +	err = create_meta_file(path_got, GOT_WORKTREE_PATH_PREFIX, absprefix);
>  	if (err)
>  		goto done;
>  
> @@ -273,13 +278,21 @@ got_worktree_match_path_prefix(int *match, struct got_
>      const char *path_prefix)
>  {
>  	char *absprefix = NULL;
> +	const char *a, *b;
> +	size_t al, bl;
>  
>  	if (!got_path_is_absolute(path_prefix)) {
>  		if (asprintf(&absprefix, "/%s", path_prefix) == -1)
>  			return got_error_from_errno("asprintf");
>  	}
> -	*match = (strcmp(absprefix ? absprefix : path_prefix,
> -	    worktree->path_prefix) == 0);
> +
> +	a = absprefix ? absprefix : path_prefix;
> +	al = strlen(a);
> +
> +	b = worktree->path_prefix;
> +	bl = strlen(b);
> +
> +	*match = (got_path_cmp(a, b, al, bl) == 0);
>  	free(absprefix);
>  	return NULL;
>  }
> @@ -6111,7 +6124,7 @@ insert_tree_entry(struct got_tree_entry *new_te,
>  	if (err)
>  		return err;
>  	if (new_pe == NULL)
> -		return got_error(GOT_ERR_TREE_DUP_ENTRY);
> +		return got_error_path(new_te->name, GOT_ERR_TREE_DUP_ENTRY);
>  	return NULL;
>  }
>  
> @@ -6868,7 +6881,6 @@ got_worktree_commit(struct got_object_id **new_commit_
>  		    head_commit_id, repo, GOT_ERR_COMMIT_OUT_OF_DATE);
>  		if (err)
>  			goto done;
> -
>  	}
>  
>  	err = commit_worktree(new_commit_id, &commitable_paths,
> commit - f90dbd6e6d9fa970e460f89a909e1b48f79c5692
> blob - 997d20f28754d7e0afc5af0f4375e2f0d19ed2d2
> file + lib/worktree_open.c
> --- lib/worktree_open.c
> +++ lib/worktree_open.c
> @@ -195,6 +195,8 @@ open_worktree(struct got_worktree **worktree, const ch
>  	    GOT_WORKTREE_PATH_PREFIX);
>  	if (err)
>  		goto done;
> +	if (!got_path_is_root_dir((*worktree)->path_prefix))
> +		got_path_strip_trailing_slashes((*worktree)->path_prefix);
>  
>  	err = read_meta_file(&base_commit_id_str, path_meta,
>  	    GOT_WORKTREE_BASE_COMMIT);
> commit - f90dbd6e6d9fa970e460f89a909e1b48f79c5692
> blob - c8651ab26909a5ce8ff104fcc69b1029cc18605e
> file + regress/cmdline/commit.sh
> --- regress/cmdline/commit.sh
> +++ regress/cmdline/commit.sh
> @@ -107,6 +107,59 @@ test_commit_subdir() {
>  	test_done "$testroot" "$ret"
>  }
>  
> +test_commit_subdirs() {
> +	local testroot=`test_init commit_subdir`
> +
> +	# trailing slash for -p on purpose, we used to have a bug
> +	# that was triggered by the presence of trailing slashes.
> +	got checkout -p epsilon/ $testroot/repo $testroot/wt > /dev/null
> +	ret=$?
> +	if [ $ret -ne 0 ]; then
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
> +	mkdir -p $testroot/wt/foo/bar
> +	mkdir -p $testroot/wt/foo/baz
> +
> +	echo omega > $testroot/wt/foo/bar/omega
> +	echo kappa > $testroot/wt/foo/bar/kappa
> +
> +	(cd $testroot/wt && got add -R foo >$testroot/stdout)
> +	ret=$?
> +	if [ $ret -ne 0 ]; then
> +		echo "got add failed unexpectedly" >&2
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
> +	echo "A  foo/bar/kappa" > $testroot/stdout.expected
> +	echo "A  foo/bar/omega" >>$testroot/stdout.expected
> +
> +	cmp -s $testroot/stdout.expected $testroot/stdout
> +	ret=$?
> +	if [ $ret -ne 0 ]; then
> +		diff -u $testroot/stdout.expected $testroot/stdout
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
> +	(cd $testroot/wt && \
> +		got commit -m 'test commit_subdirs' > $testroot/stdout)
> +
> +	local head_rev=`git_show_head $testroot/repo`
> +	echo "A  foo/bar/kappa" > $testroot/stdout.expected
> +	echo "A  foo/bar/omega" >>$testroot/stdout.expected
> +	echo "Created commit $head_rev" >> $testroot/stdout.expected
> +
> +	cmp -s $testroot/stdout.expected $testroot/stdout
> +	ret=$?
> +	if [ $ret -ne 0 ]; then
> +		diff -u $testroot/stdout.expected $testroot/stdout
> +	fi
> +	test_done "$testroot" "$ret"
> +}
> +
>  test_commit_single_file() {
>  	local testroot=`test_init commit_single_file`
>  
> @@ -2117,6 +2170,7 @@ test_parseargs "$@"
>  run_test test_commit_basic
>  run_test test_commit_new_subdir
>  run_test test_commit_subdir
> +run_test test_commit_subdirs
>  run_test test_commit_single_file
>  run_test test_commit_out_of_date
>  run_test test_commit_added_subdirs
>