Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Port git-annotate to qx{} syntax, and make default output more like CVS
From: Alex Riesen @ 2006-02-23 15:08 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, junkio
In-Reply-To: <Pine.LNX.4.63.0602231442390.24946@wbgn013.biozentrum.uni-wuerzburg.de>

On 2/23/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> This uses the qx{} syntax Alex digged up, and it makes output without "-l"
> quite similar to CVS's annotate output.
>

Works here.

^ permalink raw reply

* FYI: git-am allows creation of empty commits.
From: Eric W. Biederman @ 2006-02-23 15:33 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano


Currently I am using: git version 1.2.2.g709a.

I have been rebasing and refactoring a number of patchsets using git.

Frequently because of prior edits I will git a patch that does not
apply at all.  So all I get is the creation of the .dotest directory.
The automatic merge logic does not kick in, so I have to manually
call patch -p1 and resolve the differences but hand.  There everything
is expected and fine.  Although it might have been nice to have one
of those failed merge indicators in the index.

After fixing it up and doing all of my edits I occasionally forget
the git-update-index step, before calling git-am --resolved.  This
proceeds along it's merry way and creates an empty commit.

Is this the expected correct behavior or should git-am complain about
a situation like that?

Eric

^ permalink raw reply

* Re: [PATCH] Convert open("-|") to qx{} calls
From: Alex Riesen @ 2006-02-23 15:38 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, junkio
In-Reply-To: <Pine.LNX.4.63.0602231532470.29635@wbgn013.biozentrum.uni-wuerzburg.de>

On 2/23/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>       Since of these 4, I only use cvsimport myself, I could only test
>       that. Could someone who uses the others give them a hard beating?

I can't really test them (no svn and cvs, and locked down network), but I took
a look at the patches. Hope it helps.

git-cvsimport:

