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.
>
> 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.
> 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.
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.
Thanks for the comments.
- Fredrik
-
: 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