On Sun, Feb 26, 2023 at 06:56:41PM +0100, Stefan Sperling wrote:
> On Sun, Feb 26, 2023 at 04:37:00PM +0100, Christian Weisgerber wrote:
> > Stefan Sperling:
> What I can do for now is make sure that an obstruction is signalled.
> Going beyond that is probably far from trivial, unfortunately.

Taking a fresh look at this, I realized we can in fact allow a file
to be switched into a directory, provided the file has no local
modifications, or is missing from disk, or was already deleted.

Handling more complicated cases would need more work as explained
in my previous mail. In particular, if the file is unversioned or
contains local modifications then the file's content would be forever
lost if we deleted the file during the update.
I am adding test coverage for this case.

Your test is about the case where the file is under version control and
no local modifications are present. This can be made to pass with the
following patch and should not cause us any complications down the road.

ok?

-----------------------------------------------
 handle files changing into directories during 'got update'
 
 problem found by naddy@
 
diff 9a298e5c10f6c68afbaca853454de2787a312c81 7cbee29b204f1ae8038084a1d040434ec5167f01
commit - 9a298e5c10f6c68afbaca853454de2787a312c81
commit + 7cbee29b204f1ae8038084a1d040434ec5167f01
blob - 59440ec12abde110db7e832a12e05f5bd4bfc613
blob + c924075eacdb3d7e5ebe3fc11b3c59262e7e120f
--- got/got.1
+++ got/got.1
@@ -628,6 +628,7 @@ Show the status of each affected file, using the follo
 .It G Ta file was updated and local changes were merged cleanly
 .It C Ta file was updated and conflicts occurred during merge
 .It D Ta file was deleted
+.It d Ta file's deletion was prevented by local modifications
 .It A Ta new file was added
 .It \(a~ Ta versioned file is obstructed by a non-regular file
 .It ! Ta a missing versioned file was restored
blob - ce1ab2adbfadcbf6d57d031d9390a6d8a3940fb8
blob + 9e87414b038cb6aa0ef965f162999f1108c5d82a
--- lib/worktree.c
+++ lib/worktree.c
@@ -1408,12 +1408,14 @@ install_blob(struct got_worktree *worktree, const char
 	fd = open(ondisk_path, O_RDWR | O_CREAT | O_EXCL | O_NOFOLLOW |
 	    O_CLOEXEC, mode);
 	if (fd == -1) {
-		if (errno == ENOENT) {
+		if (errno == ENOENT || errno == ENOTDIR) {
 			char *parent;
 			err = got_path_dirname(&parent, path);
 			if (err)
 				return err;
 			err = add_dir_on_disk(worktree, parent);
+			if (err && err->code == GOT_ERR_FILE_OBSTRUCTED)
+				err = got_error_path(path, err->code);
 			free(parent);
 			if (err)
 				return err;
@@ -1883,6 +1885,8 @@ static const struct got_error *
 	return NULL;
 }
 
+static const struct got_error *remove_ondisk_file(const char *, const char *);
+
 static const struct got_error *
 update_blob(struct got_worktree *worktree,
     struct got_fileindex *fileindex, struct got_fileindex_entry *ie,
@@ -1913,7 +1917,7 @@ update_blob(struct got_worktree *worktree,
 			sb.st_mode = got_fileindex_perms_to_st(ie);
 	} else {
 		if (stat(ondisk_path, &sb) == -1) {
-			if (errno != ENOENT) {
+			if (errno != ENOENT && errno != ENOTDIR) {
 				err = got_error_from_errno2("stat",
 				    ondisk_path);
 				goto done;
@@ -1942,6 +1946,32 @@ update_blob(struct got_worktree *worktree,
 		goto done;
 	}
 
+	if (S_ISDIR(te->mode)) { /* file changing into a directory */
+		if (status == GOT_STATUS_UNVERSIONED) {
+			err = (*progress_cb)(progress_arg, status, path);
+		} else if (status != GOT_STATUS_NO_CHANGE &&
+		    status != GOT_STATUS_DELETE &&
+		    status != GOT_STATUS_NONEXISTENT &&
+		    status != GOT_STATUS_MISSING) {
+			err = (*progress_cb)(progress_arg,
+			    GOT_STATUS_CANNOT_DELETE, path);
+		} else if (ie) {
+			if (status != GOT_STATUS_DELETE &&
+			    status != GOT_STATUS_NONEXISTENT &&
+			    status != GOT_STATUS_MISSING) {
+				err = remove_ondisk_file(worktree->root_path,
+				    ie->path);
+				if (err && !(err->code == GOT_ERR_ERRNO &&
+				    errno == ENOENT))
+					goto done;
+			}
+			got_fileindex_entry_remove(fileindex, ie);
+			err = (*progress_cb)(progress_arg, GOT_STATUS_DELETE,
+			    ie->path);
+		}
+		goto done; /* nothing else to do */
+	}
+
 	if (ie && status != GOT_STATUS_MISSING && S_ISREG(sb.st_mode) &&
 	    (S_ISLNK(te->mode) ||
 	    (te->mode & S_IXUSR) == (sb.st_mode & S_IXUSR))) {
blob - 4aa5a095404e1e99181588149852bfe3df333a9c
blob + c2b798a572a28b05ce1a6dd4ea570a7728b1c23f
--- regress/cmdline/update.sh
+++ regress/cmdline/update.sh
@@ -655,11 +655,69 @@ test_update_changes_file_to_dir() {
 	(cd $testroot/wt && got update > $testroot/stdout 2> $testroot/stderr)
 	ret=$?
 	if [ $ret -ne 0 ]; then
-		ret="xfail change file into directory"
+		echo "update failed unexpectedly" >&2
+		test_done "$testroot" "1"
+		return 1
 	fi
+
+	echo "D  alpha" > $testroot/stdout.expected
+	echo "A  alpha/eta" >> $testroot/stdout.expected
+	echo -n "Updated to refs/heads/master: " >> $testroot/stdout.expected
+	git_show_head $testroot/repo >> $testroot/stdout.expected
+	echo >> $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_update_changes_modified_file_to_dir() {
+	local testroot=`test_init update_changes_modified_file_to_dir`
+
+	got checkout $testroot/repo $testroot/wt > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	git_rm $testroot/repo alpha
+	mkdir $testroot/repo/alpha
+	echo eta > $testroot/repo/alpha/eta
+	(cd $testroot/repo && git add alpha/eta)
+	git_commit $testroot/repo -m "changed alpha into directory"
+
+	echo "modified alpha" >> $testroot/wt/alpha
+	cp $testroot/wt/alpha $testroot/wt/content.expected
+	(cd $testroot/wt && got update > $testroot/stdout 2> $testroot/stderr)
+	ret=$?
+	if [ $ret -eq 0 ]; then
+		echo "update succeeded unexpectedly" >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	echo "d  alpha" > $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 "got: alpha/eta: file is obstructed" > $testroot/stderr.expected
+	cmp -s $testroot/stderr.expected $testroot/stderr
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stderr.expected $testroot/stderr
+	fi
+	test_done "$testroot" "$ret"
+}
+
 test_update_merges_file_edits() {
 	local testroot=`test_init update_merges_file_edits`
 
@@ -3120,6 +3178,7 @@ run_test test_update_merges_file_edits
 run_test test_update_creates_missing_parent_with_subdir
 run_test test_update_file_in_subsubdir
 run_test test_update_changes_file_to_dir
+run_test test_update_changes_modified_file_to_dir
 run_test test_update_merges_file_edits
 run_test test_update_keeps_xbit
 run_test test_update_clears_xbit