Hi,
On Thu, 23 Feb 2006, Fredrik Kuivinen wrote:
> On Thu, Feb 23, 2006 at 05:10:49PM -0500, Ryan Anderson wrote:
> > (Biased critique since I have the other tool in the tree, but still...)
> >
> > > + FILE* fout = fopen("/tmp/git-blame-tmp1", "w");
> >
> > Probably should be using something like mkstemp (mkstmp?) here, so if
> > someone is runnign things as root or with a malicous user around, things
> > don't collide.
> >
> > Hell, so on a multi-user machine this doesn't blow up on you.
>
> Yep, I know. The code is mostly a proof of concept. I didn't submit it
> for inclusion.
Ha ha, famous last words!
> > But, read down for a related comment.
> >
> > > + if(!fout)
> > > + die("fopen tmp1 failed: %s", strerror(errno));
> > > +
> > > + if(fwrite(info_c->buf, info_c->size, 1, fout) != 1)
> > > + die("fwrite 1 failed: %s", strerror(errno));
> > > + fclose(fout);
> > > +
> > > + fout = fopen("/tmp/git-blame-tmp2", "w");
> > > + if(!fout)
> > > + die("fopen tmp2 failed: %s", strerror(errno));
> > > +
> > > + if(fwrite(info_o->buf, info_o->size, 1, fout) != 1)
> > > + die("fwrite 2 failed: %s", strerror(errno));
> > > + fclose(fout);
> > > +
> > > + FILE* fin = popen("diff -u0 /tmp/git-blame-tmp1 /tmp/git-blame-tmp2", "r");
> > > + if(!fin)
> > > + die("popen failed: %s", strerror(errno));
> >
> > Can't git-diff-tree do this sufficiently, anyway? See my Perl script
> > for an example, you just need both commit IDs and both filenames and the
> > appropriate -M and you get the right results.
> >
> > (It's possible that's part of where the performance differences are,
> > though, not really sure at the moment.)
> >
>
> Yeah.. maybe. My first thought was to avoid forking and execing diff
> and use some C library for doing the diffing instead (libxdiff). But
> then I just wanted to get some code working and the simplest solution
> I could think of was to fork and exec diff.
git-diff-tree fork()s a diff. So, by fork()ing git-diff-tree you get two
fork()s (and no knife...).
> > I'm going to stop there for the moment, I'm not really confident in my
> > understanding of git-internals to say much more just yet.
> >
> > This could probably benefit a *LOT* from the libification project, I
> > think, though.
>
> Yes, perhaps. Some of the git-rev-list bits might simplify a couple of
> things.
The major problem is probably not solved: What Linus calls a "stream
interface".
I.e. if you pipe the output of git-rev-list to another program, you
*need* to execute the two semi-simultaneously. The "alternative" would be
to use buffers, which can get huge (and are sometimes not needed: think
git-whatchanged, which starts outputting before it's getting no more
input).
> I have found some severe problems with the code I posted, in
> particular it doesn't handle parallel development tracks at all. I am
> working on fixing it, but it isn't finished yet.
Looking forward to them!
Ciao,
Dscho
-
: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html