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

From:
Mark Jamsek <mark@jamsek.com>
Subject:
Re: fix revert and remove with missing directories
To:
gameoftrees@openbsd.org
Date:
Wed, 12 Apr 2023 23:56:51 +1000

Download raw body.

Thread
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 <fnc.bsdbox.org|got.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68