On Fri, Mar 10, 2023 at 12:52:40PM +0100, Omar Polo wrote:
> On 2023/03/10 12:29:09 +0100, Stefan Sperling <stsp@stsp.name> wrote:
> > Make test_diff_file_to_dir pass, a test which was added by naddy recently.
> > And add test_diff_dir_to_file, which covers the inverse case.
> > 
> > We display removal of the file (or all files in a directory) before displaying
> > additions of files/dirs. I hope this will allow 'got patch' and other patch
> > tools to apply such patches.
> 
> Yes, doing the removal first and the addition later is the way to go.
> I haven't tested these kind of situations with 'got patch' or patch,
> but if they fail now it's a got patch or patch(1) bug :-)
> 
> > ok?
> 
> ok op@

I missed that got log -P relies on the diff callback getting called
if we are not diffing blob contents.

With that issue fixed, got log -P shows both 'D alpha' and 'A alpha/eta'.
Previously only 'A alpha/eta' was shown. Add test coverage for this.

Still ok?

diff 9a298e5c10f6c68afbaca853454de2787a312c81 b1ac7627c5c45b470bedea8421ce5944293ffdab
commit - 9a298e5c10f6c68afbaca853454de2787a312c81
commit + b1ac7627c5c45b470bedea8421ce5944293ffdab
blob - ab4e42cf1f2915c2bca5c735f86ed6ff502f258e
blob + 5198b5ef5702bde4150b817019574f96e0b9f1f3
--- lib/diff.c
+++ lib/diff.c
@@ -669,11 +669,50 @@ diff_kind_mismatch(struct got_object_id *id1, struct g
 }
 
 static const struct got_error *
-diff_kind_mismatch(struct got_object_id *id1, struct got_object_id *id2,
+diff_kind_mismatch(struct got_tree_entry *te1, struct got_tree_entry *te2,
+    FILE *f1, FILE *f2, int fd1, int fd2,
     const char *label1, const char *label2, struct got_repository *repo,
-    got_diff_blob_cb cb, void *cb_arg)
+    got_diff_blob_cb cb, void *cb_arg, int diff_content)
 {
-	/* XXX TODO */
+	const struct got_error *err = NULL;
+
+	/* 
+	 * Handle files changing into directories and vice-versa.
+	 * Disregard edge cases with FIFOs, device nodes, etc for now.
+	 */
+	if (!S_ISDIR(te1->mode) && S_ISDIR(te2->mode)) {
+		if (S_ISREG(te1->mode)) {
+			if (diff_content) {
+				err = diff_deleted_blob(&te1->id, f1, fd1,
+				    f2, label1, te1->mode, repo, cb, cb_arg);
+			} else {
+				err = cb(cb_arg, NULL, NULL, NULL, NULL,
+				    &te1->id, NULL, label1, NULL,
+				    te1->mode, 0, repo);
+			}
+			if (err)
+				return err;
+		}
+		return diff_added_tree(&te2->id, f1, f2, fd2, label2,
+		    repo, cb, cb_arg, diff_content);
+	} else if (S_ISDIR(te1->mode) && !S_ISDIR(te2->mode)) {
+		err = diff_deleted_tree(&te1->id, f1, fd1, f2,
+		    label1, repo, cb, cb_arg, diff_content);
+		if (err)
+			return err;
+		if (S_ISREG(te2->mode)) {
+			if (diff_content) {
+				err = diff_added_blob(&te2->id, f1, f2, fd2,
+				    label2, te2->mode, repo, cb, cb_arg);
+			} else {
+				err = cb(cb_arg, NULL, NULL, NULL, NULL, NULL,
+				    &te2->id, NULL, label2, 0, te2->mode, repo);
+			}
+			if (err)
+				return err;
+		}
+	}
+
 	return NULL;
 }
 
@@ -732,8 +771,8 @@ diff_entry_old_new(struct got_tree_entry *te1, struct 
 	if (id_match)
 		return NULL;
 
-	return diff_kind_mismatch(&te1->id, &te2->id, label1, label2, repo,
-	    cb, cb_arg);
+	return diff_kind_mismatch(te1, te2, f1, f2, fd1, fd2,
+	    label1, label2, repo, cb, cb_arg, diff_content);
 }
 
 static const struct got_error *
blob - e8dc379f3766f3126a73441a2e308fd844afe31e
blob + 12bca5996322c1822c24ced40493cd7acb0ca896
--- regress/cmdline/common.sh
+++ regress/cmdline/common.sh
@@ -77,6 +77,13 @@ git_show_head()
 	(cd $repo && git rm -q "$@")
 }
 
