@matheusbsbs / matheus-bagliano / commits / fe72d420ab

Fork 0

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

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 <spearce@spearce.org> Acked-by: Tay Ray Chuan <rctay89@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Shawn O. Pearce committed Apr 19, 2010 at 07:23 UTC fe72d420ab4dda593dddece7b907ee7868ced127

2 files changed +52 -7

http.c

+37 -7

@@ -1,6 +1,7 @@

1 #include "http.h"

2 #include "pack.h"

3 #include "sideband.h"

4 +#include "run-command.h"

6 int data_received;

7 int active_requests;

@@ -998,11 +999,14 @@ void release_http_pack_request(struct http_pack_request *preq)

999

1000 int finish_http_pack_request(struct http_pack_request *preq)

1001 {

1001 - int ret;

1002 struct packed_git **lst;

1003 struct packed_git *p = preq->target;

1004 + char *tmp_idx;

1005 + struct child_process ip;

1006 + const char *ip_argv[8];

1007 +

1008 + close_pack_index(p);

1009

1005 - p->pack_size = ftell(preq->packfile);

1010 fclose(preq->packfile);

1011 preq->packfile = NULL;

1012 preq->slot->local = NULL;

@@ -1012,13 +1016,39 @@ int finish_http_pack_request(struct http_pack_request *preq)

1016 lst = &((*lst)->next);

1017 *lst = (*lst)->next;

1018

1015 - ret = move_temp_to_file(preq->tmpfile, sha1_pack_name(p->sha1));

1016 - if (ret)

1017 - return ret;

1018 - if (verify_pack(p))

1019 + tmp_idx = xstrdup(preq->tmpfile);

1020 + strcpy(tmp_idx + strlen(tmp_idx) - strlen(".pack.temp"),

1021 + ".idx.temp");

1022 +

1023 + ip_argv[0] = "index-pack";

1024 + ip_argv[1] = "-o";

1025 + ip_argv[2] = tmp_idx;

1026 + ip_argv[3] = preq->tmpfile;

1027 + ip_argv[4] = NULL;

1028 +

1029 + memset(&ip, 0, sizeof(ip));

1030 + ip.argv = ip_argv;

1031 + ip.git_cmd = 1;

1032 + ip.no_stdin = 1;

1033 + ip.no_stdout = 1;

1034 +

1035 + if (run_command(&ip)) {

1036 + unlink(preq->tmpfile);

1037 + unlink(tmp_idx);

1038 + free(tmp_idx);

1039 return -1;

1020 - install_packed_git(p);

1040 + }

1041 +

1042 + unlink(sha1_pack_index_name(p->sha1));

1043

1044 + if (move_temp_to_file(preq->tmpfile, sha1_pack_name(p->sha1))

1045 + || move_temp_to_file(tmp_idx, sha1_pack_index_name(p->sha1))) {

1046 + free(tmp_idx);

1047 + return -1;

1048 + }

1049 +

1050 + install_packed_git(p);

1051 + free(tmp_idx);

1052 return 0;

1053 }

1054

t/t5550-http-fetch.sh

+15

@@ -62,6 +62,21 @@ test_expect_success 'fetch packed objects' '

62 git clone $HTTPD_URL/dumb/repo_pack.git

63 '

65 +test_expect_success 'fetch notices corrupt pack' '

66 + cp -R "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git "$HTTPD_DOCUMENT_ROOT_PATH"/repo_bad1.git &&

67 + (cd "$HTTPD_DOCUMENT_ROOT_PATH"/repo_bad1.git &&

68 + p=`ls objects/pack/pack-*.pack` &&

69 + chmod u+w $p &&

70 + printf %0256d 0 | dd of=$p bs=256 count=1 seek=1 conv=notrunc

71 + ) &&

72 + mkdir repo_bad1.git &&

73 + (cd repo_bad1.git &&

74 + git --bare init &&

75 + test_must_fail git --bare fetch $HTTPD_URL/dumb/repo_bad1.git &&

76 + test 0 = `ls objects/pack/pack-*.pack | wc -l`

77 + )

78 +'

79 +

80 test_expect_success 'did not use upload-pack service' '

81 grep '/git-upload-pack' <"$HTTPD_ROOT_PATH"/access.log >act

82 : >exp