From: Mark Jamsek Subject: Re: fix revert and remove with missing directories To: gameoftrees@openbsd.org Date: Wed, 12 Apr 2023 23:56:51 +1000 On 23-04-11 02:12PM, Stefan Sperling wrote: > When directories in a work tree are missing from disk it is currently > impossible to remove or revert files within the directory. > With the patch below, 'got remove' will remove the missing files from > version control and 'got revert' will restore missing the files. > > robert@ noticed the 'got revert' issue. The rm.sh tests made the same > issue in 'got remove' visible once I had fixed the revert command. > > Ok? ok > ----------------------------------------------- > make 'got revert' and 'got rm' work on non-existent directories > > problem found by robert@ > > diff cc88020e952af813c1e01b91ab6516969562e972 0ef3ee5b4259299d2bef17091d336aed268c746f > commit - cc88020e952af813c1e01b91ab6516969562e972 > commit + 0ef3ee5b4259299d2bef17091d336aed268c746f > blob - 9e87414b038cb6aa0ef965f162999f1108c5d82a > blob + c6f46c807bd40c15c0cf2c5ba8b5fc399d5d6d88 > --- lib/worktree.c > +++ lib/worktree.c > @@ -3879,7 +3879,74 @@ static const struct got_error * > return err; > } > > +struct find_missing_children_args { > + const char *parent_path; > + size_t parent_len; > + struct got_pathlist_head *children; > + got_cancel_cb cancel_cb; > + void *cancel_arg; > + > +}; > + > static const struct got_error * > +find_missing_children(void *arg, struct got_fileindex_entry *ie) > +{ > + const struct got_error *err = NULL; > + struct find_missing_children_args *a = arg; > + > + if (a->cancel_cb) { > + err = a->cancel_cb(a->cancel_arg); > + if (err) > + return err; > + } > + > + if (got_path_is_child(ie->path, a->parent_path, a->parent_len)) > + err = got_pathlist_append(a->children, ie->path, NULL); > + > + return err; > +} > + > +static const struct got_error * > +report_children(struct got_pathlist_head *children, > + struct got_worktree *worktree, struct got_fileindex *fileindex, > + struct got_repository *repo, int is_root_dir, int report_unchanged, > + struct got_pathlist_head *ignores, int no_ignores, > + got_worktree_status_cb status_cb, void *status_arg, > + got_cancel_cb cancel_cb, void *cancel_arg) > +{ > + const struct got_error *err = NULL; > + struct got_pathlist_entry *pe; > + char *ondisk_path = NULL; > + > + TAILQ_FOREACH(pe, children, entry) { > + if (cancel_cb) { > + err = cancel_cb(cancel_arg); > + if (err) > + break; > + } > + > + if (asprintf(&ondisk_path, "%s%s%s", worktree->root_path, > + !is_root_dir ? "/" : "", pe->path) == -1) { > + err = got_error_from_errno("asprintf"); > + ondisk_path = NULL; > + break; > + } > + > + err = report_single_file_status(pe->path, ondisk_path, > + fileindex, status_cb, status_arg, repo, report_unchanged, > + ignores, no_ignores); > + if (err) > + break; > + > + free(ondisk_path); > + ondisk_path = NULL; > + } > + > + free(ondisk_path); > + return err; > +} > + > +static const struct got_error * > worktree_status(struct got_worktree *worktree, const char *path, > struct got_fileindex *fileindex, struct got_repository *repo, > got_worktree_status_cb status_cb, void *status_arg, > @@ -3891,10 +3958,11 @@ worktree_status(struct got_worktree *worktree, const c > struct got_fileindex_diff_dir_cb fdiff_cb; > struct diff_dir_cb_arg arg; > char *ondisk_path = NULL; > - struct got_pathlist_head ignores; > + struct got_pathlist_head ignores, missing_children; > struct got_fileindex_entry *ie; > > TAILQ_INIT(&ignores); > + TAILQ_INIT(&missing_children); > > if (asprintf(&ondisk_path, "%s%s%s", > worktree->root_path, path[0] ? "/" : "", path) == -1) > @@ -3906,6 +3974,17 @@ worktree_status(struct got_worktree *worktree, const c > fileindex, status_cb, status_arg, repo, > report_unchanged, &ignores, no_ignores); > goto done; > + } else { > + struct find_missing_children_args fmca; > + fmca.parent_path = path; > + fmca.parent_len = strlen(path); > + fmca.children = &missing_children; > + fmca.cancel_cb = cancel_cb; > + fmca.cancel_arg = cancel_arg; > + err = got_fileindex_for_each_entry_safe(fileindex, > + find_missing_children, &fmca); > + if (err) > + goto done; > } > > fd = open(ondisk_path, O_RDONLY | O_NOFOLLOW | O_DIRECTORY | O_CLOEXEC); > @@ -3920,9 +3999,21 @@ worktree_status(struct got_worktree *worktree, const c > if (err) > goto done; > } > - err = report_single_file_status(path, ondisk_path, > - fileindex, status_cb, status_arg, repo, > - report_unchanged, &ignores, no_ignores); > + if (TAILQ_EMPTY(&missing_children)) { > + err = report_single_file_status(path, > + ondisk_path, fileindex, > + status_cb, status_arg, repo, > + report_unchanged, &ignores, no_ignores); > + if (err) > + goto done; > + } else { > + err = report_children(&missing_children, > + worktree, fileindex, repo, > + (path[0] == '\0'), report_unchanged, > + &ignores, no_ignores, > + status_cb, status_arg, > + cancel_cb, cancel_arg); > + } > } > } else { > fdiff_cb.diff_old_new = status_old_new; > blob - 3ae6b968f8817e9efb7b1abb879557fe19622e08 > blob + dcea98961cf673717907f69c3761b59218c16e1a > --- regress/cmdline/revert.sh > +++ regress/cmdline/revert.sh > @@ -369,6 +369,41 @@ test_revert_patch() { > test_done "$testroot" "$ret" > } > > +test_revert_missing_directory() { > + local testroot=`test_init revert_missing_directory` > + > + got checkout $testroot/repo $testroot/wt > /dev/null > + ret=$? > + if [ $ret -ne 0 ]; then > + test_done "$testroot" "$ret" > + return 1 > + fi > + > + rm -r $testroot/wt/epsilon > + > + (cd $testroot/wt && got revert -R epsilon > $testroot/stdout) > + > + echo 'R epsilon/zeta' >> $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 > + > + echo "zeta" > $testroot/content.expected > + cat $testroot/wt/epsilon/zeta > $testroot/content > + > + cmp -s $testroot/content.expected $testroot/content > + ret=$? > + if [ $ret -ne 0 ]; then > + diff -u $testroot/content.expected $testroot/content > + fi > + > + test_done "$testroot" "$ret" > +} > + > test_revert_patch() { > local testroot=`test_init revert_patch` > > @@ -1533,6 +1568,7 @@ run_test test_revert_patch > run_test test_revert_no_arguments > run_test test_revert_directory > run_test test_revert_directory_unknown > +run_test test_revert_missing_directory > run_test test_revert_patch > run_test test_revert_patch_added > run_test test_revert_patch_removed > blob - b48ce4011c7e953f442dffb07e92ceeb2fc2a62b > blob + 6d656a615120185ccc425dcf94d85399757b7de5 > --- regress/cmdline/rm.sh > +++ regress/cmdline/rm.sh > @@ -648,7 +648,7 @@ test_rm_nonexistent_directory() { > return 1 > fi > > - echo "got: epsilon: No such file or directory" \ > + echo "got: epsilon/zeta: No such file or directory" \ > > $testroot/stderr.expected > cmp -s $testroot/stderr.expected $testroot/stderr > ret=$? > @@ -668,7 +668,7 @@ test_rm_nonexistent_directory() { > return 1 > fi > > - echo -n '' > $testroot/stdout.expected > + echo 'D epsilon/zeta' > $testroot/stdout.expected > cmp -s $testroot/stdout.expected $testroot/stdout > ret=$? > if [ $ret -ne 0 ]; then > -- Mark Jamsek GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68