* difftool -d not populating left correctly when not in git root
@ 2016-12-02 23:04 Frank Becker
2016-12-04 22:31 ` David Aguilar
0 siblings, 1 reply; 3+ messages in thread
From: Frank Becker @ 2016-12-02 23:04 UTC (permalink / raw)
To: git
Hi,
looks like this broke between 2.9.2 and 2.9.3
cat ~/.gitconfig
[difftool "diff"]
cmd = ls -l ${LOCAL}/* ${REMOTE}/*
#cmd = diff -r ${LOCAL} ${REMOTE} | less
~/stuff/gittest> ls -l *
d1:
total 8
-rw-r--r-- 1 frank staff 16 2 Dec 14:30 test.txt
d2:
total 8
-rw-r--r-- 1 frank staff 18 2 Dec 14:30 anothertest.tst
~/stuff/gittest> git status
On branch master
Changes not staged for commit:
(use "git add <file>..." to update what will be committed)
(use "git checkout -- <file>..." to discard changes in working directory)
modified: d1/test.txt
modified: d2/anothertest.tst
~/stuff/gittest> ~/stuff/git_tmp/bin/git --version
git version 2.11.0
~/stuff/gittest> ~/stuff/git_tmp/bin/git difftool -d -t diff
/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.0oGRF/left/d1:
total 8
-rw-r--r-- 1 frank staff 6 2 Dec 14:52 test.txt
/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.0oGRF/left/d2:
total 8
-rw-r--r-- 1 frank staff 7 2 Dec 14:52 anothertest.tst
/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.0oGRF/right/d1:
total 8
lrwxr-xr-x 1 frank staff 38 2 Dec 14:52 test.txt ->
/Users/frank/stuff/gittest/d1/test.txt
/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.0oGRF/right/d2:
total 8
lrwxr-xr-x 1 frank staff 45 2 Dec 14:52 anothertest.tst ->
/Users/frank/stuff/gittest/d2/anothertest.tst
cd d2
~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git difftool -d -t diff
/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.eRXhB/left/d2:
total 8
-rw-r--r-- 1 frank staff 7 2 Dec 14:52 anothertest.tst
/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.eRXhB/right/d1:
total 8
lrwxr-xr-x 1 frank staff 38 2 Dec 14:52 test.txt ->
/Users/frank/stuff/gittest/d1/test.txt
/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.eRXhB/right/d2:
total 8
lrwxr-xr-x 1 frank staff 45 2 Dec 14:52 anothertest.tst ->
/Users/frank/stuff/gittest/d2/anothertest.tst
Note that left does not contain d1
~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git --version
git version 2.9.2
~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git difftool -d -t diff
/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.YxtVw/left/d1:
total 8
-rw-r--r-- 1 frank staff 6 2 Dec 15:02 test.txt
/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.YxtVw/left/d2:
total 8
-rw-r--r-- 1 frank staff 7 2 Dec 15:02 anothertest.tst
/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.YxtVw/right/d1:
total 8
lrwxr-xr-x 1 frank staff 38 2 Dec 15:02 test.txt ->
/Users/frank/stuff/gittest/d1/test.txt
/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.YxtVw/right/d2:
total 8
lrwxr-xr-x 1 frank staff 45 2 Dec 15:02 anothertest.tst ->
/Users/frank/stuff/gittest/d2/anothertest.tst
~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git --version
git version 2.9.3
~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git difftool -d -t diff
/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.TpJ5u/left/d2:
total 8
-rw-r--r-- 1 frank staff 7 2 Dec 15:01 anothertest.tst
/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.TpJ5u/right/d1:
total 8
lrwxr-xr-x 1 frank staff 38 2 Dec 15:01 test.txt ->
/Users/frank/stuff/gittest/d1/test.txt
/var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.TpJ5u/right/d2:
total 8
lrwxr-xr-x 1 frank staff 45 2 Dec 15:01 anothertest.tst ->
/Users/frank/stuff/gittest/d2/anothertest.tst
Cheers,
Frank.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: difftool -d not populating left correctly when not in git root
2016-12-02 23:04 difftool -d not populating left correctly when not in git root Frank Becker
@ 2016-12-04 22:31 ` David Aguilar
2016-12-05 11:46 ` Johannes Schindelin
0 siblings, 1 reply; 3+ messages in thread
From: David Aguilar @ 2016-12-04 22:31 UTC (permalink / raw)
To: Frank Becker; +Cc: git, John Keeping, Johannes Schindelin, Junio C Hamano
On Fri, Dec 02, 2016 at 03:04:01PM -0800, Frank Becker wrote:
> Hi,
>
> looks like this broke between 2.9.2 and 2.9.3
>
> cat ~/.gitconfig
> [difftool "diff"]
> cmd = ls -l ${LOCAL}/* ${REMOTE}/*
> #cmd = diff -r ${LOCAL} ${REMOTE} | less
>
> ~/stuff/gittest> ls -l *
> d1:
> total 8
> -rw-r--r-- 1 frank staff 16 2 Dec 14:30 test.txt
>
> d2:
> total 8
> -rw-r--r-- 1 frank staff 18 2 Dec 14:30 anothertest.tst
>
>
> ~/stuff/gittest> git status
> On branch master
> Changes not staged for commit:
> (use "git add <file>..." to update what will be committed)
> (use "git checkout -- <file>..." to discard changes in working directory)
>
> modified: d1/test.txt
> modified: d2/anothertest.tst
>
>
> ~/stuff/gittest> ~/stuff/git_tmp/bin/git --version
> git version 2.11.0
>
> ~/stuff/gittest> ~/stuff/git_tmp/bin/git difftool -d -t diff
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.0oGRF/left/d1:
> total 8
> -rw-r--r-- 1 frank staff 6 2 Dec 14:52 test.txt
>
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.0oGRF/left/d2:
> total 8
> -rw-r--r-- 1 frank staff 7 2 Dec 14:52 anothertest.tst
>
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.0oGRF/right/d1:
> total 8
> lrwxr-xr-x 1 frank staff 38 2 Dec 14:52 test.txt ->
> /Users/frank/stuff/gittest/d1/test.txt
>
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.0oGRF/right/d2:
> total 8
> lrwxr-xr-x 1 frank staff 45 2 Dec 14:52 anothertest.tst ->
> /Users/frank/stuff/gittest/d2/anothertest.tst
>
>
> cd d2
> ~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git difftool -d -t diff
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.eRXhB/left/d2:
> total 8
> -rw-r--r-- 1 frank staff 7 2 Dec 14:52 anothertest.tst
>
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.eRXhB/right/d1:
> total 8
> lrwxr-xr-x 1 frank staff 38 2 Dec 14:52 test.txt ->
> /Users/frank/stuff/gittest/d1/test.txt
>
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.eRXhB/right/d2:
> total 8
> lrwxr-xr-x 1 frank staff 45 2 Dec 14:52 anothertest.tst ->
> /Users/frank/stuff/gittest/d2/anothertest.tst
>
>
> Note that left does not contain d1
>
>
>
> ~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git --version
> git version 2.9.2
> ~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git difftool -d -t diff
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.YxtVw/left/d1:
> total 8
> -rw-r--r-- 1 frank staff 6 2 Dec 15:02 test.txt
>
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.YxtVw/left/d2:
> total 8
> -rw-r--r-- 1 frank staff 7 2 Dec 15:02 anothertest.tst
>
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.YxtVw/right/d1:
> total 8
> lrwxr-xr-x 1 frank staff 38 2 Dec 15:02 test.txt ->
> /Users/frank/stuff/gittest/d1/test.txt
>
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.YxtVw/right/d2:
> total 8
> lrwxr-xr-x 1 frank staff 45 2 Dec 15:02 anothertest.tst ->
> /Users/frank/stuff/gittest/d2/anothertest.tst
>
>
>
> ~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git --version
> git version 2.9.3
> ~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git difftool -d -t diff
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.TpJ5u/left/d2:
> total 8
> -rw-r--r-- 1 frank staff 7 2 Dec 15:01 anothertest.tst
>
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.TpJ5u/right/d1:
> total 8
> lrwxr-xr-x 1 frank staff 38 2 Dec 15:01 test.txt ->
> /Users/frank/stuff/gittest/d1/test.txt
>
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.TpJ5u/right/d2:
> total 8
> lrwxr-xr-x 1 frank staff 45 2 Dec 15:01 anothertest.tst ->
> /Users/frank/stuff/gittest/d2/anothertest.tst
This regression was not caught by our test suite.
This looks like it's an edge case not handled by:
9ec26e797781 "difftool: fix argument handling in subdirs"
The current "rewrite difftool in C" topic may need a
similar adjustment.
The problem:
When preparing the right-side of the diff we only construct the
parts that changed. When constructing the left side we
construct a full index, but we were constructing it relative to
the subdirectory, and thus it ends up empty because we are in a
subdirectory and the paths are incorrect.
The fix seems simple -- when preparing the index files we need
to chdir to the toplevel to ensure that the index construction
steps find the correct toplevel-relative paths.
Thanks for the heads-up,
David
--- 8< ---
Date: Sun, 4 Dec 2016 14:27:17 -0800
Subject: [PATCH] difftool: properly handle being launched from a subdirectory
9ec26e797781239b36ebccb87c590e5778358007 corrected how path arguments
are handled in a subdirectory, but it introduced a regression in how
entries outside of the subdirectory are handled by the dir-diff.
When preparing the right-side of the diff we only construct the parts
that changed.
When constructing the left side we construct an index, but we were
constructing it relative to the subdirectory, and thus it ends up empty
because we are in a subdirectory and the paths are incorrect.
Teach difftool to chdir to the toplevel of the repository before
preparing its temporary indexes. This ensures that all of the
toplevel-relative paths are valid.
Add a test case to exercise this use case.
Reported-by: Frank Becker <fb@mooflu.com>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
git-difftool.perl | 4 ++++
t/t7800-difftool.sh | 7 +++++--
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index a5790d03a..959822d5f 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -182,6 +182,10 @@ EOF
}
}
+ # Go to the root of the worktree so that the left index files
+ # are properly setup -- the index is toplevel-relative.
+ chdir($workdir);
+
# Setup temp directories
my $tmpdir = tempdir('git-difftool.XXXXX', CLEANUP => 0, TMPDIR => 1);
my $ldir = "$tmpdir/left";
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 70a2de461..caab4b5ca 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -413,8 +413,11 @@ run_dir_diff_test 'difftool --dir-diff from subdirectory' '
(
cd sub &&
git difftool --dir-diff $symlinks --extcmd ls branch >output &&
- grep sub output &&
- grep file output
+ # "sub" must only exist in "right"
+ # "file" and "file2" must be listed in both "left" and "right"
+ test "1" = $(grep sub output | wc -l) &&
+ test "2" = $(grep file"$" output | wc -l) &&
+ test "2" = $(grep file2 output | wc -l)
)
'
--
2.11.0.dirty
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: difftool -d not populating left correctly when not in git root
2016-12-04 22:31 ` David Aguilar
@ 2016-12-05 11:46 ` Johannes Schindelin
0 siblings, 0 replies; 3+ messages in thread
From: Johannes Schindelin @ 2016-12-05 11:46 UTC (permalink / raw)
To: David Aguilar; +Cc: Frank Becker, git, John Keeping, Junio C Hamano
Hi David,
On Sun, 4 Dec 2016, David Aguilar wrote:
> Date: Sun, 4 Dec 2016 14:27:17 -0800
> Subject: [PATCH] difftool: properly handle being launched from a subdirectory
>
> 9ec26e797781239b36ebccb87c590e5778358007 corrected how path arguments
> are handled in a subdirectory, but it introduced a regression in how
> entries outside of the subdirectory are handled by the dir-diff.
>
> When preparing the right-side of the diff we only construct the parts
> that changed.
>
> When constructing the left side we construct an index, but we were
> constructing it relative to the subdirectory, and thus it ends up empty
> because we are in a subdirectory and the paths are incorrect.
>
> Teach difftool to chdir to the toplevel of the repository before
> preparing its temporary indexes. This ensures that all of the
> toplevel-relative paths are valid.
>
> Add a test case to exercise this use case.
>
> Reported-by: Frank Becker <fb@mooflu.com>
> Signed-off-by: David Aguilar <davvid@gmail.com>
I applied this to my builtin-difftool branch (which, as per Peff's
suggestion, runs t7800 both with and without the builtin difftool) and the
tests pass:
https://github.com/dscho/git/commit/9b9a69c5ee975a4a2565343ae0a9199a6ac89609
Which means that the builtin difftool already had fixed the bug, more by
coincidence than by design, I have to admit.
Ciao,
Johannes
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-12-05 11:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-02 23:04 difftool -d not populating left correctly when not in git root Frank Becker
2016-12-04 22:31 ` David Aguilar
2016-12-05 11:46 ` Johannes Schindelin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).