Git development
 help / color / mirror / Atom feed
* Re: GPF in index-pack
From: Jon Smirl @ 2006-08-06  2:44 UTC (permalink / raw)
  To: git
In-Reply-To: <9e4733910608051805j1192d910hf55393f1dbe1e472@mail.gmail.com>

Process size is 2.6GB when the seg fault happen. That's a lot of
memory to build a pack index over 1M objects.

I'm running a 3:1 process address space split. I wonder why it didn't
grow all the way to 3GB. I still have RAM and swap available.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: GPF in index-pack
From: Shawn Pearce @ 2006-08-06  2:48 UTC (permalink / raw)
  To: Jon Smirl; +Cc: git
In-Reply-To: <9e4733910608051944v5af7e552wd6909b9773dd516e@mail.gmail.com>

Jon Smirl <jonsmirl@gmail.com> wrote:
> Process size is 2.6GB when the seg fault happen. That's a lot of
> memory to build a pack index over 1M objects.
> 
> I'm running a 3:1 process address space split. I wonder why it didn't
> grow all the way to 3GB. I still have RAM and swap available.

Was the pack you are trying to index built with that fast-import.c
I sent last night?  Its possible its doing something weird that
pack-index can't handle, such as insert a duplicate object into
the same pack...

How big is the pack file?  I'd expect pack-index to be using
something around 24 MB of memory (24 bytes/entry) but maybe its
hanging onto a lot of data (memory leak?) as it decompresses the
entries to compute the checksums.

-- 
Shawn.

^ permalink raw reply

* git-fast-import
From: Jon Smirl @ 2006-08-06  2:51 UTC (permalink / raw)
  To: git, Shawn Pearce

git-fast-import works great. I parsed and built my pack file in
1:45hr. That's way better than 24hr. I am still IO bound but that
seems to be an issue with not being able to read ahead 150K small
files. CPU utilization averages about 50%.

I didn't bother reading the sha ids back from fast-import, instead I
computed them in the python code. Python has a C library function for
sha1. That decouple the processes from each other. They would run in
parallel on SMP.

My pack file is 980MB compared to 680MB from other attempts. I am
still missing entries for the trees and commits.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: GPF in index-pack
From: Jon Smirl @ 2006-08-06  2:58 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git
In-Reply-To: <20060806024858.GC20565@spearce.org>

On 8/5/06, Shawn Pearce <spearce@spearce.org> wrote:
> Jon Smirl <jonsmirl@gmail.com> wrote:
> > Process size is 2.6GB when the seg fault happen. That's a lot of
> > memory to build a pack index over 1M objects.
> >
> > I'm running a 3:1 process address space split. I wonder why it didn't
> > grow all the way to 3GB. I still have RAM and swap available.
>
> Was the pack you are trying to index built with that fast-import.c
> I sent last night?  Its possible its doing something weird that
> pack-index can't handle, such as insert a duplicate object into
> the same pack...

built with fast-import.

> How big is the pack file?  I'd expect pack-index to be using
> something around 24 MB of memory (24 bytes/entry) but maybe its
> hanging onto a lot of data (memory leak?) as it decompresses the
> entries to compute the checksums.

It is 934MB in size with 985,000 entries.

Why does resolve_delta in index-pack.c need to be recursive? Is there
a better way to code that routine? If it mmaps the file that uses 1GB
address space, why does it need another 1.5GB to build an index?

I had a prior 400MB pack file built with fast-import that I was able
to index ok.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: kompare won't parse git diffs
From: Paul Eggert @ 2006-08-06  3:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jakub Narebski, git
In-Reply-To: <7vy7u7t0ap.fsf@assigned-by-dhcp.cox.net>

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

> I wonder what happened to the plan to update GNU diff/patch to
> also emit/understand the c-style quoted paths.  Paul?

It's still intended, but not yet implemented unfortunately.

^ permalink raw reply

* Re: GPF in index-pack
From: Shawn Pearce @ 2006-08-06  3:32 UTC (permalink / raw)
  To: Jon Smirl; +Cc: git
In-Reply-To: <9e4733910608051958h6144add7td46de70dd8b45a5e@mail.gmail.com>

Jon Smirl <jonsmirl@gmail.com> wrote:
> On 8/5/06, Shawn Pearce <spearce@spearce.org> wrote:
> >Jon Smirl <jonsmirl@gmail.com> wrote:
> >> Process size is 2.6GB when the seg fault happen. That's a lot of
> >> memory to build a pack index over 1M objects.
> >>
> >> I'm running a 3:1 process address space split. I wonder why it didn't
> >> grow all the way to 3GB. I still have RAM and swap available.
> >
> >Was the pack you are trying to index built with that fast-import.c
> >I sent last night?  Its possible its doing something weird that
> >pack-index can't handle, such as insert a duplicate object into
> >the same pack...
> 
> built with fast-import.
> 
> >How big is the pack file?  I'd expect pack-index to be using
> >something around 24 MB of memory (24 bytes/entry) but maybe its
> >hanging onto a lot of data (memory leak?) as it decompresses the
> >entries to compute the checksums.
> 
> It is 934MB in size with 985,000 entries.
> 
> Why does resolve_delta in index-pack.c need to be recursive? Is there
> a better way to code that routine? If it mmaps the file that uses 1GB
> address space, why does it need another 1.5GB to build an index?

Probably the easiest way to code the routine.  Delta depth is
bounded; in the fast-import.c that I sent out last night I hardcoded
it to 10, which is (I believe) the default for GIT.  So long as that
routine is recursive only along a single delta chain the recursion
depth won't be very high and shouldn't be the problem.

