Git development
 help / color / mirror / Atom feed
* Re: Bottlenecks in git merge
From: Linus Torvalds @ 2006-01-31 23:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Peter Eriksen, Git Mailing List
In-Reply-To: <7vk6cgq9ny.fsf@assigned-by-dhcp.cox.net>



On Tue, 31 Jan 2006, Junio C Hamano wrote:
> 
> I am actually surprised that recursive is not much slower than
> resolve.  I expected to see bigger difference for a merge like
> this.

Well, if most of the cost is just the trivial single-file merges and the 
fact that we have to update a ton of files from an old version (and it 
probably is), the difference between the trivial and the recursive merge 
is not going to be huge.

> With an up-to-date index that has small changes from v2.6.12,
> merging these two revisions using read-tree -m to do the trivial
> merge (the part that comes before recursive/resolve) leaves
> about 850 files to be resolved in the working tree.  For these
> files, you need to do an equivalent of merge-one-file to merge
> the differences (in this particular case, most of them are
> "removed in one but unchanged in the other" kind).  In addition,
> you have to checkout the result of the merge, which means you
> need to update at least 10,723 files.

It would be interesting to see how big the "resolve 850 files" part is vs 
the "check out 10k+ files" is.

In particular, if the "resolve 850 files" is a noticeable portion of it, 
then the right thing to do may be to just re-write git-merge-one-file.sh 
in C. Right now, almost _all_ of the expense of that thing is just the 
shell interpreter startup. The actual actions it does are usually fairly 
cheap.

(yes, a real three-way merge is more expensive, but I suspect that even 
that isn't much more expensive than starting up an invocation of "bash". 
The other actions that merge-one-file does are _really_ trivial).

In fact, we could hardcode the "git-merge-one-file" behaviour inside 
"git-merge-index". 

Now, that won't help "recursive" (which doesn't use git-merge-one-file at 
all, and does it all by hand), but it would be an interesting test to 
make, becuase if it makes the simpler "-s resolve" merge even faster, then 
we know that this is likely a large portion of the time.

Then, somebody would have to consider what to do about 
git-merge-recursive. For example, if the _common_ case is "modified in 
both, but differently", and they merge cleanly, maybe the recursive merge 
could handle those separately and fast with a special "git-merge-one-file" 
invocation (just to cut down the number of files that it needs to think 
more about).

		Linus

^ permalink raw reply

* Re: [Census] So who uses git?
From: Junio C Hamano @ 2006-01-31 23:47 UTC (permalink / raw)
  To: Daniel Barkalow
  Cc: Johannes Schindelin, Carl Baldwin, Keith Packard, Martin Langhoff,
	Linus Torvalds, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0601311750270.25300@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

> I sort of suspect that "git commit some_other_file" should really read 
> HEAD into a temporary index, update "some_other_file" in that (and the 
> main index), and commit it.
> ...
> The surprising thing is that "git commit path ..." means
> "everything I've already mentioned, plus path..." not just
> "path ...", and it's particularly surprising because people
> only tend to specify paths when they've done something they
> don't want to commit.

Interesting idea, and a good point.

Not that I particularly would like to encourage people to make
partial commits by making it easier, but as long as we allow our
users to say "commit path...", your proposal would reduce the
confusion.

I wonder which is faster, to check if index differs from HEAD
and do the temporary index only when they differ, or always use
a temporary without checking?  The former needs one diff-index
--cached, zero or one read-tree, one write-tree and one
commit-tree.  The latter always needs one read-tree, one
write-tree and one commit-tree.

Wait.  We already do diff-index --cached during git-commit
anyway (it is in git-status).  Maybe with a bit of code
restructuring we can do the temporary index part optional.

^ permalink raw reply

* Re: [Census] So who uses git?
From: Linus Torvalds @ 2006-02-01  0:38 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Daniel Barkalow, Johannes Schindelin, Carl Baldwin, Keith Packard,
	Martin Langhoff, Git Mailing List
In-Reply-To: <7vek2oot7z.fsf@assigned-by-dhcp.cox.net>



On Tue, 31 Jan 2006, Junio C Hamano wrote:
> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > I sort of suspect that "git commit some_other_file" should really read 
> > HEAD into a temporary index, update "some_other_file" in that (and the 
> > main index), and commit it.
> > ...
> > The surprising thing is that "git commit path ..." means
> > "everything I've already mentioned, plus path..." not just
> > "path ...", and it's particularly surprising because people
> > only tend to specify paths when they've done something they
> > don't want to commit.
> 
> Interesting idea, and a good point.

One thing to be careful about is merges.

This actually happens to me:

	git pull ....

	.. uhhuh, trivial conflict in one file ..
	.. edit the/file/that/conflicted ..

	git commit the/file/that/conflicted

and there is no way that it would ever be correct to then just commit that 
one file. The fact that it's a merge means that the rest of the index - 
which is all from the merge, and correct - absolutely _must_ be committed 
too.

And yes, I could use "git commit -a" (and I often do), but the thing is, I 
surprisingly often have edits in unrelated files (stuff that the merge 
never touched), and doing "git commit -a" would do the wrong thing.

So the current "git commit filename" behaviour is actually the only 
possible correct one for a merge. Nothing else makes any sense 
what-so-ever.

Now, I can hear people arguing that "ok, merges are special, and for 
merges we always do it in the current index", but that makes "git commit 
pathname" act very _differently_ for a merge than for a normal commit. 
That just smells wrong to me.

So if you do this change (which may be the right one) then please make 
sure that "git commit <filename>" doesn't work _at_all_ when a merge is in 
progress (ie MERGE_HEAD exists), because it would do the wrong thing.

And yes, then I'll just have to force my fingers to do a simple

	git-update-index filename
	git commit

instead. I can do that.

Oh, one final suggestion: if you give a filename to "git commit", and you 
do the new semantics which means something _different_ than "do a 
git-update-index on that file and commit", then I'd really suggest that 
the _old_ index for that filename should match the parent exactly. 
Otherwise, you may have done a

	git diff filename

