git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* 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).