> I had a prior 400MB pack file built with fast-import that I was able
> to index ok.

Dumb luck?  Maybe that had no duplicates while this one does?

-- 
Shawn.

^ permalink raw reply

* Re: git-fast-import
From: Shawn Pearce @ 2006-08-06  3:40 UTC (permalink / raw)
  To: Jon Smirl; +Cc: git
In-Reply-To: <9e4733910608051951g42bc4a5dv560c12e0a9add163@mail.gmail.com>

Jon Smirl <jonsmirl@gmail.com> wrote:
> git-fast-import works great. I parsed and built my pack file in
> 1:45hr. That's way better than 24hr. I am still IO bound but that
> seems to be an issue with not being able to read ahead 150K small
> files. CPU utilization averages about 50%.

Excellent.  Now if only the damn RCS files were in a more suitable
format.  :-)
 
> I didn't bother reading the sha ids back from fast-import, instead I
> computed them in the python code. Python has a C library function for
> sha1. That decouple the processes from each other. They would run in
> parallel on SMP.

At least you are IO bound and not CPU bound.  But it is silly for the
importer in Python to be computing the SHA1 IDs and for fast-import
to also be computing them.  Would it help if fast-import allowed
you to feed in a tag string which it dumps to an output file listing
SHA1 and the tag?  Then you can feed that data file back into your
tree/commit processing for revision handling.

> My pack file is 980MB compared to 680MB from other attempts. I am
> still missing entries for the trees and commits.

The delta selection ain't the best.  It may be the case that prior
attempts were combining files to get better delta chains vs. staying
all in one file.  It may be the case that the branches are causing
the delta chains to not be ideal.  I guess I expected slightly
better but not that much; earlier attempts were around 700 MB so
I thought maybe you'd be in the 800 MB ballpark.  Under 1 GB is
still good though as it means its feasible to fit the damn thing
into memory on almost any system, which makes it pretty repackable
with the standard packing code.

Its possible that you are also seeing duplicates in the pack;
I actually wouldn't be surprised if at least 100 MB of that was
duplicates where the author(s) reverted a file revision to an exact
prior revision, such that the SHA1 IDs were the same.  fast-import
(as I have previously said) is stupid and will write the content
out twice rather than "reuse" the existing entry.


Tonight I'll try to improve fast-import.c to include index
generation, and at the same time perform duplicate removal.
That should get you over the GPF in index-pack.c, may reduce disk
usage a little for the new pack, and save you from having to perform
a third pass on the new pack.

-- 
Shawn.

^ permalink raw reply

* Re: GPF in index-pack
From: Jon Smirl @ 2006-08-06  4:00 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git
In-Reply-To: <20060806033222.GD20565@spearce.org>

On 8/5/06, Shawn Pearce <spearce@spearce.org> wrote:
> Jon Smirl <jonsmirl@gmail.com> wrote:
> > On 8/5/06, Shawn Pearce <spearce@spearce.org> wrote:
> > >Jon Smirl <jonsmirl@gmail.com> wrote:
> > >> Process size is 2.6GB when the seg fault happen. That's a lot of
> > >> memory to build a pack index over 1M objects.
> > >>
> > >> I'm running a 3:1 process address space split. I wonder why it didn't
> > >> grow all the way to 3GB. I still have RAM and swap available.
> > >
> > >Was the pack you are trying to index built with that fast-import.c
> > >I sent last night?  Its possible its doing something weird that
> > >pack-index can't handle, such as insert a duplicate object into
> > >the same pack...
> >
> > built with fast-import.
> >
> > >How big is the pack file?  I'd expect pack-index to be using
> > >something around 24 MB of memory (24 bytes/entry) but maybe its
> > >hanging onto a lot of data (memory leak?) as it decompresses the
> > >entries to compute the checksums.
> >
> > It is 934MB in size with 985,000 entries.
> >
> > Why does resolve_delta in index-pack.c need to be recursive? Is there
> > a better way to code that routine? If it mmaps the file that uses 1GB
> > address space, why does it need another 1.5GB to build an index?
>
> Probably the easiest way to code the routine.  Delta depth is
> bounded; in the fast-import.c that I sent out last night I hardcoded
> it to 10, which is (I believe) the default for GIT.  So long as that
> routine is recursive only along a single delta chain the recursion
> depth won't be very high and shouldn't be the problem.

When I put index-pack in gdb at the seg fault, resolve_delta had
recursed more than 20,000 times. I stopped looking after that.

> > I had a prior 400MB pack file built with fast-import that I was able
> > to index ok.
>
> Dumb luck?  Maybe that had no duplicates while this one does?

Is there a git command to list the sha1's in a pack that doesn't have
an index? I could sort it, sort it unqiue, and then diff the outputs.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: GPF in index-pack
From: Shawn Pearce @ 2006-08-06  4:08 UTC (permalink / raw)
  To: Jon Smirl; +Cc: git
In-Reply-To: <9e4733910608052100t463e6fb8gba54e4afde1ab19a@mail.gmail.com>