and you _thought_ you were committing just a two-line thing (because you 
didn't understand about the index), but another, earlier, action caused 
the index to be different from the file you had in HEAD, and in reality 
you're actually committing a much bigger diff.

In other words: if you want "git commit <filename>" to _not_ care about 
the current index, then it should make sure that the index at least 
_matches_ the current HEAD in the files mentioned.

Ie "git-diff-index --cached HEAD <filespec>" should return empty. Or 
something like that.

			Linus

^ permalink raw reply

* Re: Bottlenecks in git merge
From: Junio C Hamano @ 2006-02-01  0:43 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20060131233504.GB31278@pasky.or.cz>

Petr Baudis <pasky@suse.cz> writes:

> What about letting the file-handler actually tell merge-index what to
> do? merge-index could make a fifo at fd 3 for it (we might fork a
> special buffering process for it to avoid PIPE_BUF issues) and let it
> write there a sequence of lines like...

Yes.  That is sensible.

A merge handler that is willing to look at the current index
stages (and merge-recursive certainly is capable of doing that)
can open an outgoing pipe to 'git-update-index --index-info' and
drive it.  The command syntax is a bit different from what you
wrote in your message and I think if we go this route we should
make --index-info a bit easier to use by allowing it to accept
stage 0 entries.

-- >8 --
update-index --index-info: allow stage 0 entries.

Somehow we did not allow stuffing the index with stage 0 entries
through --index-info interface.  I do not think of a reason to
forbid it offhand.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
diff --git a/update-index.c b/update-index.c
index 2361e41..94436dd 100644
--- a/update-index.c
+++ b/update-index.c
@@ -303,7 +303,7 @@ static void read_index_info(int line_ter
 		if (!tab || tab - ptr < 41)
 			goto bad_line;
 
-		if (tab[-2] == ' ' && '1' <= tab[-1] && tab[-1] <= '3') {
+		if (tab[-2] == ' ' && '0' <= tab[-1] && tab[-1] <= '3') {
 			stage = tab[-1] - '0';
 			ptr = tab + 1; /* point at the head of path */
 			tab = tab - 2; /* point at tail of sha1 */

^ permalink raw reply related

* Re: Bottlenecks in git merge
From: Petr Baudis @ 2006-02-01  0:50 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, Peter Eriksen, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0601311533040.7301@g5.osdl.org>

Dear diary, on Wed, Feb 01, 2006 at 12:45:27AM CET, I got a letter
where Linus Torvalds <torvalds@osdl.org> said that...
> It would be interesting to see how big the "resolve 850 files" part is vs 
> the "check out 10k+ files" is.

See my other mail.

> In particular, if the "resolve 850 files" is a noticeable portion of it, 
> then the right thing to do may be to just re-write git-merge-one-file.sh 
> in C. Right now, almost _all_ of the expense of that thing is just the 
> shell interpreter startup. The actual actions it does are usually fairly 
> cheap.

Nope.

xpasky@machine[0:0]~/linux-2.6.git$ echo -e '#!/bin/sh\n/bin/true' >r && chmod a+x r
xpasky@machine[0:0]~/linux-2.6.git$ time git-merge-index -o ./r -a

real    0m3.827s
user    0m1.788s
sys     0m2.004s
xpasky@machine[0:0]~/linux-2.6.git$ time git-merge-index -o ~/git-pb/git-merge-one-file -a
[lots of "Removing"]

real    1m21.773s
user    0m30.806s
sys     0m13.248s

The costs are apparently in git-update-index, not in the shell.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Of the 3 great composers Mozart tells us what it's like to be human,
Beethoven tells us what it's like to be Beethoven and Bach tells us
what it's like to be the universe.  -- Douglas Adams

^ permalink raw reply

* Re: [Census] So who uses git?
From: Junio C Hamano @ 2006-02-01  0:52 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0601311623240.7301@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> One thing to be careful about is merges.
> ...
> So the current "git commit filename" behaviour is actually the only 
> possible correct one for a merge. Nothing else makes any sense 
> what-so-ever.

Agreed 100%, and I kind of feel silly about not mentioning that
myself.  It _might_ even make sense to reject explicit filenames
when MERGE_HEAD does not exist ;-).

> Oh, one final suggestion: if you give a filename to "git
> commit", and you do the new semantics which means something
> _different_ than "do a git-update-index on that file and
> commit", then I'd really suggest that the _old_ index for that
> filename should match the parent exactly.

That is also a good safety measure.

^ permalink raw reply

* Re: Bottlenecks in git merge
From: Linus Torvalds @ 2006-02-01  1:04 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Junio C Hamano, Peter Eriksen, Git Mailing List
In-Reply-To: <20060201005035.GD31278@pasky.or.cz>



On Wed, 1 Feb 2006, Petr Baudis wrote:
> 
> xpasky@machine[0:0]~/linux-2.6.git$ echo -e '#!/bin/sh\n/bin/true' >r && chmod a+x r
> xpasky@machine[0:0]~/linux-2.6.git$ time git-merge-index -o ./r -a
> 
> real    0m3.827s
> user    0m1.788s
> sys     0m2.004s
> xpasky@machine[0:0]~/linux-2.6.git$ time git-merge-index -o ~/git-pb/git-merge-one-file -a
> [lots of "Removing"]
> 
> real    1m21.773s
> user    0m30.806s
> sys     0m13.248s
> 
> The costs are apparently in git-update-index, not in the shell.

Btw, this is where "oprofile" really shines. You can get exact listings of 
which symbols in which programs are taking up CPU-time, even when the time 
is spent in hundreds of different executions of a program.

Me, I'm too lazy to use it often, but not too lazy to point others towards 
it.

You do need to have kernel support for it compiled in, and it works better 
on certain CPU's (that have supported counters) than on others.

		Linus

^ permalink raw reply

* Re: [PATCH] Fix HTTP request result processing after slot reuse
From: Nick Hengeveld @ 2006-02-01  1:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v64o0ulfu.fsf@assigned-by-dhcp.cox.net>

On Tue, Jan 31, 2006 at 01:39:01PM -0800, Junio C Hamano wrote:

> These static variables are probably correct, provided if
> fetch_index, fetch_indices and friends do not recurse into
> themselves, but it just gives me this funny feeling...

It's true that in the current implementation we either don't recurse
into these functions or we explicitly handle cases where we do such as
fetch_alternates.  However, I've got no argument as to why the
structures should be static and can imagine it just causing problems
down the road if we were to eg. start downloading multiple packs
concurrently.

I'll follow up with a patch.

--
For a successful technology, reality must take precedence over public
relations, for nature cannot be fooled.

^ permalink raw reply

* [PATCH] Use local structs for HTTP slot callback data
From: Nick Hengeveld @ 2006-02-01  2:00 UTC (permalink / raw)
  To: git

There's no need for these structures to be static, and it could potentially
cause problems down the road.

Signed-off-by: Nick Hengeveld <nickh@reactrix.com>


---

 http-fetch.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

6146f9e8be9d4bb49b7008af5b99853ee7c0afc1
diff --git a/http-fetch.c b/http-fetch.c
index 92326f9..97ce13c 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -375,7 +375,7 @@ static int fetch_index(struct alt_base *
 
 	FILE *indexfile;
 	struct active_request_slot *slot;
-	static struct slot_results results;
+	struct slot_results results;
 
 	if (has_pack_index(sha1))
 		return 0;
@@ -555,7 +555,7 @@ static void fetch_alternates(char *base)
 	char *url;
 	char *data;
 	struct active_request_slot *slot;
-	static struct alternates_request alt_req;
+	struct alternates_request alt_req;
 
 	/* If another request has already started fetching alternates,
 	   wait for them to arrive and return to processing this request's
@@ -618,7 +618,7 @@ static int fetch_indices(struct alt_base
 	int i = 0;
 
 	struct active_request_slot *slot;
-	static struct slot_results results;
+	struct slot_results results;
 
 	if (repo->got_indices)
 		return 0;
@@ -699,7 +699,7 @@ static int fetch_pack(struct alt_base *r
 	struct curl_slist *range_header = NULL;
 
 	struct active_request_slot *slot;
-	static struct slot_results results;
+	struct slot_results results;
 
 	if (fetch_indices(repo))
 		return -1;
@@ -900,7 +900,7 @@ int fetch_ref(char *ref, unsigned char *
         struct buffer buffer;
 	char *base = alt->base;
 	struct active_request_slot *slot;
-	static struct slot_results results;
+	struct slot_results results;
         buffer.size = 41;
         buffer.posn = 0;
         buffer.buffer = hex;
-- 
1.1.6.g1417-dirty

^ permalink raw reply related

* Re: [Census] So who uses git?
From: Linus Torvalds @ 2006-02-01  2:04 UTC (permalink / raw)
  To: Ray Lehtiniemi
  Cc: Alex Riesen, Radoslaw Szkodzinski, Keith Packard, Junio C Hamano,
	cworth, Martin Langhoff, Git Mailing List
In-Reply-To: <20060201013901.GA16832@mail.com>



On Tue, 31 Jan 2006, Ray Lehtiniemi wrote:
> 
> for what it's worth, it's certainly true here...  i'm using git to help
> me manage a similar project where i work.

Hmm.

We _could_ actually fairly easily add a flag to the index which means 
"don't even bother comparing - assume same", and then have specific 
operations to clear that flag.

That would allow people with slow filesystems (not just Windows: even 
under Linux, the cold-cache case is always going to be pretty slow) to 
have a _choice_: they could continue to use git it is done now (explicit 
checks), _or_ they could mark all their index caches as "implicitly 
up-to-date" and use a separate program to mark them as being potentially 
edited.

We still have one unused bit in the cache-entry "ce_flags", so we wouldn't 
even need to break any existing index files with it.

We'd just need to have two new (fast) operations:

 - mark one or more files as being "implicitly up-to-date"

   "git checkout" would do this if the proper flag was set in the 
   .git/config file.

   "git-update-index --refresh" would do this for files that weren't 
   already implicitly up-to-date _and_ the refresh actually showed it to 
   match (and the .git/config file said so).

 - mark one or more files as _not_ being implicitly up-to-date:

   people would do this by hand when editing a file (or when just deciding 
   that they want git to re-check everything again)

They're fast, because they are purely in the cache (well, git-update-index 
obviously isn't, but the new op wouldn't be any _slower_ than the old 
one).

Looks simple enough. The big thing to remember is to clear that 
"implicitly up-to-date" flag whenever we make changes (ie we'd probably 
make "add_cache_entry()" always clear it, possibly with a flag to add it 
as "pre-verified" which would set it).

Comments? Junio, what do you think?

> we're working on a vendor supplied tree which is also hacked upon
> by various VAR companies.  the tree in question has ~20,000 files
> totalling nearly 1.4 GB of source files, ms word docs, binary-only
> libraries for a wide array of processor variants, windows exe
> files, video clips, etc.  (however, the amount of actual source code
> interspersed in there is only about 6000 files totaling about 112 MB)
> 
> here's a repo sitting on the local linux filesystem with cold cache:
> 
>   reiserfs$ time git update-index --refresh
>    real    0m17.422s
>    user    0m0.025s
>    sys     0m0.320s

.. somewhat painful, but with enough memory this is hopefully a pretty 
rare case.

> and with hot cache
> 
>   reiserfs$ time git update-index --refresh
>    real    0m0.151s
>    user    0m0.020s
>    sys     0m0.067s

This is how it _should_ look.

But:

> for comparison, one of our sandboxes is sitting on an NTFS file system,
> accessed via SMB:
> 
>   smbfs$ time git update-index --refresh
>   real    11m36.502s
>   user    0m6.830s
>   sys     0m5.086s

Ouch, ouch, ouch.

Sounds like every single stat() will go out the wire. I forget what the 
Linux NFS client does, but I _think_ it has a metadata timeout that avoids 
this. But it might be as bad under NFS.

Has anybody used git over NFS? If it's this bad (or even close to), I 
guess the "mark files as up-to-date in the index" approach is a really 
good idea..

Of course, the whole point of git is that you should keep your repository 
close, but sometimes NFS - or similar - is enforced upon you by other 
issues, like the fact that the powers-that-be want anonymous workstations 
and everybody should work with a home-directory automounted over NFS..

			Linus

^ permalink raw reply

* Re: [Census] So who uses git?
From: Linus Torvalds @ 2006-02-01  2:09 UTC (permalink / raw)
  To: Ray Lehtiniemi
  Cc: Alex Riesen, Radoslaw Szkodzinski, Keith Packard, Junio C Hamano,
	cworth, Martin Langhoff, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0601311747360.7301@g5.osdl.org>



On Tue, 31 Jan 2006, Linus Torvalds wrote:
> 
> We still have one unused bit in the cache-entry "ce_flags", so we wouldn't 
> even need to break any existing index files with it.

In case it wasn't clear, the _core_ of this optimization would be as 
simple as something like the appended.

The real meat is just making sure that CE_VALID gets set/cleared properly.

(That's also the most complex part, of course, but this trivial patch 
might help show the basic idea)

		Linus

---
diff --git a/cache.h b/cache.h
index bdbe2d6..7adc2e6 100644
--- a/cache.h
+++ b/cache.h
@@ -91,6 +91,7 @@ struct cache_entry {
 #define CE_NAMEMASK  (0x0fff)
 #define CE_STAGEMASK (0x3000)
 #define CE_UPDATE    (0x4000)
+#define CE_VALID     (0x8000)
 #define CE_STAGESHIFT 12
 
 #define create_ce_flags(len, stage) htons((len) | ((stage) << CE_STAGESHIFT))
diff --git a/read-cache.c b/read-cache.c
index c5474d4..738fe78 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -148,7 +148,16 @@ static int ce_match_stat_basic(struct ca
 
 int ce_match_stat(struct cache_entry *ce, struct stat *st)
 {
-	unsigned int changed = ce_match_stat_basic(ce, st);
+	unsigned int changed;
+
+	/*
+	 * If it's marked as always valid in the index, it's 
+	 * valid whatever the checked-out copy says
+	 */
+	if (ce->ce_flags & htons(CE_VALID))
+		return 0;
+
+	changed = ce_match_stat_basic(ce, st);
 
 	/*
 	 * Within 1 second of this sequence:

^ permalink raw reply related

* Re: [Census] So who uses git?
From: Daniel Barkalow @ 2006-02-01  2:19 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Junio C Hamano, Johannes Schindelin, Carl Baldwin, Keith Packard,
	Martin Langhoff, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0601311623240.7301@g5.osdl.org>

On Tue, 31 Jan 2006, Linus Torvalds wrote:

> So if you do this change (which may be the right one) then please make 
> sure that "git commit <filename>" doesn't work _at_all_ when a merge is in 
> progress (ie MERGE_HEAD exists), because it would do the wrong thing.

Agreed. I suppose it could accept doing a commit of only a few files which 
weren't touched by the merge, but I don't think even you multitask enough 
to want to do that; anyway, the user can just ditch the merge, commit 
their stuff, and try the merge again. (I bet this is a case where new 
users would be really surprised by the behavior of "git commit filename", 
except that they wouldn't think it would do anything other than give an 
error.)

> And yes, then I'll just have to force my fingers to do a simple
> 
> 	git-update-index filename
> 	git commit
> 
> instead. I can do that.
>
> Oh, one final suggestion: if you give a filename to "git commit", and you 
> do the new semantics which means something _different_ than "do a 
> git-update-index on that file and commit", then I'd really suggest that 
> the _old_ index for that filename should match the parent exactly. 
> Otherwise, you may have done a
> 
> 	git diff filename
> 
> and you _thought_ you were committing just a two-line thing (because you 
> didn't understand about the index), but another, earlier, action caused 
> the index to be different from the file you had in HEAD, and in reality 
> you're actually committing a much bigger diff.
> 
> In other words: if you want "git commit <filename>" to _not_ care about 
> the current index, then it should make sure that the index at least 
> _matches_ the current HEAD in the files mentioned.
> 
> Ie "git-diff-index --cached HEAD <filespec>" should return empty. Or 
> something like that.

Agreed here, too.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* gnu/linux sued for predatory business practices
From: walt @ 2006-02-01  2:22 UTC (permalink / raw)
  To: git

Relax!  This is just a joke.  Or is it?

Fast-forward a few years (the fewer the better) to
the time when gnu/linux is the dominant operating
system in the world.  [For lawyers, 'the world' means
the USA, of course, where everyone is born a victim
and must be programmed to honor and welcome his fate!]

Now, when Micro$oft (eventually) takes its (natural)
place as a loser in the marketplace -- who will the
lawyers be anxious to sue?

Any and all reassurance will be most welcome :o)
-- 
(Any responders should include a drunkeness index
from 0 to 9.  /me=6)

^ permalink raw reply

* Re: [Census] So who uses git?
From: Junio C Hamano @ 2006-02-01  2:31 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0601311747360.7301@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> They're fast, because they are purely in the cache (well, git-update-index 
> obviously isn't, but the new op wouldn't be any _slower_ than the old 
> one).
>
> Looks simple enough. The big thing to remember is to clear that 
> "implicitly up-to-date" flag whenever we make changes (ie we'd probably 
> make "add_cache_entry()" always clear it, possibly with a flag to add it 
> as "pre-verified" which would set it).
>
> Comments? Junio, what do you think?

Somehow this reminds me of a "feature" we added quite a long
time ago to support "update-index without working tree".

I think this should work fine as a mechanism, but I am a bit
worried about the convenience and safety aspect.  It _might_
make sense to do what RCS does; check out read-only copy by
default and set the "assume unchanged" flag, to prevent people
from accidentally modifying the working tree copy without
telling the index about it.

^ permalink raw reply

* Re: [Census] So who uses git?
From: Martin Langhoff @ 2006-02-01  2:52 UTC (permalink / raw)
  To: Ray Lehtiniemi
  Cc: Alex Riesen, Linus Torvalds, Radoslaw Szkodzinski, Keith Packard,
	Junio C Hamano, cworth, Git Mailing List
In-Reply-To: <20060201013901.GA16832@mail.com>

On 2/1/06, Ray Lehtiniemi <rayl@mail.com> wrote:
> by various VAR companies.  the tree in question has ~20,000 files
> totalling nearly 1.4 GB
...
>   reiserfs$ time git update-index --refresh

If you have such a tree, your workflow _must_ be such that you know
exactly what files you have changed. Asking any tool to go out and
"find which of my 20K files has changed" is doable, but it's just
magic that it works on recent linuxes.

> for comparison, one of our sandboxes is sitting on an NTFS file system,
> accessed via SMB:

you have the samba stack, network, SMB/CIFS stack and NTFS itself in
the middle. Replace the ethernet with carrier pigeons for a more
complete picture ;-)

Perhaps a local git/cygwin on NTFS  would be more reasonable to benchmark?

cheers,


martin

^ permalink raw reply

* Re: [Census] So who uses git?
From: Linus Torvalds @ 2006-02-01  3:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v64nzollt.fsf@assigned-by-dhcp.cox.net>



On Tue, 31 Jan 2006, Junio C Hamano wrote:
> 
> I think this should work fine as a mechanism, but I am a bit
> worried about the convenience and safety aspect.  It _might_
> make sense to do what RCS does; check out read-only copy by
> default and set the "assume unchanged" flag, to prevent people
> from accidentally modifying the working tree copy without
> telling the index about it.

Yes, I think the "assume unchanged" flag goes well together with making 
sure that the checked-out file is non-writable at the time.

Of course, any number of editors and other actions won't care: if you do 
anything like

	for i in *.c
	do
		sed 's/xyzzy/bas/g' < $i > $i.new
		mv $i.new $i
	done

you'll never have even noticed that the old file was marked read-only. So 
it's obviously not in any way any guarantee, but it probably makes sense 
as a crutch.

Your point that we discussed a similar flag for the "don't require a full 
checkout" is a good one: we should try to make sure that it works for both 
uses. Although maybe we decided for some reason that nobody cared about 
the non-checked-out case?

		Linus

^ permalink raw reply

* Re: [Census] So who uses git?
From: Linus Torvalds @ 2006-02-01  3:48 UTC (permalink / raw)
  To: Martin Langhoff
  Cc: Ray Lehtiniemi, Alex Riesen, Radoslaw Szkodzinski, Keith Packard,
	Junio C Hamano, cworth, Git Mailing List
In-Reply-To: <46a038f90601311852ie8cfac0rbe92779edea4da1b@mail.gmail.com>



On Wed, 1 Feb 2006, Martin Langhoff wrote:
> 
> If you have such a tree, your workflow _must_ be such that you know
> exactly what files you have changed. Asking any tool to go out and
> "find which of my 20K files has changed" is doable, but it's just
> magic that it works on recent linuxes.

It's not magic, and it's not all that recent. Linux FS ops have always 
been pretty good, and the dentry cache was introduced in 2.0.x, I think, 
so you'd be hard-pressed to find a Linux system that doesn't have it.

Now, I bet Linux will be better (often by a factor of 2-3) than most other 
systems, but that still doesn't mean that 20k files is totally 
unreasonable on other setups. 

I suspect cygwin is worse than most because (a) the NT VFS layer is 
piss-poor and you need a kernel service to get good performance and (b) 
cygwin probably adds its own overhead for handling symlinks, so the 
"lstat()" call is probably even more expensive.

Now, the networked filesystems are a potential problem for everybody.

		Linus

^ permalink raw reply

* [Announce] gitview-0.1
From: Aneesh Kumar @ 2006-02-01  4:16 UTC (permalink / raw)
  To: git, junkio

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

Hi,

Gnome based git repository browser. The code is derived from bazaar-ng
repository browser.

To see how it looks

http://www.flickr.com/photos/17388011@N00/92918446/

I don't have a web location where i can host it so i am attaching it below.
It would be great if we can get it added to git repository

sample usage
takes same argument as git rev-list

gitview --since=2.week.ago

-aneesh

[-- Attachment #2: gitview --]
[-- Type: application/octet-stream, Size: 24075 bytes --]

#! /usr/bin/env python

# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 2 of the License, or
# (at your option) any later version.

""" gitview
GUI browser for git repository 
This program is based on bzrk by Scott James Remnant <scott@ubuntu.com>
"""
__author__    = "Aneesh Kumar K.V <aneesh.kumar@gmail.com>"


import sys
import os
import gtk
import pygtk
import pango
import re
import time
import gobject
import cairo
import math
import string

try:
    import gtksourceview
    have_gtksourceview = True
except ImportError:
    have_gtksourceview = False
    print "Running without gtksourceview module"


def list_to_string(args, skip):
	count = len(args)
	i = skip
	str_arg=" "
	while (i < count ):
		str_arg = str_arg + args[i]
		str_arg = str_arg + " "
		i = i+1

	return str_arg


class CellRendererGraph(gtk.GenericCellRenderer):
	"""Cell renderer for directed graph.

	This module contains the implementation of a custom GtkCellRenderer that
	draws part of the directed graph based on the lines suggested by the code
	in graph.py.

	Because we're shiny, we use Cairo to do this, and because we're naughty
	we cheat and draw over the bits of the TreeViewColumn that are supposed to
	just be for the background.

	Properties:
	node              (column, colour) tuple to draw revision node,
	in_lines          (start, end, colour) tuple list to draw inward lines,
	out_lines         (start, end, colour) tuple list to draw outward lines.
	"""

	__gproperties__ = {
	"node":         ( gobject.TYPE_PYOBJECT, "node",
			  "revision node instruction",
			  gobject.PARAM_WRITABLE
			),
	"in-lines":     ( gobject.TYPE_PYOBJECT, "in-lines",
			  "instructions to draw lines into the cell",
			  gobject.PARAM_WRITABLE
			),
	"out-lines":    ( gobject.TYPE_PYOBJECT, "out-lines",
			  "instructions to draw lines out of the cell",
			  gobject.PARAM_WRITABLE
			),
	}

	def do_set_property(self, property, value):
		"""Set properties from GObject properties."""
		if property.name == "node":
			self.node = value
		elif property.name == "in-lines":
			self.in_lines = value
		elif property.name == "out-lines":
			self.out_lines = value
		else:
			raise AttributeError, "no such property: '%s'" % property.name

	def box_size(self, widget):
		"""Calculate box size based on widget's font.

		Cache this as it's probably expensive to get.  It ensures that we
		draw the graph at least as large as the text.
		"""
		try:
			return self._box_size
		except AttributeError:
			pango_ctx = widget.get_pango_context()
			font_desc = widget.get_style().font_desc
			metrics = pango_ctx.get_metrics(font_desc)

			ascent = pango.PIXELS(metrics.get_ascent())
			descent = pango.PIXELS(metrics.get_descent())

			self._box_size = ascent + descent + 6
			return self._box_size

	def set_colour(self, ctx, colour, bg, fg):
		"""Set the context source colour.

		Picks a distinct colour based on an internal wheel; the bg
		parameter provides the value that should be assigned to the 'zero'
		colours and the fg parameter provides the multiplier that should be
		applied to the foreground colours.
		"""
		colours = [
		    ( 1.0, 0.0, 0.0 ),
		    ( 1.0, 1.0, 0.0 ),
		    ( 0.0, 1.0, 0.0 ),
		    ( 0.0, 1.0, 1.0 ),
		    ( 0.0, 0.0, 1.0 ),
		    ( 1.0, 0.0, 1.0 ),
		    ]

		colour %= len(colours)
		red   = (colours[colour][0] * fg) or bg
		green = (colours[colour][1] * fg) or bg
		blue  = (colours[colour][2] * fg) or bg

		ctx.set_source_rgb(red, green, blue)

	def on_get_size(self, widget, cell_area):
		"""Return the size we need for this cell.

		Each cell is drawn individually and is only as wide as it needs
		to be, we let the TreeViewColumn take care of making them all
		line up.
		"""
		box_size = self.box_size(widget)

		cols = self.node[0]
		for start, end, colour in self.in_lines + self.out_lines:
			cols = max(cols, start, end)

		width = box_size * (cols + 1)
		height = box_size

		# FIXME I have no idea how to use cell_area properly
		return (0, 0, width, height)

	def on_render(self, window, widget, bg_area, cell_area, exp_area, flags):
		"""Render an individual cell.

		Draws the cell contents using cairo, taking care to clip what we
		do to within the background area so we don't draw over other cells.
		Note that we're a bit naughty there and should really be drawing
		in the cell_area (or even the exposed area), but we explicitly don't
		want any gutter.

		We try and be a little clever, if the line we need to draw is going
		to cross other columns we actually draw it as in the .---' style
		instead of a pure diagonal ... this reduces confusion by an
		incredible amount.
		"""
		ctx = window.cairo_create()
		ctx.rectangle(bg_area.x, bg_area.y, bg_area.width, bg_area.height)
		ctx.clip()

		box_size = self.box_size(widget)

		ctx.set_line_width(box_size / 8)
		ctx.set_line_cap(cairo.LINE_CAP_SQUARE)

		# Draw lines into the cell
		for start, end, colour in self.in_lines:
			ctx.move_to(cell_area.x + box_size * start + box_size / 2,
					bg_area.y - bg_area.height / 2)

			if start - end > 1:
				ctx.line_to(cell_area.x + box_size * start, bg_area.y)
				ctx.line_to(cell_area.x + box_size * end + box_size, bg_area.y)
			elif start - end < -1:
				ctx.line_to(cell_area.x + box_size * start + box_size,
						bg_area.y)
				ctx.line_to(cell_area.x + box_size * end, bg_area.y)

		    	ctx.line_to(cell_area.x + box_size * end + box_size / 2,
					bg_area.y + bg_area.height / 2)

			self.set_colour(ctx, colour, 0.0, 0.65)
			ctx.stroke()

		# Draw lines out of the cell
		for start, end, colour in self.out_lines:
			ctx.move_to(cell_area.x + box_size * start + box_size / 2,
					bg_area.y + bg_area.height / 2)

			if start - end > 1:
				ctx.line_to(cell_area.x + box_size * start,
						bg_area.y + bg_area.height)
				ctx.line_to(cell_area.x + box_size * end + box_size,
						bg_area.y + bg_area.height)
			elif start - end < -1:
				ctx.line_to(cell_area.x + box_size * start + box_size,
						bg_area.y + bg_area.height)
				ctx.line_to(cell_area.x + box_size * end,
						bg_area.y + bg_area.height)

			ctx.line_to(cell_area.x + box_size * end + box_size / 2,
					bg_area.y + bg_area.height / 2 + bg_area.height)

			self.set_colour(ctx, colour, 0.0, 0.65)
			ctx.stroke()

		# Draw the revision node in the right column
		(column, colour) = self.node
		ctx.arc(cell_area.x + box_size * column + box_size / 2,
				cell_area.y + cell_area.height / 2,
				box_size / 4, 0, 2 * math.pi)

		self.set_colour(ctx, colour, 0.0, 0.5)
		ctx.stroke_preserve()

		self.set_colour(ctx, colour, 0.5, 1.0)
		ctx.fill()

class Commit:
	""" This represent a commit object obtained after parsing the git-rev-list 
	output """

	def __init__(self):
		self.message 		= ""
		self.author		= ""
		self.date 		= ""
		self.committer 		= ""
		self.commit_date 	= ""
		self.commit_sha1	= ""
		self.parent_sha1	= [ ]

	def get_message(self):
		fp = os.popen("git cat-file commit " + self.commit_sha1)
		message = fp.read()
		fp.close()
		return message


class DiffWindow:
	"""Diff window.
	This object represents and manages a single window containing the
	differences between two revisions on a branch.
	"""

	def __init__(self):
		self.window = gtk.Window(gtk.WINDOW_TOPLEVEL)
		self.window.set_border_width(0)
		self.window.set_title("Git reposotiry browser diff window")

		# Use two thirds of the screen by default
		screen = self.window.get_screen()
		monitor = screen.get_monitor_geometry(0)
		width = int(monitor.width * 0.66)
		height = int(monitor.height * 0.66)
		self.window.set_default_size(width, height)

		self.construct()

	def construct(self):
		"""Construct the window contents."""
		hbox = gtk.HBox(spacing=6)
		hbox.set_border_width(12)
		self.window.add(hbox)
		hbox.show()

		scrollwin = gtk.ScrolledWindow()
		scrollwin.set_policy(gtk.POLICY_AUTOMATIC, gtk.POLICY_AUTOMATIC)
		scrollwin.set_shadow_type(gtk.SHADOW_IN)
		hbox.pack_start(scrollwin, expand=True, fill=True)
		scrollwin.show()

		if have_gtksourceview:
			self.buffer = gtksourceview.SourceBuffer()
			slm = gtksourceview.SourceLanguagesManager()
			gsl = slm.get_language_from_mime_type("text/x-patch")
			self.buffer.set_highlight(True)
			self.buffer.set_language(gsl)
			sourceview = gtksourceview.SourceView(self.buffer)
		else:
			self.buffer = gtk.TextBuffer()
			sourceview = gtk.TextView(self.buffer)

		sourceview.set_editable(False)
		sourceview.modify_font(pango.FontDescription("Monospace"))
		scrollwin.add(sourceview)
		sourceview.show()


	def set_diff(self, commit_sha1, parent_sha1):
		"""Set the differences showed by this window.
		Compares the two trees and populates the window with the
		differences.
		"""
		# Diff with the first commit or the last commit shows nothing
		if (commit_sha1 == 0 or parent_sha1 == 0 ):
			return 

		fp = os.popen("git diff-tree -p " + parent_sha1 + " " + commit_sha1)
		self.buffer.set_text(fp.read())
		fp.close()
		self.window.show()


class GitView:
	""" This is the main class
	"""

	def __init__(self):
        	self.window =  	gtk.Window(gtk.WINDOW_TOPLEVEL)
        	self.window.set_border_width(0)
		self.window.set_title("Git repository browser")


        	# Use three-quarters of the screen by default
        	screen = self.window.get_screen()
        	monitor = screen.get_monitor_geometry(0)
        	width = int(monitor.width * 0.75)
        	height = int(monitor.height * 0.75)
        	self.window.set_default_size(width, height)

        	# FIXME AndyFitz!
        	icon = self.window.render_icon(gtk.STOCK_INDEX, gtk.ICON_SIZE_BUTTON)
        	self.window.set_icon(icon)

        	self.accel_group = gtk.AccelGroup()
        	self.window.add_accel_group(self.accel_group)

        	self.construct()


	def construct(self):
		"""Construct the window contents."""
		paned = gtk.VPaned()
		paned.pack1(self.construct_top(), resize=True, shrink=False)
		paned.pack2(self.construct_bottom(), resize=False, shrink=True)
		self.window.add(paned)
		paned.show()

	def construct_top(self):
		"""Construct the top-half of the window."""
		vbox = gtk.VBox(spacing=6)
		vbox.set_border_width(12)
		vbox.show()

		scrollwin = gtk.ScrolledWindow()
		scrollwin.set_policy(gtk.POLICY_NEVER, gtk.POLICY_AUTOMATIC)
		scrollwin.set_shadow_type(gtk.SHADOW_IN)
		vbox.pack_start(scrollwin, expand=True, fill=True)
		scrollwin.show()

		self.treeview = gtk.TreeView()
		self.treeview.set_rules_hint(True)
		self.treeview.set_search_column(4)
		self.treeview.connect("cursor-changed", self._treeview_cursor_cb)
		scrollwin.add(self.treeview)
		self.treeview.show()

		cell = CellRendererGraph()
		column = gtk.TreeViewColumn()
		column.set_resizable(False)
		column.pack_start(cell, expand=False)
		column.add_attribute(cell, "node", 1)
		column.add_attribute(cell, "in-lines", 2)
		column.add_attribute(cell, "out-lines", 3)
		self.treeview.append_column(column)

		cell = gtk.CellRendererText()
		cell.set_property("width-chars", 65)
		cell.set_property("ellipsize", pango.ELLIPSIZE_END)
		column = gtk.TreeViewColumn("Message")
		column.set_resizable(True)
		column.pack_start(cell, expand=True)
		column.add_attribute(cell, "text", 4)
		self.treeview.append_column(column)

		cell = gtk.CellRendererText()
		cell.set_property("width-chars", 40)
		cell.set_property("ellipsize", pango.ELLIPSIZE_END)
		column = gtk.TreeViewColumn("Author")
		column.set_resizable(True)
		column.pack_start(cell, expand=True)
		column.add_attribute(cell, "text", 5)
		self.treeview.append_column(column)

		cell = gtk.CellRendererText()
		cell.set_property("ellipsize", pango.ELLIPSIZE_END)
		column = gtk.TreeViewColumn("Date")
		column.set_resizable(True)
		column.pack_start(cell, expand=True)
		column.add_attribute(cell, "text", 6)
		self.treeview.append_column(column)

		return vbox


	def construct_bottom(self):
		"""Construct the bottom half of the window."""
		vbox = gtk.VBox(False, spacing=6)
		vbox.set_border_width(12)
		(width, height) = self.window.get_size()
		vbox.set_size_request(width, int(height / 2.5))
		vbox.show()

		self.table = gtk.Table(rows=4, columns=4)
		self.table.set_row_spacings(6)
		self.table.set_col_spacings(6)
		vbox.pack_start(self.table, expand=False, fill=True)
		self.table.show()

		align = gtk.Alignment(0.0, 0.5)
		label = gtk.Label()
		label.set_markup("<b>Revision:</b>")
		align.add(label)
		self.table.attach(align, 0, 1, 0, 1, gtk.FILL, gtk.FILL)
		label.show()
		align.show()

		align = gtk.Alignment(0.0, 0.5)
		self.revid_label = gtk.Label()
		self.revid_label.set_selectable(True)
		align.add(self.revid_label)
		self.table.attach(align, 1, 2, 0, 1, gtk.EXPAND | gtk.FILL, gtk.FILL)
		self.revid_label.show()
		align.show()

		align = gtk.Alignment(0.0, 0.5)
		label = gtk.Label()
		label.set_markup("<b>Committer:</b>")
		align.add(label)
		self.table.attach(align, 0, 1, 1, 2, gtk.FILL, gtk.FILL)
		label.show()
		align.show()

		align = gtk.Alignment(0.0, 0.5)
		self.committer_label = gtk.Label()
		self.committer_label.set_selectable(True)
		align.add(self.committer_label)
		self.table.attach(align, 1, 2, 1, 2, gtk.EXPAND | gtk.FILL, gtk.FILL)
		self.committer_label.show()
		align.show()

		align = gtk.Alignment(0.0, 0.5)
		label = gtk.Label()
		label.set_markup("<b>Timestamp:</b>")
		align.add(label)
		self.table.attach(align, 0, 1, 2, 3, gtk.FILL, gtk.FILL)
		label.show()
		align.show()

		align = gtk.Alignment(0.0, 0.5)
		self.timestamp_label = gtk.Label()
		self.timestamp_label.set_selectable(True)
		align.add(self.timestamp_label)
		self.table.attach(align, 1, 2, 2, 3, gtk.EXPAND | gtk.FILL, gtk.FILL)
		self.timestamp_label.show()
		align.show()

		align = gtk.Alignment(0.0, 0.5)
		label = gtk.Label()
		label.set_markup("<b>Parents:</b>")
		align.add(label)
		self.table.attach(align, 0, 1, 3, 4, gtk.FILL, gtk.FILL)
		label.show()
		align.show()

		self.parents_widgets = []

		scrollwin = gtk.ScrolledWindow()
		scrollwin.set_policy(gtk.POLICY_NEVER, gtk.POLICY_AUTOMATIC)
		scrollwin.set_shadow_type(gtk.SHADOW_IN)
		vbox.pack_start(scrollwin, expand=True, fill=True)
		scrollwin.show()

		self.message_buffer = gtk.TextBuffer()
		textview = gtk.TextView(self.message_buffer)
		textview.set_editable(False)
		textview.set_wrap_mode(gtk.WRAP_WORD)
		textview.modify_font(pango.FontDescription("Monospace"))
		scrollwin.add(textview)
		textview.show()

		align = gtk.Alignment(0.0, 0.5)
		label = gtk.Label()
		label.set_markup("<b>Children:</b>")
		align.add(label)
		self.table.attach(align, 2, 3, 3, 4, gtk.FILL, gtk.FILL)
		label.show()
		align.show()

		self.children_widgets = []

		return vbox

	def _treeview_cursor_cb(self, *args):
		"""Callback for when the treeview cursor changes."""
		(path, col) = self.treeview.get_cursor()
		commit = self.model[path][0]

		if commit.committer is not None:
			committer = commit.committer
			timestamp = commit.commit_date
			message   =  commit.get_message()
			revid_label = commit.commit_sha1
		else:
    			committer = ""
    			timestamp = ""
    			message = ""
			revid_label = ""

		self.revid_label.set_text(revid_label)
		self.committer_label.set_text(committer)
		self.timestamp_label.set_text(timestamp)
		self.message_buffer.set_text(message)

		for widget in self.parents_widgets:
			self.table.remove(widget)

		self.parents_widgets = []
		self.table.resize(4 + len(commit.parent_sha1) - 1, 4)
		for idx, parent_id in enumerate(commit.parent_sha1):
			self.table.set_row_spacing(idx + 3, 0)

			align = gtk.Alignment(0.0, 0.0)
			self.parents_widgets.append(align)
			self.table.attach(align, 1, 2, idx + 3, idx + 4,
					gtk.EXPAND | gtk.FILL, gtk.FILL)
			align.show()

			hbox = gtk.HBox(False, 0)
			align.add(hbox)
			hbox.show()

			label = gtk.Label(parent_id)
			label.set_selectable(True)
			hbox.pack_start(label, expand=False, fill=True)
			label.show()

			image = gtk.Image()
			image.set_from_stock(gtk.STOCK_JUMP_TO, gtk.ICON_SIZE_MENU)
			image.show()

			button = gtk.Button()
			button.add(image)
			button.set_relief(gtk.RELIEF_NONE)
			button.connect("clicked", self._go_clicked_cb, parent_id)
			hbox.pack_start(button, expand=False, fill=True)
			button.show()

			image = gtk.Image()
			image.set_from_stock(gtk.STOCK_FIND, gtk.ICON_SIZE_MENU)
			image.show()

			button = gtk.Button()
			button.add(image)
			button.set_relief(gtk.RELIEF_NONE)
			button.set_sensitive(True)
			button.connect("clicked", self._show_clicked_cb,
					commit.commit_sha1, parent_id)
			hbox.pack_start(button, expand=False, fill=True)
			button.show()

		# Populate with child details
		for widget in self.children_widgets:
			self.table.remove(widget)

		self.children_widgets = []
		try:
			child_sha1 = self.children_sha1[commit.commit_sha1]
		except KeyError:
			# We don't have child
			child_sha1 = [ 0 ]

		if ( len(child_sha1) > len(commit.parent_sha1)):
			self.table.resize(4 + len(child_sha1) - 1, 4)

		for idx, child_id in enumerate(child_sha1):
			self.table.set_row_spacing(idx + 3, 0)

			align = gtk.Alignment(0.0, 0.0)
			self.children_widgets.append(align)
			self.table.attach(align, 3, 4, idx + 3, idx + 4,
					gtk.EXPAND | gtk.FILL, gtk.FILL)
			align.show()

			hbox = gtk.HBox(False, 0)
			align.add(hbox)
			hbox.show()

			label = gtk.Label(child_id)
			label.set_selectable(True)
			hbox.pack_start(label, expand=False, fill=True)
			label.show()

			image = gtk.Image()
			image.set_from_stock(gtk.STOCK_JUMP_TO, gtk.ICON_SIZE_MENU)
			image.show()

			button = gtk.Button()
			button.add(image)
			button.set_relief(gtk.RELIEF_NONE)
			button.connect("clicked", self._go_clicked_cb, child_id)
			hbox.pack_start(button, expand=False, fill=True)
			button.show()

			image = gtk.Image()
			image.set_from_stock(gtk.STOCK_FIND, gtk.ICON_SIZE_MENU)
			image.show()

			button = gtk.Button()
			button.add(image)
			button.set_relief(gtk.RELIEF_NONE)
			button.set_sensitive(True)
			button.connect("clicked", self._show_clicked_cb,
					child_id, commit.commit_sha1)
			hbox.pack_start(button, expand=False, fill=True)
			button.show()

	def _destroy_cb(self, widget):
		"""Callback for when a window we manage is destroyed."""
		self.quit()


	def quit(self):
		"""Stop the GTK+ main loop."""
		gtk.main_quit()

	def run(self, args):
		self.set_branch(args)
        	self.window.connect("destroy", self._destroy_cb)
		self.window.show()
		gtk.main()

	def set_branch(self, args):
		"""Fill in different windows with info from the reposiroty"""
		fp = os.popen("git rev-parse --sq --default HEAD " + list_to_string(args, 1))
		git_rev_list_cmd = fp.read()
		fp.close()
		fp = os.popen("git rev-list  --header --topo-order --parents " + git_rev_list_cmd)
		self.update_window(fp)

	def update_window(self, fp):
		commit_lines = []

		self.model = gtk.ListStore(gobject.TYPE_PYOBJECT, gobject.TYPE_PYOBJECT,
				gobject.TYPE_PYOBJECT, gobject.TYPE_PYOBJECT, str, str, str)

		# used for cursor positioning 
		self.index = {}

		self.colours = {}
		self.nodepos = {}
		self.incomplete_line = {}
		self.children_sha1 = {}

		index = 0
		last_colour = 0
		last_nodepos = -1
		out_line = []	
		input_line = fp.readline()
		while (input_line != ""):
			# The commit header ends with '\0'
			# This NULL is immediately followed by the sha1 of the 
			# next commit
			if (input_line[0] != '\0'):
				commit_lines.append(input_line)
				input_line = fp.readline()
				continue;

			commit = self.parse_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

			# Skip the '\0
			commit_lines = []
			commit_lines.append(input_line[1:])
			input_line = fp.readline()

		fp.close()

		self.treeview.set_model(self.model)
		self.treeview.show()

	def draw_graph(self, commit, index, out_line, last_colour, last_nodepos):
		in_line=[]

		#   |   -> outline
		#   X
		#   |\  <- inline 

		# Reset nodepostion
		if (last_nodepos > 5):
			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:
			last_colour +=1
			self.colours[commit.commit_sha1] = last_colour
			colour =  last_colour
		try:
			node_pos = self.nodepos[commit.commit_sha1]
		except KeyError:
			last_nodepos +=1
			self.nodepos[commit.commit_sha1] = last_nodepos
			node_pos = last_nodepos

		#The first parent always continue on the same line
		try:
			# check we alreay have the value
			tmp_node_pos = self.nodepos[commit.parent_sha1[0]]
		except KeyError:
			self.colours[commit.parent_sha1[0]] = colour
			self.nodepos[commit.parent_sha1[0]] = node_pos

		in_line.append((node_pos, self.nodepos[commit.parent_sha1[0]],
					self.colours[commit.parent_sha1[0]]))

		self.add_incomplete_line(commit.parent_sha1[0], index+1)

		if (len(commit.parent_sha1) > 1):
			for parent_id in commit.parent_sha1[1:]:
				try:
					tmp_node_pos = self.nodepos[parent_id]
				except KeyError:
					last_colour += 1;
					self.colours[parent_id] = last_colour
					last_nodepos +=1
					self.nodepos[parent_id] = last_nodepos

				in_line.append((node_pos, self.nodepos[parent_id],
							self.colours[parent_id]))
				self.add_incomplete_line(parent_id, index+1)

		node = (node_pos, colour) 

		self.model.append([commit, node, out_line, in_line,
				commit.message, commit.author, commit.date]) 

		return (in_line, last_colour, last_nodepos)

	def add_incomplete_line(self, sha1, index):
		try:
			self.incomplete_line[sha1].append(self.nodepos[sha1])
		except KeyError:
			self.incomplete_line[sha1] = [self.nodepos[sha1]]


	def parse_commit(self, commit_lines):

		commit = Commit()

		# First line is the sha1 lines
		line = string.strip(commit_lines[0])
		sha1 = re.split(" ", line)
		commit.commit_sha1 = sha1[0]
		commit.parent_sha1 = sha1[1:]

		#build the child list
		for parent_id in commit.parent_sha1:
			try:
				self.children_sha1[parent_id].append(commit.commit_sha1)
			except KeyError:
				self.children_sha1[parent_id] = [commit.commit_sha1]

		# IF we don't have parent
		if (len(commit.parent_sha1) == 0):
			commit.parent_sha1 = [0]

		for line in commit_lines[1:]:
			m = re.match("^ ", line)
			if (m != None):
				# First line of the commit message used for short log
				if commit.message == "":
					commit.message = string.strip(line)
				continue

			m = re.match("tree", line)
			if (m != None):
				continue

			m = re.match("parent", line)
			if (m != None):
				continue


			m = re.match("author", line)
			if (m  != None ):
				patch_author = re.split(" ", line)	
				commit.author = list_to_string(patch_author[1:-2], 0)
				commit.date = time.strftime("%Y-%m-%d %H:%M:%S",
						time.gmtime(float(patch_author[-2])))
				continue

			m = re.match("committer", line)
			if (m  != None ):
				patch_committer = re.split(" ", line)	
				commit.committer = list_to_string(patch_committer[1:-2], 0)
				commit.commit_date = time.strftime("%Y-%m-%d %H:%M:%S",
						time.gmtime(float(patch_committer[-2])))
				continue

		return commit


	def _go_clicked_cb(self, widget, revid):
		"""Callback for when the go button for a parent is clicked."""
		try:
			self.treeview.set_cursor(self.index[revid])
		except KeyError:
			print "Revision %s not present in the list" % revid
			# revid == 0 is the parent of the first commit
			if (revid != 0 ):
				print "Try running gitview without any options"

		self.treeview.grab_focus()

	def _show_clicked_cb(self, widget,  commit_sha1, parent_sha1):
		"""Callback for when the show button for a parent is clicked."""
		window = DiffWindow()
		window.set_diff(commit_sha1, parent_sha1)
		self.treeview.grab_focus()

if __name__ == "__main__":
	view = GitView()
	view.run(sys.argv)



^ permalink raw reply

* Re: [Announce] gitview-0.1
From: Dave Jones @ 2006-02-01  4:29 UTC (permalink / raw)
  To: Aneesh Kumar; +Cc: git, junkio
In-Reply-To: <cc723f590601312016vabba201ye6d3739b3927f1a@mail.gmail.com>

On Wed, Feb 01, 2006 at 09:46:59AM +0530, Aneesh Kumar wrote:

 > I don't have a web location where i can host it so i am attaching it below.
 > It would be great if we can get it added to git repository
 > 
 > sample usage
 > takes same argument as git rev-list
 > 
 > gitview --since=2.week.ago

Nice, here's your first patch against it :-)

The one thing I like about gitk over this though is that with gitk
you don't have to click a button to see the diff.
For casual history browsing, it's much nicer to just scroll the bottom
window. Making it pop up a new window for each diff is just irritating.

		Dave


--- gitview~	2006-01-31 23:24:36.000000000 -0500
+++ gitview	2006-01-31 23:24:49.000000000 -0500
@@ -254,7 +254,7 @@ class DiffWindow:
 	def __init__(self):
 		self.window = gtk.Window(gtk.WINDOW_TOPLEVEL)
 		self.window.set_border_width(0)
-		self.window.set_title("Git reposotiry browser diff window")
+		self.window.set_title("Git repository browser diff window")
 
 		# Use two thirds of the screen by default
 		screen = self.window.get_screen()

^ permalink raw reply

* Re: [Announce] gitview-0.1
From: Aneesh Kumar @ 2006-02-01  4:41 UTC (permalink / raw)
  To: Dave Jones; +Cc: git, junkio
In-Reply-To: <20060201042930.GV16557@redhat.com>

On 2/1/06, Dave Jones <davej@redhat.com> wrote:
> On Wed, Feb 01, 2006 at 09:46:59AM +0530, Aneesh Kumar wrote:
>
>  > I don't have a web location where i can host it so i am attaching it below.
>  > It would be great if we can get it added to git repository
>  >
>  > sample usage
>  > takes same argument as git rev-list
>  >
>  > gitview --since=2.week.ago
>
> Nice, here's your first patch against it :-)
>
> The one thing I like about gitk over this though is that with gitk
> you don't have to click a button to see the diff.
> For casual history browsing, it's much nicer to just scroll the bottom
> window. Making it pop up a new window for each diff is just irritating.
>

I always browse the repository looking at the commit message and only
if interested i look at the changes.  That's why i made it look that
way.

-aneesh

^ permalink raw reply

* Re: [Census] So who uses git?
From: Linus Torvalds @ 2006-02-01  5:04 UTC (permalink / raw)
  To: Ray Lehtiniemi; +Cc: Junio C Hamano, git
In-Reply-To: <20060201045337.GC25753@mail.com>



On Tue, 31 Jan 2006, Ray Lehtiniemi wrote:
> 
> what if the user wants to change the mode bits of an assume-unchanged
> file with the twiddled permissions, but forgets to clear the flag
> first?  seems like that change is likely to get lost, especially if the
> new mode is read-only....

Remember - git only cares about execute permissions. The write permissions 
are entirely ignored by git ..

		Linus

^ permalink raw reply

* Re: [Census] So who uses git?
From: Junio C Hamano @ 2006-02-01  5:42 UTC (permalink / raw)
  To: Ray Lehtiniemi; +Cc: Linus Torvalds, git
In-Reply-To: <20060201045337.GC25753@mail.com>

Ray Lehtiniemi <rayl@mail.com> writes:

> what if the user wants to change the mode bits of an assume-unchanged
> file with the twiddled permissions, but forgets to clear the flag
> first?  seems like that change is likely to get lost, especially if the
> new mode is read-only....

No problem, since we only record u+x bit and nothing else.  Most
importantly, we do not record any of the +w bits.

^ permalink raw reply

* Re: [Census] So who uses git?
From: Junio C Hamano @ 2006-02-01  6:42 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0601311623240.7301@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> Oh, one final suggestion: if you give a filename to "git commit", and you 
> do the new semantics which means something _different_ than "do a 
> git-update-index on that file and commit", then I'd really suggest that 
> the _old_ index for that filename should match the parent exactly. 
> Otherwise, you may have done a
>
> 	git diff filename
>
> and you _thought_ you were committing just a two-line thing (because you 
> didn't understand about the index), but another, earlier, action caused 
> the index to be different from the file you had in HEAD, and in reality 
> you're actually committing a much bigger diff.

This "I thought I was only checking in the two-liner I did as
the last step but you committed the whole thing, stupid git!"
confusion feels to be a parallel of "I thought I was only
checking in the files I specified on the command line but you
also committed the files I earlier git-add'ed, stupid git!"
confusion.

Taken together with your "during a partially conflicted merge"
example, it feels to me that the simplest safety valve would be
to refuse "git commit paths..." if the index does not exactly
match HEAD.  Not just mentioned paths but anywhere.

People who do not like this can set in their config file some
flag, say, 'core.index = understood', to get the current
behaviour.

The reason I am bringing this up is because of this command
sequence:

	# start from a clean tree, after 'git reset --hard'
        $ create a-new-file
        $ git add a-new-file
        $ edit existing-file
        $ edit another-file
        $ git commit existing-file

There is no question we do not commit "another-file" and we do
commit changes to the "existing-file" as a whole.  What should
we do to "a-new-file", and how do we explain why we do so to
novices?

We can argue it either way.  We could say we shouldn't because
"commit" argument does not mention it.  We could say we should
because the user already told that he wants to add that file to
git.  Either makes sort-of sense from what the end user did.

I think a file "cvs add"ed is committed if whole subdirectory
commit (similar to our "commit -a") is done or the file is
explicitly specified on the "cvs commit" command line, and that
may match people's expectations.  That's an argument for not
committing "a-new-file".  But to be consistent with that, this
should not commit anything:

        # the same clean tree.
	$ create a-new-file
        $ git add a-new-file
        $ git commit

Which is counterintuitive to me by now (because I played too
long with git).

We could make "git commit" without paths to mean the current
"-a" behaviour, which would match CVS behaviour more closely.
However, it would make commit after a merge conflict resolution
in a dirty working tree _very_ dangerous -- it may give more
familiar feel to CVS people, but it is not an improvement for
git people at all.  I would rather not.

Right now, "git add" means "stage this for the next commit in
the index".  If we change the semantics of "git add" to mean "I
am not adding it for the next commit yet; I am just letting you
know there is a file in the working tree so that you can keep an
eye on it for me", using the intent-to-add index entry I've
mentioned a couple of times, I think the above problem might
naturally be solved.  For people who do not use update-index,
"commit -a" and "commit paths..." are the only two ways to
actually check-in anything to the index file for the next
commit ("git add" alone does not count).  "commit -a" would do
the equivalent of current "update all the not-up-to-date file to
the index and then commit", which would include the intent-to-add
paths.

^ permalink raw reply

* Re: [Census] So who uses git?
From: Junio C Hamano @ 2006-02-01  7:03 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0601311938130.7301@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> Your point that we discussed a similar flag for the "don't require a full 
> checkout" is a good one: we should try to make sure that it works for both 
> uses. Although maybe we decided for some reason that nobody cared about 
> the non-checked-out case?

We gave them a way to add --cacheinfo but did not do any more
than that, because they are independently coming up with some
hash (not necessarily be a proper git blob object name), they
did not have the huge blob data with the working tree anyway,
and the only thing they cared about was which paths changed and
they did not even want to see how the contents changed.
I.e. "diff-tree -r" was the only thing they cared about.

If we end up doing "assume unchanged", I should remember to do a
sensible thing for "diff-index" without --cached.  It should not
look at the working tree file for paths marked as such.  This
implies one optimization in "diff-index -p" and "diff-tree -p"
may need to be disabled.  They cheat and avoid expanding blob
objects when their cache entries are clean and required blobs
are in the working tree.  If "assume unchanged" path was
actually changed, such a diff would show up as a confusing
unexpected change.

Well, the user is asking for it, so that confusion is not _my_
problem, though ;-).

^ permalink raw reply

* Re: [Census] So who uses git?
From: linux @ 2006-02-01  7:08 UTC (permalink / raw)
  To: torvalds; +Cc: git, linux

> Yes, I think the "assume unchanged" flag goes well together with making 
> sure that the checked-out file is non-writable at the time.
> 
> Of course, any number of editors and other actions won't care: if you do 
> anything like
> 
> 	for i in *.c
> 	do
> 		sed 's/xyzzy/bas/g' < $i > $i.new
> 		mv $i.new $i
> 	done
> 
> you'll never have even noticed that the old file was marked read-only. So 
> it's obviously not in any way any guarantee, but it probably makes sense 
> as a crutch.

At the risk of complicating something already very complicated, and
possibly breaking on Microsoft file systems, that case can be detected
by reading the directory and noticing that the inode number changed.

Would it be worth validating the inode numbers (which can be retrieved
in a batch) even if you don't do a full lstat()?

Or is that too Unix-centric and prone to performance problems on other
file systems?  I'd think that, even if a file system used fake inode
numbers, they'd be pretty consistent if you didn't touch the file at all,
and being different would just cause a more expensive validation.
Which would be okay as long as it's infrequent.

^ 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