+git_rmdir()
+{
+	local repo="$1"
+	shift
+	(cd $repo && git rm -q -r "$@")
+}
+
 git_show_head()
 {
 	local repo="$1"
blob - fe9c943bc19d1b0b6905788633041f88ae69282b
blob + 74c5c3277048c41e7d957d274e2fd708fbd7e022
--- regress/cmdline/diff.sh
+++ regress/cmdline/diff.sh
@@ -1796,6 +1796,7 @@ test_diff_file_to_dir() {
 test_diff_file_to_dir() {
 	local testroot=`test_init diff_file_to_dir`
 	local commit_id0=`git_show_head $testroot/repo`
+	local alpha_blobid=`get_blob_id $testroot/repo "" alpha`
 
 	got checkout $testroot/repo $testroot/wt > /dev/null
 	ret=$?
@@ -1810,20 +1811,155 @@ test_diff_file_to_dir() {
 	(cd $testroot/repo && git add alpha/eta)
 	git_commit $testroot/repo -m "changed alpha into directory"
 	local commit_id1=`git_show_head $testroot/repo`
+	local alpha_eta_blobid=`get_blob_id $testroot/repo alpha eta`
 
-	echo "diff $commit_id0 $commit_id1" > $testroot/stdout.expected
-	echo "commit - $commit_id0" >> $testroot/stdout.expected
-	echo "commit + $commit_id1" >> $testroot/stdout.expected
+	cat <<EOF >$testroot/stdout.expected
+diff $commit_id0 $commit_id1
+commit - $commit_id0
+commit + $commit_id1
+blob - $alpha_blobid (mode 644)
+blob + /dev/null
+--- alpha
++++ /dev/null
+@@ -1 +0,0 @@
+-alpha
+blob - /dev/null
+blob + $alpha_eta_blobid (mode 644)
+--- /dev/null
++++ alpha/eta
+@@ -0,0 +1 @@
++eta
+EOF
 	got diff -r $testroot/repo $commit_id0 $commit_id1 > $testroot/stdout
-	# Diff should not be empty
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "diff failed unexpectedly" >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+
 	cmp -s $testroot/stdout.expected $testroot/stdout
 	ret=$?
-	if [ $ret -eq 0 ]; then
-		ret="xfail file to directory"
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
 	fi
+
+	local author_time=`git_show_author_time $testroot/repo`
+	d=`date -u -r $author_time +"%a %b %e %X %Y UTC"`
+	cat <<EOF >$testroot/stdout.expected
+-----------------------------------------------
+commit $commit_id1 (master)
+from: $GOT_AUTHOR
+date: $d
+ 
+ changed alpha into directory
+ 
+ D  alpha
+ A  alpha/eta
+
+EOF
+
+	got log -P -r $testroot/repo -l1 -c $commit_id1 > $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "diff failed unexpectedly" >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	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_diff_dir_to_file() {
+	local testroot=`test_init diff_file_to_dir`
+	local commit_id0=`git_show_head $testroot/repo`
+	local epsilon_zeta_blobid=`get_blob_id $testroot/repo epsilon zeta`
+
+	got checkout $testroot/repo $testroot/wt > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	git_rmdir $testroot/repo epsilon
+	echo epsilon > $testroot/repo/epsilon
+	(cd $testroot/repo && git add epsilon)
+	git_commit $testroot/repo -m "changed epsilon into file"
+	local commit_id1=`git_show_head $testroot/repo`
+	local epsilon_blobid=`get_blob_id $testroot/repo "" epsilon`
+
+	cat <<EOF >$testroot/stdout.expected
+diff $commit_id0 $commit_id1
+commit - $commit_id0
+commit + $commit_id1
+blob - $epsilon_zeta_blobid (mode 644)
+blob + /dev/null
+--- epsilon/zeta
++++ /dev/null
+@@ -1 +0,0 @@
+-zeta
+blob - /dev/null
+blob + $epsilon_blobid (mode 644)
+--- /dev/null
++++ epsilon
+@@ -0,0 +1 @@
++epsilon
+EOF
+	got diff -r $testroot/repo $commit_id0 $commit_id1 > $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "diff failed unexpectedly" >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	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
+
+	local author_time=`git_show_author_time $testroot/repo`
+	d=`date -u -r $author_time +"%a %b %e %X %Y UTC"`
+	cat <<EOF >$testroot/stdout.expected
+-----------------------------------------------
+commit $commit_id1 (master)
+from: $GOT_AUTHOR
+date: $d
+ 
+ changed epsilon into file
+ 
+ D  epsilon/zeta
+ A  epsilon
+
+EOF
+
+	got log -P -r $testroot/repo -l1 -c $commit_id1 > $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "diff failed unexpectedly" >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	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_diff_basic
 run_test test_diff_shows_conflict
@@ -1841,3 +1977,4 @@ run_test test_diff_file_to_dir
 run_test test_diff_commit_diffstat
 run_test test_diff_worktree_diffstat
 run_test test_diff_file_to_dir
+run_test test_diff_dir_to_file