From 6e180cdcecbb3e828aa892925d7ef67abf81ad80 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 24 Feb 2009 23:11:29 -0800 Subject: Make sure objects/pack exists before creating a new pack In a repository created with git older than f49fb35 (git-init-db: create "pack" subdirectory under objects, 2005-06-27), objects/pack/ directory is not created upon initialization. It was Ok because subdirectories are created as needed inside directories init-db creates, and back then, packfiles were recent invention. After the said commit, new codepaths started relying on the presense of objects/pack/ directory in the repository. This was exacerbated with 8b4eb6b (Do not perform cross-directory renames when creating packs, 2008-09-22) that moved the location temporary pack files are created from objects/ directory to objects/pack/ directory, because moving temporary to the final location was done carefully with lazy leading directory creation. Many packfile related operations in such an old repository can fail mysteriously because of this. This commit introduces two helper functions to make things work better. - odb_mkstemp() is a specialized version of mkstemp() to refactor the code and teach it to create leading directories as needed; - odb_pack_keep() refactors the code to create a ".keep" file while create leading directories as needed. Signed-off-by: Junio C Hamano --- builtin-pack-objects.c | 5 ++--- fast-import.c | 14 +++++--------- git-compat-util.h | 2 ++ index-pack.c | 23 ++++++++++++----------- pack-write.c | 6 ++---- t/t5300-pack-object.sh | 17 +++++++++++++++++ wrapper.c | 32 ++++++++++++++++++++++++++++++++ 7 files changed, 72 insertions(+), 27 deletions(-) diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index fb5e14d56e..7518d536c3 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -473,9 +473,8 @@ static void write_pack_file(void) } else { char tmpname[PATH_MAX]; int fd; - snprintf(tmpname, sizeof(tmpname), - "%s/pack/tmp_pack_XXXXXX", get_object_directory()); - fd = xmkstemp(tmpname); + fd = odb_mkstemp(tmpname, sizeof(tmpname), + "pack/tmp_pack_XXXXXX"); pack_tmp_name = xstrdup(tmpname); f = sha1fd(fd, pack_tmp_name); } diff --git a/fast-import.c b/fast-import.c index d58cca58e5..f0114948ec 100644 --- a/fast-import.c +++ b/fast-import.c @@ -815,9 +815,8 @@ static void start_packfile(void) struct pack_header hdr; int pack_fd; - snprintf(tmpfile, sizeof(tmpfile), - "%s/pack/tmp_pack_XXXXXX", get_object_directory()); - pack_fd = xmkstemp(tmpfile); + pack_fd = odb_mkstemp(tmpfile, sizeof(tmpfile), + "pack/tmp_pack_XXXXXX"); p = xcalloc(1, sizeof(*p) + strlen(tmpfile) + 2); strcpy(p->pack_name, tmpfile); p->pack_fd = pack_fd; @@ -877,9 +876,8 @@ static char *create_index(void) c = next; } - snprintf(tmpfile, sizeof(tmpfile), - "%s/pack/tmp_idx_XXXXXX", get_object_directory()); - idx_fd = xmkstemp(tmpfile); + idx_fd = odb_mkstemp(tmpfile, sizeof(tmpfile), + "pack/tmp_idx_XXXXXX"); f = sha1fd(idx_fd, tmpfile); sha1write(f, array, 256 * sizeof(int)); SHA1_Init(&ctx); @@ -905,9 +903,7 @@ static char *keep_pack(char *curr_index_name) chmod(pack_data->pack_name, 0444); chmod(curr_index_name, 0444); - snprintf(name, sizeof(name), "%s/pack/pack-%s.keep", - get_object_directory(), sha1_to_hex(pack_data->sha1)); - keep_fd = open(name, O_RDWR|O_CREAT|O_EXCL, 0600); + keep_fd = odb_pack_keep(name, sizeof(name), pack_data->sha1); if (keep_fd < 0) die("cannot create keep file"); write_or_die(keep_fd, keep_msg, strlen(keep_msg)); diff --git a/git-compat-util.h b/git-compat-util.h index cf89cdf459..758a880862 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -294,6 +294,8 @@ extern ssize_t xwrite(int fd, const void *buf, size_t len); extern int xdup(int fd); extern FILE *xfdopen(int fd, const char *mode); extern int xmkstemp(char *template); +extern int odb_mkstemp(char *template, size_t limit, const char *pattern); +extern int odb_pack_keep(char *name, size_t namesz, unsigned char *sha1); static inline size_t xsize_t(off_t len) { diff --git a/index-pack.c b/index-pack.c index c99a1a152c..745ac0bfa0 100644 --- a/index-pack.c +++ b/index-pack.c @@ -171,9 +171,8 @@ static char *open_pack_file(char *pack_name) input_fd = 0; if (!pack_name) { static char tmpfile[PATH_MAX]; - snprintf(tmpfile, sizeof(tmpfile), - "%s/pack/tmp_pack_XXXXXX", get_object_directory()); - output_fd = xmkstemp(tmpfile); + output_fd = odb_mkstemp(tmpfile, sizeof(tmpfile), + "pack/tmp_pack_XXXXXX"); pack_name = xstrdup(tmpfile); } else output_fd = open(pack_name, O_CREAT|O_EXCL|O_RDWR, 0600); @@ -795,22 +794,24 @@ static void final(const char *final_pack_name, const char *curr_pack_name, if (keep_msg) { int keep_fd, keep_msg_len = strlen(keep_msg); - if (!keep_name) { - snprintf(name, sizeof(name), "%s/pack/pack-%s.keep", - get_object_directory(), sha1_to_hex(sha1)); - keep_name = name; - } - keep_fd = open(keep_name, O_RDWR|O_CREAT|O_EXCL, 0600); + + if (!keep_name) + keep_fd = odb_pack_keep(name, sizeof(name), sha1); + else + keep_fd = open(keep_name, O_RDWR|O_CREAT|O_EXCL, 0600); + if (keep_fd < 0) { if (errno != EEXIST) - die("cannot write keep file"); + die("cannot write keep file '%s' (%s)", + keep_name, strerror(errno)); } else { if (keep_msg_len > 0) { write_or_die(keep_fd, keep_msg, keep_msg_len); write_or_die(keep_fd, "\n", 1); } if (close(keep_fd) != 0) - die("cannot write keep file"); + die("cannot close written keep file '%s' (%s)", + keep_name, strerror(errno)); report = "keep"; } } diff --git a/pack-write.c b/pack-write.c index 3621f1dd32..e82c025457 100644 --- a/pack-write.c +++ b/pack-write.c @@ -44,9 +44,7 @@ char *write_idx_file(char *index_name, struct pack_idx_entry **objects, if (!index_name) { static char tmpfile[PATH_MAX]; - snprintf(tmpfile, sizeof(tmpfile), - "%s/pack/tmp_idx_XXXXXX", get_object_directory()); - fd = xmkstemp(tmpfile); + fd = odb_mkstemp(tmpfile, sizeof(tmpfile), "pack/tmp_idx_XXXXXX"); index_name = xstrdup(tmpfile); } else { unlink(index_name); @@ -239,7 +237,7 @@ char *index_pack_lockfile(int ip_out) char packname[46]; /* - * The first thing we expects from index-pack's output + * The first thing we expect 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 diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh index 2852a03265..73d871c713 100755 --- a/t/t5300-pack-object.sh +++ b/t/t5300-pack-object.sh @@ -180,6 +180,23 @@ test_expect_success \ unset GIT_OBJECT_DIRECTORY +test_expect_success 'survive missing objects/pack directory' ' + ( + rm -fr missing-pack && + mkdir missing-pack && + cd missing-pack && + git init && + GOP=.git/objects/pack + rm -fr $GOP && + git index-pack --stdin --keep=test <../test-3-${packname_3}.pack && + test -f $GOP/pack-${packname_3}.pack && + test_cmp $GOP/pack-${packname_3}.pack ../test-3-${packname_3}.pack && + test -f $GOP/pack-${packname_3}.idx && + test_cmp $GOP/pack-${packname_3}.idx ../test-3-${packname_3}.idx && + test -f $GOP/pack-${packname_3}.keep + ) +' + test_expect_success \ 'verify pack' \ 'git verify-pack test-1-${packname_1}.idx \ diff --git a/wrapper.c b/wrapper.c index 93562f03ee..231a58f1a9 100644 --- a/wrapper.c +++ b/wrapper.c @@ -196,3 +196,35 @@ int xmkstemp(char *template) die("Unable to create temporary file: %s", strerror(errno)); return fd; } + +int odb_mkstemp(char *template, size_t limit, const char *pattern) +{ + int fd; + + snprintf(template, limit, "%s/%s", + get_object_directory(), pattern); + fd = mkstemp(template); + if (0 <= fd) + return fd; + + /* slow path */ + safe_create_leading_directories(template); + snprintf(template, limit, "%s/%s", + get_object_directory(), pattern); + return xmkstemp(template); +} + +int odb_pack_keep(char *name, size_t namesz, unsigned char *sha1) +{ + int fd; + + snprintf(name, namesz, "%s/pack/pack-%s.keep", + get_object_directory(), sha1_to_hex(sha1)); + fd = open(name, O_RDWR|O_CREAT|O_EXCL, 0600); + if (0 <= fd) + return fd; + + /* slow path */ + safe_create_leading_directories(name); + return open(name, O_RDWR|O_CREAT|O_EXCL, 0600); +} -- cgit v1.2.1 From 2c626e5fa8a46f647b88fb32d7b28d573e8631bf Mon Sep 17 00:00:00 2001 From: Mike Ralphson Date: Thu, 26 Feb 2009 16:31:52 +0100 Subject: Fix odb_mkstemp() on AIX The AIX mkstemp() modifies its template parameter to an empty string if the call fails. The existing code had already recomputed the template, but too late to be good. See also 6ff6af62, which fixed this problem in a different spot. Signed-off-by: Mike Ralphson Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- wrapper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/wrapper.c b/wrapper.c index 231a58f1a9..5e9de294d5 100644 --- a/wrapper.c +++ b/wrapper.c @@ -208,9 +208,10 @@ int odb_mkstemp(char *template, size_t limit, const char *pattern) return fd; /* slow path */ - safe_create_leading_directories(template); + /* some mkstemp implementations erase template on failure */ snprintf(template, limit, "%s/%s", get_object_directory(), pattern); + safe_create_leading_directories(template); return xmkstemp(template); } -- cgit v1.2.1