commit c6e8a8268ec4f4240d51dcfd54d05c5370060747
from: Stefan Sperling <stsp@stsp.name>
date: Mon Apr 05 17:55:12 2021 UTC

do not update symlinks which are already up-to-date

This fixes spurious 'U' notifications for symlinks during 'got update'
that occurred even when the work tree was fully up-to-date.

Observed on a work tree of the FreeBSD src repo and reproduced in our
test suite by adding a no-op update at the end of a test which deals
with updating symlinks.

commit - 0f58026f98fdad502497af6b7bb1e8778ee88b42
commit + c6e8a8268ec4f4240d51dcfd54d05c5370060747
blob - d3e48372f3109c8b5259a25c8bced11886caa2a1
blob + d2012785f43070b2587e5ab750848b6dbce1f336
--- lib/worktree.c
+++ lib/worktree.c
@@ -1244,14 +1244,16 @@ install_blob(struct got_worktree *worktree, const char
  * safe location in the work tree!
  */
 static const struct got_error *
-replace_existing_symlink(const char *ondisk_path, const char *target_path,
-    size_t target_len)
+replace_existing_symlink(int *did_something, const char *ondisk_path,
+    const char *target_path, size_t target_len)
 {
 	const struct got_error *err = NULL;
 	ssize_t elen;
 	char etarget[PATH_MAX];
 	int fd;
 
+	*did_something = 0;
+
 	/*
 	 * "Bad" symlinks (those pointing outside the work tree or into the
 	 * .got directory) are installed in the work tree as a regular file
@@ -1275,6 +1277,7 @@ replace_existing_symlink(const char *ondisk_path, cons
 			return NULL; /* nothing to do */
 	}
 
+	*did_something = 1;
 	err = update_symlink(ondisk_path, target_path, target_len);
 	if (fd != -1 && close(fd) == -1 && err == NULL)
 		err = got_error_from_errno2("close", ondisk_path);
@@ -1396,7 +1399,6 @@ install_symlink(int *is_bad_symlink, struct got_worktr
 
 	if (*is_bad_symlink) {
 		/* install as a regular file */
-		*is_bad_symlink = 1;
 		got_object_blob_rewind(blob);
 		err = install_blob(worktree, ondisk_path, path,
 		    GOT_DEFAULT_FILE_MODE, GOT_DEFAULT_FILE_MODE, blob,
@@ -1407,20 +1409,26 @@ install_symlink(int *is_bad_symlink, struct got_worktr
 
 	if (symlink(target_path, ondisk_path) == -1) {
 		if (errno == EEXIST) {
+			int symlink_replaced;
 			if (path_is_unversioned) {
 				err = (*progress_cb)(progress_arg,
 				    GOT_STATUS_UNVERSIONED, path);
 				goto done;
 			}
-			err = replace_existing_symlink(ondisk_path,
-			    target_path, target_len);
+			err = replace_existing_symlink(&symlink_replaced,
+			    ondisk_path, target_path, target_len);
 			if (err)
 				goto done;
 			if (progress_cb) {
-				err = (*progress_cb)(progress_arg,
-				    reverting_versioned_file ?
-				    GOT_STATUS_REVERT : GOT_STATUS_UPDATE,
-				    path);
+				if (symlink_replaced) {
+					err = (*progress_cb)(progress_arg,
+					    reverting_versioned_file ?
+					    GOT_STATUS_REVERT :
+					    GOT_STATUS_UPDATE, path);
+				} else {
+					err = (*progress_cb)(progress_arg,
+					    GOT_STATUS_EXISTS, path);
+				}
 			}
 			goto done; /* Nothing else to do. */
 		}
@@ -1928,11 +1936,19 @@ update_blob(struct got_worktree *worktree,
 		goto done;
 	}
 
-	if (ie && status != GOT_STATUS_MISSING &&
-	    (te->mode & S_IXUSR) == (sb.st_mode & S_IXUSR)) {
+	if (ie && status != GOT_STATUS_MISSING && S_ISREG(sb.st_mode) &&
+	    (S_ISLNK(te->mode) ||
+	    (te->mode & S_IXUSR) == (sb.st_mode & S_IXUSR))) {
+		/*
+		 * This is a regular file or an installed bad symlink.
+		 * If the file index indicates that this file is already
+		 * up-to-date with respect to the repository we can skip
+		 * updating contents of this file.
+		 */
 		if (got_fileindex_entry_has_commit(ie) &&
 		    memcmp(ie->commit_sha1, worktree->base_commit_id->sha1,
 		    SHA1_DIGEST_LENGTH) == 0) {
+			/* Same commit. */
 			err = sync_timestamps(worktree->root_fd,
 			    path, status, ie, &sb);
 			if (err)
@@ -1944,6 +1960,7 @@ update_blob(struct got_worktree *worktree,
 		if (got_fileindex_entry_has_blob(ie) &&
 		    memcmp(ie->blob_sha1, te->id.sha1,
 		    SHA1_DIGEST_LENGTH) == 0) {
+			/* Different commit but the same blob. */
 			err = sync_timestamps(worktree->root_fd,
 			    path, status, ie, &sb);
 			if (err)
blob - 2623bfe8749df2799a7d5181fce77b05f162548a
blob + c90dd13dc4e8de96ade487d14ace7bf99feecb82
--- regress/cmdline/update.sh
+++ regress/cmdline/update.sh
@@ -1929,7 +1929,18 @@ test_update_adds_symlink() {
 	ret="$?"
 	if [ "$ret" != "0" ]; then
 		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
 	fi
+
+	# Updating an up-to-date symlink should be a no-op.
+	echo 'Already up-to-date' > $testroot/stdout.expected
+	(cd $testroot/wt && got update > $testroot/stdout)
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+	fi
 	test_done "$testroot" "$ret"
 }