> -               open(F,"git-cat-file commit $ftag |");
> -               while(<F>) {
> +               foreach (qx{git-cat-file commit $ftag}) {
>                         next unless /^author\s.*\s(\d+)\s[-+]\d{4}$/;

Are you sure you don't need quoting/safe pipe here?
Or is it a CVS tag?

> +} else {
> +    @input = qx{cvsps --norc opt -u -A --root $opt_d $cvs_tree};
> +    !$? or exit $?;

Same here. $cvs_tree can contain any filesystem-allowed character.

git-svnimport:

> -                       my $sha = <$F>;
> +                       my $sha = qx{git-hash-object -w $tmpname};
> +                       !$? or exit $?;

Is $tmpname safe?

> -       my $sha = <$F>;
> +       my $sha = qx{git-hash-object -w $name};
> +       !$? or exit $?;

Is $name safe?

> -       while(<$f>) {
> +       foreach (qx{git-ls-tree -r -z $gitrev $srcpath}) {
>                 chomp;

Is $srcpath safe?

> -                       while(<$F>) {
> +                       foreach (qx{git-ls-files -z @o1}) {

@o1 must contain filenames. Can be dangerous

^ permalink raw reply

* Re: [PATCH] Convert open("-|") to qx{} calls
From: Randal L. Schwartz @ 2006-02-23 16:07 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Johannes Schindelin, git, junkio
In-Reply-To: <81b0412b0602230738s3445bd86h2d1d670e0ef5daed@mail.gmail.com>

>>>>> "Alex" == Alex Riesen <raa.lkml@gmail.com> writes:

Alex> Is $tmpname safe?

>> -       my $sha = <$F>;
>> +       my $sha = qx{git-hash-object -w $name};
>> +       !$? or exit $?;

Alex> Is $name safe?

>> -       while(<$f>) {
>> +       foreach (qx{git-ls-tree -r -z $gitrev $srcpath}) {
>> chomp;

Alex> Is $srcpath safe?

>> -                       while(<$F>) {
>> +                       foreach (qx{git-ls-files -z @o1}) {

Alex> @o1 must contain filenames. Can be dangerous

Convert all of these to use "safe_qx" (perl 5.6 compatible):

    sub safe_qx {
      defined (my $pid = open my $kid, "-|") or die "Cannot fork: $!";
      unless ($pid) { # child does:
        exec @_;
        die "Cannot exec @_: $!";
      }
      my $result = do { local $/; <$kid> };
      close $kid;                   # sets $?
      return $result;
    }

my $result = safe_qx('some shell command');
my $other_result = safe_qx('git-ls-tree', '-r', '-z', $gitrev, $srcpath);

Args are safe, as if being passed to system/exec, so a single arg
can be a shell command, multiargs are passed arg-by-arg to a single
exec target.  $? is set correctly.

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
From: Linus Torvalds @ 2006-02-23 17:13 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Andreas Ericsson, Junio C Hamano, git
In-Reply-To: <81b0412b0602230607n22146a77k36929f0ad9e44d53@mail.gmail.com>



On Thu, 23 Feb 2006, Alex Riesen wrote:
>
> Someday we'll have to start dropping features on Windows or restrict them
> beyond their usefullness.

One thing that would help a bit would be to avoid shell.

There are many portable interpreters out there, and I don't mean perl. And 
writing a small "specialized for git" one isn't even that hard. In fact, 
most of the shell (and bash) hackery we do now would be unnecessary if we 
just made a small "git interpreter" that ran "git scripts".

The fact that it would also help portability is just an added advantage.

		Linus

^ permalink raw reply

* Re: [PATCH] Convert open("-|") to qx{} calls
From: Junio C Hamano @ 2006-02-23 17:53 UTC (permalink / raw)
  To: Randal L. Schwartz; +Cc: Alex Riesen, Johannes Schindelin, git
In-Reply-To: <86hd6qgit5.fsf@blue.stonehenge.com>

merlyn@stonehenge.com (Randal L. Schwartz) writes:

> Convert all of these to use "safe_qx" (perl 5.6 compatible):
>
>     sub safe_qx {
>       defined (my $pid = open my $kid, "-|") or die "Cannot fork: $!";
>...

IIRC, that is backwards.  The thread's conversion is not about
5.6 vs 5.8.  The conversion like what you suggested above was
done, but the thing is, and Alex's Perl is unhappy about it.

The version of Perl Alex has to use claims to be 5.8, but does
not understand open($kid, '-|'), and he is trying to come up
with a workaround.

I wish Perl had a stricter trademark policy that required
language features to be fully ported for an implementation to
use that name ;-).

^ permalink raw reply

* Re: [PATCH] Convert open("-|") to qx{} calls
From: Randal L. Schwartz @ 2006-02-23 18:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alex Riesen, Johannes Schindelin, git
In-Reply-To: <7v1wxuhsgw.fsf@assigned-by-dhcp.cox.net>

>>>>> "Junio" == Junio C Hamano <junkio@cox.net> writes:

Junio> The version of Perl Alex has to use claims to be 5.8, but does
Junio> not understand open($kid, '-|'), and he is trying to come up
Junio> with a workaround.

Ahh, the problem is activestate then.  If that's the case, then amend the code
with a check for $^O (operating system) that falls back to a qx if on
activestate, and hope that filenames aren't a problem.  Unfortunately, I don't
know enough about that to fix it.

But whatever you do, *don't* replace safe_qx with qx() for all other systems,
or you'll be opening up a can of worms for those of us on sensible systems.

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

^ permalink raw reply

* gitview: Display the lines joining commit nodes clearly.
From: Aneesh Kumar K.V @ 2006-02-23 19:29 UTC (permalink / raw)
  To: git, Junio C Hamano, Aneesh Kumar K.V

[-- Attachment #1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2: 0001-gitview-Display-the-lines-joining-commit-nodes-clearly.txt --]
[-- Type: text/plain, Size: 3255 bytes --]


Since i wanted to limit the graph box size i was resetting
the window after an index of 5. This result in line joining
commit nodes to pass over nodes which are not related. The
changes fixes the same

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@gmail.com>

---

 contrib/gitview/gitview |   48 ++++++++++++++++++++++++++++++++++-------------
 1 files changed, 35 insertions(+), 13 deletions(-)

3fcd411c65b2c6f84052d69ca05766e45a34682e
diff --git a/contrib/gitview/gitview b/contrib/gitview/gitview
index 4b52eb7..b04df74 100755
--- a/contrib/gitview/gitview
+++ b/contrib/gitview/gitview
@@ -823,6 +823,7 @@ class GitView:
 		self.colours = {}
 		self.nodepos = {}
 		self.incomplete_line = {}
+		self.commits = []
 
 		index = 0
 		last_colour = 0
@@ -840,12 +841,7 @@ class GitView:
 
 			commit = Commit(commit_lines)
 			if (commit != None ):
-				(out_line, last_colour, last_nodepos) = self.draw_graph(commit,
-										index, out_line,
-										last_colour,
-										last_nodepos)
-				self.index[commit.commit_sha1] = index
-				index += 1
+				self.commits.append(commit)
 
 			# Skip the '\0
 			commit_lines = []
@@ -854,6 +850,14 @@ class GitView:
 
 		fp.close()
 
+		for commit in self.commits:
+			(out_line, last_colour, last_nodepos) = self.draw_graph(commit,
+										index, out_line,
+										last_colour,
+										last_nodepos)
+			self.index[commit.commit_sha1] = index
+			index += 1
+
 		self.treeview.set_model(self.model)
 		self.treeview.show()
 
@@ -869,13 +873,6 @@ class GitView:
 			last_nodepos = 0
 
 		# Add the incomplete lines of the last cell in this
-		for sha1 in self.incomplete_line.keys():
-			if ( sha1 != commit.commit_sha1):
-				for pos in self.incomplete_line[sha1]:
-					in_line.append((pos, pos, self.colours[sha1]))
-			else:
-				del self.incomplete_line[sha1]
-
 		try:
 			colour = self.colours[commit.commit_sha1]
 		except KeyError:
@@ -897,6 +894,14 @@ class GitView:
 			self.colours[commit.parent_sha1[0]] = colour
 			self.nodepos[commit.parent_sha1[0]] = node_pos
 
+		for sha1 in self.incomplete_line.keys():
+			if ( sha1 != commit.commit_sha1):
+				self.draw_incomplete_line(sha1, node_pos,
+						out_line, in_line, index)
+			else:
+				del self.incomplete_line[sha1]
+
+
 		in_line.append((node_pos, self.nodepos[commit.parent_sha1[0]],
 					self.colours[commit.parent_sha1[0]]))
 
@@ -936,6 +941,23 @@ class GitView:
 		except KeyError:
 			self.incomplete_line[sha1] = [self.nodepos[sha1]]
 
+	def draw_incomplete_line(self, sha1, node_pos, out_line, in_line, index):
+		for idx, pos in enumerate(self.incomplete_line[sha1]):
+			if(pos == node_pos):
+				out_line.append((pos,
+					pos+0.5, self.colours[sha1]))
+				self.incomplete_line[sha1][idx] = pos = pos+0.5
+			try:
+				next_commit = self.commits[index+1]
+				if (next_commit.commit_sha1 == sha1 and pos != int(pos)):
+				# join the line back to the node point 
+				# This need to be done only if we modified it
+					in_line.append((pos, pos-0.5, self.colours[sha1]))
+					continue;
+			except IndexError:
+				pass
+			in_line.append((pos, pos, self.colours[sha1]))
+
 
 	def _go_clicked_cb(self, widget, revid):
 		"""Callback for when the go button for a parent is clicked."""
-- 
1.2.3.g2cf3-dirty


^ permalink raw reply related

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
From: Junio C Hamano @ 2006-02-23 19:32 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0602230911410.3771@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> There are many portable interpreters out there, and I don't mean perl. And 
> writing a small "specialized for git" one isn't even that hard. In fact, 
> most of the shell (and bash) hackery we do now would be unnecessary if we 
> just made a small "git interpreter" that ran "git scripts".

Before anybody mentions tcl ;-).

I agree with the above in principle, but I am afraid that is
only half of the solution to the problem Alex is having.

In the longer term, libified git with script language bindings
would make the way git things work together a lot better.  I've
always wanted to make merge-base a subroutine callable from
other things, so that I can say "git diff A...B" to mean "diff
up to B since B forked from A" ;-).

That way, we would eliminate the current common pattern of
piping rev-list output to diff-tree, or ls-files/diff-files
output to update-index --stdin.  These components live in the
single process, a calling "git script", and will talk with each
other internally.

But we do need to talk to non-git things.  git-grep needs a way
for ls-files to drive xargs/grep, for example.  diff --cc reads
from GNU diff output.  And for these external tools, the way
they expect the input to be fed to them or their output is taken
out is via UNIXy pipe.

And the breakage Alex wants to work around is that the platform
is not friendly to pipes, if you deny Cygwin.  So I suspect
avoiding shell would not help much.

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
From: Johannes Schindelin @ 2006-02-23 19:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7virr5hnw4.fsf@assigned-by-dhcp.cox.net>

Hi,

On Thu, 23 Feb 2006, Junio C Hamano wrote:

> Linus Torvalds <torvalds@osdl.org> writes:
> 
> > There are many portable interpreters out there, and I don't mean perl. And 
> > writing a small "specialized for git" one isn't even that hard. In fact, 
> > most of the shell (and bash) hackery we do now would be unnecessary if we 
> > just made a small "git interpreter" that ran "git scripts".
> 
> Before anybody mentions tcl ;-).

Darn, I had my suggestion sent out: Java ;-)

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Convert open("-|") to qx{} calls
From: Johannes Schindelin @ 2006-02-23 19:41 UTC (permalink / raw)
  To: Randal L. Schwartz; +Cc: Junio C Hamano, Alex Riesen, git
In-Reply-To: <863bi9hq6u.fsf@blue.stonehenge.com>

Hi,

On Thu, 23 Feb 2006, Randal L. Schwartz wrote:

> >>>>> "Junio" == Junio C Hamano <junkio@cox.net> writes:
> 
> Junio> The version of Perl Alex has to use claims to be 5.8, but does
> Junio> not understand open($kid, '-|'), and he is trying to come up
> Junio> with a workaround.
> 
> Ahh, the problem is activestate then.  If that's the case, then amend 
> the code with a check for $^O (operating system) that falls back to a qx 
> if on activestate, and hope that filenames aren't a problem.  
> Unfortunately, I don't know enough about that to fix it.

Now that our local Perl guru joined the discussion, may I ask what is, and 
what is not quoted when put inside qx{}? I had the impression that all 
arguments are quoted, except that variables are resolved first. Was that 
wrong? IOW does

	qx{bash $variable}

quote the value of $variable, or not?

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
From: Linus Torvalds @ 2006-02-23 19:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7virr5hnw4.fsf@assigned-by-dhcp.cox.net>



On Thu, 23 Feb 2006, Junio C Hamano wrote:
>
> Linus Torvalds <torvalds@osdl.org> writes: 
> > There are many portable interpreters out there, and I don't mean perl. And 
> > writing a small "specialized for git" one isn't even that hard. In fact, 
> > most of the shell (and bash) hackery we do now would be unnecessary if we 
> > just made a small "git interpreter" that ran "git scripts".
> 
> Before anybody mentions tcl ;-).

Well, I was thinking more of the "embeddable" ones - things that are so 
small that they can be compiled with the project. Things like Lua.

Now, Lua is not really very useful for this use case: our scripts are much 
more about combining other programs - piping the output from one to the 
other - than about any traditional scripting. Which, afaik, Lua isn't good 
at.

> I agree with the above in principle, but I am afraid that is
> only half of the solution to the problem Alex is having.
> 
> In the longer term, libified git with script language bindings
> would make the way git things work together a lot better.  I've
> always wanted to make merge-base a subroutine callable from
> other things, so that I can say "git diff A...B" to mean "diff
> up to B since B forked from A" ;-).

Yeah, we should libify some of it, to make things easier. That said, I 
don't belive in the "big-picture" libification. The fact is, a lot of git 
really _is_ about piping things from one part to another, and library 
interfaces work horribly badly for that. You really want more of a 
"stream" interface, and that's just not something I see happening.

I think one of the strengths of git is that you can use it in a very 
traditional UNIX manner, and do your own pipelines. And that will 
obviously NEVER work well under Windows, if only because it's not the 
natural way to do things.

Again, libification does nothing for that thing.

What I'd suggest using an embedded interpreter for is literally just the 
common helper scripts. We'll never make 

	git-rev-list --header a..b -- tree | 
		grep -z '^author.*torvalds' |
		..

style interesting power-user pipelines work in windows, but we _can_ make 
the things like "git commit" work natively in windows without having to 
re-write it in C by just having an embedded interpreter.

And I very much mean _embedded_. Otherwise we'll just have all the same 
problems with perl and bash and versioning. 

> But we do need to talk to non-git things.  git-grep needs a way
> for ls-files to drive xargs/grep, for example.  diff --cc reads
> from GNU diff output.  And for these external tools, the way
> they expect the input to be fed to them or their output is taken
> out is via UNIXy pipe.

I was really thinking more of a simple shell-like script interpreter. 
Something that we can make portable, by virtue of it _not_ being real 
shell. For example, the "find | xargs" stuff we do is really not that hard 
to do portably even on windows using standard C, it's just that you can't 
do it THAT WAY portably without assuming that it's a full cygwin thing.

		Linus

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
From: Linus Torvalds @ 2006-02-23 19:54 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.63.0602232037260.30630@wbgn013.biozentrum.uni-wuerzburg.de>



On Thu, 23 Feb 2006, Johannes Schindelin wrote:
> > 
> > Before anybody mentions tcl ;-).
> 
> Darn, I had my suggestion sent out: Java ;-)

I do see the smileys, but the fact is, "perl" is a hell of a lot more 
portable than either, if we want to talk executing processes and pipelines 
etc. But even perl is clearly not portable enough, and has tons of version 
skew.

Java, afaik, has absolutely _zero_ support for creating a new process and 
piping its output to another one and doing things like safe argument 
expansion. Which is what almost all of the git scripts are all about.

		Linus

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
From: Johannes Schindelin @ 2006-02-23 20:19 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0602231151580.3771@g5.osdl.org>

Hi,

On Thu, 23 Feb 2006, Linus Torvalds wrote:

> On Thu, 23 Feb 2006, Johannes Schindelin wrote:
> > > 
> > > Before anybody mentions tcl ;-).
> > 
> > Darn, I had my suggestion sent out: Java ;-)
> 
> I do see the smileys, but the fact is, "perl" is a hell of a lot more 
> portable than either, if we want to talk executing processes and pipelines 
> etc. But even perl is clearly not portable enough, and has tons of version 
> skew.
> 
> Java, afaik, has absolutely _zero_ support for creating a new process and 
> piping its output to another one and doing things like safe argument 
> expansion. Which is what almost all of the git scripts are all about.

You are right, but for the wrong reason. Java is actually a wonderful 
thing to create new processes and talk between threads.

But Java is HUGE. No, it is rather HOOODGEEE.

And I don't know if something like Lua does any good. The problem is not 
so much the language. It is the fork().

AFAIAC, cygwin is pretty good at hiding Windows behind sortofa POSIX 
layer. <tongue-in-cheek>It hides it behind a POSIX layer *and* a 
performance hit.</tongue-in-cheek>

I would rather like to see how all the fork()ing and |'ing can be done 
with MinGW32.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
From: Sam Vilain @ 2006-02-23 20:31 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0602231143290.3771@g5.osdl.org>

Linus Torvalds wrote:
>>>There are many portable interpreters out there, and I don't mean perl. And 
>>>writing a small "specialized for git" one isn't even that hard. In fact, 
>>>most of the shell (and bash) hackery we do now would be unnecessary if we 
>>>just made a small "git interpreter" that ran "git scripts".
>>Before anybody mentions tcl ;-).
> Well, I was thinking more of the "embeddable" ones - things that are so 
> small that they can be compiled with the project. Things like Lua.
   [...]
> I was really thinking more of a simple shell-like script interpreter. 

I like the term "Domain Specific Language" to refer to this sort of 
thing.  It even hints at using the right kind of tools to achieve it, too :)

Sam.

^ permalink raw reply

* [PATCH] New git-seek command with documentation and test.
From: Carl Worth @ 2006-02-23 20:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Linus Torvalds
In-Reply-To: <7vu0b1pntl.fsf@assigned-by-dhcp.cox.net>

[-- Attachment #1: Type: text/plain, Size: 10708 bytes --]

Add git-seek which allows for temporary excursions through the
revision history. With "git seek <revision>" one gets a working tree
corresponding to <revision>. When done with the excursion "git seek"
returns back to the original branch from where the first seek began.

Signed-off-by: Carl Worth <cworth@cworth.org>

---

 git-seek could be used as a new basis for git-bisect. This patch does
 not do that, but even so, git-bisect and git-seek should play nicely
 with each other, (in the sense that either will refuse to do anything
 if .git/head-name already exists).

 On Tue, 14 Feb 2006 14:39:34 -0800, Junio C Hamano wrote:
 > Carl Worth <cworth@cworth.org> writes:
 > > [arguments in favor of a new git-seek] 
 > 
 > I think this is a very valid point and I am happy to accept a
 > workable proposal (does not have to be a working patch, but a
 > general semantics that covers most of if not all the corner
 > cases).
 
 I had planned to just let this drop as my original need was some
 historical exploration that I've already finished. But now I've found
 a common use case in my everyday workflow that could benefit from
 git-seek. Here it is:
 
 I receive a bug-fix patch that updates a test case to demonstrate the
 bug. I can apply both the fix and the test case and see it succeed.
 But what I really want to do is first commit the test case, see it
 fail, and only then commit the fix and see the test now succeed.  I'd
 also like the history to reflect that order. So what I do is:
 
 	$ git-am
 	$ git update-index test.c ; git commit -m "Update test"
 	$ git update-index buggy.c ; git commit -m "Fix bug"
 
 At that point, without git-seek I can get by with:
 
 	$ git checkout -b tmp HEAD^
 	$ make check # to see failure
 	$ git checkout <branch_I_was_on_to_begin_with>
 	$ git branch -d tmp # easy to forget, but breaks the next time otherwise
 	$ make check # to see success
 
 But what I'd really like to do, (and can with the attached patch), is:
 
 	$ git seek HEAD^
 	$ make check # to see failure
 	$ git seek
 	$ make check # to see success
 
 This avoids me having to: 1) invent a throwaway name, 2) remember the
 branch I started on, 3) remember to actually throwaway the temporary
 branch.

 I've documented git-seek quite carefully and added a test that tries
 to cover every documented failure mode.

 -Carl

 .gitignore                 |    1 
 Documentation/git-seek.txt |   44 +++++++++++++++++++++
 Makefile                   |    4 +-
 git-seek.sh                |   94 ++++++++++++++++++++++++++++++++++++++++++++
 t/t3800-seek.sh            |   82 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 223 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/git-seek.txt
 create mode 100644 git-seek.sh
 create mode 100755 t/t3800-seek.sh

