Re: [PATCH] difftool: support repositories with .git-files
Am 25.02.2014 22:12, schrieb Junio C Hamano: Jens Lehmann jens.lehm...@web.de writes: +test_expect_success PERL 'difftool properly honours gitlink and core.worktree' ' + git submodule add ./. submod/ule + ( + cd submod/ule + git difftool --tool=echo --dir-diff --cached In the context of this fix, finishing with 0 exit status may be all we care about, but do we also care about things like in what directory the tool is invoked in, what arguments and extra environment settings (if any) it is given, and stuff like that? Sure. But I just intended to test the fix (and the test can easily be extended by people who know more about difftool than I do). Yes, we need to start somewhere and I'd agree that it was a good starting point. Right, using echo was not the best choice here. I used it to avoid the dependency to meld... Perhaps like this then? This is an a monkey sees what difftool_test_setup does and then mimics patch ;-). Nicely done :-) t/t7800-difftool.sh | 13 + 1 file changed, 13 insertions(+) diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index 2418528..595f808 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -434,4 +434,17 @@ test_expect_success PERL 'difftool --no-symlinks detects conflict ' ' ) ' +test_expect_success PERL 'difftool properly honours gitlink and core.worktree' ' + git submodule add ./. submod/ule + ( + cd submod/ule + git config diff.tool checktrees + git config difftool.checktrees.cmd '\'' + test -d $LOCAL test -d $REMOTE + '\'' + echo further file + git difftool --tool=checktrees --dir-diff + ) +' + test_done -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] difftool: support repositories with .git-files
Jens Lehmann jens.lehm...@web.de writes: Am 24.02.2014 17:55, schrieb Junio C Hamano: David Aguilar dav...@gmail.com writes: Modern versions of git submodule use .git-files to setup the submodule directory. When run in a git submodule-created repository git difftool --dir-diff dies with the following error: $ git difftool -d HEAD~ fatal: This operation must be run in a work tree diff --raw --no-abbrev -z HEAD~: command returned error: 128 core.worktree is relative to the .git directory but the logic in find_worktree() does not account for it. Use `git rev-parse --show-toplevel` to find the worktree so that the dir-diff feature works inside a submodule. Reported-by: Gábor Lipták gabor.lip...@gmail.com Helped-by: Jens Lehmann jens.lehm...@web.de Helped-by: John Keeping j...@keeping.me.uk Signed-off-by: David Aguilar dav...@gmail.com --- Looks good; thanks. FWIW: Tested-by: Jens Lehmann jens.lehm...@web.de What about squashing this in to detect any future regressions? diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index 2418528..d86ad68 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -434,4 +434,12 @@ test_expect_success PERL 'difftool --no-symlinks detects conflict ' ' ) ' +test_expect_success PERL 'difftool properly honours gitlink and core.worktree' ' + git submodule add ./. submod/ule + ( + cd submod/ule + git difftool --tool=echo --dir-diff --cached In the context of this fix, finishing with 0 exit status may be all we care about, but do we also care about things like in what directory the tool is invoked in, what arguments and extra environment settings (if any) it is given, and stuff like that? In fact, the echo in the above is very misleading. The test relies on the fact that immediately after the submod/ule is cloned, diff --cached does not have to call any tool backend---if you modify some tracked file in its working tree and dropped --cached on the command line, the command will fail with Huh? I do not know what 'echo' diff/merge backend is, no? + ) +' + test_done -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] difftool: support repositories with .git-files
Am 25.02.2014 18:02, schrieb Junio C Hamano: Jens Lehmann jens.lehm...@web.de writes: Am 24.02.2014 17:55, schrieb Junio C Hamano: David Aguilar dav...@gmail.com writes: Modern versions of git submodule use .git-files to setup the submodule directory. When run in a git submodule-created repository git difftool --dir-diff dies with the following error: $ git difftool -d HEAD~ fatal: This operation must be run in a work tree diff --raw --no-abbrev -z HEAD~: command returned error: 128 core.worktree is relative to the .git directory but the logic in find_worktree() does not account for it. Use `git rev-parse --show-toplevel` to find the worktree so that the dir-diff feature works inside a submodule. Reported-by: Gábor Lipták gabor.lip...@gmail.com Helped-by: Jens Lehmann jens.lehm...@web.de Helped-by: John Keeping j...@keeping.me.uk Signed-off-by: David Aguilar dav...@gmail.com --- Looks good; thanks. FWIW: Tested-by: Jens Lehmann jens.lehm...@web.de What about squashing this in to detect any future regressions? diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index 2418528..d86ad68 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -434,4 +434,12 @@ test_expect_success PERL 'difftool --no-symlinks detects conflict ' ' ) ' +test_expect_success PERL 'difftool properly honours gitlink and core.worktree' ' +git submodule add ./. submod/ule +( +cd submod/ule +git difftool --tool=echo --dir-diff --cached In the context of this fix, finishing with 0 exit status may be all we care about, but do we also care about things like in what directory the tool is invoked in, what arguments and extra environment settings (if any) it is given, and stuff like that? Sure. But I just intended to test the fix (and the test can easily be extended by people who know more about difftool than I do). In fact, the echo in the above is very misleading. The test relies on the fact that immediately after the submod/ule is cloned, diff --cached does not have to call any tool backend---if you modify some tracked file in its working tree and dropped --cached on the command line, the command will fail with Huh? I do not know what 'echo' diff/merge backend is, no? Right, using echo was not the best choice here. I used it to avoid the dependency to meld in the example of the OP (maybe using true as tool would have indicated that the tool is not important here, but looking into this again a simple git difftool --dir-diff without any further arguments also shows that the fix is working). Aas mentioned above, I'm not familiar with difftool and just wanted to share an easy way to test the fix. But I do not care too deeply about this test, so feel free to ignore it. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] difftool: support repositories with .git-files
Jens Lehmann jens.lehm...@web.de writes: +test_expect_success PERL 'difftool properly honours gitlink and core.worktree' ' + git submodule add ./. submod/ule + ( + cd submod/ule + git difftool --tool=echo --dir-diff --cached In the context of this fix, finishing with 0 exit status may be all we care about, but do we also care about things like in what directory the tool is invoked in, what arguments and extra environment settings (if any) it is given, and stuff like that? Sure. But I just intended to test the fix (and the test can easily be extended by people who know more about difftool than I do). Yes, we need to start somewhere and I'd agree that it was a good starting point. Right, using echo was not the best choice here. I used it to avoid the dependency to meld... Perhaps like this then? This is an a monkey sees what difftool_test_setup does and then mimics patch ;-). t/t7800-difftool.sh | 13 + 1 file changed, 13 insertions(+) diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index 2418528..595f808 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -434,4 +434,17 @@ test_expect_success PERL 'difftool --no-symlinks detects conflict ' ' ) ' +test_expect_success PERL 'difftool properly honours gitlink and core.worktree' ' + git submodule add ./. submod/ule + ( + cd submod/ule + git config diff.tool checktrees + git config difftool.checktrees.cmd '\'' + test -d $LOCAL test -d $REMOTE + '\'' + echo further file + git difftool --tool=checktrees --dir-diff + ) +' + test_done -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] difftool: support repositories with .git-files
David Aguilar dav...@gmail.com writes:
Modern versions of git submodule use .git-files to setup the
submodule directory. When run in a git submodule-created
repository git difftool --dir-diff dies with the following
error:
$ git difftool -d HEAD~
fatal: This operation must be run in a work tree
diff --raw --no-abbrev -z HEAD~: command returned error: 128
core.worktree is relative to the .git directory but the logic
in find_worktree() does not account for it.
Use `git rev-parse --show-toplevel` to find the worktree so that
the dir-diff feature works inside a submodule.
Reported-by: Gábor Lipták gabor.lip...@gmail.com
Helped-by: Jens Lehmann jens.lehm...@web.de
Helped-by: John Keeping j...@keeping.me.uk
Signed-off-by: David Aguilar dav...@gmail.com
---
Looks good; thanks.
git-difftool.perl | 18 ++
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index e57d3d1..18ca61e 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -39,24 +39,10 @@ USAGE
sub find_worktree
{
- my ($repo) = @_;
-
# Git-repository-wc_path() does not honor changes to the working
# tree location made by $ENV{GIT_WORK_TREE} or the 'core.worktree'
# config variable.
- my $worktree;
- my $env_worktree = $ENV{GIT_WORK_TREE};
- my $core_worktree = Git::config('core.worktree');
-
- if (defined($env_worktree) and (length($env_worktree) 0)) {
- $worktree = $env_worktree;
- } elsif (defined($core_worktree) and (length($core_worktree) 0)) {
- $worktree = $core_worktree;
- } else {
- $worktree = $repo-wc_path();
- }
-
- return $worktree;
+ return Git::command_oneline('rev-parse', '--show-toplevel');
}
sub print_tool_help
@@ -418,7 +404,7 @@ sub dir_diff
my $rc;
my $error = 0;
my $repo = Git-repository();
- my $workdir = find_worktree($repo);
+ my $workdir = find_worktree();
my ($a, $b, $tmpdir, @worktree) =
setup_dir_diff($repo, $workdir, $symlinks);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] difftool: support repositories with .git-files
Am 24.02.2014 17:55, schrieb Junio C Hamano: David Aguilar dav...@gmail.com writes: Modern versions of git submodule use .git-files to setup the submodule directory. When run in a git submodule-created repository git difftool --dir-diff dies with the following error: $ git difftool -d HEAD~ fatal: This operation must be run in a work tree diff --raw --no-abbrev -z HEAD~: command returned error: 128 core.worktree is relative to the .git directory but the logic in find_worktree() does not account for it. Use `git rev-parse --show-toplevel` to find the worktree so that the dir-diff feature works inside a submodule. Reported-by: Gábor Lipták gabor.lip...@gmail.com Helped-by: Jens Lehmann jens.lehm...@web.de Helped-by: John Keeping j...@keeping.me.uk Signed-off-by: David Aguilar dav...@gmail.com --- Looks good; thanks. FWIW: Tested-by: Jens Lehmann jens.lehm...@web.de What about squashing this in to detect any future regressions? diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index 2418528..d86ad68 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -434,4 +434,12 @@ test_expect_success PERL 'difftool --no-symlinks detects conflict ' ' ) ' +test_expect_success PERL 'difftool properly honours gitlink and core.worktree' ' + git submodule add ./. submod/ule + ( + cd submod/ule + git difftool --tool=echo --dir-diff --cached + ) +' + test_done -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
