patch, 3 partshttp*: 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:

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:

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 <Johannes.Schindelin@gmx.de> wrote:

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?

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

[...]

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

Hi,

On Wed, May 20, 2009 at 3:43 PM, Mike Hommey <mh@glandium.org> wrote:

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.

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.