2656ffb6e3fcbd9443c22b4675b13f23c031600e
diff --git a/.gitignore b/.gitignore
index 94f66d5..55484b0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -85,6 +85,7 @@ git-rev-list
 git-rev-parse
 git-revert
 git-rm
+git-seek
 git-send-email
 git-send-pack
 git-sh-setup
diff --git a/Documentation/git-seek.txt b/Documentation/git-seek.txt
new file mode 100644
index 0000000..cb5c13d
--- /dev/null
+++ b/Documentation/git-seek.txt
@@ -0,0 +1,44 @@
+git-bisect(1)
+=============
+
+NAME
+----
+git-seek - Provide a temporary excursion through the revision history.
+
+
+SYNOPSIS
+--------
+'git seek' [<revision>]
+
+DESCRIPTION
+-----------
+When given a <revision>, git-seek updates the files in the working
+tree to the state of the given revision. It will do this by performing
+a checkout of <revision> to a new branch named "seek", or by resetting
+the seek branch if it already exists.
+
+When run with with no <revision> argument, git-seek will return to the
+original branch from which the initial git-seek operation was
+performed, (this original branch name is saved in $GIT_DIR/head-name).
+
+git-seek refuses to do anything if the working tree or index are
+modified with respect to HEAD. If you want to carry modifications
+around, use git-checkout rather than git-seek.
+
+git-seek will also fail if GIT_DIR/head-name exists when a seek is not
+already in progress, or if a seek branch already exists that is not a
+subset of the current branch, (that is, if it has unmerged commits).
+
+Author
+------
+Written by Carl Worth <cworth@cworth.org>, based on git-bisect by
+Linus Torvalds <torvalds@osdl.org>
+
+Documentation
+-------------
+Documentation by Carl Worth and the git-list <git@vger.kernel.org>.
+
+GIT
+---
+Part of the gitlink:git[7] suite
+
diff --git a/Makefile b/Makefile
index 8e6bbce..f3383d8 100644
--- a/Makefile
+++ b/Makefile
@@ -120,8 +120,8 @@ SCRIPT_SH = \
 	git-merge-one-file.sh git-parse-remote.sh \
 	git-prune.sh git-pull.sh git-push.sh git-rebase.sh \
 	git-repack.sh git-request-pull.sh git-reset.sh \