Jon Smirl <jonsmirl@gmail.com> wrote:
> On 8/5/06, Shawn Pearce <spearce@spearce.org> wrote:
> >Jon Smirl <jonsmirl@gmail.com> wrote:
> >> On 8/5/06, Shawn Pearce <spearce@spearce.org> wrote:
> >> >Jon Smirl <jonsmirl@gmail.com> wrote:
> >> >> Process size is 2.6GB when the seg fault happen. That's a lot of
> >> >> memory to build a pack index over 1M objects.
> >> >>
> >> >> I'm running a 3:1 process address space split. I wonder why it didn't
> >> >> grow all the way to 3GB. I still have RAM and swap available.
> >> >
> >> >Was the pack you are trying to index built with that fast-import.c
> >> >I sent last night?  Its possible its doing something weird that
> >> >pack-index can't handle, such as insert a duplicate object into
> >> >the same pack...
> >>
> >> built with fast-import.
> >>
> >> >How big is the pack file?  I'd expect pack-index to be using
> >> >something around 24 MB of memory (24 bytes/entry) but maybe its
> >> >hanging onto a lot of data (memory leak?) as it decompresses the
> >> >entries to compute the checksums.
> >>
> >> It is 934MB in size with 985,000 entries.
> >>
> >> Why does resolve_delta in index-pack.c need to be recursive? Is there
> >> a better way to code that routine? If it mmaps the file that uses 1GB
> >> address space, why does it need another 1.5GB to build an index?
> >
> >Probably the easiest way to code the routine.  Delta depth is
> >bounded; in the fast-import.c that I sent out last night I hardcoded
> >it to 10, which is (I believe) the default for GIT.  So long as that
> >routine is recursive only along a single delta chain the recursion
> >depth won't be very high and shouldn't be the problem.
> 
> When I put index-pack in gdb at the seg fault, resolve_delta had
> recursed more than 20,000 times. I stopped looking after that.

Ouch.  I'm not familiar with this code, but looking at right now its
also not entirely obviously what its recursing for.  Plus dinner
is trying to be burned on the grill, so my attention is on that
more than on GIT.  :-)
 
> >> I had a prior 400MB pack file built with fast-import that I was able
> >> to index ok.
> >
> >Dumb luck?  Maybe that had no duplicates while this one does?
> 
> Is there a git command to list the sha1's in a pack that doesn't have
> an index? I could sort it, sort it unqiue, and then diff the outputs.

Not that I know of.  Packs themselves don't have the SHA1 values and
getting them from a pack without an index is a painful exercise as
you don't know where the base of an object resides within the pack
when you need it to generate the object's raw content to determine
its ID.

Yea, I'm sure the pack format has some great properties, but storing
the base as a 20 byte object ID rather the offset of the base within
the pack has its limitations...

-- 
Shawn.

^ permalink raw reply

* Re: git-fast-import
From: Jon Smirl @ 2006-08-06  4:09 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git
In-Reply-To: <20060806034009.GE20565@spearce.org>

On 8/5/06, Shawn Pearce <spearce@spearce.org> wrote:
> Jon Smirl <jonsmirl@gmail.com> wrote:
> > git-fast-import works great. I parsed and built my pack file in
> > 1:45hr. That's way better than 24hr. I am still IO bound but that
> > seems to be an issue with not being able to read ahead 150K small
> > files. CPU utilization averages about 50%.
>
> Excellent.  Now if only the damn RCS files were in a more suitable
> format.  :-)
>
> > I didn't bother reading the sha ids back from fast-import, instead I
> > computed them in the python code. Python has a C library function for
> > sha1. That decouple the processes from each other. They would run in
> > parallel on SMP.
>
> At least you are IO bound and not CPU bound.  But it is silly for the
> importer in Python to be computing the SHA1 IDs and for fast-import
> to also be computing them.  Would it help if fast-import allowed
> you to feed in a tag string which it dumps to an output file listing
> SHA1 and the tag?  Then you can feed that data file back into your
> tree/commit processing for revision handling.

I am IO bound, there is plenty of CPU and I am on a 2.8Ghz single processor.
The sha1 is getting stored into an internal Python structure. The
structures then get sliced and diced a thousand ways to compute the
change sets.

The real goal of this is to use the cvs2svn code for change set
detection. Look at how much work these guys have put into it making it
work on the various messed up CVS repositories.
http://git.catalyst.net.nz/gitweb?p=cvs2svn.git;a=shortlog;h=a9167614a7acec27e122ccf948d1602ffe5a0c4b

cvs2svn is the only tool that read and built change sets for Moz CVS
on the first try.

> > My pack file is 980MB compared to 680MB from other attempts. I am
> > still missing entries for the trees and commits.
>
> The delta selection ain't the best.  It may be the case that prior
> attempts were combining files to get better delta chains vs. staying

My suspicion is that prior attempts weren't capturing all of the
revisions. I know cvsps (the 680MB repo) was throwing away branches
that it didn't understand. I don't think anyone got parsecvs to run to
completion. MozCVS has 1,500 branches.

> all in one file.  It may be the case that the branches are causing
> the delta chains to not be ideal.  I guess I expected slightly
> better but not that much; earlier attempts were around 700 MB so
> I thought maybe you'd be in the 800 MB ballpark.  Under 1 GB is
> still good though as it means its feasible to fit the damn thing
> into memory on almost any system, which makes it pretty repackable
> with the standard packing code.

I am still missing all of the commits and trees. Don't know how much
they will add yet.

> Its possible that you are also seeing duplicates in the pack;
> I actually wouldn't be surprised if at least 100 MB of that was
> duplicates where the author(s) reverted a file revision to an exact
> prior revision, such that the SHA1 IDs were the same.  fast-import
> (as I have previously said) is stupid and will write the content
> out twice rather than "reuse" the existing entry.
>
> Tonight I'll try to improve fast-import.c to include index
> generation, and at the same time perform duplicate removal.
> That should get you over the GPF in index-pack.c, may reduce disk
> usage a little for the new pack, and save you from having to perform
> a third pass on the new pack.

