commit af57b12ab516c7fa5ecc8bd00db5637240411ed7
from: Stefan Sperling <stsp@stsp.name>
date: Thu Jul 23 14:21:30 2020 UTC

add symlink support to 'got cherrypick'; test non-conflict cases only for now

commit - 40dde666c0e7cae797b8652b1f4368e52c2c7b13
commit + af57b12ab516c7fa5ecc8bd00db5637240411ed7
blob - fe23bf04f80b2f9cb6cd0a084b66aa3656096f18
blob + 3adf5f3dbc4de118965bdc9066822242aff95942
--- include/got_object.h
+++ include/got_object.h
@@ -279,6 +279,16 @@ void got_object_blob_rewind(struct got_blob_object *);
  */
 const struct got_error *got_object_blob_dump_to_file(size_t *, int *,
     off_t **, FILE *, struct got_blob_object *);
+
+/*
+ * Read the entire content of a blob into a newly allocated string buffer
+ * and terminate it with '\0'. This is intended for blobs which contain a
+ * symlink target path. It should not be used to process arbitrary blobs.
+ * Use got_object_blob_dump_to_file() or got_tree_entry_get_symlink_target()
+ * instead if possible. The caller must dispose of the string with free(3).
+ */
+const struct got_error *got_object_blob_read_to_str(char **,
+    struct got_blob_object *);
 
 /*
  * Attempt to open a tag object in a repository.
blob - eaff0579c403d60f90fdc01f50fde183091286a2
blob + 102953c89fd18f475a1f3d905c286a6f7fb4fa9b
--- lib/object.c
+++ lib/object.c
@@ -864,22 +864,11 @@ got_tree_entry_get_id(struct got_tree_entry *te)
 }
 
 const struct got_error *
-got_tree_entry_get_symlink_target(char **link_target, struct got_tree_entry *te,
-    struct got_repository *repo)
+got_object_blob_read_to_str(char **s, struct got_blob_object *blob)
 {
 	const struct got_error *err = NULL;
-	struct got_blob_object *blob = NULL;
 	size_t len, totlen, hdrlen, offset;
 
-	*link_target = NULL;
-
-	if (!got_object_tree_entry_is_symlink(te))
-		return got_error(GOT_ERR_TREE_ENTRY_TYPE);
-
-	err = got_object_open_as_blob(&blob, repo,
-	    got_tree_entry_get_id(te), PATH_MAX);
-	if (err)
-		return err;
 	hdrlen = got_object_blob_get_hdrlen(blob);
 	totlen = 0;
 	offset = 0;
@@ -888,28 +877,50 @@ got_tree_entry_get_symlink_target(char **link_target, 
 
 		err = got_object_blob_read_block(&len, blob);
 		if (err)
-			goto done;
+			return err;
 
 		if (len == 0)
 			break;
 
 		totlen += len - hdrlen;
-		p = realloc(*link_target, totlen + 1);
+		p = realloc(*s, totlen + 1);
 		if (p == NULL) {
 			err = got_error_from_errno("realloc");
-			goto done;
+			free(*s);
+			*s = NULL;
+			return err;
 		}
-		*link_target = p;
+		*s = p;
 		/* Skip blob object header first time around. */
-		memcpy(*link_target + offset,
+		memcpy(*s + offset,
 		    got_object_blob_get_read_buf(blob) + hdrlen, len - hdrlen);
 		hdrlen = 0;
 		offset = totlen;
 	} while (len > 0);
