git-send-pack segfaulting on DebianPPC (was: Re: cg-clone, tag objects and cg-push/git-push don't play nice)

git-send-pack segfaulting on DebianPPC (was: Re: cg-clone, tag objects and cg-push/git-push don't play nice)

[Martin Langhoff]

On 10/19/05, Junio C Hamano <junkio@cox.net> wrote:
> Although I do not follow Cogito development closely, I seem to
> recall that it fetched tags without making them complete at some
> point in the past; I hope it is now fixed but I am not sure.

Ok -- I can now shed some light on this. This was happening on a
DebianPPC machine, but I didn't have clear evidence of that being a
factor (specially when Linus is widely known to use a PPC, and I
hadn't seen problems in other ppc boxen around). Now I do.

For all my finger-pointing, cg-fetch was trying to fetch the refernces
properly. However, git-fetch-pack is segfaulting on this Debian PPC
(etch). This is true of the current git "master" and 0.99.8.c. I
don'thave strace on the box until tomorrow, so I can't tell you more
about it.

On Debian i386 and Ubuntu i386, the exact same versions work correctly.

I'll post more info as soon as I can get my hands on strace,

cheers,



martin

Re: git-send-pack segfaulting on DebianPPC

[Junio C Hamano]

Martin Langhoff <martin.langhoff@gmail.com> writes:

> ... However, git-fetch-pack is segfaulting on this Debian PPC
> (etch). This is true of the current git "master" and 0.99.8.c. I
> don'thave strace on the box until tomorrow, so I can't tell you more
> about it.

Thanks, that is good to know.  As I said in another message, I
looked at cg-fetch's current tag completion code while working
on the ref^{tree} thing, and saw it used commit walker with '-a'
option (IOW, not producing an incomplete tag) to download
everything.

Even after fetch-pack segfaulted, since it does not update any
refs itself (it only writes them to stdout for the wrapper to
process), it should not have resulted in the repository that has
refs pointing at objects it does not have. In fact, it writes
its "SHA1 - name" list only after seeing unpack-objects exit
successfully, that should not have resulted in a corrupt
repository that records incomplete refs.  I suspect there is
something else going on here.

In cg-fetch, fetch-pack is used only in one location, and it
does not have anything to do with the tag completion code.
Presumably that explains why removing tags did not have any
effect, although it does not explain why removing some objects
did -- maybe those object files you needed to remove were
corrupt?

Re: git-send-pack segfaulting on DebianPPC

[Martin Langhoff]

On 10/19/05, Junio C Hamano <junkio@cox.net> wrote:
> Martin Langhoff <martin.langhoff@gmail.com> writes:
>
> > ... However, git-fetch-pack is segfaulting on this Debian PPC
> > (etch). This is true of the current git "master" and 0.99.8.c. I
> > don'thave strace on the box until tomorrow, so I can't tell you more
> > about it.
>
> Thanks, that is good to know.  As I said in another message, I
> looked at cg-fetch's current tag completion code while working
> on the ref^{tree} thing, and saw it used commit walker with '-a'
> option (IOW, not producing an incomplete tag) to download
> everything.

Ok -- I'm at home now so I can't test it any further, but reading
cg-fetch, it would actually be running git-ssh-fetch which was failing
(but cogito hides STDERR). When I tried it by hand, I used
git-fetch-pack, which segfaulted on Debian PPC and worked on i386.

> Even after fetch-pack segfaulted, since it does not update any
> refs itself (it only writes them to stdout for the wrapper to
> process), it should not have resulted in the repository that has
> refs pointing at objects it does not have. In fact, it writes
> its "SHA1 - name" list only after seeing unpack-objects exit
> successfully, that should not have resulted in a corrupt
> repository that records incomplete refs.  I suspect there is
> something else going on here.

Hmmm. You're right, it only spits out a list of things to retrieve...

> In cg-fetch, fetch-pack is used only in one location, and it
> does not have anything to do with the tag completion code.
> Presumably that explains why removing tags did not have any
> effect, although it does not explain why removing some objects
> did -- maybe those object files you needed to remove were
> corrupt?

Well, I'll strace the execution tomorrow and let you know. The tag
objects, however, are there. I can reliably git-cat-file tag <sha1>
them and they look pretty normal. The commit objects they refer to are
missing, though.

cheers,


martin

Re: git-send-pack segfaulting on DebianPPC (was: Re: cg-clone, tag objects and cg-push/git-push don't play nice)

[Linus Torvalds]

On Wed, 19 Oct 2005, Martin Langhoff wrote:
> 
> On Debian i386 and Ubuntu i386, the exact same versions work correctly.

Interesting.

As you say, I'm obviously testing on ppc all the time, and if 
git-fetch-pack has problems on ppc, I haven't seen them. And I fetch a 
lot.

It might be just the repo you're using. I can't test, since I obviously 
cannot ssh into the repo you pointed at (I tried to see if you had a 
git-daemon running, but no such luck).

> I'll post more info as soon as I can get my hands on strace,

Even more interesting than strace might be for you to try to run 
git-send-pack (even on an x86) under valgrind. valgrind really is a 
wonderful tool.

One other potential issue: I'm not running debian. I've run YDL, but right 
now the machine I use is FC4. There are likely _lots_ of library 
differences etc there.

		Linus

Re: git-send-pack segfaulting on DebianPPC (was: Re: cg-clone, tag objects and cg-push/git-push don't play nice)

[Martin Langhoff]

On 10/20/05, Linus Torvalds <torvalds@osdl.org> wrote:
> Interesting.
>
> As you say, I'm obviously testing on ppc all the time, and if
> git-fetch-pack has problems on ppc, I haven't seen them. And I fetch a
> lot.
>
> It might be just the repo you're using. I can't test, since I obviously
> cannot ssh into the repo you pointed at (I tried to see if you had a
> git-daemon running, but no such luck).

Ok -- I seem to be wrong in blaming PPC, so it's back to cg-fetch, and
I think I have a patch that does the right thing.

To recap: repo is slightly broken because cg-fetch has fetched tag
objects, but hasn't followed them through to the commit objects they
refer to.

Internally cg-fetch is actually using git-ssh-fetch (I misreported it
using git-fetch-pack), which is working correctly. However, cg-fetch
attempts to optimize the fetch process, by not calling git-ssh-fetch
if it has the tagobj that the ref points to. How those tag objects get
there without commits in the first place I don't know. So I've removed
the optimization and life is much better.

There is a second bug during the tag fetch. Some of the references
(created by git-cvsimport) are multiline, and break cg-fetch. It's
probably a bug in git-cvsimport, but I'm fixing cg-fetch to use head
-n1 instead of cat. I'll deal with git-cvsimport later.

On this broken-ish repo, git-fetch-pack segfaults as I've reported.
Running it under strace shows that it dies walking the local repo,
trying to find a missing commit. Unlikely that this is PPC only ;-)

cheers,



martin

Re: git-send-pack segfaulting on DebianPPC (was: Re: cg-clone, tag objects and cg-push/git-push don't play nice)

[Linus Torvalds]

On Thu, 20 Oct 2005, Martin Langhoff wrote:
> 
> On this broken-ish repo, git-fetch-pack segfaults as I've reported.
> Running it under strace shows that it dies walking the local repo,
> trying to find a missing commit. Unlikely that this is PPC only ;-)

Ok, that's not horribly surprising, but it's wrong.

Since I'm a lazy bastard, and couldn't necessarily re-create the right 
kind of broken repository even if I weren't, could you just enable 
core-dumps, and run gdb on the thing and report what "where" says is the 
call-chain.

It's almost certainly something that looks up an object and dereferences 
it without checking whether the object lookup succeeded or not. But if you 
can say where the SIGSEGV happens, it will be easier to figure out where.

Of course, what I really _should_ do is improve "sparse" to the point 
where I can mark functions as returning "unsafe" pointers, and then have 
sparse statically find all the dangerous users. I should do that anyway, 
since it would be useful for the kernel too, but it's actually quite 
nontrivial.

		Linus "being lazy is not a state of
			mind - it's a way of life" Torvalds

Re: git-send-pack segfaulting on DebianPPC

[Junio C Hamano]

Martin Langhoff <martin.langhoff@gmail.com> writes:

> There is a second bug during the tag fetch. Some of the references
> (created by git-cvsimport) are multiline, and break cg-fetch. It's
> probably a bug in git-cvsimport, but I'm fixing cg-fetch to use head
> -n1 instead of cat. I'll deal with git-cvsimport later.

When you update git-cvsimport, please make the tags it generates
git-check-ref-format happy.  The rules are:

 - No pathname component starting with a dot '.' (cg-fetch uses
   refs/heads/.$name-fetching to store the SHA1 it has not fully
   completed; for_each_ref() deliberately ignores path component
   that start with a dot to ignore them, probably to work this
   around).

 - No double-dots anywhere (ref1..ref2 notation is used on the
   command line and ref with embedded double-dots introduces
   ambiguities in parsing).

 - No byte with value lower than \040 (SP) nor \177 (DEL).

 - No tilde '~', caret '^' or colon ':' (the first two are used
   in extended SHA1 expressions; colon is used as src:dst
   notation in push and pull).

 - Not ending with a slash '/'.

> On this broken-ish repo, git-fetch-pack segfaults as I've reported.
> Running it under strace shows that it dies walking the local repo,
> trying to find a missing commit. Unlikely that this is PPC only ;-)

Re: git-send-pack segfaulting on DebianPPC (was: Re: cg-clone, tag objects and cg-push/git-push don't play nice)

[Petr Baudis]

Dear diary, on Wed, Oct 19, 2005 at 10:56:35PM CEST, I got a letter
where Martin Langhoff <martin.langhoff@gmail.com> told me that...
> To recap: repo is slightly broken because cg-fetch has fetched tag
> objects, but hasn't followed them through to the commit objects they
> refer to.

That must be leftover of some old fetch, before cg-fetch got fixed wrt.
this issue.

> Internally cg-fetch is actually using git-ssh-fetch (I misreported it
> using git-fetch-pack), which is working correctly. However, cg-fetch
> attempts to optimize the fetch process, by not calling git-ssh-fetch
> if it has the tagobj that the ref points to. How those tag objects get
> there without commits in the first place I don't know. So I've removed
> the optimization and life is much better.

The "optimization" or something alike needs to be there at least for the
user interface, so that we can actually say what tags are we
downloading; besides, there can be a *lot* of tags. But I'm not sure how
moot will this all be after Cogito moves to the remote peeking (Real
Soon Now, promise ;-).

> There is a second bug during the tag fetch. Some of the references
> (created by git-cvsimport) are multiline, and break cg-fetch. It's
> probably a bug in git-cvsimport, but I'm fixing cg-fetch to use head
> -n1 instead of cat. I'll deal with git-cvsimport later.

Eek. Did I miss something and are multiline refs meaningful now? If not,
they are pretty bad and I'd imagine other parts of Cogito would be
pretty confused about that. I'd call this a corrupted repository,
tough. Perhaps a check should be added to fsck.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.

Re: git-send-pack segfaulting on DebianPPC (was: Re: cg-clone, tag objects and cg-push/git-push don't play nice)

[Martin Langhoff]

On 10/20/05, Petr Baudis <pasky@suse.cz> wrote:
> Dear diary, on Wed, Oct 19, 2005 at 10:56:35PM CEST, I got a letter
> where Martin Langhoff <martin.langhoff@gmail.com> told me that...
> > To recap: repo is slightly broken because cg-fetch has fetched tag
> > objects, but hasn't followed them through to the commit objects they
> > refer to.
>
> That must be leftover of some old fetch, before cg-fetch got fixed wrt.
> this issue.

Yes, I'm trying to figure out if I can fix it...

> > Internally cg-fetch is actually using git-ssh-fetch (I misreported it
> > using git-fetch-pack), which is working correctly. However, cg-fetch
> > attempts to optimize the fetch process, by not calling git-ssh-fetch
> > if it has the tagobj that the ref points to. How those tag objects get
> > there without commits in the first place I don't know. So I've removed
> > the optimization and life is much better.
>
> The "optimization" or something alike needs to be there at least for the
> user interface, so that we can actually say what tags are we
> downloading; besides, there can be a *lot* of tags. But I'm not sure how
> moot will this all be after Cogito moves to the remote peeking (Real
> Soon Now, promise ;-).
>
> > There is a second bug during the tag fetch. Some of the references
> > (created by git-cvsimport) are multiline, and break cg-fetch. It's
> > probably a bug in git-cvsimport, but I'm fixing cg-fetch to use head
> > -n1 instead of cat. I'll deal with git-cvsimport later.
>
> Eek. Did I miss something and are multiline refs meaningful now? If not,
> they are pretty bad and I'd imagine other parts of Cogito would be
> pretty confused about that. I'd call this a corrupted repository,
> tough. Perhaps a check should be added to fsck.

Actually, all the tagsrefs have only one line, but something is going
weird around tagid=$(cat $tag) as $tag ends up containing many
filenames. So I undid my initial change to "head -n1", and I'm trying
to fix the loop.

cheers,


martin

Re: git-send-pack segfaulting on DebianPPC (was: Re: cg-clone, tag objects and cg-push/git-push don't play nice)

[Petr Baudis]

Dear diary, on Thu, Oct 20, 2005 at 01:23:11AM CEST, I got a letter
where Martin Langhoff <martin.langhoff@gmail.com> told me that...
> Actually, all the tagsrefs have only one line, but something is going
> weird around tagid=$(cat $tag) as $tag ends up containing many
> filenames. So I undid my initial change to "head -n1", and I'm trying
> to fix the loop.

Hmm, what bash version are you using? It's enclosed in

	for tag in *

and perhaps your bash misunderstood.

Anyway, this should've been quoted. I've fixed that in my tree, will
push out soon.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.

Re: git-send-pack segfaulting on DebianPPC

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> Hmm, what bash version are you using? It's enclosed in
>
> 	for tag in *
>
> and perhaps your bash misunderstood.

It probably is your cat not quoting $tag, which you just fixed.
Martin's repo has a conversion from CVS, with bunch of tags
whose names are like "Foo **INVALID**" (that's whitespace and
asterisks).

Re: git-send-pack segfaulting on DebianPPC

[Petr Baudis]

Dear diary, on Thu, Oct 20, 2005 at 02:03:52AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> told me that...
> Petr Baudis <pasky@suse.cz> writes:
> 
> > Hmm, what bash version are you using? It's enclosed in
> >
> > 	for tag in *
> >
> > and perhaps your bash misunderstood.
> 
> It probably is your cat not quoting $tag, which you just fixed.
> Martin's repo has a conversion from CVS, with bunch of tags
> whose names are like "Foo **INVALID**" (that's whitespace and
> asterisks).

Eww. Well, I pushed out the fix.

BTW (for anyone reading this), in the near future, I would like to do a
complete quoting review of Cogito, since while I'm trying to be careful
now, it wasn't always so, especially early on. Of course, *I* would also
like to do many other things... *hint* *hint* ;-)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.

Re: git-send-pack segfaulting on DebianPPC (was: Re: cg-clone, tag objects and cg-push/git-push don't play nice)

[Martin Langhoff]

On 10/20/05, Petr Baudis <pasky@suse.cz> wrote:
> Dear diary, on Thu, Oct 20, 2005 at 01:23:11AM CEST, I got a letter
> where Martin Langhoff <martin.langhoff@gmail.com> told me that...
> > Actually, all the tagsrefs have only one line, but something is going
> > weird around tagid=$(cat $tag) as $tag ends up containing many
> > filenames. So I undid my initial change to "head -n1", and I'm trying
> > to fix the loop.
>
> Hmm, what bash version are you using? It's enclosed in
>
>         for tag in *
>
> and perhaps your bash misunderstood.

After a bit of head-scratching, I found out what was causing this one:
legacy tagnames from git-cvsimport that contain '*' in the tagname. So

   tagid=$(cat SOME_TAG **INVALID**)

gives us some really nasty surprises.

cheers,


martin