Sounds like a good plan.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: GPF in index-pack
From: Jon Smirl @ 2006-08-06  4:16 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git
In-Reply-To: <20060806040848.GF20565@spearce.org>

On 8/6/06, Shawn Pearce <spearce@spearce.org> wrote:
> > When I put index-pack in gdb at the seg fault, resolve_delta had
> > recursed more than 20,000 times. I stopped looking after that.
>
> Ouch.  I'm not familiar with this code, but looking at right now its
> also not entirely obviously what its recursing for.  Plus dinner
> is trying to be burned on the grill, so my attention is on that
> more than on GIT.  :-)

It has recursed 131,000 times before running out of memory. Something
must be wrong in the resolve_delta() logic.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: GPF in index-pack
From: Jon Smirl @ 2006-08-06  4:22 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git
In-Reply-To: <9e4733910608052116u4d58908cj654ea94716b546e6@mail.gmail.com>

I started another run and added code to record the object_names. I can
use the sort utilities then to figure out how many duplicates there
are. It takes two hours for the run to finish so I won't have results
until tomorrow.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [PATCH] git-status: colorize status output
From: Joel Becker @ 2006-08-06  5:01 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20060805031418.GA11102@coredump.intra.peff.net>

On Fri, Aug 04, 2006 at 11:14:19PM -0400, Jeff King wrote:
> The git-status output can sometimes be very verbose, making it difficult to
> quickly see whether your files are updated in the index. This adds 4 levels
> of colorizing to the status output:
>   - general header (defaults to normal white)
>   - updated but not committed (defaults to green)
>   - changed but not updated (defaults to red)
>   - untracked files (defaults to red)

Please do one of two things:
1) Add code to discover a terminal is white-on-black and use bright colors
or
2) Default this to off.

I like the idea of colors, but the colors most people use in
black-on-white terminals are invisible on my screen.  colorls has the
same problem.

Joel

-- 

"What no boss of a programmer can ever understand is that a programmer
 is working when he's staring out of the window"
	- With apologies to Burton Rascoe


Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

^ permalink raw reply

* Re: git-svn added files to my commit?!
From: Seth Falcon @ 2006-08-06  5:22 UTC (permalink / raw)
  To: Eric Wong; +Cc: git
In-Reply-To: <20060805211337.GA31264@hand.yhbt.net>

[cc'ing the list in case others are seeing similar, etc]

Eric Wong <normalperson@yhbt.net> writes:
> Seth Falcon <sethfalcon@gmail.com> wrote:
>> Hi Eric,
>> 
>> I just used git-svn commit to commit a change back to an svn
>> repository and a few files made it into the commit that didn't appear
>> in the git diff output.  This resulted in a "dirty" commit to the svn
>> repository.  Perhaps it is user error, but I would appreciate it if
>> you could take a quick look.
>> 
>> Here is the log for my git-svn branch:
>> 
>>    $ git log --stat remotes/git-svn
>>    commit 6a376fa49bb5e9917c6272d61c6ee0769674da34
>>    Merge: c40e1e6... e4a5813...
>>    Author: sethf <sethf@bc3139a8-67e5-0310-9ffc-ced21a209358>
>>    Date:   Sat Aug 5 18:02:01 2006 +0000
>>    
>>        Fixed bug in certain vectorized uses of removeEdge
>>        
>>        Thanks to Denise Scholtens for reporting.
>>        
>>        
>>        git-svn-id: https://hedgehog.fhcrc.org/bioconductor/trunk/madman/Rpacks/graph@190
>>    commit e4a581328e194d545eb2e6a0e61f55c6d8ae56b8
>>    Author: Seth Falcon <seth@ziti.local>
>>    Date:   Sat Aug 5 11:00:59 2006 -0700
>>    
>>        Fixed bug in certain vectorized uses of removeEdge
>>        
>>        Thanks to Denise Scholtens for reporting.
>>    
>>     DESCRIPTION                    |    2 +-
>>     R/graphNEL.R                   |   23 ++++++++++++++++-------
>>     inst/unitTests/graphNEL_test.R |   40 +++++++++++++++++++++++++++++++++++++++-
>>     3 files changed, 56 insertions(+), 9 deletions(-)
>> 
>> git-svn log has:
>> 
>>    ------------------------------------------------------------------------
>>    r19045 | sethf | 2006-08-05 11:02:01 -0700 (Sat, 05 Aug 2006) | 4 lines
>>    
>>    Fixed bug in certain vectorized uses of removeEdge
>>    
>>    Thanks to Denise Scholtens for reporting.
>> 
>> Ok, looks good.  But in an updated svn working copy, I get:
>> 
>> svn log -v:
>>     ------------------------------------------------------------------------
>>     r19045 | sethf | 2006-08-05 11:02:01 -0700 (Sat, 05 Aug 2006) | 4 lines
>>     Changed paths:
>>        M /trunk/madman/Rpacks/graph/DESCRIPTION
>>        M /trunk/madman/Rpacks/graph/R/graphNEL.R
>>        M /trunk/madman/Rpacks/graph/inst/GXL/complexExample.gxl
>>        M /trunk/madman/Rpacks/graph/inst/GXL/simplExample.gxl.www
>>        M /trunk/madman/Rpacks/graph/inst/GXL/simpleExample.gxl
>>        M /trunk/madman/Rpacks/graph/inst/doc/GraphClass.tex
>>        M /trunk/madman/Rpacks/graph/inst/doc/GraphDesign.tex
>>        M /trunk/madman/Rpacks/graph/inst/unitTests/graphNEL_test.R
>>     
>>     Fixed bug in certain vectorized uses of removeEdge
>>     
>>     Thanks to Denise Scholtens for reporting.
>> 
>> Here is where you can see that a number of files "snuck" into the
>> commit.  When I looked at the diffs, it seemed to be line ending
>> changes only, but I was very surprised because 'git diff
>> remotes/git-svn..master' didn't show these, so I wasn't expecting them
>> to be part of the commit.
>
> That's very strange, git diff not showing anything suspicious.  The
> modified files have eol-style:native set on them, so it could be an
> issue with git-svn's eol-style handling.  From the info below, I assume
> you're _not_ using the SVN:: libraries, correct?

Right, I haven't installed any Perl libs out of laziness.  What are
the advantages of using the SVN:: libs?

> Looks like a bug, might as well cc the ml as it's a public repo.  I had
> issues accessing it with the SVN:: libraries, though (probably a
> permissions handling issue with git-svn) and had to use the
> command-line.

The repos is public, but you need --username=readonly
--password=readonly.

I think this is related to git-svn eol handling.  I tried the
following experiment:

Create a fresh git-svn repos along with a vanilla svn working copy.
Then diff.  I get differences in eol and I think there should be no
differences.

 ## create svn working copy
 svn co \
   https://hedgehog.fhcrc.org/bioconductor/trunk/madman/Rpacks/graph graph-svn
 cd graph-svn
 svn update -r18986

 ## create git-svn repos
 cd ~/
 mkdir graph-git
 cd graph-git
 git-svn init https://hedgehog.fhcrc.org/bioconductor/trunk/madman/Rpacks/graph
 git-svn fetch -r18300:18986
 git checkout master
 git diff remotes/git-svn..master  ## no output, as expected

 diff -ur inst/doc ~/graph-svn/inst/doc ## lots of output, unexpected


>> git-svn --version
>> git-svn version 1.4.2.rc3.g7e18e
>> 
>> git --version
>> git version 1.4.2.rc3.g7e18e
>> 
>> svn --version
>> svn, version 1.3.1 (r19032)
>> 
>> OSX 10.4.7


+ seth

^ permalink raw reply

* Re: [PATCH] git-status: colorize status output
From: Jeff King @ 2006-08-06  6:16 UTC (permalink / raw)
  To: Joel Becker; +Cc: Junio C Hamano, git
In-Reply-To: <20060806050138.GN23246@ca-server1.us.oracle.com>

On Sat, Aug 05, 2006 at 10:01:38PM -0700, Joel Becker wrote:

> Please do one of two things:
> 1) Add code to discover a terminal is white-on-black and use bright colors
> or
> 2) Default this to off.