-	(*link_target)[totlen] = '\0';
-done:
-	if (blob)
-		got_object_blob_close(blob);
+
+	(*s)[totlen] = '\0';
+	return NULL;
+}
+
+const struct got_error *
+got_tree_entry_get_symlink_target(char **link_target, struct got_tree_entry *te,
+    struct got_repository *repo)
+{
+	const struct got_error *err = NULL;
+	struct got_blob_object *blob = NULL;
+
+	*link_target = NULL;
+
+	if (!got_object_tree_entry_is_symlink(te))
+		return got_error(GOT_ERR_TREE_ENTRY_TYPE);
+
+	err = got_object_open_as_blob(&blob, repo,
+	    got_tree_entry_get_id(te), PATH_MAX);
+	if (err)
+		return err;
+
+	err = got_object_blob_read_to_str(link_target, blob);
+	got_object_blob_close(blob);
 	if (err) {
 		free(*link_target);
 		*link_target = NULL;
blob - 365bc046c6f9d63a9505071b9fcfa2ed056c4931
blob + 18b7915d9fcf0ce6358373186138b917bbf2f1ba
--- lib/worktree.c
+++ lib/worktree.c
@@ -826,7 +826,92 @@ done:
 	if (blob_orig_path) {
 		unlink(blob_orig_path);
 		free(blob_orig_path);
+	}
+	return err;
+}
+
+static const struct got_error *
+update_symlink(const char *ondisk_path, const char *target_path,
+    size_t target_len)
+{
+	/* This is not atomic but matches what 'ln -sf' does. */
+	if (unlink(ondisk_path) == -1)
+		return got_error_from_errno2("unlink", ondisk_path);
+	if (symlink(target_path, ondisk_path) == -1)
+		return got_error_from_errno3("symlink", target_path,
+		    ondisk_path);
+	return NULL;
+}
+
+/*
+ * Merge a symlink into the work tree, where blob_orig acts as the common
+ * ancestor, blob_deriv acts as the first derived version, and the symlink
+ * on disk acts as the second derived version.
+ * Assume that contents of both blobs represent symlinks.
+ */
+static const struct got_error *
+merge_symlink(struct got_worktree *worktree,
+    struct got_blob_object *blob_orig, const char *ondisk_path,
+    const char *path, uint16_t st_mode, const char *label_orig,
+    struct got_blob_object *blob_deriv,
+    struct got_object_id *deriv_base_commit_id, struct got_repository *repo,
+    got_worktree_checkout_cb progress_cb, void *progress_arg)
+{
+	const struct got_error *err = NULL;
+	char *ancestor_target = NULL, *deriv_target = NULL;
+	struct stat sb;
+	ssize_t ondisk_len;
+	char ondisk_target[PATH_MAX];
+
+	if (lstat(ondisk_path, &sb) == -1)
+		return got_error_from_errno2("lstat", ondisk_path);
+
+	if (!S_ISLNK(sb.st_mode)) {
+		/* TODO symlink is obstructed; do something */
+		return got_error_path(ondisk_path, GOT_ERR_FILE_OBSTRUCTED);
+	}
+
+	ondisk_len = readlink(ondisk_path, ondisk_target,
+	    sizeof(ondisk_target));
+	if (ondisk_len == -1) {
+		err = got_error_from_errno2("readlink",
+		    ondisk_path);
+		goto done;
+	}
+
+	err = got_object_blob_read_to_str(&ancestor_target, blob_orig);
+	if (err)
+		goto done;
+
+	err = got_object_blob_read_to_str(&deriv_target, blob_deriv);
+	if (err)
+		goto done;
+
+	if (ondisk_len != strlen(ancestor_target) ||
+	    memcmp(ondisk_target, ancestor_target, ondisk_len) != 0) {
+		/*
+		 * The symlink has changed on-disk (second derived version).
+		 * Keep that change and discard the incoming change (first
+		 * derived version).
+		 * TODO: Need tree-conflict resolution to handle this.
+		 */
+		err = (*progress_cb)(progress_arg, GOT_STATUS_OBSTRUCTED,
+		    path);
+	} else if (ondisk_len == strlen(deriv_target) &&
+	    memcmp(ondisk_target, deriv_target, ondisk_len) == 0) {
+		/* Both versions made the same change. */
+		err = (*progress_cb)(progress_arg, GOT_STATUS_MERGE, path);
+	} else {
+		/* Apply the incoming change. */
+		err = update_symlink(ondisk_path, deriv_target,
+		    strlen(deriv_target));
+		if (err)
+			goto done;
+		err = (*progress_cb)(progress_arg, GOT_STATUS_MERGE, path);
 	}
+done:
+	free(ancestor_target);
+	free(deriv_target);
 	return err;
 }
 
@@ -1069,17 +1154,10 @@ install_symlink(struct got_worktree *worktree, const c
 				err = NULL; /* nothing to do */
 				goto done;
 			} else {
-				if (unlink(ondisk_path) == -1) {
-					err = got_error_from_errno2("unlink",
-					    ondisk_path);
-					goto done;
-				}
-				if (symlink(target_path, ondisk_path) == -1) {
-					err = got_error_from_errno3("symlink",
-					    target_path, ondisk_path);
+				err = update_symlink(ondisk_path, target_path,
+				    target_len);
+				if (err)
 					goto done;
-				}
-
 				err = (*progress_cb)(progress_arg,
 				    GOT_STATUS_UPDATE, path);
 				goto done;
@@ -2381,9 +2459,17 @@ merge_file_cb(void *arg, struct got_blob_object *blob1
 			goto done;
 		}
 
-		err = merge_blob(&local_changes_subsumed, a->worktree, blob1,
-		    ondisk_path, path2, sb.st_mode, a->label_orig, blob2,
-		    a->commit_id2, repo, a->progress_cb, a->progress_arg);
+		if (S_ISLNK(mode1) && S_ISLNK(mode2)) {
+			err = merge_symlink(a->worktree, blob1,
+			    ondisk_path, path2, sb.st_mode, a->label_orig,
+			    blob2, a->commit_id2, repo, a->progress_cb,
+			    a->progress_arg);
+		} else {
+			err = merge_blob(&local_changes_subsumed, a->worktree,
+			    blob1, ondisk_path, path2, sb.st_mode,
+			    a->label_orig, blob2, a->commit_id2, repo,
+			    a->progress_cb, a->progress_arg);
+		}
 	} else if (blob1) {
 		ie = got_fileindex_entry_get(a->fileindex, path1,
 		    strlen(path1));
blob - 15366064a879e3cb8137061a6408c3504af03578
blob + f5bdf9bfbbdf46e5e6221be22cfab48b7c15d9ff
--- regress/cmdline/cherrypick.sh
+++ regress/cmdline/cherrypick.sh
@@ -343,6 +343,119 @@ function test_cherrypick_conflict_wt_file_vs_repo_subm
 		diff -u $testroot/stdout.expected $testroot/stdout
 	fi
 	test_done "$testroot" "$ret"
+}
+
+function test_cherrypick_modified_symlinks {
+	local testroot=`test_init cherrypick_modified_symlinks`
+
+	(cd $testroot/repo && ln -s alpha alpha.link)
+	(cd $testroot/repo && ln -s epsilon epsilon.link)
+	(cd $testroot/repo && ln -s /etc/passwd passwd.link)
+	(cd $testroot/repo && ln -s ../beta epsilon/beta.link)
+	(cd $testroot/repo && ln -s nonexistent nonexistent.link)
+	(cd $testroot/repo && git add .)
+	git_commit $testroot/repo -m "add symlinks"
+	local commit_id1=`git_show_head $testroot/repo`
+
+	got branch -r $testroot/repo foo
+
+	got checkout -b foo $testroot/repo $testroot/wt > /dev/null
+
+	(cd $testroot/repo && ln -sf beta alpha.link)
+	(cd $testroot/repo && ln -sfh gamma epsilon.link)
+	(cd $testroot/repo && ln -sf ../gamma/delta epsilon/beta.link)
+	(cd $testroot/repo && ln -sf .got/bar $testroot/repo/dotgotfoo.link)
+	(cd $testroot/repo && git rm -q nonexistent.link)
+	(cd $testroot/repo && ln -sf epsilon/zeta zeta.link)
+	(cd $testroot/repo && git add .)
+	git_commit $testroot/repo -m "change symlinks"
+	local commit_id2=`git_show_head $testroot/repo`
+
+	(cd $testroot/wt && got cherrypick $commit_id2 > $testroot/stdout)
+
+	echo "G  alpha.link" > $testroot/stdout.expected
+	echo "G  epsilon/beta.link" >> $testroot/stdout.expected
+	echo "A  dotgotfoo.link" >> $testroot/stdout.expected
+	echo "G  epsilon.link" >> $testroot/stdout.expected
+	echo "D  nonexistent.link" >> $testroot/stdout.expected
+	echo "A  zeta.link" >> $testroot/stdout.expected
+	echo "Merged commit $commit_id2" >> $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	if ! [ -h $testroot/wt/alpha.link ]; then
+		echo "alpha.link is not a symlink"
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	readlink $testroot/wt/alpha.link > $testroot/stdout
+	echo "beta" > $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	if ! [ -h $testroot/wt/epsilon.link ]; then
+		echo "epsilon.link is not a symlink"
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	readlink $testroot/wt/epsilon.link > $testroot/stdout
+	echo "gamma" > $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	if [ -h $testroot/wt/passwd.link ]; then
+		echo -n "passwd.link symlink points outside of work tree: " >&2
+		readlink $testroot/wt/passwd.link >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	echo -n "/etc/passwd" > $testroot/content.expected
+	cp $testroot/wt/passwd.link $testroot/content
+
+	cmp -s $testroot/content.expected $testroot/content
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/content.expected $testroot/content
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	readlink $testroot/wt/epsilon/beta.link > $testroot/stdout
+	echo "../gamma/delta" > $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	if [ -h $testroot/wt/nonexistent.link ]; then
+		echo -n "nonexistent.link still exists on disk: " >&2
+		readlink $testroot/wt/nonexistent.link >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	test_done "$testroot" "0"
 }
 
 run_test test_cherrypick_basic
@@ -351,3 +464,4 @@ run_test test_cherrypick_into_work_tree_with_conflicts
 run_test test_cherrypick_modified_submodule
 run_test test_cherrypick_added_submodule
 run_test test_cherrypick_conflict_wt_file_vs_repo_submodule
+run_test test_cherrypick_modified_symlinks