HTTP questions

HTTP questions

[Nick Hengeveld]

While testing recent http-fetch changes with a current checkout of next,
I noticed a couple of things:

- "git push" seems to pass --thin by default to http-push, which
  subsequently barfs because that's not a valid http-push option.
  Should it be?  Should it be silently ignored?  Should git-push not
  default to --thin when pushing with HTTP transport?

- when I clone, http-fetch outputs a whole bunch of 
  "error: Could not read ..." messages - is that expected?

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

Re: HTTP questions

[Junio C Hamano]

Nick Hengeveld <nickh@reactrix.com> writes:

> - "git push" seems to pass --thin by default to http-push, which
>   subsequently barfs because that's not a valid http-push option.
>   Should it be?  Should it be silently ignored?  Should git-push not
>   default to --thin when pushing with HTTP transport?

The way I understand http-push works is that it does not use
packed transfer, so I presume even if we give --thin as a hint
it cannot even take advantage of it.  Probably we should stop
passing --thin to http transport (git native one uses it as a
cue that it can generate baseless deltas in the resulting pack).

> - when I clone, http-fetch outputs a whole bunch of 
>   "error: Could not read ..." messages - is that expected?

The clone over http seems to be severely broken in "next" right
now.  The one in "master" seems to be OK.  bisecting suggests
the breakage is coming from the tree parser rewrite.

bisect points at 136f2e548a34f1f504b0f062f87ddf33e8d6e83b.

http-fetch troubles

[Junio C Hamano]

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

> Nick Hengeveld <nickh@reactrix.com> writes:
>
>> - "git push" seems to pass --thin by default to http-push, which
>>   subsequently barfs because that's not a valid http-push option.
>>   Should it be?  Should it be silently ignored?  Should git-push not
>>   default to --thin when pushing with HTTP transport?
>
> The way I understand http-push works is that it does not use
> packed transfer, so I presume even if we give --thin as a hint
> it cannot even take advantage of it.  Probably we should stop
> passing --thin to http transport (git native one uses it as a
> cue that it can generate baseless deltas in the resulting pack).
>
>> - when I clone, http-fetch outputs a whole bunch of 
>>   "error: Could not read ..." messages - is that expected?
>
> The clone over http seems to be severely broken in "next" right
> now.  The one in "master" seems to be OK.  bisecting suggests
> the breakage is coming from the tree parser rewrite.
>
> bisect points at 136f2e548a34f1f504b0f062f87ddf33e8d6e83b.

I've pushed out Nick's http-fetch fixes in "master" to see if it
fixes problems for people.  Right now the one in "next" branch
seems to be having unrelated problems coming from another topic
which I haven't started tracking down yet.

Re: http-fetch troubles

[Junio C Hamano]

I think this fixes the http trouble with tree parser change in
the "next" branch.

-- >8 --
fetch.c: do not call process_tree() from process_tree().

This function reads a freshly fetched tree object, and schedules
the objects pointed by it for further fetching, so calling lookup-tree
and doing process_tree() recursively from there does not make
much sense.  We need to use process() on it to make sure we
fetch it first.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
diff --git a/fetch.c b/fetch.c
index ec2d8c3..107504b 100644
--- a/fetch.c
+++ b/fetch.c
@@ -46,13 +46,20 @@ static int process_tree(struct tree *tre
 	desc.buf = tree->buffer;
 	desc.size = tree->size;
 	while (tree_entry(&desc, &entry)) {
+		struct object *obj = NULL;
+
 		if (S_ISDIR(entry.mode)) {
 			struct tree *tree = lookup_tree(entry.sha1);
-			process_tree(tree);
-		} else {
+			if (tree)
+				obj = &tree->object;
+		}
+		else {
 			struct blob *blob = lookup_blob(entry.sha1);
-			process(&blob->object);
+			if (blob)
+				obj = &blob->object;
 		}
+		if (!obj || process(obj))
+			return -1;
 	}
 	free(tree->buffer);
 	tree->buffer = NULL;

Re: http-fetch troubles

[Linus Torvalds]

On Fri, 2 Jun 2006, Junio C Hamano wrote:
>
> I think this fixes the http trouble with tree parser change in
> the "next" branch.

Ahh, my bad. That happened as a bug in my original understanding of that 
fetch.c thing.

Your fix looks obviously fine,

               Linus

Re: http-fetch troubles

[Becky Bruce]

Woohoo!  The stuff you moved to master (which is what I was building  
from, not "next", as Nick pointed out) has fixed the git-http-fetch  
segfault problem I was seeing.

Thanks!
-Becky

On Jun 2, 2006, at 12:38 PM, Junio C Hamano wrote:

> Junio C Hamano <junkio@cox.net> writes:
>
>> Nick Hengeveld <nickh@reactrix.com> writes:
>>
>>> - "git push" seems to pass --thin by default to http-push, which
>>>   subsequently barfs because that's not a valid http-push option.
>>>   Should it be?  Should it be silently ignored?  Should git-push not
>>>   default to --thin when pushing with HTTP transport?
>>
>> The way I understand http-push works is that it does not use
>> packed transfer, so I presume even if we give --thin as a hint
>> it cannot even take advantage of it.  Probably we should stop
>> passing --thin to http transport (git native one uses it as a
>> cue that it can generate baseless deltas in the resulting pack).
>>
>>> - when I clone, http-fetch outputs a whole bunch of
>>>   "error: Could not read ..." messages - is that expected?
>>
>> The clone over http seems to be severely broken in "next" right
>> now.  The one in "master" seems to be OK.  bisecting suggests
>> the breakage is coming from the tree parser rewrite.
>>
>> bisect points at 136f2e548a34f1f504b0f062f87ddf33e8d6e83b.
>
> I've pushed out Nick's http-fetch fixes in "master" to see if it
> fixes problems for people.  Right now the one in "next" branch
> seems to be having unrelated problems coming from another topic
> which I haven't started tracking down yet.
>
>

Re: http-fetch troubles

[Junio C Hamano]

Becky Bruce <Becky.Bruce@freescale.com> writes:

> Woohoo!  The stuff you moved to master (which is what I was building
> from, not "next", as Nick pointed out) has fixed the git-http-fetch
> segfault problem I was seeing.
>
> Thanks!

Thanks Nick for fixing, and Becky for confirming.

... and I take all the credit ;-).

I haven't pushed it out yet, but I believe "next" is also good
to go.

Re: http-fetch troubles

[Linus Torvalds]

On Fri, 2 Jun 2006, Junio C Hamano wrote:
> 
> ... and I take all the credit ;-).

I always knew you'd work out as a maintainer.

		Linus

Re: http-fetch troubles

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> On Fri, 2 Jun 2006, Junio C Hamano wrote:
>> 
>> ... and I take all the credit ;-).
>
> I always knew you'd work out as a maintainer.
>
> 		Linus

Thanks for the compliment.  I realize that is your way to take
the credit ;-).