It defaults to off. The colors are also easily customizable should you
want to use it.

-Peff

^ permalink raw reply

* Re: [PATCH 1/6] gitweb: Refactor untabifying - converting tabs to spaces
From: Matthias Lederhofer @ 2006-08-06  8:31 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200608060208.31862.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> wrote:
> +		# one out of three implementations used ($pos - 1) instead of $pos
Perhaps this should be in the commit message instead of the source?
I think no one will later understand this comment without the commit.

^ permalink raw reply

* Re: [RFC/PATCH] Fix "grep -w"
From: Junio C Hamano @ 2006-08-06  8:39 UTC (permalink / raw)
  To: git; +Cc: Morten Welinder
In-Reply-To: <7vlkq2dh6b.fsf@assigned-by-dhcp.cox.net>

We used to find the first match of the pattern and then if the
match is not for the entire word, declared that the whole line
does not match.

But that is wrong.  The command "git grep -w -e mmap" should
find that a line "foo_mmap bar mmap baz" matches, by tring the
second instance of pattern "mmap" on the same line.

Problems an earlier round of "fix" had were pointed out by Morten
Welinder, which have been incorporated in the t7002 tests.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 * Second try.

 builtin-grep.c  |   35 ++++++++++++++++-------
 t/t7002-grep.sh |   85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 110 insertions(+), 10 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index 69b7c48..93b7e07 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -410,8 +410,10 @@ static int fixmatch(const char *pattern,
 static int match_one_pattern(struct grep_opt *opt, struct grep_pat *p, char *bol, char *eol)
 {
 	int hit = 0;
+	int at_true_bol = 1;
 	regmatch_t pmatch[10];
 
+ again:
 	if (!opt->fixed) {
 		regex_t *exp = &p->regexp;
 		hit = !regexec(exp, bol, ARRAY_SIZE(pmatch),
@@ -422,22 +424,35 @@ static int match_one_pattern(struct grep
 	}
 
 	if (hit && opt->word_regexp) {
-		/* Match beginning must be either
-		 * beginning of the line, or at word
-		 * boundary (i.e. the last char must
-		 * not be alnum or underscore).
-		 */
 		if ((pmatch[0].rm_so < 0) ||
 		    (eol - bol) <= pmatch[0].rm_so ||
 		    (pmatch[0].rm_eo < 0) ||
 		    (eol - bol) < pmatch[0].rm_eo)
 			die("regexp returned nonsense");
-		if (pmatch[0].rm_so != 0 &&
-		    word_char(bol[pmatch[0].rm_so-1]))
-			hit = 0;
-		if (pmatch[0].rm_eo != (eol-bol) &&
-		    word_char(bol[pmatch[0].rm_eo]))
+
+		/* Match beginning must be either beginning of the
+		 * line, or at word boundary (i.e. the last char must
+		 * not be a word char).  Similarly, match end must be
+		 * either end of the line, or at word boundary
+		 * (i.e. the next char must not be a word char).
+		 */
+		if ( ((pmatch[0].rm_so == 0 && at_true_bol) ||
+		      !word_char(bol[pmatch[0].rm_so-1])) &&
+		     ((pmatch[0].rm_eo == (eol-bol)) ||
+		      !word_char(bol[pmatch[0].rm_eo])) )
+			;
+		else
 			hit = 0;
+
+		if (!hit && pmatch[0].rm_so + bol + 1 < eol) {
+			/* There could be more than one match on the
+			 * line, and the first match might not be
+			 * strict word match.  But later ones could be!
+			 */
+			bol = pmatch[0].rm_so + bol + 1;
+			at_true_bol = 0;
+			goto again;
+		}
 	}
 	return hit;
 }
diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
new file mode 100755
index 0000000..00a7d76
--- /dev/null
+++ b/t/t7002-grep.sh
@@ -0,0 +1,85 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Junio C Hamano
+#
+
+test_description='git grep -w
+'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	{
+		echo foo mmap bar
+		echo foo_mmap bar
+		echo foo_mmap bar mmap
+		echo foo mmap bar_mmap
+		echo foo_mmap bar mmap baz
+	} >file &&
+	echo x x xx x >x &&
+	echo y yy >y &&
+	echo zzz > z &&
+	git add file x y z &&
+	git commit -m initial
+'
+
+for H in HEAD ''
+do
+	case "$H" in
+	HEAD)	HC='HEAD:' L='HEAD' ;;
+	'')	HC= L='in working tree' ;;
+	esac
+
+	test_expect_success "grep -w $L" '
+		{
+			echo ${HC}file:1:foo mmap bar
+			echo ${HC}file:3:foo_mmap bar mmap
+			echo ${HC}file:4:foo mmap bar_mmap
+			echo ${HC}file:5:foo_mmap bar mmap baz
+		} >expected &&
+		git grep -n -w -e mmap $H >actual &&
+		diff expected actual
+	'
+
+	test_expect_success "grep -w $L (x)" '
+		{
+			echo ${HC}x:1:x x xx x
+		} >expected &&
+		git grep -n -w -e "x xx* x" $H >actual &&
+		diff expected actual
+	'
+
+	test_expect_success "grep -w $L (y-1)" '
+		{
+			echo ${HC}y:1:y yy
+		} >expected &&
+		git grep -n -w -e "^y" $H >actual &&
+		diff expected actual
+	'
+
+	test_expect_success "grep -w $L (y-2)" '
+		: >expected &&
+		if git grep -n -w -e "^y y" $H >actual
+		then
+			echo should not have matched
+			cat actual
+			false
+		else
+			diff expected actual
+		fi
+	'
+
+	test_expect_success "grep -w $L (z)" '
+		: >expected &&
+		if git grep -n -w -e "^z" $H >actual
+		then
+			echo should not have matched
+			cat actual
+			false
+		else
+			diff expected actual
+		fi
+	'
+done
+
+test_done
-- 
1.4.2.rc3.g19a8a

