* Using Filemerge.app as a git-diff viewer
@ 2007-11-21 10:31 Toby White
2007-11-21 11:20 ` Jonathan del Strother
2007-11-21 11:27 ` Jeff King
0 siblings, 2 replies; 9+ messages in thread
From: Toby White @ 2007-11-21 10:31 UTC (permalink / raw)
To: git; +Cc: Wincent Colaiuta
Mac OS X bundles a rather nice graphical diff viewer (Filemerge.app)
with its developer tools.
While git-merge knows how to use this as a merge tool, I couldn't
find any way to easily use Filemerge as a viewer for the output
of git-diff.
(http://thread.gmane.org/gmane.comp.version-control.git/58702
discusses the problem, and recommends piping git-diff into kompare.
Filemerge unfortunately won't accept diff output on stdin)
So I wrote a quick script (below) which does what I need. Of all
the available git-diff flags, it only understands "--cached", and
up to two commit objects, and no paths, but that's enough for me.
Within those constraints, it has the same semantics as git-diff.
It's not very nice, but in case anyone else wants this:
#!/bin/sh
#
# Filemerge.app must not already be open before running
# this script, or opendiff below will return immediately,
# and the TMPDIRs deleted before it gets the chance to read
# them.
if test $# = 0; then
OLD=`git-write-tree`
elif test "$1" = --cached; then
OLD=HEAD
NEW=`git-write-tree`
shift
fi
if test $# -gt 0; then
OLD="$1"; shift
fi
test $# -gt 0 && test -z "$CACHED" && NEW="$1"
TMPDIR1=`mktemp -d`
git-archive --format=tar $OLD | (cd $TMPDIR1; tar xf -)
if test -z "$NEW"; then
TMPDIR2=$(git rev-parse --show-cdup)
test -z "$cdup" && TMPDIR2=.
else
TMPDIR2=`mktemp -d`
git-archive --format=tar $NEW | (cd $TMPDIR2; tar xf -)
fi
opendiff $TMPDIR1 $TMPDIR2 | cat
rm -rf $TMPDIR1
test ! -z "$NEW" && rm -rf $TMPDIR2
--
Dr. Toby O. H. White
Dept. Earth Sciences,
Downing Street,
Cambridge CB2 3EQ
United Kingdom
Tel: +44 1223 333464
Fax: +44 1223 333450
Web: http://uszla.me.uk
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Using Filemerge.app as a git-diff viewer
2007-11-21 10:31 Using Filemerge.app as a git-diff viewer Toby White
@ 2007-11-21 11:20 ` Jonathan del Strother
2007-11-21 11:27 ` Jeff King
1 sibling, 0 replies; 9+ messages in thread
From: Jonathan del Strother @ 2007-11-21 11:20 UTC (permalink / raw)
To: Toby White; +Cc: git, Wincent Colaiuta
On 21 Nov 2007, at 10:31, Toby White wrote:
> Mac OS X bundles a rather nice graphical diff viewer (Filemerge.app)
> with its developer tools.
>
> While git-merge knows how to use this as a merge tool, I couldn't
> find any way to easily use Filemerge as a viewer for the output
> of git-diff.
>
> (http://thread.gmane.org/gmane.comp.version-control.git/58702
> discusses the problem, and recommends piping git-diff into kompare.
> Filemerge unfortunately won't accept diff output on stdin)
>
> So I wrote a quick script (below) which does what I need. Of all
> the available git-diff flags, it only understands "--cached", and
> up to two commit objects, and no paths, but that's enough for me.
> Within those constraints, it has the same semantics as git-diff.
> ...
Handy, thanks.
Just as a note to anyone who runs into the same problems as me - I
couldn't persuade this script to work at first. On my Leopard
install, mktemp needs a template argument to run. After replacing
both `mktemp -d`s in this script with `mktemp -d /tmp/diff.XXXXX`, it
works nicely.
Jon
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Using Filemerge.app as a git-diff viewer
2007-11-21 10:31 Using Filemerge.app as a git-diff viewer Toby White
2007-11-21 11:20 ` Jonathan del Strother
@ 2007-11-21 11:27 ` Jeff King
2007-11-21 12:59 ` Wincent Colaiuta
2007-11-21 13:10 ` Toby White
1 sibling, 2 replies; 9+ messages in thread
From: Jeff King @ 2007-11-21 11:27 UTC (permalink / raw)
To: Toby White; +Cc: git, Wincent Colaiuta
On Wed, Nov 21, 2007 at 10:31:46AM +0000, Toby White wrote:
> So I wrote a quick script (below) which does what I need. Of all
> the available git-diff flags, it only understands "--cached", and
> up to two commit objects, and no paths, but that's enough for me.
> Within those constraints, it has the same semantics as git-diff.
Have you looked at the documentation for GIT_EXTERNAL_DIFF (try git(7))?
I think it is a cleaner way of doing what you want (although I think you
will get each file diffed individually, which is perhaps not what you
want).
Something like:
$ cat >merge.sh <<EOF
#!/bin/sh
opendiff "$1" "$2"
EOF
$ GIT_EXTERNAL_DIFF=./merge.sh git-diff ...
> #!/bin/sh
> #
> # Filemerge.app must not already be open before running
> # this script, or opendiff below will return immediately,
> # and the TMPDIRs deleted before it gets the chance to read
> # them.
>
> if test $# = 0; then
> OLD=`git-write-tree`
> elif test "$1" = --cached; then
> OLD=HEAD
> NEW=`git-write-tree`
> shift
> fi
> if test $# -gt 0; then
> OLD="$1"; shift
> fi
> test $# -gt 0 && test -z "$CACHED" && NEW="$1"
write-tree? Yikes. If you want to diff against the working tree, then do
that. If you want to diff against the index, then you probably want to
git-checkout-index to a tmpdir, and diff against that.
> git-archive --format=tar $OLD | (cd $TMPDIR1; tar xf -)
Again, this could be simpler and faster by using git-checkout-index
(preceded by git-read-tree into a temp index, if you are comparing
against a tree).
-Peff
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Using Filemerge.app as a git-diff viewer
2007-11-21 11:27 ` Jeff King
@ 2007-11-21 12:59 ` Wincent Colaiuta
2007-11-21 13:04 ` Jeff King
2007-11-21 13:58 ` Jakub Narebski
2007-11-21 13:10 ` Toby White
1 sibling, 2 replies; 9+ messages in thread
From: Wincent Colaiuta @ 2007-11-21 12:59 UTC (permalink / raw)
To: Jeff King; +Cc: Toby White, git
El 21/11/2007, a las 12:27, Jeff King escribió:
> On Wed, Nov 21, 2007 at 10:31:46AM +0000, Toby White wrote:
>
>> So I wrote a quick script (below) which does what I need. Of all
>> the available git-diff flags, it only understands "--cached", and
>> up to two commit objects, and no paths, but that's enough for me.
>> Within those constraints, it has the same semantics as git-diff.
>
> Have you looked at the documentation for GIT_EXTERNAL_DIFF (try
> git(7))?
> I think it is a cleaner way of doing what you want (although I think
> you
> will get each file diffed individually, which is perhaps not what you
> want).
>
> Something like:
>
> $ cat >merge.sh <<EOF
> #!/bin/sh
> opendiff "$1" "$2"
> EOF
> $ GIT_EXTERNAL_DIFF=./merge.sh git-diff ...
A few problems with that:
- FileMerge is broken, and crashes if you pass /dev/null as a param
(which happens for new/deleted files)
- you need to escape those $-signs
- the params you're interested in are actually $2 and $5, not $1 and
$2, according to git(7)
- you need to handle the clean tree case (no params are passed)
- and also the 1-param case, "for unmerged paths", whatever that means
- will only work when run from the top of the tree (path parameters
are passed in relative to that)
So here's a less-broken version of your suggestion, but it's still
broken; a relatively complex wrapper is required to do this right:
$ cat >merge.sh <<EOF
#!/bin/sh
[ \$# -eq 7 ] && opendiff "\$2" "\$5"
EOF
chmod +x merge.sh
GIT_EXTERNAL_DIFF=./merge.sh git-diff ...
Cheers,
Wincent
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Using Filemerge.app as a git-diff viewer
2007-11-21 12:59 ` Wincent Colaiuta
@ 2007-11-21 13:04 ` Jeff King
2007-11-21 13:58 ` Jakub Narebski
1 sibling, 0 replies; 9+ messages in thread
From: Jeff King @ 2007-11-21 13:04 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: Toby White, git
On Wed, Nov 21, 2007 at 01:59:37PM +0100, Wincent Colaiuta wrote:
> So here's a less-broken version of your suggestion, but it's still broken; a
> relatively complex wrapper is required to do this right:
Thanks. My "something like" should more accurately have been "I really
didn't think about this at all." But I do think a GIT_EXTERNAL_DIFF-type
approach (and if GIT_EXTERNAL_DIFF isn't powerful enough, considering
some alternate interface) is nicer than trying to wrap git-diff.
> $ cat >merge.sh <<EOF
> #!/bin/sh
> [ \$# -eq 7 ] && opendiff "\$2" "\$5"
> EOF
This is probably easier to read using <<'EOF' to quote (but I failed to
do even that in my example).
-Peff
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Using Filemerge.app as a git-diff viewer
2007-11-21 11:27 ` Jeff King
2007-11-21 12:59 ` Wincent Colaiuta
@ 2007-11-21 13:10 ` Toby White
2007-11-21 13:28 ` Wincent Colaiuta
2007-11-21 14:36 ` Jeff King
1 sibling, 2 replies; 9+ messages in thread
From: Toby White @ 2007-11-21 13:10 UTC (permalink / raw)
To: Jeff King; +Cc: git, Wincent Colaiuta
Jeff King wrote:
> Have you looked at the documentation for GIT_EXTERNAL_DIFF (try git(7))?
> I think it is a cleaner way of doing what you want (although I think you
> will get each file diffed individually, which is perhaps not what you
> want).
>
> Something like:
>
> $ cat >merge.sh <<EOF
> #!/bin/sh
> opendiff "$1" "$2"
> EOF
> $ GIT_EXTERNAL_DIFF=./merge.sh git-diff ...
I hadn't seen that, no, but that's not quite right.
(Wincent pointed out its flaws better than me. Basically,
opendiff is not really diff-like enough.)
And in any case, that launches Filemerge repeatedly
on every file separately, which makes reviewing a large diff
time-consuming and not very helpful.
> write-tree? Yikes. If you want to diff against the working tree, then do
> that. If you want to diff against the index, then you probably want to
> git-checkout-index to a tmpdir, and diff against that.
Am I misunderstanding the documentation? From man git-write-tree
"Conceptually, git-write-tree sync()s the current index contents into a
set of tree files. In order to have that match what is actually in your
directory right now, you need to have done a git-update-index phase
before you did the git-write-tree."
So git-write-tree precisely does give you the index not the working tree,
by my reading.
>> git-archive --format=tar $OLD | (cd $TMPDIR1; tar xf -)
>
> Again, this could be simpler and faster by using git-checkout-index
> (preceded by git-read-tree into a temp index, if you are comparing
> against a tree).
Erm, ok, this is rapidly approaching the limit of my git knowledge,
but while I can see git-read-tree will write a tree-ish into a temp
index,
(so presumably
git-read-tree --index-output=$TMPFILE <commit>
ought to work. Except it doesn't, I get the error message
fatal: unable to write new index file
),
I can't see how to make git-checkout-index read from a temp index.
And I'm assuming I don't want to go stomping all over the current
index just in order to do a diff, which shouldn't change the state
of my repository.
Is there a canonical way to checkout a given commit object into
a fresh directory?
--
Dr. Toby O. H. White
Dept. Earth Sciences,
Downing Street,
Cambridge CB2 3EQ
United Kingdom
Tel: +44 1223 333464
Fax: +44 1223 333450
Web: http://uszla.me.uk
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Using Filemerge.app as a git-diff viewer
2007-11-21 13:10 ` Toby White
@ 2007-11-21 13:28 ` Wincent Colaiuta
2007-11-21 14:36 ` Jeff King
1 sibling, 0 replies; 9+ messages in thread
From: Wincent Colaiuta @ 2007-11-21 13:28 UTC (permalink / raw)
To: Toby White; +Cc: Jeff King, git
El 21/11/2007, a las 14:10, Toby White escribió:
> (Wincent pointed out its flaws better than me. Basically,
> opendiff is not really diff-like enough.)
>
> And in any case, that launches Filemerge repeatedly
> on every file separately, which makes reviewing a large diff
> time-consuming and not very helpful.
I know it's not much help to you right now, but when I first asked
about a side-by-side diff viewer back in September I explored creating
a wrapper using GIT_EXTERNAL_DIFF and was basically unsatisfied with
the results.
So I decided to write a very simple native Cocoa app (I'm working on
Mac OS X); it was to be a diff viewer and nothing more (not a
repository browser). I was able to spend a few hours on it back in
September but then other things came up so I haven't been able to
finish it, but I do intend to come back to it when time permits. You
can browse the work in progress at:
<http://git.wincent.com/gdiff.git>
Basically consists of a fast Ragel-generated state machine for parsing
git-diff output, patches and email messages, and the beginnings of a
UI. But like I said, not much help to you right now as it can't yet be
used to do anything useful. My goal for it is to provide a nice
experience, above all when reviewing patches on a mailing list (ie.
where your local repository doesn't have the proposed commits in it
yet). In fact, even when run from outside of any Git repo I want it to
at least show the known parts of the files (the context) and provide a
shaded background for the unknown bits.
Cheers,
Wincent
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Using Filemerge.app as a git-diff viewer
2007-11-21 12:59 ` Wincent Colaiuta
2007-11-21 13:04 ` Jeff King
@ 2007-11-21 13:58 ` Jakub Narebski
1 sibling, 0 replies; 9+ messages in thread
From: Jakub Narebski @ 2007-11-21 13:58 UTC (permalink / raw)
To: git
Wincent Colaiuta wrote:
> - you need to escape those $-signs
[...]
> $ cat >merge.sh <<EOF
Or use
$ cat >merge.sh <<\EOF
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Using Filemerge.app as a git-diff viewer
2007-11-21 13:10 ` Toby White
2007-11-21 13:28 ` Wincent Colaiuta
@ 2007-11-21 14:36 ` Jeff King
1 sibling, 0 replies; 9+ messages in thread
From: Jeff King @ 2007-11-21 14:36 UTC (permalink / raw)
To: Toby White; +Cc: git, Wincent Colaiuta
On Wed, Nov 21, 2007 at 01:10:47PM +0000, Toby White wrote:
> And in any case, that launches Filemerge repeatedly
> on every file separately, which makes reviewing a large diff
> time-consuming and not very helpful.
I think that particular complaint of GIT_EXTERNAL_DIFF has come up
before. It probably wouldn't be too hard to support a separate
environment variable to mean "diff these two trees" (or tree/cache, or
cache/working tree, etc) rather than "diff these two files".
> Am I misunderstanding the documentation? From man git-write-tree
>
> "Conceptually, git-write-tree sync()s the current index contents into a
> set of tree files. In order to have that match what is actually in your
> directory right now, you need to have done a git-update-index phase
> before you did the git-write-tree."
>
> So git-write-tree precisely does give you the index not the working tree,
> by my reading.
What you are doing isn't _wrong_, but it is not the most efficient way
of doing it, and it is a little bit awkward, I think.
There are basically three conceptual "spots" for files in git:
- in the object database (i.e., tree objects pointing to blobs)
- in the index (the blobs are actually in the object db)
- in the working tree (i.e., actual files)
git-write-tree moves data from the index into the object database
(git-commit is more or less git-write-tree followed by git-commit-tree).
git-archive (piped to tar) moves data from the object database (because
it takes a tree object name) into the working tree.
So you are basically moving data into the object db, and then out to the
working tree. But there is a command that moves data directly from the
index to the working tree: git-checkout-index.
Now, what you are doing is not terribly inefficient in that sense, since
creating a tree object is usually pretty inexpensive. But it does
involve writing to the object db for a diff, which is conceptually a
read-only operation.
However, git-checkout-index also avoids piping through tar, which is
likely to be faster for a large data set.
> Erm, ok, this is rapidly approaching the limit of my git knowledge,
BTW, I don't mean to nitpick your script, which obviously works for you.
I'm just trying to increase your git knowledge. :)
> but while I can see git-read-tree will write a tree-ish into a temp
> index,
>
> (so presumably
> git-read-tree --index-output=$TMPFILE <commit>
> ought to work. Except it doesn't, I get the error message
> fatal: unable to write new index file
> ),
Are you perhaps running against the rename(2)-ability boundaries
mentioned in the man page? --index-output is relatively new, and I'm not
sure of the reasons surrounding it. I think what you really want to work
with a temp index is to set GIT_INDEX_FILE.
> I can't see how to make git-checkout-index read from a temp index.
It should read from GIT_INDEX_FILE. So the correct incantation for
checking out a tree should be:
GIT_INDEX_FILE=$TMPFILE git-read-tree <commit>
GIT_INDEX_FILE=$TMPFILE git-checkout-index -a --prefix=$TMPDIR/
and of course, for the --cached case, you can just use the existing
index:
git-checkout-index -a --prefix=$TMPDIR/
(btw, I have no idea if creating a temp index is actually faster than
piping through tar. I suspect it is, but haven't tested. So it may be
sane to use checkout-index for the --cached case, which is what I was
originally suggesting, and simply use git-archive for the tree case).
> And I'm assuming I don't want to go stomping all over the current
> index just in order to do a diff, which shouldn't change the state
> of my repository.
Correct.
> Is there a canonical way to checkout a given commit object into
> a fresh directory?
git-archive piped to tar is the canonical porcelain way (again, I was
suggesting checkout-index mainly for the --cached case). The way I
showed above is the "plumbing" way (and is what underlies git-checkout,
for example, though of course a temp index is not needed in that case).
Hope that make sense.
-Peff
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2007-11-21 14:37 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-11-21 10:31 Using Filemerge.app as a git-diff viewer Toby White
2007-11-21 11:20 ` Jonathan del Strother
2007-11-21 11:27 ` Jeff King
2007-11-21 12:59 ` Wincent Colaiuta
2007-11-21 13:04 ` Jeff King
2007-11-21 13:58 ` Jakub Narebski
2007-11-21 13:10 ` Toby White
2007-11-21 13:28 ` Wincent Colaiuta
2007-11-21 14:36 ` Jeff King
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).