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

From:
Omar Polo <op@omarpolo.com>
Subject:
Re: got patch: add -c to apply at specified commit
To:
Stefan Sperling <stsp@stsp.name>
Cc:
gameoftrees@openbsd.org
Date:
Wed, 27 Jul 2022 16:46:39 +0200

Download raw body.

Thread
Stefan Sperling <stsp@stsp.name> wrote:
> Yes, I have just some documentation nits:
> 
> > @@ -1401,6 +1401,16 @@ The options for
> >  .Cm got patch
> >  are as follows:
> >  .Bl -tag -width Ds
> > +.It Fl c Ar commit
> > +Use files in the specified
> 
> s/files in/files found in/ ?

sure, it's better

> > +.Ar commit
> > +to find a merge-base, overriding the meta-data in
> 
> "use files ... to find a merge-base" could be more accurately expressed
> like this: "use files ... as a merge base for 3-way merges".

agreed, it's more clear the text then

> Perhaps we should add a hint that this is an optional flag, and that patch
> will work well without it? Otherwise people might wonder if it is required
> in case the patch does not carry meta-data?
> 
> "In case no merge-base is available for a file,
> .Cm got patch
> will attempt to apply the changes without doing a 3-way merge."

agreed also on this

> > +.Ar patchfile
> > +.Pq if any .
> > +This is useful to apply an old patch that wasn't generated by
> > +.Cm got diff
> > +or
> > +.Xr git-diff 1 .
> 
> s/an old patch/a patch/ ? I don't see why such a patch would be "old".
> There are many ways to produce patches aside from 'got diff' and 'git diff'
> and people will keep using them. Consider 'diff -u file.c.orig file.c'.

it's poorly written, sorry.  with "old" i meant that, no matter how it
was generated, if a patch is "recent enough" it'll probably apply
without a diff3-merge, while if it's "old" (i.e. the latest revision
diverged enough from the version the patch was generated) then a dumb
search/replace will have more issue to apply it.

i dropped that sentence thought as it didn't bring in anything and was
poorly written.

diff refs/heads/main refs/heads/pc
commit - 615e455c6bdddbd4e59d5d0dece41eb9953b6336
commit + 6334862bab97cd016628d6b9b5230904f1ebc231
blob - d97716d4af2c458376bb73bebfca2e1ff3725410
blob + 4e7fe9ac12d23a680dee56b299c0b2566d3c6da0
--- got/got.1
+++ got/got.1
@@ -1337,7 +1337,7 @@ option)
 .El
 .El
 .Tg pa
-.It Cm patch Oo Fl n Oc Oo Fl p Ar strip-count Oc Oo Fl R Oc Op Ar patchfile
+.It Cm patch Oo Fl c Ar commit Oc Oo Fl n Oc Oo Fl p Ar strip-count Oc Oo Fl R Oc Op Ar patchfile
 .Dl Pq alias: Cm pa
 Apply changes from
 .Ar patchfile
@@ -1401,6 +1401,15 @@ The options for
 .Cm got patch
 are as follows:
 .Bl -tag -width Ds
+.It Fl c Ar commit
+Use files found in in the specified
+.Ar commit
+as a merge base for 3-way merges, overriding the meta-data in
+.Ar patchfile
+.Pq if any .
+In case no merge-base is available for a file
+.Cm got patch
+will attempt to apply the changes without doing a 3-way merge.
 .It Fl n
 Do not make any modifications to the work tree.
 This can be used to check whether a patch would apply without issues.