^ permalink raw reply related

* Re: [PATCH 2/6] gitweb: Simplify git_diff_print
From: Junio C Hamano @ 2006-08-06  9:26 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200608060211.38604.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> Copy to temporaty file more directly, not using temporary variable @file.
> Use list form of open for diff invocation (we cannot use git-diff because
> first it doesn't support -L/--label option, and we cannot generate diff
> between /dev/null and blob given by it's sha1 identifier). 
>
> Use "local $/ = undef;" for (temporary) slurp mode.
>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>

I think this part of the gitweb code predates the git-diff
support by the core level.  If I recall correctly, back then we
did not even have "git-diff-{tree,files,index}" when this part
was written.

I think you could use "git-diff $from $to" instead of using
temporary files and running /usr/bin/diff on it with today's
git.  Which is a big win from both security and ease of
administration point of view.  Not having to write anything into
temporary file from gitweb.cgi means you do not have to worry
about stray gitweb process leaving temporary files dangling
behind, for example.

You obviously would need to emulate the -L$from_name -L$to_name
part by hand in the loop that reads from the diff output.

One worrysome pathological case is what happens if files being
tracked happen to have names that consist of 40-byte hexdigits.
I think "git-diff $from $to --" should do the right thing for
gitweb use ($from and $to should not mistaken taken as pathnames
with the explicit -- disambiguator) and "git-diff -- $from $to"
also should do the right thing to take $from and $to as pathnames
even if there are revisions with the same object names, but
somebody may want to verify that (and send patches to fix if it
is not the case).

^ permalink raw reply

* Re: [PATCH 0/6] gitweb: Further refactoring
From: Junio C Hamano @ 2006-08-06 10:01 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200608060206.49086.jnareb@gmail.com>

The earlier 9-series looked fine, so I intend to apply them all
to "next".  However, they were all tab damaged and it was very
painful to fix up the patch.

This series, PATCH 1/5 (6?) looks fine (it is a bugfix for
git_diff_print), but among others, one of them had a seriously
fishy preimage (e.g. as you can see in "next", I haven't
accepted the function rename patch but 3/6 talks about
format_mark_referencing), and at that point my patience ran out.

Please check what I pushed out on "next"/"pu" tonight, and
consider patches missing from there dropped on the floor for now
(which does not mean they are rejected, except for the one that
touches diff_print and still uses /usr/bin/diff, which I
commented on).  It probably is a good idea to make sure further
patches resent are based on (or at least apply cleanly to) the
commit f16db173a4680aebc2f14c103a1e125c3f4d4531.

Please note that I am primarily concentrating on polishing what
is in "master" to finish 1.4.2 for now, so I might have missed
breakage in these gitweb patches that I queued for "next".

Comments, tests and fixups are all very welcomed.

^ permalink raw reply

* Re: [PATCH 2/6] gitweb: Simplify git_diff_print
From: Jakub Narebski @ 2006-08-06 10:22 UTC (permalink / raw)
  To: git
In-Reply-To: <7v8xm2b4g3.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:

> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Copy to temporaty file more directly, not using temporary variable @file.
>> Use list form of open for diff invocation (we cannot use git-diff because
>> first it doesn't support -L/--label option, and we cannot generate diff
>> between /dev/null and blob given by it's sha1 identifier). 
>>
>> Use "local $/ = undef;" for (temporary) slurp mode.
>>
>> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> 
> I think this part of the gitweb code predates the git-diff
> support by the core level.  If I recall correctly, back then we
> did not even have "git-diff-{tree,files,index}" when this part
> was written.
> 
> I think you could use "git-diff $from $to" instead of using
> temporary files and running /usr/bin/diff on it with today's
> git.  Which is a big win from both security and ease of
> administration point of view.  Not having to write anything into
> temporary file from gitweb.cgi means you do not have to worry
> about stray gitweb process leaving temporary files dangling
> behind, for example.

If I remember gitweb code correctly, diff to /dev/null is called
_only_ in commitdiff, where we can use git-diff -p to generate patch
directly, instead of generating difftree and calling diff on entries.

Still it would be nice if git-diff could diff between blob and arbitrary
file on filesystem (including /dev/null), and acquired -L/--label option.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* [PATCH] Add gitweb.cgi to .gitignore
From: Matthias Kestenholz @ 2006-08-06 10:47 UTC (permalink / raw)
  To: junkio; +Cc: git, Matthias Kestenholz

Signed-off-by: Matthias Kestenholz <matthias@spinlock.ch>
---
 .gitignore |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/.gitignore b/.gitignore
index 2299e36..b2feba5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -126,6 +126,7 @@ git-verify-tag
 git-whatchanged
 git-write-tree
 git-core-*/?*
+gitweb/gitweb.cgi
 test-date
 test-delta
 test-dump-cache-tree
-- 
1.4.2.rc3.g59e1

^ permalink raw reply related

* Re: [PATCH 0/6] gitweb: Further refactoring
From: Jakub Narebski @ 2006-08-06 10:52 UTC (permalink / raw)
  To: git
In-Reply-To: <7vmzai89o1.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:

> The earlier 9-series looked fine, so I intend to apply them all
> to "next".  However, they were all tab damaged and it was very
> painful to fix up the patch.

I'm sorry for that. As far as I checked, KNode (news reader which I used for
sending 9-(micro)series) preserves tabs when attaching file inline. I guess
that GMane news-to-mail gateway might munge whitespace. I won't use KNode
to send patches, then.

By the way, can anyone tell me how to setup git-send-email to send email via
GMail (authenticated SMTP, via STARTTLS/SSL, on ports 25 or 465 or 587)?

> This series, PATCH 1/5 (6?) looks fine (it is a bugfix for
> git_diff_print), but among others, one of them had a seriously
> fishy preimage (e.g. as you can see in "next", I haven't
> accepted the function rename patch but 3/6 talks about
> format_mark_referencing), and at that point my patience ran out.

I _have_ notified that this 6-series is on top of previous series,
namely '[PATCH 7/5] Merge changes in "split patch 1" series' patch.

But I try to redo not accepted changes on top of next, and put
"gitweb: Great subroutines renaming" on top of series. (BTW. this 
is simple search'n'replace on subroutine names... well, perhaps after
1.4.2...).

[...]
> Please note that I am primarily concentrating on polishing what
> is in "master" to finish 1.4.2 for now, so I might have missed
> breakage in these gitweb patches that I queued for "next".

I'm trying to check my gitweb patches, by checking the output of all
possible actions, althought I think it doesn't encompass all the cases...

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* [PATCH] gitweb: fix $project usage
From: Matthias Lederhofer @ 2006-08-06 11:25 UTC (permalink / raw)
  To: git

There were some places where $project was used even if it was not
defined.

Signed-off-by: Matthias Lederhofer <matled@gmx.net>
---
 gitweb/gitweb.perl |   17 ++++++++++++-----
 1 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index d0672cd..fa9ecce 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -85,7 +85,10 @@ if (defined $action) {
 }
 
 our $project = ($cgi->param('p') || $ENV{'PATH_INFO'});
-$project =~ s|^/||; $project =~ s|/$||;
+if (defined $project) {
+	$project =~ s|^/||;
+	$project =~ s|/$||;
+}
 if (defined $project && $project) {
 	if (!validate_input($project)) {
 		die_error(undef, "Invalid project parameter");
@@ -874,11 +877,15 @@ sub git_header_html {
 <title>$title</title>
 <link rel="stylesheet" type="text/css" href="$stylesheet"/>
 EOF
-	print "<link rel=\"alternate\" title=\"" . esc_param($project) . " log\" href=\"" .
-	      "$my_uri?" . esc_param("p=$project;a=rss") . "\" type=\"application/rss+xml\"/>\n" .
-	      "</head>\n";
+	if (defined $project) {
+		printf('<link rel="alternate" title="%s log" '.
+		       'href="%s" type="application/rss+xml"/>'."\n",
+		       esc_param($project),
+		       esc_param("$my_uri?p=$project;a=rss"));
+	}
 
-	print "<body>\n" .
+	print "</head>\n" .
+	      "<body>\n" .
 	      "<div class=\"page_header\">\n" .
 	      "<a href=\"http://www.kernel.org/pub/software/scm/git/docs/\" title=\"git documentation\">" .
 	      "<img src=\"$logo\" width=\"72\" height=\"27\" alt=\"git\" style=\"float:right; border-width:0px;\"/>" .
-- 
1.4.2.rc3.g2f52

^ permalink raw reply related

* fast-import and unique objects.
From: Jon Smirl @ 2006-08-06 12:32 UTC (permalink / raw)
  To: git, Shawn Pearce

This model has a lot of object duplication. I generated 949,305
revisions, but only 754,165 are unique. I'll modify my code to build a
hash of the objects it has seen and then not send the duplicates to
fast-import. Those 195,140 duplicated objects may be what is tripping
index-pack up.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: GPF in index-pack
From: Sergey Vlasov @ 2006-08-06 13:46 UTC (permalink / raw)
  To: Jon Smirl; +Cc: Shawn Pearce, git
In-Reply-To: <20060806040848.GF20565@spearce.org>

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

On Sun, 6 Aug 2006 00:08:48 -0400 Shawn Pearce wrote:

> Jon Smirl <jonsmirl@gmail.com> wrote:
> > On 8/5/06, Shawn Pearce <spearce@spearce.org> wrote:
> > >Jon Smirl <jonsmirl@gmail.com> wrote:
[...]
> > >> Why does resolve_delta in index-pack.c need to be recursive? Is there
> > >> a better way to code that routine? If it mmaps the file that uses 1GB
> > >> address space, why does it need another 1.5GB to build an index?
> > >
> > >Probably the easiest way to code the routine.  Delta depth is
> > >bounded; in the fast-import.c that I sent out last night I hardcoded
> > >it to 10, which is (I believe) the default for GIT.  So long as that
> > >routine is recursive only along a single delta chain the recursion
> > >depth won't be very high and shouldn't be the problem.
> > 
> > When I put index-pack in gdb at the seg fault, resolve_delta had
> > recursed more than 20,000 times. I stopped looking after that.
> 
> Ouch.  I'm not familiar with this code, but looking at right now its
> also not entirely obviously what its recursing for.  Plus dinner
> is trying to be burned on the grill, so my attention is on that
> more than on GIT.  :-)

git-pack-objects never creates a pack file with duplicate objects,
therefore git-index-pack was never tested on such pack files - no wonder
that it breaks.

The case of patch revert (A -> B -> A again) is probably the problem -
your dumb pack generator will probably write this:

	A
	B (delta based on A)
	A (delta based on B)

git-index-pack will first unpack the first copy of A, then notice that A
is used as a delta base for B and apply the delta, then it will find the
second copy of A and apply that delta, and then it will find B again...

Please try the patch at the end of this message - it should help to
avoid the infinite recursion in git-index-pack.  However, I'm not sure
that other git parts won't do something bad when they encounter an index
with duplicate sha1 entries (and git-index-pack cannot remove these
duplicates, because the number of index entries must match the pack
header).

> > >> I had a prior 400MB pack file built with fast-import that I was able
> > >> to index ok.
> > >
> > >Dumb luck?  Maybe that had no duplicates while this one does?
> > 
> > Is there a git command to list the sha1's in a pack that doesn't have
> > an index? I could sort it, sort it unqiue, and then diff the outputs.

git-index-pack :)

(git-unpack-objects will also have all sha1 values at the end, but the
side effect of unpacking all objects to separate files is probably not
what you want to get.)

> Not that I know of.  Packs themselves don't have the SHA1 values and
> getting them from a pack without an index is a painful exercise as
> you don't know where the base of an object resides within the pack
> when you need it to generate the object's raw content to determine
> its ID.

Yes, this is a problem.  git-unpack-objects can unpack all objects in a
single pass, but only because it temporarily saves all deltas which
cannot be resolved yet, and can read back objects which it has written
before.  git-index-pack needs to work without modifying the object
database, so it works in two passes:

	/*
	 * First pass:
	 * - find locations of all objects;
	 * - calculate SHA1 of all non-delta objects;
	 * - remember base SHA1 for all deltas.
	 */

	/*
	 * Second pass:
	 * - for all non-delta objects, look if it is used as a base for
	 *   deltas;
	 * - if used as a base, uncompress the object and apply all deltas,
	 *   recursively checking if the resulting object is used as a base
	 *   for some more deltas.
	 */

-----------------------------------------------------------------------

From: Sergey Vlasov <vsu@altlinux.ru>
Date: Sun, 6 Aug 2006 17:28:29 +0400
Subject: [PATCH] git-index-pack: Avoid infinite recursion if the pack has duplicate objects

Although git-pack-objects never creates packs which contain the same
object more than once, other tools may be not so careful, so add a check
to prevents infinite recursion of resolve_delta() for such packs.

Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>
---
 index-pack.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/index-pack.c b/index-pack.c
index b39953d..a8e3b1f 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -257,6 +257,13 @@ static void resolve_delta(struct delta_e
 	unsigned long next_obj_offset;
 	int j, first, last;
 
+	/*
+	 * Do nothing if this delta was resolved earlier (this can happen if
+	 * the pack contains duplicate objects for some reason).
+	 */
+	if (obj->real_type != OBJ_DELTA)
+		return;
+
 	obj->real_type = type;
 	delta_data = unpack_raw_entry(obj->offset, &delta_type,
 				      &delta_size, base_sha1,
-- 
1.4.2.rc3.g23aa

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

^ permalink raw reply related


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