From: Stefan Sperling <stsp@stsp.name>
Subject: Re: got patch: resolve paths from $PWD
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Sat, 23 Apr 2022 08:54:53 +0200

On Fri, Apr 22, 2022 at 10:58:54PM +0200, Omar Polo wrote:
> The paths are computed in worktree.c:got_worktree_patch_check_path via
> patch_check_path, and they're build off got_worktree_resolve_path.
> 
> I'm still keeping the paths relative to the worktree root in the
> pathlists, otherwise I'd broke everything.  I'm then building (again)
> the absolute paths in apply_patch before running the patch machinery.
> 
> I'm only feeding absolute paths to lstat/fopen/rename & co.
> Yes.  To be fair it could be somehow avoided right now, I just need to
> propagate the path I build in worktree.c:patch_check_path up until
> apply_patch.  (it's the "ondisk_path" from patch_check_path).  but that
> can be done in a follow-up commit, diff belows already has three
> different fixes, it's probably enough ;)

This seems great. ok stsp@

> diff refs/heads/main refs/heads/ppath2
> blob - 9b49b4b1349c2d2994dfa6e667c9265975aea642
> blob + adf9459c88ef262d9070733fe62620364926f5db
> --- lib/patch.c
> +++ lib/patch.c
> @@ -565,13 +565,14 @@ patch_add(void *arg, unsigned char status, const char 
>  
>  static const struct got_error *
>  apply_patch(struct got_worktree *worktree, struct got_repository *repo,
> -    const char *oldpath, const char *newpath, struct got_patch *p,
> -    int nop, struct patch_args *pa, got_cancel_cb cancel_cb, void *cancel_arg)
> +    const char *old, const char *new, struct got_patch *p, int nop,
> +    struct patch_args *pa, got_cancel_cb cancel_cb, void *cancel_arg)
>  {
>  	const struct got_error *err = NULL;
>  	struct got_pathlist_head oldpaths, newpaths;
>  	struct got_pathlist_entry *pe;
>  	int file_renamed = 0;
> +	char *oldpath = NULL, *newpath = NULL;
>  	char *tmppath = NULL, *template = NULL, *parent = NULL;;
>  	FILE *tmp = NULL;
>  	mode_t mode = GOT_DEFAULT_FILE_MODE;
> @@ -579,13 +580,25 @@ apply_patch(struct got_worktree *worktree, struct got_
>  	TAILQ_INIT(&oldpaths);
>  	TAILQ_INIT(&newpaths);
>  
> -	err = got_pathlist_insert(&pe, &oldpaths, oldpath, NULL);
> +	err = got_pathlist_insert(&pe, &oldpaths, old, NULL);
>  	if (err)
>  		goto done;
> -	err = got_pathlist_insert(&pe, &newpaths, newpath, NULL);
> +	err = got_pathlist_insert(&pe, &newpaths, new, NULL);
>  	if (err)
>  		goto done;
>  
> +	if (asprintf(&oldpath, "%s/%s", got_worktree_get_root_path(worktree),
> +	    old) == -1) {
> +		err = got_error_from_errno("asprintf");
> +		goto done;
> +	}
> +
> +	if (asprintf(&newpath, "%s/%s", got_worktree_get_root_path(worktree),
> +	    new) == -1) {
> +		err = got_error_from_errno("asprintf");
> +		goto done;
> +	}
> +
>  	file_renamed = strcmp(oldpath, newpath);
>  
>  	if (asprintf(&template, "%s/got-patch",
> @@ -612,7 +625,7 @@ apply_patch(struct got_worktree *worktree, struct got_
>  	}
>  
>  	if (fchmod(fileno(tmp), mode) == -1) {
> -		err = got_error_from_errno2("chmod", newpath);
> +		err = got_error_from_errno2("chmod", tmppath);
>  		goto done;
>  	}
>  
> @@ -650,8 +663,7 @@ apply_patch(struct got_worktree *worktree, struct got_
>  		if (err)
>  			unlink(newpath);
>  	} else
> -		err = report_progress(pa, oldpath, newpath, GOT_STATUS_MODIFY,
> -		    NULL);
> +		err = report_progress(pa, old, new, GOT_STATUS_MODIFY, NULL);
>  
>  done:
>  	got_pathlist_free(&oldpaths);
> @@ -661,6 +673,8 @@ done:
>  	if (tmppath != NULL)
>  		unlink(tmppath);
>  	free(tmppath);
> +	free(oldpath);
> +	free(newpath);
>  	return err;
>  }
>  
> blob - bfebb46526cc0f3f0deda90d0868edad7c6946c9
> blob + bbe95ae655a519c5a9d4dc6eac25f7ace38c7452
> --- lib/worktree.c
> +++ lib/worktree.c
> @@ -8795,7 +8795,8 @@ patch_check_path(const char *p, char **path, unsigned 
>  	ie = got_fileindex_entry_get(fileindex, *path, strlen(*path));
>  	if (ie) {
>  		*staged_status = get_staged_status(ie);
> -		err = get_file_status(status, &sb, ie, *path, -1, NULL, repo);
> +		err = get_file_status(status, &sb, ie, ondisk_path, -1, NULL,
> +		    repo);
>  		if (err)
>  			goto done;
>  	} else {
> blob - b3f10d2dc6e282f6548c695befab125a7f8f680e
> blob + af1fd72e33001d46b8f822e0b0a7f00a5b1ad7b7
> --- regress/cmdline/patch.sh
> +++ regress/cmdline/patch.sh
> @@ -1273,6 +1273,46 @@ EOF
>  	test_done $testroot 0
>  }
>  
> +test_patch_relative_paths() {
> +	local testroot=`test_init patch_orig`
> +
> +	got checkout $testroot/repo $testroot/wt > /dev/null
> +	ret=$?
> +	if [ $ret -ne 0 ]; then
> +		test_done $testroot $ret
> +		return 1
> +	fi
> +
> +	cat <<EOF > $testroot/wt/gamma/patch
> +--- delta
> ++++ delta
> +@@ -1 +1 @@
> +-delta
> ++DELTA
> +--- /dev/null
> ++++ eta
> +@@ -0,0 +1 @@
> ++eta
> +EOF
> +
> +	(cd $testroot/wt/gamma && got patch patch) > $testroot/stdout
> +	ret=$?
> +	if [ $ret -ne 0 ]; then
> +		test_done $testroot $ret
> +		return 1
> +	fi
> +
> +	echo 'M  gamma/delta' > $testroot/stdout.expected
> +	echo 'A  gamma/eta' >> $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_parseargs "$@"
>  run_test test_patch_simple_add_file
>  run_test test_patch_simple_rm_file
> @@ -1294,3 +1334,4 @@ run_test test_patch_with_offset
>  run_test test_patch_prefer_new_path
>  run_test test_patch_no_newline
>  run_test test_patch_strip
> +run_test test_patch_relative_paths
> 
> 
>