On Thu, Jan 07, 2021 at 05:25:52PM +0200, Timo Myyrä wrote:
> Stefan Sperling <stsp@stsp.name> [2021-01-05, 22:38 +0100]:
> 
> > On Tue, Jan 05, 2021 at 10:53:05PM +0200, Timo Myyrä wrote:
> >
> >> Stefan Sperling <stsp@stsp.name> [2021-01-04, 11:42 +0100]:
> >> 
> >> > On Sun, Jan 03, 2021 at 10:13:07PM +0200, Timo Myyrä wrote:
> >> >
> >> >> Hi,
> >> >> 
> >> >> While figuring out how to get single files contents at given commit I
> >> >> attempted to use update command. It seems to behave oddly when running
> >> >> repeatedly with same arguments.
> >> >> 
> >> >> Is this expected?
> >> >
> >> > What did you expect instead?
> >> >
> >> 
> >> Hmm, well looking at man page I would have expected either the just
> >> README.org be reverted to commit
> >> bf832809c3806e9b69db972abaa3d38fd427573e if straight file can be given
> >> as path argument. Or if file is not an valid path argument I would have
> >> expected some error or nothing happening. Now it would seem to ...
> >> delete one file from work tree after each invocation of the command
> >> which is the odd part.
> >> 
> >> timo
> >
> > OK, I see it now, thank you!
> > I did not notice the mismatch between the path argument and the file
> > which was deleted.
> >
> > It looks like update -c with a path isn't covered by the test suite.
> > I have tried to add test coverage for this bug but I cannot trigger it.
> > My attempt is shown below.
> >
> > Could you try to write a script that triggers the bug, starting with
> > an empty repo? Or even add a test case to our test suite (which is
> > just a shell script)?
> >
> > In any case, thanks for pointing this out! :)
> >
> > diff 1255c02f3e117fa4fd07f9cc6fbcf62383755e5d /home/stsp/src/got
> > blob - a52e26753f5fdda870b159596d715fa7bae988c0
> > file + regress/cmdline/update.sh
> > --- regress/cmdline/update.sh
> > +++ regress/cmdline/update.sh
> > @@ -1089,6 +1089,7 @@ test_update_conflict_wt_rm_vs_repo_rm() {
> >  
> >  test_update_partial() {
> >  	local testroot=`test_init update_partial`
> > +	local base_commit=`git_show_head $testroot/repo`
> >  
> >  	got checkout $testroot/repo $testroot/wt > /dev/null
> >  	ret="$?"
> > @@ -1156,6 +1157,29 @@ test_update_partial() {
> >  		return 1
> >  	fi
> >  
> > +	echo "U  alpha" > $testroot/stdout.expected
> > +	echo "Updated to commit $base_commit" >> $testroot/stdout.expected
> > +
> > +	(cd $testroot/wt && got update -c $base_commit alpha > $testroot/stdout)
> > +
> > +	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
> > +
> > +	echo "alpha" > $testroot/content.expected
> > +	cat $testroot/wt/alpha > $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
> >  	test_done "$testroot" "$ret"
> >  }
> >  
> >
> >
> 
> I'm not sure what the end result should be with the command.
> I can repro it with:
> ```
> git clone --bare git@github.com:zmyrgel/dotfiles.git
> got checkout dotfiles.git dotfiles-got
> cd dotfiles-got
> got update -c bf832809c3806e9b69db972abaa3d38fd427573e README.org
> ```
> 
> I checked that it loops the files with both default git repository and
> bare git repository.

Thanks! Base on the above I could reproduce this problem in a test case:

test_update_single_file --- /tmp/got-test-update_single_file-AdsXe3EN/stdout.expected   Tue Jan 19 08:21:03 2021
+++ /tmp/got-test-update_single_file-AdsXe3EN/stdout    Tue Jan 19 08:21:03 2021
@@ -1,2 +1,2 @@
-U  c
+D  a
 Updated to commit 423644b4e16bd49ac2bd4e5b4b8b1edc78cefca8
test failed; leaving test data in /tmp/got-test-update_single_file-AdsXe3EN

I've just committed the following fix + test:

-----------------------------------------------
commit 194cb7cb2a0f223daf0baa74a1e85bbbb44c10fd (main, origin/main)
from: Stefan Sperling <stsp@stsp.name>
date: Tue Jan 19 07:38:37 2021 UTC
 
 fix bug where 'got up -c commit path' deleted unrelated files from work tree
 
 Problem reported by Timo Myyrä
 
diff a2f506ff027fb051b191436c5d48c660233e2312 582f68222c402626b5381c919a019e7d0d63889d
blob - 5796af500a49c702c5c1d95f32551f881ad0f9b5
blob + d1e3839c06a7f70e71410592024f426053df2ced
--- lib/fileindex.c
+++ lib/fileindex.c
@@ -848,8 +848,7 @@ diff_fileindex_tree(struct got_fileindex *fileindex,
 			} else if (cmp < 0) {
 				next = walk_fileindex(fileindex, *ie);
 				if (got_path_is_child((*ie)->path, path,
-				    path_len) && (entry_name == NULL ||
-				    strcmp(te_name, entry_name) == 0)) {
+				    path_len) && entry_name == NULL) {
 					err = cb->diff_old(cb_arg, *ie, path);
 					if (err || entry_name)
 						break;
blob - a52e26753f5fdda870b159596d715fa7bae988c0
blob + dd8f31f1ab6cb4e1c7bba94b8c2abc6f0e08b67a
--- regress/cmdline/update.sh
+++ regress/cmdline/update.sh
@@ -2254,6 +2254,80 @@ test_update_symlink_conflicts() {
 
 }
 
+test_update_single_file() {
+	local testroot=`test_init update_single_file 1`
+
+	echo c1 > $testroot/repo/c
+	(cd $testroot/repo && git add .)
+	git_commit $testroot/repo -m "adding executable file"
+	local commit_id1=`git_show_head $testroot/repo`
+
+	echo a > $testroot/repo/a
+	echo b > $testroot/repo/b
+	echo c2 > $testroot/repo/c
+	(cd $testroot/repo && git add .)
+	git_commit $testroot/repo -m "adding executable file"
+	local commit_id2=`git_show_head $testroot/repo`
+
+	got checkout $testroot/repo $testroot/wt > /dev/null
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo "U  c" > $testroot/stdout.expected
+	echo -n "Updated to commit $commit_id1" >> $testroot/stdout.expected
+	echo >> $testroot/stdout.expected
+
+	(cd $testroot/wt && got update -c $commit_id1 c \
+		> $testroot/stdout)
+
+	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
+
+	echo c1 > $testroot/content.expected
+	cat $testroot/wt/c > $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
+
+	echo "U  c" > $testroot/stdout.expected
+	echo -n "Updated to commit $commit_id2" >> $testroot/stdout.expected
+	echo >> $testroot/stdout.expected
+
+	(cd $testroot/wt && got update c > $testroot/stdout)
+
+	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
+
+	echo c2 > $testroot/content.expected
+	cat $testroot/wt/c > $testroot/content
+
+	cmp -s $testroot/content.expected $testroot/content
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/content.expected $testroot/content
+	fi
+	test_done "$testroot" "$ret"
+}
+
+
 test_parseargs "$@"
 run_test test_update_basic
 run_test test_update_adds_file
@@ -2293,3 +2367,4 @@ run_test test_update_conflict_wt_file_vs_repo_submodul
 run_test test_update_adds_symlink
 run_test test_update_deletes_symlink
 run_test test_update_symlink_conflicts
+run_test test_update_single_file