git/http.c, branch v1.7.2

Merge branch 'sp/maint-dumb-http-pack-reidx'

2010-05-21T11:02:19Z

* sp/maint-dumb-http-pack-reidx: http.c::new_http_pack_request: do away with the temp variable filename http-fetch: Use temporary files for pack-*.idx until verified http-fetch: Use index-pack rather than verify-pack to check packs Allow parse_pack_index on temporary files Extract verify_pack_index for reuse from verify_pack Introduce close_pack_index to permit replacement http.c: Remove unnecessary strdup of sha1_to_hex result http.c: Don't store destination name in request structures http.c: Drop useless != NULL test in finish_http_pack_request http.c: Tiny refactoring of finish_http_pack_request t5550-http-fetch: Use subshell for repository operations http.c: Remove bad free of static block

Merge branch 'rc/maint-curl-helper'

2010-05-09T05:37:24Z

* rc/maint-curl-helper: remote-curl: ensure that URLs have a trailing slash http: make end_url_with_slash() public t5541-http-push: add test for URLs with trailing slash Conflicts: remote-curl.c

http.c::new_http_pack_request: do away with the temp variable filename

2010-04-20T00:57:50Z

Now that the temporary variable char *filename is only used in one place, do away with it and just call sha1_pack_name() directly. Signed-off-by: Tay Ray Chuan Acked-by: Shawn O. Pearce Signed-off-by: Junio C Hamano

http-fetch: Use temporary files for pack-*.idx until verified

2010-04-20T00:56:29Z

Verify that a downloaded pack-*.idx file is consistent and valid as an index file before we rename it into its final destination. This prevents a corrupt index file from later being treated as a usable file, confusing readers. Check that we do not have the pack index file before invoking fetch_pack_index(); that way, we can do without the has_pack_index() check in fetch_pack_index(). Signed-off-by: Shawn O. Pearce Signed-off-by: Junio C Hamano

http-fetch: Use index-pack rather than verify-pack to check packs

2010-04-20T00:56:20Z

To ensure we don't leave a corrupt pack file positioned as though it were a valid pack file, run index-pack on the temporary pack before we rename it to its final name. If index-pack crashes out when it discovers file corruption (e.g. GitHub's error HTML at the end of the file), simply delete the temporary files to cleanup. By waiting until the pack has been validated before we move it to its final name, we eliminate a race condition where another concurrent reader might try to access the pack at the same time that we are still trying to verify its not corrupt. Switching from verify-pack to index-pack is a change in behavior, but it should turn out better for users. The index-pack algorithm tries to minimize disk seeks, as well as the number of times any given object is inflated, by organizing its work along delta chains. The verify-pack logic does not attempt to do this, thrashing the delta base cache and the filesystem cache. By recreating the index file locally, we also can automatically upgrade from a v1 pack table of contents to v2. This makes the CRC32 data available for use during later repacks, even if the server didn't have them on hand. Signed-off-by: Shawn O. Pearce Acked-by: Tay Ray Chuan Signed-off-by: Junio C Hamano

Allow parse_pack_index on temporary files

2010-04-20T00:56:17Z

The easiest way to verify a pack index is to open it through the standard parse_pack_index function, permitting the header check to happen when the file is mapped. However, the dumb HTTP client needs to verify a pack index before its moved into its proper file name within the objects/pack directory, to prevent a corrupt index from being made available. So permit the caller to specify the exact path of the index file. For now we're still using the final destination name within the sole call site in http.c, but eventually we will start to parse the temporary path instead. Signed-off-by: Shawn O. Pearce Signed-off-by: Junio C Hamano

http.c: Remove unnecessary strdup of sha1_to_hex result

2010-04-20T00:55:59Z

Most of the time the dumb HTTP transport is run without the verbose flag set, so we only need the result of sha1_to_hex(sha1) once, to construct the pack URL. Don't bother with an unnecessary malloc, copy, free chain of this buffer. If verbose is set, we'll format the SHA-1 twice now. But this tiny extra CPU time spent is nothing compared to the slowdown that is usually imposed by the verbose messages being sent to the tty, and is entirely trivial compared to the latency involved with the remote HTTP server sending something as big as a pack file. Signed-off-by: Shawn O. Pearce Acked-by: Tay Ray Chuan Signed-off-by: Junio C Hamano

http.c: Don't store destination name in request structures

2010-04-17T20:55:46Z

The destination name within the object store is easily computed on demand, reusing a static buffer held by sha1_file.c. We don't need to copy the entire path into the request structure for safe keeping, when it can be easily reformatted after the download has been completed. This reduces the size of the per-request structure, and removes yet another PATH_MAX based limit. Signed-off-by: Shawn O. Pearce Signed-off-by: Junio C Hamano

http.c: Drop useless != NULL test in finish_http_pack_request

2010-04-17T20:55:46Z

The test preq->packfile != NULL is always true. If packfile was actually NULL when entering this function the ftell() above would crash out with a SIGSEGV, resulting in never reaching this point. Simplify the code by just removing the conditional. Signed-off-by: Shawn O. Pearce Signed-off-by: Junio C Hamano

http.c: Tiny refactoring of finish_http_pack_request

2010-04-17T20:55:45Z

Always remove the struct packed_git from the active list, even if the rename of the temporary file fails. While we are here, simplify the code a bit by using a common local variable name ("p") to hold the relevant packed_git. Signed-off-by: Shawn O. Pearce Signed-off-by: Junio C Hamano