{"thread":{"id":"44602","subject":"difftool -d not populating left correctly when not in git root","startedAt":"2016-12-02T23:04:09Z","lastAt":"2016-12-05T11:49:08Z","messageCount":3,"participants":["Frank Becker","David Aguilar","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"306827","messageId":"68f49f5e-4e71-fd52-cd6f-64e92face962@mooflu.com","threadId":"44602","inReplyTo":null,"subject":"difftool -d not populating left correctly when not in git root","fromName":"Frank Becker","fromEmail":"fb@mooflu.com","sentAt":"2016-12-02T23:04:01Z","receivedAt":"2016-12-02T23:04:09Z","isPatch":false,"sender":{"key":"fb@mooflu.com","avatar":null},"body":"Hi,\n\nlooks like this broke between 2.9.2 and 2.9.3\n\ncat ~/.gitconfig\n[difftool \"diff\"]\n     cmd = ls -l ${LOCAL}/* ${REMOTE}/*\n     #cmd = diff -r ${LOCAL} ${REMOTE} | less\n\n~/stuff/gittest> ls -l *\nd1:\ntotal 8\n-rw-r--r--  1 frank  staff  16  2 Dec 14:30 test.txt\n\nd2:\ntotal 8\n-rw-r--r--  1 frank  staff  18  2 Dec 14:30 anothertest.tst\n\n\n~/stuff/gittest> git status\nOn branch master\nChanges not staged for commit:\n   (use \"git add <file>...\" to update what will be committed)\n   (use \"git checkout -- <file>...\" to discard changes in working directory)\n\n     modified:   d1/test.txt\n     modified:   d2/anothertest.tst\n\n\n~/stuff/gittest> ~/stuff/git_tmp/bin/git --version\ngit version 2.11.0\n\n~/stuff/gittest> ~/stuff/git_tmp/bin/git difftool -d -t diff\n/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.0oGRF/left/d1:\ntotal 8\n-rw-r--r--  1 frank  staff  6  2 Dec 14:52 test.txt\n\n/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.0oGRF/left/d2:\ntotal 8\n-rw-r--r--  1 frank  staff  7  2 Dec 14:52 anothertest.tst\n\n/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.0oGRF/right/d1:\ntotal 8\nlrwxr-xr-x  1 frank  staff  38  2 Dec 14:52 test.txt -> \n/Users/frank/stuff/gittest/d1/test.txt\n\n/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.0oGRF/right/d2:\ntotal 8\nlrwxr-xr-x  1 frank  staff  45  2 Dec 14:52 anothertest.tst -> \n/Users/frank/stuff/gittest/d2/anothertest.tst\n\n\ncd d2\n~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git difftool -d -t diff\n/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.eRXhB/left/d2:\ntotal 8\n-rw-r--r--  1 frank  staff  7  2 Dec 14:52 anothertest.tst\n\n/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.eRXhB/right/d1:\ntotal 8\nlrwxr-xr-x  1 frank  staff  38  2 Dec 14:52 test.txt -> \n/Users/frank/stuff/gittest/d1/test.txt\n\n/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.eRXhB/right/d2:\ntotal 8\nlrwxr-xr-x  1 frank  staff  45  2 Dec 14:52 anothertest.tst -> \n/Users/frank/stuff/gittest/d2/anothertest.tst\n\n\nNote that left does not contain d1\n\n\n\n~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git --version\ngit version 2.9.2\n~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git difftool -d -t diff\n/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.YxtVw/left/d1:\ntotal 8\n-rw-r--r--  1 frank  staff  6  2 Dec 15:02 test.txt\n\n/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.YxtVw/left/d2:\ntotal 8\n-rw-r--r--  1 frank  staff  7  2 Dec 15:02 anothertest.tst\n\n/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.YxtVw/right/d1:\ntotal 8\nlrwxr-xr-x  1 frank  staff  38  2 Dec 15:02 test.txt -> \n/Users/frank/stuff/gittest/d1/test.txt\n\n/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.YxtVw/right/d2:\ntotal 8\nlrwxr-xr-x  1 frank  staff  45  2 Dec 15:02 anothertest.tst -> \n/Users/frank/stuff/gittest/d2/anothertest.tst\n\n\n\n~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git --version\ngit version 2.9.3\n~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git difftool -d -t diff\n/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.TpJ5u/left/d2:\ntotal 8\n-rw-r--r--  1 frank  staff  7  2 Dec 15:01 anothertest.tst\n\n/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.TpJ5u/right/d1:\ntotal 8\nlrwxr-xr-x  1 frank  staff  38  2 Dec 15:01 test.txt -> \n/Users/frank/stuff/gittest/d1/test.txt\n\n/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.TpJ5u/right/d2:\ntotal 8\nlrwxr-xr-x  1 frank  staff  45  2 Dec 15:01 anothertest.tst -> \n/Users/frank/stuff/gittest/d2/anothertest.tst\n\n\n\nCheers,\nFrank.\n\n\n\n\n"},{"id":"306904","messageId":"20161204223139.kk2ejrfc5elxmsro@gmail.com","threadId":"44602","inReplyTo":"68f49f5e-4e71-fd52-cd6f-64e92face962@mooflu.com","subject":"Re: difftool -d not populating left correctly when not in git root","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2016-12-04T22:31:39Z","receivedAt":"2016-12-04T22:32:26Z","isPatch":false,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Fri, Dec 02, 2016 at 03:04:01PM -0800, Frank Becker wrote:\n> Hi,\n> \n> looks like this broke between 2.9.2 and 2.9.3\n> \n> cat ~/.gitconfig\n> [difftool \"diff\"]\n>     cmd = ls -l ${LOCAL}/* ${REMOTE}/*\n>     #cmd = diff -r ${LOCAL} ${REMOTE} | less\n> \n> ~/stuff/gittest> ls -l *\n> d1:\n> total 8\n> -rw-r--r--  1 frank  staff  16  2 Dec 14:30 test.txt\n> \n> d2:\n> total 8\n> -rw-r--r--  1 frank  staff  18  2 Dec 14:30 anothertest.tst\n> \n> \n> ~/stuff/gittest> git status\n> On branch master\n> Changes not staged for commit:\n>   (use \"git add <file>...\" to update what will be committed)\n>   (use \"git checkout -- <file>...\" to discard changes in working directory)\n> \n>     modified:   d1/test.txt\n>     modified:   d2/anothertest.tst\n> \n> \n> ~/stuff/gittest> ~/stuff/git_tmp/bin/git --version\n> git version 2.11.0\n> \n> ~/stuff/gittest> ~/stuff/git_tmp/bin/git difftool -d -t diff\n> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.0oGRF/left/d1:\n> total 8\n> -rw-r--r--  1 frank  staff  6  2 Dec 14:52 test.txt\n> \n> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.0oGRF/left/d2:\n> total 8\n> -rw-r--r--  1 frank  staff  7  2 Dec 14:52 anothertest.tst\n> \n> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.0oGRF/right/d1:\n> total 8\n> lrwxr-xr-x  1 frank  staff  38  2 Dec 14:52 test.txt ->\n> /Users/frank/stuff/gittest/d1/test.txt\n> \n> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.0oGRF/right/d2:\n> total 8\n> lrwxr-xr-x  1 frank  staff  45  2 Dec 14:52 anothertest.tst ->\n> /Users/frank/stuff/gittest/d2/anothertest.tst\n> \n> \n> cd d2\n> ~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git difftool -d -t diff\n> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.eRXhB/left/d2:\n> total 8\n> -rw-r--r--  1 frank  staff  7  2 Dec 14:52 anothertest.tst\n> \n> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.eRXhB/right/d1:\n> total 8\n> lrwxr-xr-x  1 frank  staff  38  2 Dec 14:52 test.txt ->\n> /Users/frank/stuff/gittest/d1/test.txt\n> \n> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.eRXhB/right/d2:\n> total 8\n> lrwxr-xr-x  1 frank  staff  45  2 Dec 14:52 anothertest.tst ->\n> /Users/frank/stuff/gittest/d2/anothertest.tst\n> \n> \n> Note that left does not contain d1\n> \n> \n> \n> ~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git --version\n> git version 2.9.2\n> ~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git difftool -d -t diff\n> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.YxtVw/left/d1:\n> total 8\n> -rw-r--r--  1 frank  staff  6  2 Dec 15:02 test.txt\n> \n> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.YxtVw/left/d2:\n> total 8\n> -rw-r--r--  1 frank  staff  7  2 Dec 15:02 anothertest.tst\n> \n> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.YxtVw/right/d1:\n> total 8\n> lrwxr-xr-x  1 frank  staff  38  2 Dec 15:02 test.txt ->\n> /Users/frank/stuff/gittest/d1/test.txt\n> \n> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.YxtVw/right/d2:\n> total 8\n> lrwxr-xr-x  1 frank  staff  45  2 Dec 15:02 anothertest.tst ->\n> /Users/frank/stuff/gittest/d2/anothertest.tst\n> \n> \n> \n> ~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git --version\n> git version 2.9.3\n> ~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git difftool -d -t diff\n> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.TpJ5u/left/d2:\n> total 8\n> -rw-r--r--  1 frank  staff  7  2 Dec 15:01 anothertest.tst\n> \n> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.TpJ5u/right/d1:\n> total 8\n> lrwxr-xr-x  1 frank  staff  38  2 Dec 15:01 test.txt ->\n> /Users/frank/stuff/gittest/d1/test.txt\n> \n> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.TpJ5u/right/d2:\n> total 8\n> lrwxr-xr-x  1 frank  staff  45  2 Dec 15:01 anothertest.tst ->\n> /Users/frank/stuff/gittest/d2/anothertest.tst\n\nThis regression was not caught by our test suite.\n\nThis looks like it's an edge case not handled by:\n9ec26e797781 \"difftool: fix argument handling in subdirs\"\nThe current \"rewrite difftool in C\" topic may need a\nsimilar adjustment.\n\nThe problem:\n\nWhen preparing the right-side of the diff we only construct the\nparts that changed.  When constructing the left side we\nconstruct a full index, but we were constructing it relative to\nthe subdirectory, and thus it ends up empty because we are in a\nsubdirectory and the paths are incorrect.\n\nThe fix seems simple -- when preparing the index files we need\nto chdir to the toplevel to ensure that the index construction\nsteps find the correct toplevel-relative paths.\n\nThanks for the heads-up,\nDavid\n\n--- 8< ---\nDate: Sun, 4 Dec 2016 14:27:17 -0800\nSubject: [PATCH] difftool: properly handle being launched from a subdirectory\n\n9ec26e797781239b36ebccb87c590e5778358007 corrected how path arguments\nare handled in a subdirectory, but it introduced a regression in how\nentries outside of the subdirectory are handled by the dir-diff.\n\nWhen preparing the right-side of the diff we only construct the parts\nthat changed.\n\nWhen constructing the left side we construct an index, but we were\nconstructing it relative to the subdirectory, and thus it ends up empty\nbecause we are in a subdirectory and the paths are incorrect.\n\nTeach difftool to chdir to the toplevel of the repository before\npreparing its temporary indexes.  This ensures that all of the\ntoplevel-relative paths are valid.\n\nAdd a test case to exercise this use case.\n\nReported-by: Frank Becker <fb@mooflu.com>\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n git-difftool.perl   | 4 ++++\n t/t7800-difftool.sh | 7 +++++--\n 2 files changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/git-difftool.perl b/git-difftool.perl\nindex a5790d03a..959822d5f 100755\n--- a/git-difftool.perl\n+++ b/git-difftool.perl\n@@ -182,6 +182,10 @@ EOF\n \t\t}\n \t}\n \n+\t# Go to the root of the worktree so that the left index files\n+\t# are properly setup -- the index is toplevel-relative.\n+\tchdir($workdir);\n+\n \t# Setup temp directories\n \tmy $tmpdir = tempdir('git-difftool.XXXXX', CLEANUP => 0, TMPDIR => 1);\n \tmy $ldir = \"$tmpdir/left\";\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex 70a2de461..caab4b5ca 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -413,8 +413,11 @@ run_dir_diff_test 'difftool --dir-diff from subdirectory' '\n \t(\n \t\tcd sub &&\n \t\tgit difftool --dir-diff $symlinks --extcmd ls branch >output &&\n-\t\tgrep sub output &&\n-\t\tgrep file output\n+\t\t# \"sub\" must only exist in \"right\"\n+\t\t# \"file\" and \"file2\" must be listed in both \"left\" and \"right\"\n+\t\ttest \"1\" = $(grep sub output | wc -l) &&\n+\t\ttest \"2\" = $(grep file\"$\" output | wc -l) &&\n+\t\ttest \"2\" = $(grep file2 output | wc -l)\n \t)\n '\n \n-- \n2.11.0.dirty\n"},{"id":"306929","messageId":"alpine.DEB.2.20.1612051222560.117539@virtualbox","threadId":"44602","inReplyTo":"20161204223139.kk2ejrfc5elxmsro@gmail.com","subject":"Re: difftool -d not populating left correctly when not in git root","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-12-05T11:46:14Z","receivedAt":"2016-12-05T11:49:08Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi David,\n\nOn Sun, 4 Dec 2016, David Aguilar wrote:\n\n> Date: Sun, 4 Dec 2016 14:27:17 -0800\n> Subject: [PATCH] difftool: properly handle being launched from a subdirectory\n> \n> 9ec26e797781239b36ebccb87c590e5778358007 corrected how path arguments\n> are handled in a subdirectory, but it introduced a regression in how\n> entries outside of the subdirectory are handled by the dir-diff.\n> \n> When preparing the right-side of the diff we only construct the parts\n> that changed.\n> \n> When constructing the left side we construct an index, but we were\n> constructing it relative to the subdirectory, and thus it ends up empty\n> because we are in a subdirectory and the paths are incorrect.\n> \n> Teach difftool to chdir to the toplevel of the repository before\n> preparing its temporary indexes.  This ensures that all of the\n> toplevel-relative paths are valid.\n> \n> Add a test case to exercise this use case.\n> \n> Reported-by: Frank Becker <fb@mooflu.com>\n> Signed-off-by: David Aguilar <davvid@gmail.com>\n\nI applied this to my builtin-difftool branch (which, as per Peff's\nsuggestion, runs t7800 both with and without the builtin difftool) and the\ntests pass:\n\nhttps://github.com/dscho/git/commit/9b9a69c5ee975a4a2565343ae0a9199a6ac89609\n\nWhich means that the builtin difftool already had fixed the bug, more by\ncoincidence than by design, I have to admit.\n\nCiao,\nJohannes\n"}]}