threads / discuss / 4371

HTTP questions

Subject: HTTP questions

## tl;dr

9 messages between Jun 1, 2006 and Jun 3, 2006.

replies: 8people: 4as markdown or json

Nick Hengeveld· Jun 1, 2006, 23:24 UTC · lore

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.
Junio C Hamano· Jun 2, 2006, 06:38 UTC · re: Nick Hengeveld · lore

Re: HTTP questions

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.
Junio C Hamano· Jun 2, 2006, 17:38 UTC · re: Junio C Hamano · lore

http-fetch troubles

Junio C Hamano <junkio@cox.net> writes:
Show 21 quoted lines
> 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.

Junio C Hamano· Jun 2, 2006, 22:15 UTC · re: Junio C Hamano · lore

Re: http-fetch troubles

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;
Linus Torvalds· Jun 2, 2006, 22:34 UTC · re: Junio C Hamano · lore

Re: http-fetch troubles

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
Becky Bruce· Jun 3, 2006, 01:13 UTC · re: Junio C Hamano · lore

Re: http-fetch troubles

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:
Show 30 quoted lines
> 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.
>
>
Junio C Hamano· Jun 3, 2006, 01:24 UTC · re: Becky Bruce · lore

Re: http-fetch troubles

Becky Bruce <Becky.Bruce@freescale.com> writes:
Show 5 quoted lines
> 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.

Junio C Hamano· Jun 3, 2006, 07:58 UTC · re: Linus Torvalds · lore

Re: http-fetch troubles

Linus Torvalds <torvalds@osdl.org> writes:
Show 7 quoted lines
> 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 ;-).

← back to recent threads