* [PATCH 1/6] Allow pack header preprocessing before unpack-objects/index-pack. @ 2006-11-01 22:06 Nicolas Pitre 2006-11-01 22:06 ` [PATCH 2/6] Teach receive-pack how to keep pack files based on object count Nicolas Pitre 2006-11-01 23:50 ` [PATCH 1/6] Allow pack header preprocessing before unpack-objects/index-pack Junio C Hamano 0 siblings, 2 replies; 13+ messages in thread From: Nicolas Pitre @ 2006-11-01 22:06 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Nicolas Pitre Some applications which invoke unpack-objects or index-pack --stdin may want to examine the pack header to determine the number of objects contained in the pack and use that value to determine which executable to invoke to handle the rest of the pack stream. However if the caller consumes the pack header from the input stream then its no longer available for unpack-objects or index-pack --stdin, both of which need the version and object count to process the stream. This change introduces --pack_header=ver,cnt as a command line option that the caller can supply to indicate it has already consumed the pack header and what version and object count were found in that header. As this option is only meant for low level applications such as receive-pack we are not documenting it at this time. Signed-off-by: Nicolas Pitre <nico@cam.org> --- Patch description text shamelessly stolen from a similar patch from Shawn Pearce <spearce@spearce.org>. builtin-unpack-objects.c | 15 +++++++++++++++ index-pack.c | 13 +++++++++++++ 2 files changed, 28 insertions(+), 0 deletions(-) diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c index 74a90c1..e6d7574 100644 --- a/builtin-unpack-objects.c +++ b/builtin-unpack-objects.c @@ -371,6 +371,21 @@ int cmd_unpack_objects(int argc, const c recover = 1; continue; } + if (!strncmp(arg, "--pack_header=", 14)) { + struct pack_header *hdr; + char *c; + + hdr = (struct pack_header *)buffer; + hdr->hdr_signature = htonl(PACK_SIGNATURE); + hdr->hdr_version = htonl(strtoul(arg + 14, &c, 10)); + if (*c != ',') + die("bad %s", arg); + hdr->hdr_entries = htonl(strtoul(c + 1, &c, 10)); + if (*c) + die("bad %s", arg); + len = sizeof(*hdr); + continue; + } usage(unpack_usage); } diff --git a/index-pack.c b/index-pack.c index b37dd78..a3b55f9 100644 --- a/index-pack.c +++ b/index-pack.c @@ -841,6 +841,19 @@ int main(int argc, char **argv) keep_msg = ""; } else if (!strncmp(arg, "--keep=", 7)) { keep_msg = arg + 7; + } else if (!strncmp(arg, "--pack_header=", 14)) { + struct pack_header *hdr; + char *c; + + hdr = (struct pack_header *)input_buffer; + hdr->hdr_signature = htonl(PACK_SIGNATURE); + hdr->hdr_version = htonl(strtoul(arg + 14, &c, 10)); + if (*c != ',') + die("bad %s", arg); + hdr->hdr_entries = htonl(strtoul(c + 1, &c, 10)); + if (*c) + die("bad %s", arg); + input_len = sizeof(*hdr); } else if (!strcmp(arg, "-v")) { verbose = 1; } else if (!strcmp(arg, "-o")) { -- 1.4.3.3.g87b2-dirty ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/6] Teach receive-pack how to keep pack files based on object count. 2006-11-01 22:06 [PATCH 1/6] Allow pack header preprocessing before unpack-objects/index-pack Nicolas Pitre @ 2006-11-01 22:06 ` Nicolas Pitre 2006-11-01 22:06 ` [PATCH 3/6] git-fetch can use both --thin and --keep with fetch-pack now Nicolas Pitre 2006-11-01 23:50 ` [PATCH 1/6] Allow pack header preprocessing before unpack-objects/index-pack Junio C Hamano 1 sibling, 1 reply; 13+ messages in thread From: Nicolas Pitre @ 2006-11-01 22:06 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Shawn Pearce From: Shawn Pearce <spearce@spearce.org> Since keeping a pushed pack or exploding it into loose objects should be a local repository decision this teaches receive-pack to decide if it should call unpack-objects or index-pack --stdin --fix-thin based on the setting of receive.unpackLimit and the number of objects contained in the received pack. If the number of objects (hdr_entries) in the received pack is below the value of receive.unpackLimit (which is 5000 by default) then we unpack-objects as we have in the past. If the hdr_entries >= receive.unpackLimit then we call index-pack and ask it to include our pid and hostname in the .keep file to make it easier to identify why a given pack has been kept in the repository. Currently this leaves every received pack as a kept pack. We really don't want that as received packs will tend to be small. Instead we want to delete the .keep file automatically after all refs have been updated. That is being left as room for future improvement. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- Documentation/config.txt | 11 +++++- cache.h | 1 + receive-pack.c | 98 +++++++++++++++++++++++++++++++++++++--------- sha1_file.c | 2 +- 4 files changed, 91 insertions(+), 21 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index d9e73da..9d3c71c 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -301,7 +301,16 @@ imap:: The configuration variables in the 'imap' section are described in gitlink:git-imap-send[1]. -receive.denyNonFastforwads:: +receive.unpackLimit:: + If the number of objects received in a push is below this + limit then the objects will be unpacked into loose object + files. However if the number of received objects equals or + exceeds this limit then the received pack will be stored as + a pack, after adding any missing delta bases. Storing the + pack from a push can make the push operation complete faster, + especially on slow filesystems. + +receive.denyNonFastForwards:: If set to true, git-receive-pack will deny a ref update which is not a fast forward. Use this to prevent such an update via a push, even if that push is forced. This configuration variable is diff --git a/cache.h b/cache.h index e997a85..6cb7e1d 100644 --- a/cache.h +++ b/cache.h @@ -376,6 +376,7 @@ extern struct packed_git *parse_pack_ind char *idx_path); extern void prepare_packed_git(void); +extern void reprepare_packed_git(void); extern void install_packed_git(struct packed_git *pack); extern struct packed_git *find_sha1_pack(const unsigned char *sha1, diff --git a/receive-pack.c b/receive-pack.c index e2ba8a8..be4c3fb 100644 --- a/receive-pack.c +++ b/receive-pack.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "pack.h" #include "refs.h" #include "pkt-line.h" #include "run-command.h" @@ -7,9 +8,8 @@ static const char receive_pack_usage[] = "git-receive-pack <git-dir>"; -static const char *unpacker[] = { "unpack-objects", NULL }; - static int deny_non_fast_forwards = 0; +static int unpack_limit = 5000; static int report_status; static char capabilities[] = "report-status"; @@ -25,6 +25,12 @@ static int receive_pack_config(const cha return 0; } + if (strcmp(var, "receive.unpacklimit") == 0) + { + unpack_limit = git_config_int(var, value); + return 0; + } + return 0; } @@ -227,27 +233,81 @@ static void read_head_info(void) } } +static const char *parse_pack_header(struct pack_header *hdr) +{ + char *c = (char*)hdr; + ssize_t remaining = sizeof(struct pack_header); + do { + ssize_t r = xread(0, c, remaining); + if (r <= 0) + return "eof before pack header was fully read"; + remaining -= r; + c += r; + } while (remaining > 0); + if (hdr->hdr_signature != htonl(PACK_SIGNATURE)) + return "protocol error (pack signature mismatch detected)"; + if (!pack_version_ok(hdr->hdr_version)) + return "protocol error (pack version unsupported)"; + return NULL; +} + static const char *unpack(void) { - int code = run_command_v_opt(1, unpacker, RUN_GIT_CMD); + struct pack_header hdr; + const char *hdr_err; + char hdr_arg[38]; + int code; + + hdr_err = parse_pack_header(&hdr); + if (hdr_err) + return hdr_err; + snprintf(hdr_arg, sizeof(hdr_arg), "--pack_header=%u,%u", + ntohl(hdr.hdr_version), ntohl(hdr.hdr_entries)); + + if (ntohl(hdr.hdr_entries) < unpack_limit) { + const char *unpacker[3]; + unpacker[0] = "unpack-objects"; + unpacker[1] = hdr_arg; + unpacker[2] = NULL; + code = run_command_v_opt(1, unpacker, RUN_GIT_CMD); + } else { + const char *keeper[6]; + char my_host[255], keep_arg[128 + 255]; + + if (gethostname(my_host, sizeof(my_host))) + strcpy(my_host, "localhost"); + snprintf(keep_arg, sizeof(keep_arg), + "--keep=receive-pack %i on %s", + getpid(), my_host); + + keeper[0] = "index-pack"; + keeper[1] = "--stdin"; + keeper[2] = "--fix-thin"; + keeper[3] = hdr_arg; + keeper[4] = keep_arg; + keeper[5] = NULL; + code = run_command_v_opt(1, keeper, RUN_GIT_CMD); + if (!code) + reprepare_packed_git(); + } switch (code) { - case 0: - return NULL; - case -ERR_RUN_COMMAND_FORK: - return "unpack fork failed"; - case -ERR_RUN_COMMAND_EXEC: - return "unpack execute failed"; - case -ERR_RUN_COMMAND_WAITPID: - return "waitpid failed"; - case -ERR_RUN_COMMAND_WAITPID_WRONG_PID: - return "waitpid is confused"; - case -ERR_RUN_COMMAND_WAITPID_SIGNAL: - return "unpacker died of signal"; - case -ERR_RUN_COMMAND_WAITPID_NOEXIT: - return "unpacker died strangely"; - default: - return "unpacker exited with error code"; + case 0: + return NULL; + case -ERR_RUN_COMMAND_FORK: + return "unpack fork failed"; + case -ERR_RUN_COMMAND_EXEC: + return "unpack execute failed"; + case -ERR_RUN_COMMAND_WAITPID: + return "waitpid failed"; + case -ERR_RUN_COMMAND_WAITPID_WRONG_PID: + return "waitpid is confused"; + case -ERR_RUN_COMMAND_WAITPID_SIGNAL: + return "unpacker died of signal"; + case -ERR_RUN_COMMAND_WAITPID_NOEXIT: + return "unpacker died strangely"; + default: + return "unpacker exited with error code"; } } diff --git a/sha1_file.c b/sha1_file.c index 5e6c8b8..7bda2d4 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -663,7 +663,7 @@ void prepare_packed_git(void) prepare_packed_git_run_once = 1; } -static void reprepare_packed_git(void) +void reprepare_packed_git(void) { prepare_packed_git_run_once = 0; prepare_packed_git(); -- 1.4.3.3.g87b2-dirty ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/6] git-fetch can use both --thin and --keep with fetch-pack now 2006-11-01 22:06 ` [PATCH 2/6] Teach receive-pack how to keep pack files based on object count Nicolas Pitre @ 2006-11-01 22:06 ` Nicolas Pitre 2006-11-01 22:06 ` [PATCH 4/6] improve fetch-pack's handling of kept packs Nicolas Pitre 0 siblings, 1 reply; 13+ messages in thread From: Nicolas Pitre @ 2006-11-01 22:06 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Nicolas Pitre Signed-off-by: Nicolas Pitre <nico@cam.org> --- git-fetch.sh | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/git-fetch.sh b/git-fetch.sh index 539dff6..2b5538f 100755 --- a/git-fetch.sh +++ b/git-fetch.sh @@ -20,7 +20,7 @@ verbose= update_head_ok= exec= upload_pack= -keep=--thin +keep= while case "$#" in 0) break ;; esac do case "$1" in @@ -370,7 +370,7 @@ fetch_main () { ( : subshell because we muck with IFS IFS=" $LF" ( - git-fetch-pack $exec $keep "$remote" $rref || echo failed "$remote" + git-fetch-pack --thin $exec $keep "$remote" $rref || echo failed "$remote" ) | while read sha1 remote_name do -- 1.4.3.3.g87b2-dirty ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/6] improve fetch-pack's handling of kept packs 2006-11-01 22:06 ` [PATCH 3/6] git-fetch can use both --thin and --keep with fetch-pack now Nicolas Pitre @ 2006-11-01 22:06 ` Nicolas Pitre 2006-11-01 22:06 ` [PATCH 5/6] have index-pack create .keep file more carefully Nicolas Pitre 0 siblings, 1 reply; 13+ messages in thread From: Nicolas Pitre @ 2006-11-01 22:06 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Nicolas Pitre Since functions in fetch-clone.c were only used from fetch-pack.c, its content has been merged with fetch-pack.c. This allows for better coupling of features with much simpler implementations. One new thing is that the (abscence of) --thin also enforce it on index-pack now, such that index-pack will abort if a thin pack was _not_ asked for. The -k or --keep, when provided twice, now causes the fetched pack to be left as a kept pack just like receive-pack currently does. Eventually this will be used to close a race against concurrent repacking. Signed-off-by: Nicolas Pitre <nico@cam.org> --- Documentation/git-fetch-pack.txt | 3 +- Makefile | 2 +- cache.h | 4 -- fetch-clone.c | 87 ------------------------------ fetch-pack.c | 110 +++++++++++++++++++++++++++++++++++--- 5 files changed, 106 insertions(+), 100 deletions(-) diff --git a/Documentation/git-fetch-pack.txt b/Documentation/git-fetch-pack.txt index bff9aa6..3e6cd88 100644 --- a/Documentation/git-fetch-pack.txt +++ b/Documentation/git-fetch-pack.txt @@ -32,7 +32,8 @@ OPTIONS -k:: Do not invoke 'git-unpack-objects' on received data, but create a single packfile out of it instead, and store it - in the object database. + in the object database. If provided twice then the pack is + locked against repacking. --exec=<git-upload-pack>:: Use this to specify the path to 'git-upload-pack' on the diff --git a/Makefile b/Makefile index 28a5d91..80d4a4e 100644 --- a/Makefile +++ b/Makefile @@ -260,7 +260,7 @@ LIB_OBJS = \ quote.o read-cache.o refs.o run-command.o dir.o object-refs.o \ server-info.o setup.o sha1_file.o sha1_name.o strbuf.o \ tag.o tree.o usage.o config.o environment.o ctype.o copy.o \ - fetch-clone.o revision.o pager.o tree-walk.o xdiff-interface.o \ + revision.o pager.o tree-walk.o xdiff-interface.o \ write_or_die.o trace.o list-objects.o grep.o \ alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \ color.o wt-status.o archive-zip.o archive-tar.o diff --git a/cache.h b/cache.h index 6cb7e1d..f2ec5c8 100644 --- a/cache.h +++ b/cache.h @@ -416,10 +416,6 @@ extern int copy_fd(int ifd, int ofd); extern void write_or_die(int fd, const void *buf, size_t count); extern int write_or_whine(int fd, const void *buf, size_t count, const char *msg); -/* Finish off pack transfer receiving end */ -extern int receive_unpack_pack(int fd[2], const char *me, int quiet, int); -extern int receive_keep_pack(int fd[2], const char *me, int quiet, int); - /* pager.c */ extern void setup_pager(void); extern int pager_in_use; diff --git a/fetch-clone.c b/fetch-clone.c deleted file mode 100644 index f629d8d..0000000 --- a/fetch-clone.c +++ /dev/null @@ -1,87 +0,0 @@ -#include "cache.h" -#include "exec_cmd.h" -#include "pkt-line.h" -#include "sideband.h" -#include <sys/wait.h> - -static pid_t setup_sideband(int sideband, const char *me, int fd[2], int xd[2]) -{ - pid_t side_pid; - - if (!sideband) { - fd[0] = xd[0]; - fd[1] = xd[1]; - return 0; - } - /* xd[] is talking with upload-pack; subprocess reads from - * xd[0], spits out band#2 to stderr, and feeds us band#1 - * through our fd[0]. - */ - if (pipe(fd) < 0) - die("%s: unable to set up pipe", me); - side_pid = fork(); - if (side_pid < 0) - die("%s: unable to fork off sideband demultiplexer", me); - if (!side_pid) { - /* subprocess */ - close(fd[0]); - if (xd[0] != xd[1]) - close(xd[1]); - if (recv_sideband(me, xd[0], fd[1], 2)) - exit(1); - exit(0); - } - close(xd[0]); - close(fd[1]); - fd[1] = xd[1]; - return side_pid; -} - -static int get_pack(int xd[2], const char *me, int sideband, const char **argv) -{ - int status; - pid_t pid, side_pid; - int fd[2]; - - side_pid = setup_sideband(sideband, me, fd, xd); - pid = fork(); - if (pid < 0) - die("%s: unable to fork off %s", me, argv[0]); - if (!pid) { - dup2(fd[0], 0); - close(fd[0]); - close(fd[1]); - execv_git_cmd(argv); - die("%s exec failed", argv[0]); - } - close(fd[0]); - close(fd[1]); - while (waitpid(pid, &status, 0) < 0) { - if (errno != EINTR) - die("waiting for %s: %s", argv[0], strerror(errno)); - } - if (WIFEXITED(status)) { - int code = WEXITSTATUS(status); - if (code) - die("%s died with error code %d", argv[0], code); - return 0; - } - if (WIFSIGNALED(status)) { - int sig = WTERMSIG(status); - die("%s died of signal %d", argv[0], sig); - } - die("%s died of unnatural causes %d", argv[0], status); -} - -int receive_unpack_pack(int xd[2], const char *me, int quiet, int sideband) -{ - const char *argv[3] = { "unpack-objects", quiet ? "-q" : NULL, NULL }; - return get_pack(xd, me, sideband, argv); -} - -int receive_keep_pack(int xd[2], const char *me, int quiet, int sideband) -{ - const char *argv[5] = { "index-pack", "--stdin", "--fix-thin", - quiet ? NULL : "-v", NULL }; - return get_pack(xd, me, sideband, argv); -} diff --git a/fetch-pack.c b/fetch-pack.c index 36ea092..0a169dc 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -3,6 +3,9 @@ #include "pkt-line.h" #include "commit.h" #include "tag.h" +#include "exec_cmd.h" +#include "sideband.h" +#include <sys/wait.h> static int keep_pack; static int quiet; @@ -416,6 +419,103 @@ static int everything_local(struct ref * return retval; } +static pid_t setup_sideband(int fd[2], int xd[2]) +{ + pid_t side_pid; + + if (!use_sideband) { + fd[0] = xd[0]; + fd[1] = xd[1]; + return 0; + } + /* xd[] is talking with upload-pack; subprocess reads from + * xd[0], spits out band#2 to stderr, and feeds us band#1 + * through our fd[0]. + */ + if (pipe(fd) < 0) + die("fetch-pack: unable to set up pipe"); + side_pid = fork(); + if (side_pid < 0) + die("fetch-pack: unable to fork off sideband demultiplexer"); + if (!side_pid) { + /* subprocess */ + close(fd[0]); + if (xd[0] != xd[1]) + close(xd[1]); + if (recv_sideband("fetch-pack", xd[0], fd[1], 2)) + exit(1); + exit(0); + } + close(xd[0]); + close(fd[1]); + fd[1] = xd[1]; + return side_pid; +} + +static int get_pack(int xd[2], const char **argv) +{ + int status; + pid_t pid, side_pid; + int fd[2]; + + side_pid = setup_sideband(fd, xd); + pid = fork(); + if (pid < 0) + die("fetch-pack: unable to fork off %s", argv[0]); + if (!pid) { + dup2(fd[0], 0); + close(fd[0]); + close(fd[1]); + execv_git_cmd(argv); + die("%s exec failed", argv[0]); + } + close(fd[0]); + close(fd[1]); + while (waitpid(pid, &status, 0) < 0) { + if (errno != EINTR) + die("waiting for %s: %s", argv[0], strerror(errno)); + } + if (WIFEXITED(status)) { + int code = WEXITSTATUS(status); + if (code) + die("%s died with error code %d", argv[0], code); + return 0; + } + if (WIFSIGNALED(status)) { + int sig = WTERMSIG(status); + die("%s died of signal %d", argv[0], sig); + } + die("%s died of unnatural causes %d", argv[0], status); +} + +static int explode_rx_pack(int xd[2]) +{ + const char *argv[3] = { "unpack-objects", quiet ? "-q" : NULL, NULL }; + return get_pack(xd, argv); +} + +static int keep_rx_pack(int xd[2]) +{ + const char *argv[6]; + char keep_arg[256]; + int n = 0; + + argv[n++] = "index-pack"; + argv[n++] = "--stdin"; + if (!quiet) + argv[n++] = "-v"; + if (use_thin_pack) + argv[n++] = "--fix-thin"; + if (keep_pack > 1) { + int s = sprintf(keep_arg, "--keep=fetch-pack %i on ", getpid()); + if (gethostname(keep_arg + s, sizeof(keep_arg) - s)) + strcpy(keep_arg + s, "localhost"); + argv[n++] = keep_arg; + } + argv[n] = NULL; + return get_pack(xd, argv); +} + static int fetch_pack(int fd[2], int nr_match, char **match) { struct ref *ref; @@ -447,17 +547,13 @@ static int fetch_pack(int fd[2], int nr_ goto all_done; } if (find_common(fd, sha1, ref) < 0) - if (!keep_pack) + if (keep_pack != 1) /* When cloning, it is not unusual to have * no common commit. */ fprintf(stderr, "warning: no common commits\n"); - if (keep_pack) - status = receive_keep_pack(fd, "git-fetch-pack", quiet, use_sideband); - else - status = receive_unpack_pack(fd, "git-fetch-pack", quiet, use_sideband); - + status = (keep_pack) ? keep_rx_pack(fd) : explode_rx_pack(fd); if (status) die("git-fetch-pack: fetch failed."); @@ -494,7 +590,7 @@ int main(int argc, char **argv) continue; } if (!strcmp("--keep", arg) || !strcmp("-k", arg)) { - keep_pack = 1; + keep_pack++; continue; } if (!strcmp("--thin", arg)) { -- 1.4.3.3.g87b2-dirty ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5/6] have index-pack create .keep file more carefully 2006-11-01 22:06 ` [PATCH 4/6] improve fetch-pack's handling of kept packs Nicolas Pitre @ 2006-11-01 22:06 ` Nicolas Pitre 2006-11-01 22:06 ` [PATCH 6/6] remove .keep pack lock files when done with refs update Nicolas Pitre 0 siblings, 1 reply; 13+ messages in thread From: Nicolas Pitre @ 2006-11-01 22:06 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Nicolas Pitre If by chance we receive a pack which content (list of objects) matches another pack that we already have, and if that pack is marked with a .keep file, then we should not overwrite it. Signed-off-by: Nicolas Pitre <nico@cam.org> --- index-pack.c | 17 ++++++++++------- 1 files changed, 10 insertions(+), 7 deletions(-) diff --git a/index-pack.c b/index-pack.c index a3b55f9..8d64a88 100644 --- a/index-pack.c +++ b/index-pack.c @@ -788,14 +788,17 @@ static void final(const char *final_pack get_object_directory(), sha1_to_hex(sha1)); keep_name = name; } - keep_fd = open(keep_name, O_RDWR | O_CREAT, 0600); - if (keep_fd < 0) - die("cannot write keep file"); - if (keep_msg_len > 0) { - write_or_die(keep_fd, keep_msg, keep_msg_len); - write_or_die(keep_fd, "\n", 1); + keep_fd = open(keep_name, O_RDWR|O_CREAT|O_EXCL, 0600); + if (keep_fd < 0) { + if (errno != EEXIST) + die("cannot write keep file"); + } else { + if (keep_msg_len > 0) { + write_or_die(keep_fd, keep_msg, keep_msg_len); + write_or_die(keep_fd, "\n", 1); + } + close(keep_fd); } - close(keep_fd); } if (final_pack_name != curr_pack_name) { -- 1.4.3.3.g87b2-dirty ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 6/6] remove .keep pack lock files when done with refs update 2006-11-01 22:06 ` [PATCH 5/6] have index-pack create .keep file more carefully Nicolas Pitre @ 2006-11-01 22:06 ` Nicolas Pitre 2006-11-02 5:18 ` Shawn Pearce 2006-11-02 5:27 ` Junio C Hamano 0 siblings, 2 replies; 13+ messages in thread From: Nicolas Pitre @ 2006-11-01 22:06 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Nicolas Pitre This makes both git-fetch and git-push (fetch-pack and receive-pack) safe against a possible race with aparallel git-repack -a -d that could prune the new pack while it is not yet referenced, and remove the .keep file after refs have been updated. Signed-off-by: Nicolas Pitre <nico@cam.org> --- Documentation/git-index-pack.txt | 11 ++++ git-fetch.sh | 10 ++++- index-pack.c | 38 +++++++++------ receive-pack.c | 96 +++++++++++++++++++++++++++++--------- 4 files changed, 116 insertions(+), 39 deletions(-) diff --git a/Documentation/git-index-pack.txt b/Documentation/git-index-pack.txt index 1235416..2229ee8 100644 --- a/Documentation/git-index-pack.txt +++ b/Documentation/git-index-pack.txt @@ -69,6 +69,17 @@ OPTIONS locate any which have outlived their usefulness. +Note +---- + +Once the index has been created, the list of object names is sorted +and the SHA1 hash of that list is printed to stdout. If --stdin was +also used then this is prefixed by either "pack\t", or "keep\t" if a +new .keep file was successfully created. This is useful to remove a +.keep file used as a lock to prevent the race with gitlink:git-repack[1] +mentioned above. + + Author ------ Written by Sergey Vlasov <vsu@altlinux.ru> diff --git a/git-fetch.sh b/git-fetch.sh index 2b5538f..e8ce296 100755 --- a/git-fetch.sh +++ b/git-fetch.sh @@ -51,7 +51,7 @@ do verbose=Yes ;; -k|--k|--ke|--kee|--keep) - keep=--keep + keep=-k -k ;; --reflog-action=*) rloga=`expr "z$1" : 'z-[^=]*=\(.*\)'` @@ -368,6 +368,7 @@ fetch_main () { ;; # we are already done. *) ( : subshell because we muck with IFS + pack_lockfile= IFS=" $LF" ( git-fetch-pack --thin $exec $keep "$remote" $rref || echo failed "$remote" @@ -378,6 +379,12 @@ fetch_main () { failed) echo >&2 "Fetch failure: $remote" exit 1 ;; + # special line coming from index-pack with the pack name + pack) + continue ;; + keep) + pack_lockfile="$GIT_OBJECT_DIRECTORY/pack/pack-$remote_name.keep" + continue ;; esac found= single_force= @@ -408,6 +415,7 @@ fetch_main () { append_fetch_head "$sha1" "$remote" \ "$remote_name" "$remote_nick" "$local_name" "$not_for_merge" done + if [ "$pack_lockfile" ]; then rm -f "$pack_lockfile"; fi ) || exit ;; esac diff --git a/index-pack.c b/index-pack.c index 8d64a88..042aea8 100644 --- a/index-pack.c +++ b/index-pack.c @@ -757,6 +757,7 @@ static void final(const char *final_pack const char *keep_name, const char *keep_msg, unsigned char *sha1) { + char *report = "pack"; char name[PATH_MAX]; int err; @@ -767,18 +768,6 @@ static void final(const char *final_pack if (err) die("error while closing pack file: %s", strerror(errno)); chmod(curr_pack_name, 0444); - - /* - * Let's just mimic git-unpack-objects here and write - * the last part of the buffer to stdout. - */ - while (input_len) { - err = xwrite(1, input_buffer + input_offset, input_len); - if (err <= 0) - break; - input_len -= err; - input_offset += err; - } } if (keep_msg) { @@ -798,6 +787,7 @@ static void final(const char *final_pack write_or_die(keep_fd, "\n", 1); } close(keep_fd); + report = "keep"; } } @@ -821,6 +811,27 @@ static void final(const char *final_pack if (move_temp_to_file(curr_index_name, final_index_name)) die("cannot store index file"); } + + if (!from_stdin) { + printf("%s\n", sha1_to_hex(sha1)); + } else { + char buf[48]; + int len = snprintf(buf, sizeof(buf), "%s\t%s\n", + report, sha1_to_hex(sha1)); + xwrite(1, buf, len); + + /* + * Let's just mimic git-unpack-objects here and write + * the last part of the input buffer to stdout. + */ + while (input_len) { + err = xwrite(1, input_buffer + input_offset, input_len); + if (err <= 0) + break; + input_len -= err; + input_offset += err; + } + } } int main(int argc, char **argv) @@ -937,8 +948,5 @@ int main(int argc, char **argv) free(index_name_buf); free(keep_name_buf); - if (!from_stdin) - printf("%s\n", sha1_to_hex(sha1)); - return 0; } diff --git a/receive-pack.c b/receive-pack.c index be4c3fb..8bb5683 100644 --- a/receive-pack.c +++ b/receive-pack.c @@ -3,8 +3,10 @@ #include "refs.h" #include "pkt-line.h" #include "run-command.h" +#include "exec_cmd.h" #include "commit.h" #include "object.h" +#include <sys/wait.h> static const char receive_pack_usage[] = "git-receive-pack <git-dir>"; @@ -251,12 +253,13 @@ static const char *parse_pack_header(str return NULL; } +static const char *pack_lockfile; + static const char *unpack(void) { struct pack_header hdr; const char *hdr_err; char hdr_arg[38]; - int code; hdr_err = parse_pack_header(&hdr); if (hdr_err) @@ -265,33 +268,13 @@ static const char *unpack(void) ntohl(hdr.hdr_version), ntohl(hdr.hdr_entries)); if (ntohl(hdr.hdr_entries) < unpack_limit) { + int code; const char *unpacker[3]; unpacker[0] = "unpack-objects"; unpacker[1] = hdr_arg; unpacker[2] = NULL; code = run_command_v_opt(1, unpacker, RUN_GIT_CMD); - } else { - const char *keeper[6]; - char my_host[255], keep_arg[128 + 255]; - - if (gethostname(my_host, sizeof(my_host))) - strcpy(my_host, "localhost"); - snprintf(keep_arg, sizeof(keep_arg), - "--keep=receive-pack %i on %s", - getpid(), my_host); - - keeper[0] = "index-pack"; - keeper[1] = "--stdin"; - keeper[2] = "--fix-thin"; - keeper[3] = hdr_arg; - keeper[4] = keep_arg; - keeper[5] = NULL; - code = run_command_v_opt(1, keeper, RUN_GIT_CMD); - if (!code) - reprepare_packed_git(); - } - - switch (code) { + switch (code) { case 0: return NULL; case -ERR_RUN_COMMAND_FORK: @@ -308,6 +291,71 @@ static const char *unpack(void) return "unpacker died strangely"; default: return "unpacker exited with error code"; + } + } else { + const char *keeper[6]; + int fd[2], s, len, status; + pid_t pid; + char keep_arg[256]; + char packname[46]; + + s = sprintf(keep_arg, "--keep=receive-pack %i on ", getpid()); + if (gethostname(keep_arg + s, sizeof(keep_arg) - s)) + strcpy(keep_arg + s, "localhost"); + + keeper[0] = "index-pack"; + keeper[1] = "--stdin"; + keeper[2] = "--fix-thin"; + keeper[3] = hdr_arg; + keeper[4] = keep_arg; + keeper[5] = NULL; + + if (pipe(fd) < 0) + return "index-pack pipe failed"; + pid = fork(); + if (pid < 0) + return "index-pack fork failed"; + if (!pid) { + dup2(fd[1], 1); + close(fd[1]); + close(fd[0]); + execv_git_cmd(keeper); + die("execv of index-pack failed"); + } + close(fd[1]); + + /* + * The first thing we expects from index-pack's output + * is "pack\t%40s\n" or "keep\t%40s\n" (46 bytes) where + * %40s is the newly created pack SHA1 name. In the "keep" + * case, we need it to remove the corresponding .keep file + * later on. If we don't get that then tough luck with it. + */ + for (len = 0; + len < 46 && (s = xread(fd[0], packname+len, 46-len)) > 0; + len += s); + close(fd[0]); + if (len == 46 && packname[45] == '\n' && + memcmp(packname, "keep\t", 5) == 0) { + char path[PATH_MAX]; + packname[45] = 0; + snprintf(path, sizeof(path), "%s/pack/pack-%s.keep", + get_object_directory(), packname + 5); + pack_lockfile = xstrdup(path); + } + + /* Then wrap our index-pack process. */ + while (waitpid(pid, &status, 0) < 0) + if (errno != EINTR) + return "waitpid failed"; + if (WIFEXITED(status)) { + int code = WEXITSTATUS(status); + if (code) + return "index-pack exited with error code"; + reprepare_packed_git(); + return NULL; + } + return "index-pack abnormal exit"; } } @@ -363,6 +411,8 @@ int main(int argc, char **argv) const char *unpack_status = unpack(); if (!unpack_status) execute_commands(); + if (pack_lockfile) + unlink(pack_lockfile); if (report_status) report(unpack_status); } -- 1.4.3.3.g87b2-dirty ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 6/6] remove .keep pack lock files when done with refs update 2006-11-01 22:06 ` [PATCH 6/6] remove .keep pack lock files when done with refs update Nicolas Pitre @ 2006-11-02 5:18 ` Shawn Pearce 2006-11-02 5:27 ` Junio C Hamano 1 sibling, 0 replies; 13+ messages in thread From: Shawn Pearce @ 2006-11-02 5:18 UTC (permalink / raw) To: Nicolas Pitre; +Cc: Junio C Hamano, git Nicolas Pitre <nico@cam.org> wrote: > This makes both git-fetch and git-push (fetch-pack and receive-pack) > safe against a possible race with aparallel git-repack -a -d that could > prune the new pack while it is not yet referenced, and remove the .keep > file after refs have been updated. Thanks for finishing this series out. I've gotten sidetracked the past two days and wasn't able to finish it myself. Nicely done too, your code looks much better than my drafts... :-) -- ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 6/6] remove .keep pack lock files when done with refs update 2006-11-01 22:06 ` [PATCH 6/6] remove .keep pack lock files when done with refs update Nicolas Pitre 2006-11-02 5:18 ` Shawn Pearce @ 2006-11-02 5:27 ` Junio C Hamano 2006-11-02 15:09 ` Nicolas Pitre 1 sibling, 1 reply; 13+ messages in thread From: Junio C Hamano @ 2006-11-02 5:27 UTC (permalink / raw) To: Nicolas Pitre; +Cc: git Nicolas Pitre <nico@cam.org> writes: > --- a/git-fetch.sh > +++ b/git-fetch.sh > @@ -51,7 +51,7 @@ do > verbose=Yes > ;; > -k|--k|--ke|--kee|--keep) > - keep=--keep > + keep=-k -k > ;; > --reflog-action=*) > rloga=`expr "z$1" : 'z-[^=]*=\(.*\)'` Micronit. This cannot be right without quoting. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 6/6] remove .keep pack lock files when done with refs update 2006-11-02 5:27 ` Junio C Hamano @ 2006-11-02 15:09 ` Nicolas Pitre 2006-11-02 17:25 ` Nicolas Pitre 0 siblings, 1 reply; 13+ messages in thread From: Nicolas Pitre @ 2006-11-02 15:09 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Wed, 1 Nov 2006, Junio C Hamano wrote: > Nicolas Pitre <nico@cam.org> writes: > > > --- a/git-fetch.sh > > +++ b/git-fetch.sh > > @@ -51,7 +51,7 @@ do > > verbose=Yes > > ;; > > -k|--k|--ke|--kee|--keep) > > - keep=--keep > > + keep=-k -k > > ;; > > --reflog-action=*) > > rloga=`expr "z$1" : 'z-[^=]*=\(.*\)'` > > Micronit. This cannot be right without quoting. OK. Shell programming isn't really my turf. Another area of that patch I'm not really sure of is this: @@ -408,6 +415,7 @@ fetch_main () { append_fetch_head "$sha1" "$remote" \ "$remote_name" "$remote_nick" "$local_name" "$not_for_merge" done + if [ "$pack_lockfile" ]; then rm -f "$pack_lockfile"; fi ) || exit ;; esac How can I make sure to not clobber a non-zero return code with the rm that would prevent the exit while still performing the rm in all cases? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 6/6] remove .keep pack lock files when done with refs update 2006-11-02 15:09 ` Nicolas Pitre @ 2006-11-02 17:25 ` Nicolas Pitre 2006-11-03 9:35 ` Junio C Hamano 0 siblings, 1 reply; 13+ messages in thread From: Nicolas Pitre @ 2006-11-02 17:25 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Thu, 2 Nov 2006, Nicolas Pitre wrote: > Another area of that patch I'm not really sure of is this: > > @@ -408,6 +415,7 @@ fetch_main () { > append_fetch_head "$sha1" "$remote" \ > "$remote_name" "$remote_nick" "$local_name" "$not_for_merge" > done > + if [ "$pack_lockfile" ]; then rm -f "$pack_lockfile"; fi > ) || exit ;; > esac > > How can I make sure to not clobber a non-zero return code with the rm > that would prevent the exit while still performing the rm in all cases? OK looking at it again I think the || exit is only meant to match the exit 1 before that point. Since this is a subshell then the exit 1 wouldn't exit it all on its own. Otherwise there is no errors expected to trickle out of the while loop and I _think_ the above is correct. Do you still want a new patch for the micronit quoting issue? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 6/6] remove .keep pack lock files when done with refs update 2006-11-02 17:25 ` Nicolas Pitre @ 2006-11-03 9:35 ` Junio C Hamano 0 siblings, 0 replies; 13+ messages in thread From: Junio C Hamano @ 2006-11-03 9:35 UTC (permalink / raw) To: Nicolas Pitre; +Cc: git Nicolas Pitre <nico@cam.org> writes: > On Thu, 2 Nov 2006, Nicolas Pitre wrote: > >> Another area of that patch I'm not really sure of is this: >> >> @@ -408,6 +415,7 @@ fetch_main () { >> append_fetch_head "$sha1" "$remote" \ >> "$remote_name" "$remote_nick" "$local_name" "$not_for_merge" >> done >> + if [ "$pack_lockfile" ]; then rm -f "$pack_lockfile"; fi >> ) || exit ;; >> esac >> >> How can I make sure to not clobber a non-zero return code with the rm >> that would prevent the exit while still performing the rm in all cases? > > OK looking at it again I think the || exit is only meant to match the > exit 1 before that point. Since this is a subshell then the exit 1 > wouldn't exit it all on its own. Otherwise there is no errors expected > to trickle out of the while loop and I _think_ the above is correct. I think so too. > Do you still want a new patch for the micronit quoting issue? Not needed, will park them in "pu" and play with them -- hopefully in "next" by the end of the week. Thanks. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/6] Allow pack header preprocessing before unpack-objects/index-pack. 2006-11-01 22:06 [PATCH 1/6] Allow pack header preprocessing before unpack-objects/index-pack Nicolas Pitre 2006-11-01 22:06 ` [PATCH 2/6] Teach receive-pack how to keep pack files based on object count Nicolas Pitre @ 2006-11-01 23:50 ` Junio C Hamano 2006-11-02 3:58 ` Nicolas Pitre 1 sibling, 1 reply; 13+ messages in thread From: Junio C Hamano @ 2006-11-01 23:50 UTC (permalink / raw) To: Nicolas Pitre; +Cc: git Rather nicely done. I see upto 2/6 are identical to what has sitting in "pu" for the past few days, modulo some indentation? I wonder how much testing has this series seen, by the way. Also, I wonder what happens after 5/6 errors out. It dies and the caller (typically receive-pack) reports it back, which would fail the push (and does not update the refs). Retrying in such a case would probably use the same set of refs on both ends, resulting in exactly the same pack... ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/6] Allow pack header preprocessing before unpack-objects/index-pack. 2006-11-01 23:50 ` [PATCH 1/6] Allow pack header preprocessing before unpack-objects/index-pack Junio C Hamano @ 2006-11-02 3:58 ` Nicolas Pitre 0 siblings, 0 replies; 13+ messages in thread From: Nicolas Pitre @ 2006-11-02 3:58 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Wed, 1 Nov 2006, Junio C Hamano wrote: > Rather nicely done. I see upto 2/6 are identical to what has > sitting in "pu" for the past few days, modulo some indentation? Yes. I just inserted them in the serie as I didn't see them in pu until a very recent pull. > I wonder how much testing has this series seen, by the way. It has been tested a bit. At least until it started behaving correctly for me. There might still be bugs remaining, but nothing that should screw a repository up. I think it should be good enough for next. > Also, I wonder what happens after 5/6 errors out. It dies and > the caller (typically receive-pack) reports it back, which would > fail the push (and does not update the refs). Retrying in such > a case would probably use the same set of refs on both ends, > resulting in exactly the same pack... Well, the purpose of 5/6 is to avoid creating a .keep file if it already exists. This is more a security thing so if you decide to mark one of your pack with .keep (say an history pack that doesn't need to be repacked) then you don't want a funny git server to send you the same pack just for the purpose of overwriting the same .keep file (that could be removed right away if it ends up being considered as a lock file). This is made more obvious with 6/6 as the .keep file is not removed if it was there before. So this is not actually an error if creating a .keep file fails with EEXIST. As for index-pack dying, it will be reported the same way as unpack-objects dying -- no difference there. Of course if it dies on the first push, it is likely to die on all subsequent pushes since the condition for that to happen is most probably due to a serious problem (out of disk space, corruption in the existing objects in the repository, etc). The pack is put in place with move_temp_to_file() so if a previous attempt already stored a pack with the same name before successfully updating corresponding refs then the old pack will silently be kept since we know both packs contain the same objects when they have the same name. When a pack is moved it is always complete so there shouldn't be any (new) instance of repository corruption in normal circumbstances even in the presence of index-pack dying. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2006-11-03 9:35 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-11-01 22:06 [PATCH 1/6] Allow pack header preprocessing before unpack-objects/index-pack Nicolas Pitre 2006-11-01 22:06 ` [PATCH 2/6] Teach receive-pack how to keep pack files based on object count Nicolas Pitre 2006-11-01 22:06 ` [PATCH 3/6] git-fetch can use both --thin and --keep with fetch-pack now Nicolas Pitre 2006-11-01 22:06 ` [PATCH 4/6] improve fetch-pack's handling of kept packs Nicolas Pitre 2006-11-01 22:06 ` [PATCH 5/6] have index-pack create .keep file more carefully Nicolas Pitre 2006-11-01 22:06 ` [PATCH 6/6] remove .keep pack lock files when done with refs update Nicolas Pitre 2006-11-02 5:18 ` Shawn Pearce 2006-11-02 5:27 ` Junio C Hamano 2006-11-02 15:09 ` Nicolas Pitre 2006-11-02 17:25 ` Nicolas Pitre 2006-11-03 9:35 ` Junio C Hamano 2006-11-01 23:50 ` [PATCH 1/6] Allow pack header preprocessing before unpack-objects/index-pack Junio C Hamano 2006-11-02 3:58 ` Nicolas Pitre
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).