-	git-resolve.sh git-revert.sh git-rm.sh git-sh-setup.sh \
-	git-tag.sh git-verify-tag.sh git-whatchanged.sh \
+	git-resolve.sh git-revert.sh git-rm.sh git-seek.sh \
+	git-sh-setup.sh git-tag.sh git-verify-tag.sh git-whatchanged.sh \
 	git-applymbox.sh git-applypatch.sh git-am.sh \
 	git-merge.sh git-merge-stupid.sh git-merge-octopus.sh \
 	git-merge-resolve.sh git-merge-ours.sh git-grep.sh \
diff --git a/git-seek.sh b/git-seek.sh
new file mode 100644
index 0000000..26f0b76
--- /dev/null
+++ b/git-seek.sh
@@ -0,0 +1,94 @@
+#!/bin/sh
+
+USAGE='[<revision>]'
+LONG_USAGE='git-seek provides a temporary excursion through the revision history.
+
+When given a <revision>, git-seek updates the files in the working
+tree to the state of the given revision. It will do this by performing
+a checkout of <revision> to a new branch named "seek", or by resetting
+the seek branch if it already exists.
+
+When run with with no <revision> argument, git-seek will return to the
+original branch from which the initial git-seek operation was
+performed, (this original branch name is saved in $GIT_DIR/head-name).
+
+git-seek refuses to do anything if the working tree or index are
+modified with respect to HEAD. If you want to carry modifications
+around, use git-checkout rather than git-seek.
+
+git-seek will also fail if GIT_DIR/head-name exists when a seek is not
+already in progress, or if a seek branch already exists that is not a
+subset of the current branch, (that is, if it has unmerged commits).'
+
+. git-sh-setup
+
+# Does $GIT_DIR/head-name contain the given revision
+# We use git-rev-parse to correctly resolve any aliases through references.
+head_name_contains() {
+	old_head=$(git-rev-parse $(cat "$GIT_DIR/head-name"))
+	new_head=$(git-rev-parse "$1")
+	[ "$old_head" = "$new_head" ]
+}
+
+seek_to() {
+	target="$1"
+	head=$(GIT_DIR="$GIT_DIR" git-symbolic-ref HEAD) ||
+	die "Bad HEAD - I need a symbolic ref"
+	case "$head" in
+	refs/heads/seek)
+		# An explicit seek to head-name is treated as a reset
+		if head_name_contains "$target"; then
+			seek_reset
+		else
+			git reset --hard $target
+		fi
+		;;
+	refs/heads/*)
+		[ -s "$GIT_DIR/head-name" ] && die "Will not seek: $GIT_DIR/head-name is already in use"
+		echo "$head" | sed 's#^refs/heads/##' >"$GIT_DIR/head-name"
+		if git-rev-parse --verify seek >&/dev/null ; then
+			git-branch -d seek || exit
+		fi
+		git checkout -b seek $target
+		;;
+	*)
+		die "Bad HEAD - strange symbolic ref"
+		;;
+	esac
+}
+
+seek_reset() {
+	if [ -s "$GIT_DIR/head-name" ]; then
+		source=$(cat "$GIT_DIR/head-name") || exit
+	else
+		echo >&2 "No seek is in progress: returning to master."
+		source 
+	fi
+	git checkout "$source" &&
+	(git branch -d seek || err=$? ; git checkout seek ; exit $err) &&
+	rm -f "$GIT_DIR/head-name"
+}
+
+head=$(git-rev-parse --verify HEAD) || die "You do not have a valid HEAD"
+
+files_dirty=$(git-diff-index --name-only $head) || exit
+index_dirty=$(git-diff-index --cached --name-only $head) || exit
+if [ "$files_dirty" -o "$index_dirty" ]; then
+	die "Will not seek from a dirty state:
+	${index_dirty:+(dirty in index: $index_dirty)} ${files_dirty:+(dirty in working tree: $files_dirty)}
+You may want to commit these changes first or perhaps use git-checkout
+-m instead of git-seek."
+fi
+
+case "$#" in
+0)
+	seek_reset
+	;;
+1)
+	seek_to "$1"
+	;;
+*)
+	usage 
+	;;
+esac
+
diff --git a/t/t3800-seek.sh b/t/t3800-seek.sh
new file mode 100755
index 0000000..e5d8f90
--- /dev/null
+++ b/t/t3800-seek.sh
@@ -0,0 +1,82 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Carl D. Worth
+#
+
+test_description='Test of git-seek and all documented failure modes.'
+
+. ./test-lib.sh
+
+echo "first" > file
+git-add file && git-commit -m "add first revision of file"
+echo "second" > file
+git-commit -a -m "commit second revision"
+git tag second
+echo "third" > file
+git-commit -a -m "commit third revision"
+
+verify_revision() {
+    contents=$(cat file) && [ "$contents" = "$1" ]
+}
+
+test_expect_success \
+    'Test of initial "git-seek <revision>"' \
+    'git-seek HEAD~2 && verify_revision first'
+
+test_expect_success \
+    'Test of "git-seek <revision>" during seek' \
+    'git-seek second && verify_revision second'
+
+test_expect_success \
+    'Test that "git-seek" returns to starting point and resets seek state' \
+    'git-seek && verify_revision third &&
+     [ ! -f .git/refs/seek ] &&
+     [ ! -f .git/head-name ]'
+
+test_expect_success \
+    'Test that "git-seek master" also resets seek state' \
+    'git seek HEAD^1 &&
+     git seek master && verify_revision third &&
+     [ ! -f .git/refs/seek ] &&
+     [ ! -f .git/head-name ]'
+
+test_expect_success \
+    'Test that "git-seek <revision>" which aliases to master also resets seek state' \
+    'source=$(git-rev-parse HEAD) &&
+     git seek HEAD^1 &&
+     git seek $source && verify_revision third &&
+     [ ! -f .git/refs/seek ] &&
+     [ ! -f .git/head-name ]'
+
+echo modified > file
+test_expect_failure \
+    'Test that git-seek fails with local file modification' \
+    'git-seek HEAD^'
+git-reset --hard master
+
+echo modified > file
+git-update-index file
+test_expect_failure \
+    'Test that git-seek fails with a modified index' \
+    'git-seek HEAD^'
+git-reset --hard master
+
+echo master > .git/head-name
+test_expect_failure \
+    'Test that git-seek fails when .git/head-name exists and not seeking' \
+    'git-seek HEAD^'
+rm .git/head-name
+
+git-seek HEAD^
+echo new > new; git-add new; git-commit -m "Commit new file to seek branch"
+test_expect_failure \
+    'Test that git-seek fails when there are unmerged commits on seek branch' \
+    'git-seek'
+
+git checkout master
+git-pull . seek >&/dev/null
+test_expect_success \
+    'Test that git-seek works again after merging in the seek branch' \
+    'git-seek'
+
+test_done
-- 
1.2.3.g207a-dirty





[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply related

* Re: [PATCH] Convert open("-|") to qx{} calls
From: Randal L. Schwartz @ 2006-02-23 20:41 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Alex Riesen, git
In-Reply-To: <Pine.LNX.4.63.0602232039160.30630@wbgn013.biozentrum.uni-wuerzburg.de>

>>>>> "Johannes" == Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Johannes> Now that our local Perl guru joined the discussion, may I ask what
Johannes> is, and what is not quoted when put inside qx{}?

Nothing is quoted.  Your string acts as if it was XXX in:

        sh -c 'XXX'

so any quoting is entirely on your own.  Thus, without the multi-arg exec in
my proposed replacement, you can get shell-ish interactions that can ruin your
day pretty bad.

Johannes>  I had the
Johannes> impression that all arguments are quoted, except that variables are
Johannes> resolved first. Was that wrong? IOW does

Johannes> 	qx{bash $variable}

Johannes> quote the value of $variable, or not?

The Perl $variable is expanded to its current contents.  But suppose the
contents are `date` (including the backquotes).  That would mean that a shell
would execute a date command, and *its* output would then contribute further
to the command invocation.

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

^ permalink raw reply

* Re: [PATCH] Convert open("-|") to qx{} calls
From: Alex Riesen @ 2006-02-23 21:14 UTC (permalink / raw)
  To: Randal L. Schwartz; +Cc: Johannes Schindelin, Junio C Hamano, git
In-Reply-To: <86lkw1g647.fsf@blue.stonehenge.com>

Randal L. Schwartz, Thu, Feb 23, 2006 21:41:44 +0100:
> Johannes> Now that our local Perl guru joined the discussion, may I ask what
> Johannes> is, and what is not quoted when put inside qx{}?
> 
> Nothing is quoted.  Your string acts as if it was XXX in:
> 
>         sh -c 'XXX'
> 

Not so for ActiveState. It'll just run the first non-whitespace word
passing the rest of the line in its command-line.
It's not even worse then to pass it all to cmd/command :)

^ permalink raw reply

* Re: [PATCH] Convert open("-|") to qx{} calls
From: Randal L. Schwartz @ 2006-02-23 21:15 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Johannes Schindelin, Junio C Hamano, git
In-Reply-To: <20060223211403.GB5827@steel.home>

>>>>> "Alex" == Alex Riesen <raa.lkml@gmail.com> writes:

Alex> Randal L. Schwartz, Thu, Feb 23, 2006 21:41:44 +0100:
Johannes> Now that our local Perl guru joined the discussion, may I ask what
Johannes> is, and what is not quoted when put inside qx{}?
>> 
>> Nothing is quoted.  Your string acts as if it was XXX in:
>> 
>> sh -c 'XXX'
>> 

Alex> Not so for ActiveState. It'll just run the first non-whitespace word
Alex> passing the rest of the line in its command-line.
Alex> It's not even worse then to pass it all to cmd/command :)

Right.  That's why I suggest (in a later message) that safe_qx merely
fall back to qx() on Activestate.  Can't go much more wrong. :)

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
From: Alex Riesen @ 2006-02-23 21:43 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Andreas Ericsson, Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0602230911410.3771@g5.osdl.org>

Linus Torvalds, Thu, Feb 23, 2006 18:13:43 +0100:
> > Someday we'll have to start dropping features on Windows or restrict them
> > beyond their usefullness.
> 
> One thing that would help a bit would be to avoid shell.
> 
> There are many portable interpreters out there, and I don't mean perl. And 
> writing a small "specialized for git" one isn't even that hard. In fact, 
> most of the shell (and bash) hackery we do now would be unnecessary if we 
> just made a small "git interpreter" that ran "git scripts".
> 
> The fact that it would also help portability is just an added advantage.
> 

I actually was dreaming about taking a vacation and rewrite at least
the most important scripts in C, but without cygwin. Implement the
needed subset of POSIX in compat/, workaround fork.

That'd help me to present git to my collegues without requiring them
to install cygwin, perl and python first. It is a real problem to
explain why a new tool is better than the old one if the problem start
right from installation, and it probably wont matter how bad the old
tool is (it is, they know that too, but it has windows, doors and a
mostly running man for busy-waiting cursor).

A gits own interpreter would be more than, of course.

^ permalink raw reply

* Re: [PATCH] Add git-annotate, a tool for assigning blame.
From: Ryan Anderson @ 2006-02-23 22:10 UTC (permalink / raw)
  To: Fredrik Kuivinen; +Cc: Junio C Hamano, git
In-Reply-To: <20060220234054.GA7903@c165.ib.student.liu.se>

(Biased critique since I have the other tool in the tree, but still...)

On Tue, Feb 21, 2006 at 12:40:54AM +0100, Fredrik Kuivinen wrote:
> diff --git a/blame.c b/blame.c
> new file mode 100644
> index 0000000..d4a2fad
> --- /dev/null
> +++ b/blame.c
> @@ -0,0 +1,444 @@
> +#include <assert.h>
> +
> +#include "cache.h"
> +#include "refs.h"
> +#include "tag.h"
> +#include "commit.h"
> +#include "tree.h"
> +#include "blob.h"
> +#include "epoch.h"
> +#include "diff.h"
> +
> +#define DEBUG 0
> +
> +struct commit** blame_lines;
> +int num_blame_lines;
> +
> +struct util_info
> +{
> +    int* line_map;
> +    int num_lines;
> +    unsigned char sha1[20]; /* blob sha, not commit! */
> +    char* buf;
> +    unsigned long size;
> +//    const char* path;
> +};
> +
> +struct chunk
> +{
> +    int off1, len1; // ---
> +    int off2, len2; // +++
> +};
> +
> +struct patch
> +{
> +    struct chunk* chunks;
> +    int num;
> +};
> +
> +static void get_blob(struct commit* commit);
> +
> +int num_get_patch = 0;
> +int num_commits = 0;
> +
> +struct patch* get_patch(struct commit* commit, struct commit* other)
> +{
> +    struct patch* ret = xmalloc(sizeof(struct patch));
> +    ret->chunks = NULL;
> +    ret->num = 0;
> +
> +    struct util_info* info_c = (struct util_info*) commit->object.util;
> +    struct util_info* info_o = (struct util_info*) other->object.util;
> +
> +    if(!memcmp(info_c->sha1, info_o->sha1, 20))
> +        return ret;
> +
> +    get_blob(commit);
> +    get_blob(other);
> +
> +    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.

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.)

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.


