From: Stefan Sperling Subject: Re: fix duplicated tree item with prefix checkout To: Omar Polo Cc: gameoftrees@openbsd.org Date: Sat, 18 Jul 2026 19:27:07 +0200 On Sat, Jul 18, 2026 at 07:25:29PM +0200, Omar Polo wrote: > "Omar Polo" 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 >