[PATCH 0/3] http*: refactor fetching code (v2)

This patch series applies on pu.

The major differences between this series and the 11-patch long one
(dated May 15, 2009) are:

 *check 'preq' pointer before using it (for fetching packs) [1]
 *moved free(url) before 'abort' label to stop -Wuninitialized warning
  (for fetching loose objects) [2]
 *use unlink_or_warn, after 691f1a2 ("replace direct calls to unlink
  (2) with unlink_or_warn")

Junio: I've rebased the patch series on pu, hopefully this stops the
attribute warnings for patch 3.

Tay Ray Chuan (3):
  http*: add helper methods for fetching packs
  http*: add helper methods for fetching objects/info/packs
  http*: add helper methods for fetching objects (loose)

 http-push.c   |  378 ++++++++--------------------------------------------
 http-walker.c |  407 +++++++++++---------------------------------------------
 http.c        |  415 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 http.h        |   72 ++++++++++-
 4 files changed, 616 insertions(+), 656 deletions(-)

Footnotes:
[1] @@ -595,17 +595,21 @@ static void finish_request(struct transfer_request *request)
     			start_fetch_packed(request);
     
     	} else if (request->state == RUN_FETCH_PACKED) {
    +		int fail = 1;
     		if (request->curl_result != CURLE_OK) {
     			fprintf(stderr, "Unable to get pack file %s\n%s",
     				request->url, curl_errorstr);
    -			repo->can_update_info_refs = 0;
     		} else {
     			preq = (struct http_pack_request *)request->userData;
     
    -			if (finish_http_pack_request(preq))
    -				repo->can_update_info_refs = 0;
    +			if (preq) {
    +				if (finish_http_pack_request(preq) > 0)
    +					fail = 0;
    +				release_http_pack_request(preq);
    +			}
     		}
    -		release_http_pack_request(preq);
    +		if (fail)
    +			repo->can_update_info_refs = 0;
     		release_request(request);
     	}
     }
    
[2] @@ -1167,9 +1167,9 @@ struct http_object_request
    *new_http_object_request(const char *base_url, 
     	return freq;
     
    +	free(url);
     abort:
     	free(filename);
    -	free(url);
     	free(freq);
     	return NULL;
     }

On Mon, May 18, 2009 at 04:30:25PM +0800, Tay Ray Chuan wrote:

This patch series applies on pu.

The major differences between this series and the 11-patch long one
(dated May 15, 2009) are:

 *check 'preq' pointer before using it (for fetching packs) [1]
 *moved free(url) before 'abort' label to stop -Wuninitialized warning
  (for fetching loose objects) [2]
 *use unlink_or_warn, after 691f1a2 ("replace direct calls to unlink
  (2) with unlink_or_warn")

Junio: I've rebased the patch series on pu, hopefully this stops the
attribute warnings for patch 3.

Tay Ray Chuan (3):
  http*: add helper methods for fetching packs
  http*: add helper methods for fetching objects/info/packs
  http*: add helper methods for fetching objects (loose)

I do think these would be even better if they were integrated with
http://kerneltrap.org/mailarchive/git/2009/1/18/4757804.

Speaking of which, do we have a better http test suite now ? If we do, I
can try to (finally) finalize my work.

Mike

Hi,

On Wed, 20 May 2009, Mike Hommey wrote:

do we have a better http test suite now ? If we do, I can try to 
(finally) finalize my work.

Oops, sorry, I know I promised to work on that, but I simply lack the 
time.

Sorry,
Dscho

Hi,

On Wed, May 20, 2009 at 6:21 PM, Johannes Schindelin
[off-list ref] wrote:

Oops, sorry, I know I promised to work on that, but I simply lack the
time.

a quick list of tests available for http* (I left out some minor ones):

t5540-http-push:
 *push to remote repository with packed refs
 *push to remote repository with unpacked refs
 *http-push fetches unpacked objects
 *http-push fetches packed objects
 *create and delete remote branch

t5550-http-fetch:
 *fetch changes via http
 *http remote detects correct HEAD
 *fetch packed objects

Mike and Johannes, what else do you have in mind?

-- 
Cheers,
Ray Chuan

Hi,

On Wed, May 20, 2009 at 3:43 PM, Mike Hommey [off-list ref] wrote:

I do think these would be even better if they were integrated with
http://kerneltrap.org/mailarchive/git/2009/1/18/4757804.

yes, I agree very much with your proposed http API, with methods for
fetching to files and strbufs. Please think of my patch series as an
intermediate step for your patches, by reducing the amount of code
involved in this API-ish refactoring.

I'm presently working on this.

-- 
Cheers,
Ray Chuan

On this topic, some of my friends have complained that git doesn't
automatically try appending .git to a URL when it fails to find a
bare git repository on the server.

Tony.
-- 
f.anthony.n.finch  [off-list ref]  http://dotat.at/
GERMAN BIGHT HUMBER: SOUTHWEST 5 TO 7. MODERATE OR ROUGH. SQUALLY SHOWERS.
MODERATE OR GOOD.

On Wed, May 20, 2009 at 08:14:59PM +0800, Tay Ray Chuan wrote:

a quick list of tests available for http* (I left out some minor ones):

[...]

Mike and Johannes, what else do you have in mind?

As I understand it, there is a bunch of error regressions in Mike's patches.
Having tests for those would help.