-- 

Ryan Anderson
  sometimes Pug Majere

^ permalink raw reply

* [PATCH] fix warning from pack-objects.c
From: Luck, Tony @ 2006-02-23 22:42 UTC (permalink / raw)
  To: git

When compiling on ia64 I get this warning (from gcc 3.4.3):

gcc -o pack-objects.o -c -g -O2 -Wall -DSHA1_HEADER='<openssl/sha.h>'  pack-objects.c
pack-objects.c: In function `pack_revindex_ix':
pack-objects.c:94: warning: cast from pointer to integer of different size

A double cast (first to long, then to int) shuts gcc up, but is there
a better way?

Signed-off-by: Tony Luck <tony.luck@intel.com>

---

diff --git a/pack-objects.c b/pack-objects.c
index 8f352aa..c985fab 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -91,7 +91,7 @@ static int reused_delta = 0;
 
 static int pack_revindex_ix(struct packed_git *p)
 {
-	unsigned int ui = (unsigned int) p;
+	unsigned int ui = (unsigned int)(long)p;
 	int i;
 
 	ui = ui ^ (ui >> 16); /* defeat structure alignment */

^ permalink raw reply related

* Re: [PATCH] fix warning from pack-objects.c
From: Andreas Ericsson @ 2006-02-23 22:51 UTC (permalink / raw)
  To: Luck, Tony; +Cc: git
In-Reply-To: <200602232242.k1NMgdAJ018406@agluck-lia64.sc.intel.com>

Luck, Tony wrote:
> When compiling on ia64 I get this warning (from gcc 3.4.3):
> 
> gcc -o pack-objects.o -c -g -O2 -Wall -DSHA1_HEADER='<openssl/sha.h>'  pack-objects.c
> pack-objects.c: In function `pack_revindex_ix':
> pack-objects.c:94: warning: cast from pointer to integer of different size
> 
> A double cast (first to long, then to int) shuts gcc up, but is there
> a better way?
> 