blob - 3e86ff1ba0e1405d13dce902d5e91d684aef8bb7
blob + 52a212802c5d2d319bd43f2fa914eb2f48d10b90
--- got/got.c
+++ got/got.c
@@ -7851,7 +7851,7 @@ done:
 __dead static void
 usage_patch(void)
 {
-	fprintf(stderr, "usage: %s patch [-n] [-p strip-count] "
+	fprintf(stderr, "usage: %s patch [-c commit] [-n] [-p strip-count] "
 	    "[-R] [patchfile]\n", getprogname());
 	exit(1);
 }
@@ -7941,6 +7941,9 @@ cmd_patch(int argc, char *argv[])
 	const struct got_error *error = NULL, *close_error = NULL;
 	struct got_worktree *worktree = NULL;
 	struct got_repository *repo = NULL;
+	struct got_reflist_head refs;
+	struct got_object_id *commit_id = NULL;
+	const char *commit_id_str = NULL;
 	struct stat sb;
 	const char *errstr;
 	char *cwd = NULL;
@@ -7948,14 +7951,19 @@ cmd_patch(int argc, char *argv[])
 	int patchfd;
 	int *pack_fds = NULL;
 
+	TAILQ_INIT(&refs);
+
 #ifndef PROFILE
 	if (pledge("stdio rpath wpath cpath fattr proc exec sendfd flock "
 	    "unveil", NULL) == -1)
 		err(1, "pledge");
 #endif
 
-	while ((ch = getopt(argc, argv, "np:R")) != -1) {
+	while ((ch = getopt(argc, argv, "c:np:R")) != -1) {
 		switch (ch) {
+		case 'c':
+			commit_id_str = optarg;
+			break;
 		case 'n':
 			nop = 1;
 			break;
@@ -8021,10 +8029,23 @@ cmd_patch(int argc, char *argv[])
 	if (error != NULL)
 		goto done;
 
+	error = got_ref_list(&refs, repo, NULL, got_ref_cmp_by_name, NULL);
+	if (error)
+		goto done;
+
+	if (commit_id_str != NULL) {
+		error = got_repo_match_object_id(&commit_id, NULL,
+		    commit_id_str, GOT_OBJ_TYPE_COMMIT, &refs, repo);
+		if (error)
+			goto done;
+	}
+
 	error = got_patch(patchfd, worktree, repo, nop, strip, reverse,
-	    &patch_progress, NULL, check_cancelled, NULL);
+	    commit_id, &patch_progress, NULL, check_cancelled, NULL);
 
 done:
+	got_ref_list_free(&refs);
+	free(commit_id);
 	if (repo) {
 		close_error = got_repo_close(repo);
 		if (error == NULL)
blob - 3d08eea48e4a52a62094553e103775d364d22f90
blob + f2558175dfefdb0894ec38dbd84fee193f313732
--- include/got_patch.h
+++ include/got_patch.h
@@ -32,4 +32,5 @@ typedef const struct got_error *(*got_patch_progress_c
  */
 const struct got_error *
 got_patch(int, struct got_worktree *, struct got_repository *, int, int,
-    int, got_patch_progress_cb, void *, got_cancel_cb, void *);
+    int, struct got_object_id *, got_patch_progress_cb, void *,
+    got_cancel_cb, void *);
blob - 57e6ce78b6ed68c545f6e3c07cca28d1833f1df6
blob + b662fd9440dd44cb727518f3d66665658c6eea34
--- lib/patch.c
+++ lib/patch.c
@@ -726,10 +726,54 @@ done:
 }
 
 static const struct got_error *
+prepare_merge(int *do_merge, char **apath, FILE **afile,
+    struct got_worktree *worktree, struct got_repository *repo,
+    struct got_patch *p, struct got_object_id *commit_id,
+    struct got_tree_object *tree, const char *path)
+{
+	const struct got_error *err = NULL;
+
+	*do_merge = 0;
+	*apath = NULL;
+	*afile = NULL;
+
+	/* don't run the diff3 merge on creations/deletions */
+	if (p->old == NULL || p->new == NULL)
+		return NULL;
+
+	if (commit_id) {
+		struct got_object_id *id;
+
+		err = got_object_tree_find_path(&id, NULL, repo, tree, path);
+		if (err)
+			return err;
+		got_sha1_digest_to_str(id->sha1, p->blob, sizeof(p->blob));
+		got_sha1_digest_to_str(commit_id->sha1, p->cid, sizeof(p->cid));
+		free(id);
+		err = open_blob(apath, afile, p->blob, repo);
+		*do_merge = err == NULL;
+	} else if (*p->blob != '\0') {
+		err = open_blob(apath, afile, p->blob, repo);
+		/*
+		 * ignore failures to open this blob, we might have
+		 * parsed gibberish.
+		 */
+		if (err && !(err->code == GOT_ERR_ERRNO && errno == ENOENT) &&
+		    err->code != GOT_ERR_NO_OBJ)
+			return err;
+		*do_merge = err == NULL;
+		err = NULL;
+	}
+
+	return err;
+}
+
+static const struct got_error *
 apply_patch(int *overlapcnt, struct got_worktree *worktree,
     struct got_repository *repo, struct got_fileindex *fileindex,
     const char *old, const char *new, struct got_patch *p, int nop,
-    int reverse, struct patch_args *pa,
+    int reverse, struct got_object_id *commit_id,
+    struct got_tree_object *tree, struct patch_args *pa,
     got_cancel_cb cancel_cb, void *cancel_arg)
 {
 	const struct got_error *err = NULL;
@@ -746,20 +790,10 @@ apply_patch(int *overlapcnt, struct got_worktree *work
 
 	*overlapcnt = 0;
 
-	/* don't run the diff3 merge on creations/deletions */
-	if (*p->blob != '\0' && p->old != NULL && p->new != NULL) {
-		err = open_blob(&apath, &afile, p->blob, repo);
-		/*
-		 * ignore failures to open this blob, we might have
-		 * parsed gibberish.
-		 */
-		if (err && !(err->code == GOT_ERR_ERRNO && errno == ENOENT) &&
-		    err->code != GOT_ERR_NO_OBJ)
-			return err;
-		else if (err == NULL)
-			do_merge = 1;
-		err = NULL;
-	}
+	err = prepare_merge(&do_merge, &apath, &afile, worktree, repo, p,
+	    commit_id, tree, old);
+	if (err)
+		return err;
 
 	if (reverse && !do_merge)
 		reverse_patch(p);
@@ -957,11 +991,14 @@ done:
 
 const struct got_error *
 got_patch(int fd, struct got_worktree *worktree, struct got_repository *repo,
-    int nop, int strip, int reverse, got_patch_progress_cb progress_cb,
-    void *progress_arg, got_cancel_cb cancel_cb, void *cancel_arg)
+    int nop, int strip, int reverse, struct got_object_id *commit_id,
+    got_patch_progress_cb progress_cb, void *progress_arg,
+    got_cancel_cb cancel_cb, void *cancel_arg)
 {
 	const struct got_error *err = NULL, *complete_err = NULL;
 	struct got_fileindex *fileindex = NULL;
+	struct got_commit_object *commit = NULL;
+	struct got_tree_object *tree = NULL;
 	char *fileindex_path = NULL;
 	char *oldpath, *newpath;
 	struct imsgbuf *ibuf;
@@ -1007,6 +1044,16 @@ got_patch(int fd, struct got_worktree *worktree, struc
 	if (err)
 		goto done;
 
+	if (commit_id) {
+		err = got_object_open_as_commit(&commit, repo, commit_id);
+		if (err)
+			goto done;
+
+		err = got_object_open_as_tree(&tree, repo, commit->tree_id);
+		if (err)
+			goto done;
+	}
+
 	while (!done && err == NULL) {
 		struct got_patch p;
 		struct patch_args pa;
@@ -1024,7 +1071,7 @@ got_patch(int fd, struct got_worktree *worktree, struc
 		if (err == NULL)
 			err = apply_patch(&overlapcnt, worktree, repo,
 			    fileindex, oldpath, newpath, &p, nop, reverse,
-			    &pa, cancel_cb, cancel_arg);
+			    commit_id, tree, &pa, cancel_cb, cancel_arg);
 		if (err != NULL) {
 			failed = 1;
 			/* recoverable errors */
@@ -1054,6 +1101,10 @@ done:
 	if (complete_err && err == NULL)
 		err = complete_err;
 	free(fileindex_path);
+	if (tree)
+		got_object_tree_close(tree);
+	if (commit)
+		got_object_commit_close(commit);
 	if (fd != -1 && close(fd) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	if (ibuf != NULL)
blob - 5b3df4e132acb5f7a05acfcdaca7e9d1eb7ee25a
blob + 82c4dc94b64ce6ac9282519c38c6ca795845e651
--- regress/cmdline/patch.sh
+++ regress/cmdline/patch.sh
@@ -1661,6 +1661,66 @@ test_patch_merge_gitdiff() {
 	test_done $testroot $ret
 }
 
+test_patch_merge_base_provided() {
+	local testroot=`test_init patch_merge_base_provided`
+
+	got checkout $testroot/repo $testroot/wt >/dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	jot 10 > $testroot/wt/numbers
+	(cd $testroot/wt && got add numbers && got commit -m +numbers) \
+		>/dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	local commit_id=`git_show_head $testroot/repo`
+
+	jot 10 | sed s/4/four/ > $testroot/wt/numbers
+
+	# get rid of the metadata
+	(cd $testroot/wt && got diff | sed -n '/^---/,$p' > patch) \
+		>/dev/null
+
+	jot 10 | sed s/6/six/ > $testroot/wt/numbers
+	(cd $testroot/wt && got commit -m 'edit numbers') >/dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	(cd $testroot/wt && got patch -c $commit_id patch) >$testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	echo 'G  numbers' > $testroot/stdout.expected
+	cmp -s $testroot/stdout $testroot/stdout.expected
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout $testroot/stdout.expected
+		test_done $testroot $ret
+		return 1
+	fi
+
+	jot 10 | sed -e s/4/four/ -e s/6/six/ > $testroot/wt/numbers.expected
+	cmp -s $testroot/wt/numbers $testroot/wt/numbers.expected
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/wt/numbers $testroot/wt/numbers.expected
+	fi
+	test_done $testroot $ret
+}
+
 test_patch_merge_conflict() {
 	local testroot=`test_init patch_merge_conflict`
 
@@ -1977,6 +2037,7 @@ run_test test_patch_relpath_with_path_prefix
 run_test test_patch_reverse
 run_test test_patch_merge_simple
 run_test test_patch_merge_gitdiff
+run_test test_patch_merge_base_provided
 run_test test_patch_merge_conflict
 run_test test_patch_merge_unknown_blob
 run_test test_patch_merge_reverse