Make ui and i unsigned long and cast p to unsigned long (or perhaps 
ptrdiff_t is preferred?). On 32-bit archs it's no difference, but 64-bit 
archs can work with their native size. It's slightly faster (although I 
expect the compiler takes care of that anyways).

I noticed this earlier on my shiny AMD FX2 64-bit. The tests run just 
fine with my solution. I expect they will with yours as well.

> Signed-off-by: Tony Luck <tony.luck@intel.com>
> 
> ---
> 
> diff --git a/pack-objects.c b/pack-objects.c
> index 8f352aa..c985fab 100644
> --- a/pack-objects.c
> +++ b/pack-objects.c
> @@ -91,7 +91,7 @@ static int reused_delta = 0;
>  
>  static int pack_revindex_ix(struct packed_git *p)
>  {
> -	unsigned int ui = (unsigned int) p;
> +	unsigned int ui = (unsigned int)(long)p;
>  	int i;
>  
>  	ui = ui ^ (ui >> 16); /* defeat structure alignment */
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: [PATCH] Add git-annotate, a tool for assigning blame.
From: Fredrik Kuivinen @ 2006-02-23 22:55 UTC (permalink / raw)
  To: Ryan Anderson; +Cc: Fredrik Kuivinen, Junio C Hamano, git
In-Reply-To: <20060223221048.GA6423@mythryan2.michonline.com>

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

^ permalink raw reply

* Re: [PATCH] Add git-annotate, a tool for assigning blame.
From: Johannes Schindelin @ 2006-02-24  0:00 UTC (permalink / raw)
  To: Fredrik Kuivinen; +Cc: Ryan Anderson, Junio C Hamano, git
In-Reply-To: <20060223225547.GB8673@c165.ib.student.liu.se>

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

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox