{"thread":{"id":"17992","subject":"git-push error: Cannot write keep file","startedAt":"2009-02-24T17:04:55Z","lastAt":"2009-02-26T17:07:33Z","messageCount":10,"participants":["Rafael Darder Calvo","Junio C Hamano","Johannes Sixt","Mike Ralphson"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"106079","messageId":"49A428B7.4000003@spiralti.com","threadId":"17992","inReplyTo":null,"subject":"git-push error: Cannot write keep file","fromName":"Rafael Darder Calvo","fromEmail":"rdarder@spiralti.com","sentAt":"2009-02-24T17:04:55Z","receivedAt":"2009-02-24T17:04:55Z","isPatch":false,"sender":{"key":"rdarder@spiralti.com","avatar":null},"body":"Hello,\n\tI am having the following error when I try to git push.\n\n\nrdarder@shiny:~/Sources/promotoras$ git push origin ranto:ranto\nCounting objects: 256, done.\nCompressing objects: 100% (199/199), done.\nWriting objects: 100% (213/213), 216.94 KiB, done.\nTotal 213 (delta 60), reused 1 (delta 0)\n\nfatal: cannot write keep file\n\nerror: unpack failed: index-pack abnormal exit\nTo ssh://fherrero@10.7.1.20:2222/var/www/promotoras.git\n ! [remote rejected] ranto -> ranto (n/a (unpacker error))\nerror: failed to push some refs to \n'ssh://fherrero@10.7.1.20:2222/var/www/promotoras.git'\n\n\nI couldn't find any significant description on the error \"cannot write \nkeep file\". git-fsck passes without errors in both repositories, and I \nfind no permission problems. Can anyone give me some directions on how \nto diagnose this?\n\nRegards,\n\nRafael\n\n\nbegin:vcard\nfn:Rafael Darder Calvo\nn:Darder Calvo;Rafael\norg:;Infraestructura\nadr:;;Mitre 1017 Piso 5;Rosario;Santa Fe;S2000COU;Argentina\nemail;internet:rdarder@spiralti.com\ntitle:Spiralti Rosario\ntel;work:+54 341 5302041\nurl:www.spiralti.com\nversion:2.1\nend:vcard\n\n"},{"id":"106080","messageId":"7v7i3fivx2.fsf@gitster.siamese.dyndns.org","threadId":"17992","inReplyTo":"49A428B7.4000003@spiralti.com","subject":"Re: git-push error: Cannot write keep file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-24T17:31:21Z","receivedAt":"2009-02-24T17:31:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rafael Darder Calvo writes:\n\n> Hello,\n> \tI am having the following error when I try to git push.\n>\n>\n> rdarder@shiny:~/Sources/promotoras$ git push origin ranto:ranto\n> Counting objects: 256, done.\n> Compressing objects: 100% (199/199), done.\n> Writing objects: 100% (213/213), 216.94 KiB, done.\n> Total 213 (delta 60), reused 1 (delta 0)\n>\n> fatal: cannot write keep file\n>\n> error: unpack failed: index-pack abnormal exit\n> To ssh://fherrero@10.7.1.20:2222/var/www/promotoras.git\n> ! [remote rejected] ranto -> ranto (n/a (unpacker error))\n> error: failed to push some refs to\n> 'ssh://fherrero@10.7.1.20:2222/var/www/promotoras.git'\n>\n>\n> I couldn't find any significant description on the error \"cannot write\n> keep file\". git-fsck passes without errors in both repositories, and I\n> find no permission problems. Can anyone give me some directions on how\n> to diagnose this?\n\nIf you have access to the receiving side of the repository and the machine\nthat hosts it, the debug patch attached may help.\n\nOne possibility is the receiving repository was initialized long time ago\nwith an ancient git (ealier than f49fb35 (git-init-db: create \"pack\"\nsubdirectory under objects, 2005-06-27), and never had a packfile in it\nsince then. We started creating '.git/objects/pack/' subdirectory in\ngit-init only after that commit. It was Ok for a long time because we\nlazily create \"pack\" subdirectory as needed, but a recent change 8b4eb6b\n(Do not perform cross-directory renames when creating packs, 2008-09-22)\ncarelessly assumed that .git/objects/pack/ directory would always exist\nand tries to create files in there without making sure the leading\ndirectories exist. See $gmane/110621\n\n Subject: [PATCH] Make sure objects/pack exists before creating a new pack\n To: git@vger.kernel.org\n Date: Wed, 18 Feb 2009 20:48:07 -0800\n Message-ID: <7vr61vnibc.fsf@gitster.siamese.dyndns.org>\n\nfor details.\n\nAnd the debug patch...\n\n index-pack.c | 8 ++++++--\n 1 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/index-pack.c b/index-pack.c\nindex f7a3807..acdc85f 100644\n--- a/index-pack.c\n+++ b/index-pack.c\n@@ -802,14 +802,18 @@ static void final(const char *final_pack_name, const char *curr_pack_name,\n \t\tkeep_fd = open(keep_name, O_RDWR|O_CREAT|O_EXCL, 0600);\n \t\tif (keep_fd < 0) {\n \t\t\tif (errno != EEXIST)\n-\t\t\t\tdie(\"cannot write keep file\");\n+\t\t\t\tdie(\"cannot write keep file '%s' (%s)\",\n+\t\t\t\t keep_name,\n+\t\t\t\t strerror(errno));\n \t\t} else {\n \t\t\tif (keep_msg_len > 0) {\n \t\t\t\twrite_or_die(keep_fd, keep_msg, keep_msg_len);\n \t\t\t\twrite_or_die(keep_fd, \"\\n\", 1);\n \t\t\t}\n \t\t\tif (close(keep_fd) != 0)\n-\t\t\t\tdie(\"cannot write keep file\");\n+\t\t\t\tdie(\"cannot close the written keep file '%s' (%s)\",\n+\t\t\t\t keep_name,\n+\t\t\t\t strerror(errno));\n \t\t\treport = \"keep\";\n \t\t}\n \t}\n"},{"id":"106106","messageId":"49A464E6.3080802@spiralti.com","threadId":"17992","inReplyTo":"7v7i3fivx2.fsf@gitster.siamese.dyndns.org","subject":"Re: git-push error: Cannot write keep file","fromName":"Rafael Darder Calvo","fromEmail":"rdarder@spiralti.com","sentAt":"2009-02-24T21:21:42Z","receivedAt":"2009-02-24T21:21:42Z","isPatch":false,"sender":{"key":"rdarder@spiralti.com","avatar":null},"body":"Junio C Hamano wrote:\n> Rafael Darder Calvo writes:\n> \n>> Hello,\n>> \tI am having the following error when I try to git push.\n>>\n>>\n>> rdarder@shiny:~/Sources/promotoras$ git push origin ranto:ranto\n>> Counting objects: 256, done.\n>> Compressing objects: 100% (199/199), done.\n>> Writing objects: 100% (213/213), 216.94 KiB, done.\n>> Total 213 (delta 60), reused 1 (delta 0)\n>>\n>> fatal: cannot write keep file\n>>\n>> error: unpack failed: index-pack abnormal exit\n>> To ssh://fherrero@10.7.1.20:2222/var/www/promotoras.git\n>> ! [remote rejected] ranto -> ranto (n/a (unpacker error))\n>> error: failed to push some refs to\n>> 'ssh://fherrero@10.7.1.20:2222/var/www/promotoras.git'\n>>\n>>\n>> I couldn't find any significant description on the error \"cannot write\n>> keep file\". git-fsck passes without errors in both repositories, and I\n>> find no permission problems. Can anyone give me some directions on how\n>> to diagnose this?\n> \n> If you have access to the receiving side of the repository and the machine\n> that hosts it, the debug patch attached may help.\n> \n> One possibility is the receiving repository was initialized long time ago\n> with an ancient git (ealier than f49fb35 (git-init-db: create \"pack\"\n> subdirectory under objects, 2005-06-27), and never had a packfile in it\n> since then. We started creating '.git/objects/pack/' subdirectory in\n> git-init only after that commit. It was Ok for a long time because we\n> lazily create \"pack\" subdirectory as needed, but a recent change 8b4eb6b\n> (Do not perform cross-directory renames when creating packs, 2008-09-22)\n> carelessly assumed that .git/objects/pack/ directory would always exist\n> and tries to create files in there without making sure the leading\n> directories exist. See $gmane/110621\n\nThe receiving repository was indeed lacking a .git/objects/pack/ \ndirectory, but it was created with a recent (1.5.4.3) git version. I \nsuspect the repo owner has deleted de dir by mistake or something.\n\nThank you very much for your help.\n\n> \n> Subject: [PATCH] Make sure objects/pack exists before creating a new pack\n> To: git@vger.kernel.org\n> Date: Wed, 18 Feb 2009 20:48:07 -0800\n> Message-ID: <7vr61vnibc.fsf@gitster.siamese.dyndns.org>\n> \n> for details.\n> \n> And the debug patch...\n> \n> index-pack.c | 8 ++++++--\n> 1 files changed, 6 insertions(+), 2 deletions(-)\n> \n> diff --git a/index-pack.c b/index-pack.c\n> index f7a3807..acdc85f 100644\n> --- a/index-pack.c\n> +++ b/index-pack.c\n> @@ -802,14 +802,18 @@ static void final(const char *final_pack_name, const char *curr_pack_name,\n> \t\tkeep_fd = open(keep_name, O_RDWR|O_CREAT|O_EXCL, 0600);\n> \t\tif (keep_fd < 0) {\n> \t\t\tif (errno != EEXIST)\n> -\t\t\t\tdie(\"cannot write keep file\");\n> +\t\t\t\tdie(\"cannot write keep file '%s' (%s)\",\n> +\t\t\t\t keep_name,\n> +\t\t\t\t strerror(errno));\n> \t\t} else {\n> \t\t\tif (keep_msg_len > 0) {\n> \t\t\t\twrite_or_die(keep_fd, keep_msg, keep_msg_len);\n> \t\t\t\twrite_or_die(keep_fd, \"\\n\", 1);\n> \t\t\t}\n> \t\t\tif (close(keep_fd) != 0)\n> -\t\t\t\tdie(\"cannot write keep file\");\n> +\t\t\t\tdie(\"cannot close the written keep file '%s' (%s)\",\n> +\t\t\t\t keep_name,\n> +\t\t\t\t strerror(errno));\n> \t\t\treport = \"keep\";\n> \t\t}\n> \t}\n> \n> \n> \n> \n> \n\n\n\nbegin:vcard\nfn:Rafael Darder Calvo\nn:Darder Calvo;Rafael\norg:;Infraestructura\nadr:;;Mitre 1017 Piso 5;Rosario;Santa Fe;S2000COU;Argentina\nemail;internet:rdarder@spiralti.com\ntitle:Spiralti Rosario\ntel;work:+54 341 5302041\nurl:www.spiralti.com\nversion:2.1\nend:vcard\n\n"},{"id":"106144","messageId":"7vocwrc7oe.fsf_-_@gitster.siamese.dyndns.org","threadId":"17992","inReplyTo":"7v7i3fivx2.fsf@gitster.siamese.dyndns.org","subject":"[PATCH v2] Make sure objects/pack exists before creating a new pack","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-25T07:11:29Z","receivedAt":"2009-02-25T07:11:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"In a repository created with git older than f49fb35 (git-init-db: create\n\"pack\" subdirectory under objects, 2005-06-27), objects/pack/ directory is\nnot created upon initialization. It was Ok because subdirectories are\ncreated as needed inside directories init-db creates, and back then,\npackfiles were recent invention.\n\nAfter the said commit, new codepaths started relying on the presense of\nobjects/pack/ directory in the repository. This was exacerbated with\n8b4eb6b (Do not perform cross-directory renames when creating packs,\n2008-09-22) that moved the location temporary pack files are created from\nobjects/ directory to objects/pack/ directory, because moving temporary to\nthe final location was done carefully with lazy leading directory creation.\n\nMany packfile related operations in such an old repository can fail\nmysteriously because of this.\n\nThis commit introduces two helper functions to make things work better.\n\n - odb_mkstemp() is a specialized version of mkstemp() to refactor the\n code and teach it to create leading directories as needed;\n\n - odb_pack_keep() refactors the code to create a \".keep\" file while\n create leading directories as needed.\n\nSigned-off-by: Junio C Hamano \n---\n\n * This is a re-roll of the earlier patch in response to Fedora bugzilla\n entry https://bugzilla.redhat.com/show_bug.cgi?id=483546\n\n The earlier one didn't cover the objects/pack/pack-*.keep codepaths, and\n wouldn't have been sufficient to fix the breakage reported by\n $gmane/111316.\n\n This is meant to apply cleanly to the v1.6.0.X maintenance track and\n upwards.\n\n builtin-pack-objects.c | 5 ++---\n fast-import.c | 14 +++++---------\n git-compat-util.h | 2 ++\n index-pack.c | 23 ++++++++++++-----------\n pack-write.c | 6 ++----\n t/t5300-pack-object.sh | 17 +++++++++++++++++\n wrapper.c | 32 ++++++++++++++++++++++++++++++++\n 7 files changed, 72 insertions(+), 27 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex fb5e14d..7518d53 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -473,9 +473,8 @@ static void write_pack_file(void)\n \t\t} else {\n \t\t\tchar tmpname[PATH_MAX];\n \t\t\tint fd;\n-\t\t\tsnprintf(tmpname, sizeof(tmpname),\n-\t\t\t\t \"%s/pack/tmp_pack_XXXXXX\", get_object_directory());\n-\t\t\tfd = xmkstemp(tmpname);\n+\t\t\tfd = odb_mkstemp(tmpname, sizeof(tmpname),\n+\t\t\t\t\t \"pack/tmp_pack_XXXXXX\");\n \t\t\tpack_tmp_name = xstrdup(tmpname);\n \t\t\tf = sha1fd(fd, pack_tmp_name);\n \t\t}\ndiff --git a/fast-import.c b/fast-import.c\nindex 5bc9ce2..3f92194 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -815,9 +815,8 @@ static void start_packfile(void)\n \tstruct pack_header hdr;\n \tint pack_fd;\n \n-\tsnprintf(tmpfile, sizeof(tmpfile),\n-\t\t\"%s/pack/tmp_pack_XXXXXX\", get_object_directory());\n-\tpack_fd = xmkstemp(tmpfile);\n+\tpack_fd = odb_mkstemp(tmpfile, sizeof(tmpfile),\n+\t\t\t \"pack/tmp_pack_XXXXXX\");\n \tp = xcalloc(1, sizeof(*p) + strlen(tmpfile) + 2);\n \tstrcpy(p->pack_name, tmpfile);\n \tp->pack_fd = pack_fd;\n@@ -877,9 +876,8 @@ static char *create_index(void)\n \t\tc = next;\n \t}\n \n-\tsnprintf(tmpfile, sizeof(tmpfile),\n-\t\t\"%s/pack/tmp_idx_XXXXXX\", get_object_directory());\n-\tidx_fd = xmkstemp(tmpfile);\n+\tidx_fd = odb_mkstemp(tmpfile, sizeof(tmpfile),\n+\t\t\t \"pack/tmp_idx_XXXXXX\");\n \tf = sha1fd(idx_fd, tmpfile);\n \tsha1write(f, array, 256 * sizeof(int));\n \tSHA1_Init(&ctx);\n@@ -905,9 +903,7 @@ static char *keep_pack(char *curr_index_name)\n \tchmod(pack_data->pack_name, 0444);\n \tchmod(curr_index_name, 0444);\n \n-\tsnprintf(name, sizeof(name), \"%s/pack/pack-%s.keep\",\n-\t\t get_object_directory(), sha1_to_hex(pack_data->sha1));\n-\tkeep_fd = open(name, O_RDWR|O_CREAT|O_EXCL, 0600);\n+\tkeep_fd = odb_pack_keep(name, sizeof(name), pack_data->sha1);\n \tif (keep_fd < 0)\n \t\tdie(\"cannot create keep file\");\n \twrite_or_die(keep_fd, keep_msg, strlen(keep_msg));\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex cf89cdf..758a880 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -294,6 +294,8 @@ extern ssize_t xwrite(int fd, const void *buf, size_t len);\n extern int xdup(int fd);\n extern FILE *xfdopen(int fd, const char *mode);\n extern int xmkstemp(char *template);\n+extern int odb_mkstemp(char *template, size_t limit, const char *pattern);\n+extern int odb_pack_keep(char *name, size_t namesz, unsigned char *sha1);\n \n static inline size_t xsize_t(off_t len)\n {\ndiff --git a/index-pack.c b/index-pack.c\nindex c99a1a1..745ac0b 100644\n--- a/index-pack.c\n+++ b/index-pack.c\n@@ -171,9 +171,8 @@ static char *open_pack_file(char *pack_name)\n \t\tinput_fd = 0;\n \t\tif (!pack_name) {\n \t\t\tstatic char tmpfile[PATH_MAX];\n-\t\t\tsnprintf(tmpfile, sizeof(tmpfile),\n-\t\t\t\t \"%s/pack/tmp_pack_XXXXXX\", get_object_directory());\n-\t\t\toutput_fd = xmkstemp(tmpfile);\n+\t\t\toutput_fd = odb_mkstemp(tmpfile, sizeof(tmpfile),\n+\t\t\t\t\t\t\"pack/tmp_pack_XXXXXX\");\n \t\t\tpack_name = xstrdup(tmpfile);\n \t\t} else\n \t\t\toutput_fd = open(pack_name, O_CREAT|O_EXCL|O_RDWR, 0600);\n@@ -795,22 +794,24 @@ static void final(const char *final_pack_name, const char *curr_pack_name,\n \n \tif (keep_msg) {\n \t\tint keep_fd, keep_msg_len = strlen(keep_msg);\n-\t\tif (!keep_name) {\n-\t\t\tsnprintf(name, sizeof(name), \"%s/pack/pack-%s.keep\",\n-\t\t\t\t get_object_directory(), sha1_to_hex(sha1));\n-\t\t\tkeep_name = name;\n-\t\t}\n-\t\tkeep_fd = open(keep_name, O_RDWR|O_CREAT|O_EXCL, 0600);\n+\n+\t\tif (!keep_name)\n+\t\t\tkeep_fd = odb_pack_keep(name, sizeof(name), sha1);\n+\t\telse\n+\t\t\tkeep_fd = open(keep_name, O_RDWR|O_CREAT|O_EXCL, 0600);\n+\n \t\tif (keep_fd < 0) {\n \t\t\tif (errno != EEXIST)\n-\t\t\t\tdie(\"cannot write keep file\");\n+\t\t\t\tdie(\"cannot write keep file '%s' (%s)\",\n+\t\t\t\t keep_name, strerror(errno));\n \t\t} else {\n \t\t\tif (keep_msg_len > 0) {\n \t\t\t\twrite_or_die(keep_fd, keep_msg, keep_msg_len);\n \t\t\t\twrite_or_die(keep_fd, \"\\n\", 1);\n \t\t\t}\n \t\t\tif (close(keep_fd) != 0)\n-\t\t\t\tdie(\"cannot write keep file\");\n+\t\t\t\tdie(\"cannot close written keep file '%s' (%s)\",\n+\t\t\t\t keep_name, strerror(errno));\n \t\t\treport = \"keep\";\n \t\t}\n \t}\ndiff --git a/pack-write.c b/pack-write.c\nindex 3621f1d..e82c025 100644\n--- a/pack-write.c\n+++ b/pack-write.c\n@@ -44,9 +44,7 @@ char *write_idx_file(char *index_name, struct pack_idx_entry **objects,\n \n \tif (!index_name) {\n \t\tstatic char tmpfile[PATH_MAX];\n-\t\tsnprintf(tmpfile, sizeof(tmpfile),\n-\t\t\t \"%s/pack/tmp_idx_XXXXXX\", get_object_directory());\n-\t\tfd = xmkstemp(tmpfile);\n+\t\tfd = odb_mkstemp(tmpfile, sizeof(tmpfile), \"pack/tmp_idx_XXXXXX\");\n \t\tindex_name = xstrdup(tmpfile);\n \t} else {\n \t\tunlink(index_name);\n@@ -239,7 +237,7 @@ char *index_pack_lockfile(int ip_out)\n \tchar packname[46];\n \n \t/*\n-\t * The first thing we expects from index-pack's output\n+\t * The first thing we expect from index-pack's output\n \t * is \"pack\\t%40s\\n\" or \"keep\\t%40s\\n\" (46 bytes) where\n \t * %40s is the newly created pack SHA1 name. In the \"keep\"\n \t * case, we need it to remove the corresponding .keep file\ndiff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\nindex 2852a03..73d871c 100755\n--- a/t/t5300-pack-object.sh\n+++ b/t/t5300-pack-object.sh\n@@ -180,6 +180,23 @@ test_expect_success \\\n \n unset GIT_OBJECT_DIRECTORY\n \n+test_expect_success 'survive missing objects/pack directory' '\n+\t(\n+\t\trm -fr missing-pack &&\n+\t\tmkdir missing-pack &&\n+\t\tcd missing-pack &&\n+\t\tgit init &&\n+\t\tGOP=.git/objects/pack\n+\t\trm -fr $GOP &&\n+\t\tgit index-pack --stdin --keep=test <../test-3-${packname_3}.pack &&\n+\t\ttest -f $GOP/pack-${packname_3}.pack &&\n+\t\ttest_cmp $GOP/pack-${packname_3}.pack ../test-3-${packname_3}.pack &&\n+\t\ttest -f $GOP/pack-${packname_3}.idx &&\n+\t\ttest_cmp $GOP/pack-${packname_3}.idx ../test-3-${packname_3}.idx &&\n+\t\ttest -f $GOP/pack-${packname_3}.keep\n+\t)\n+'\n+\n test_expect_success \\\n 'verify pack' \\\n 'git verify-pack\ttest-1-${packname_1}.idx \\\ndiff --git a/wrapper.c b/wrapper.c\nindex 93562f0..231a58f 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -196,3 +196,35 @@ int xmkstemp(char *template)\n \t\tdie(\"Unable to create temporary file: %s\", strerror(errno));\n \treturn fd;\n }\n+\n+int odb_mkstemp(char *template, size_t limit, const char *pattern)\n+{\n+\tint fd;\n+\n+\tsnprintf(template, limit, \"%s/%s\",\n+\t\t get_object_directory(), pattern);\n+\tfd = mkstemp(template);\n+\tif (0 <= fd)\n+\t\treturn fd;\n+\n+\t/* slow path */\n+\tsafe_create_leading_directories(template);\n+\tsnprintf(template, limit, \"%s/%s\",\n+\t\t get_object_directory(), pattern);\n+\treturn xmkstemp(template);\n+}\n+\n+int odb_pack_keep(char *name, size_t namesz, unsigned char *sha1)\n+{\n+\tint fd;\n+\n+\tsnprintf(name, namesz, \"%s/pack/pack-%s.keep\",\n+\t\t get_object_directory(), sha1_to_hex(sha1));\n+\tfd = open(name, O_RDWR|O_CREAT|O_EXCL, 0600);\n+\tif (0 <= fd)\n+\t\treturn fd;\n+\n+\t/* slow path */\n+\tsafe_create_leading_directories(name);\n+\treturn open(name, O_RDWR|O_CREAT|O_EXCL, 0600);\n+}\n-- \n1.6.2.rc1.113.ga620b\n"},{"id":"106145","messageId":"7vk57fc7gz.fsf@gitster.siamese.dyndns.org","threadId":"17992","inReplyTo":"7vocwrc7oe.fsf_-_@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2] Make sure objects/pack exists before creating a new pack","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-25T07:15:56Z","receivedAt":"2009-02-25T07:15:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This is *not* to be applied, but to document and demonstrate how I tested\nthe previous fix. It disables the creation of .git/objects/pack/\ndirectory when \"git init\" is run, and adds minimum workaround in the tests\nthat assume the directory is always created.\n\n---\n builtin-init-db.c | 2 +-\n t/t0000-basic.sh | 2 +-\n t/t5300-pack-object.sh | 1 +\n 3 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-init-db.c b/builtin-init-db.c\nindex d30c3fe..e5fc4c1 100644\n--- a/builtin-init-db.c\n+++ b/builtin-init-db.c\n@@ -301,7 +301,7 @@ int init_db(const char *template_dir, unsigned int flags)\n \n \tsafe_create_dir(sha1_dir, 1);\n \tstrcpy(path+len, \"/pack\");\n-\tsafe_create_dir(path, 1);\n+/*\tsafe_create_dir(path, 1); */\n \tstrcpy(path+len, \"/info\");\n \tsafe_create_dir(path, 1);\n \ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex 70df15c..6240ca4 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -42,7 +42,7 @@ test_expect_success \\\n # also it should have 2 subdirectories; no fan-out anymore, pack, and info.\n # 3 is counting \"objects\" itself\n find .git/objects -type d -print >full-of-directories\n-test_expect_success \\\n+: test_expect_success \\\n '.git/objects should have 3 subdirectories.' \\\n 'test $(wc -l < full-of-directories) = 3'\n \ndiff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\nindex 73d871c..e7d8daf 100755\n--- a/t/t5300-pack-object.sh\n+++ b/t/t5300-pack-object.sh\n@@ -138,6 +138,7 @@ test_expect_success \\\n 'GIT_OBJECT_DIRECTORY=.git2/objects &&\n export GIT_OBJECT_DIRECTORY &&\n git init &&\n+ mkdir -p .git2/objects/pack &&\n cp test-1-${packname_1}.pack test-1-${packname_1}.idx .git2/objects/pack && {\n \t git diff-tree --root -p $commit &&\n \t while read object\n"},{"id":"106306","messageId":"49A65EBF.10307@viscovery.net","threadId":"17992","inReplyTo":"7vocwrc7oe.fsf_-_@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2] Make sure objects/pack exists before creating a new pack","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-02-26T09:19:59Z","receivedAt":"2009-02-26T09:19:59Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Junio C Hamano schrieb:\n> @@ -795,22 +794,24 @@ static void final(const char *final_pack_name, const char *curr_pack_name,\n> \n> \tif (keep_msg) {\n> \t\tint keep_fd, keep_msg_len = strlen(keep_msg);\n> -\t\tif (!keep_name) {\n> -\t\t\tsnprintf(name, sizeof(name), \"%s/pack/pack-%s.keep\",\n> -\t\t\t\t get_object_directory(), sha1_to_hex(sha1));\n> -\t\t\tkeep_name = name;\n\nYou should not have removed this line...\n\n> -\t\t}\n> -\t\tkeep_fd = open(keep_name, O_RDWR|O_CREAT|O_EXCL, 0600);\n> +\n> +\t\tif (!keep_name)\n> +\t\t\tkeep_fd = odb_pack_keep(name, sizeof(name), sha1);\n> +\t\telse\n> +\t\t\tkeep_fd = open(keep_name, O_RDWR|O_CREAT|O_EXCL, 0600);\n> +\n> \t\tif (keep_fd < 0) {\n> \t\t\tif (errno != EEXIST)\n> -\t\t\t\tdie(\"cannot write keep file\");\n> +\t\t\t\tdie(\"cannot write keep file '%s' (%s)\",\n> +\t\t\t\t keep_name, strerror(errno));\n\n... we need keep_name for this error message.\n\nWarning: I did not test the failure path; but I made sure that these\ntests pass on Windows:\n\ncd t && make *fetch* *pack* *index* *pull*\n\n-- snip --\nFrom: Johannes Sixt \nSubject: [PATCH] index-pack: Set keep_name if a new keep file was allocated\n\n6e180cdc (Make sure objects/pack exists before creating a new pack,\n2009-02-25) simplified the code in this area, but it also extended an\nerror message to include the file name that caused the error. However,\none code path did not set file name correctly.\n\nSigned-off-by: Johannes Sixt \n---\n index-pack.c | 5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/index-pack.c b/index-pack.c\nindex 7fee872..93a09c6 100644\n--- a/index-pack.c\n+++ b/index-pack.c\n@@ -794,9 +794,10 @@ static void final(const char *final_pack_name, const char *curr_pack_name,\n \tif (keep_msg) {\n \t\tint keep_fd, keep_msg_len = strlen(keep_msg);\n\n-\t\tif (!keep_name)\n+\t\tif (!keep_name) {\n \t\t\tkeep_fd = odb_pack_keep(name, sizeof(name), sha1);\n-\t\telse\n+\t\t\tkeep_name = name;\n+\t\t} else\n \t\t\tkeep_fd = open(keep_name, O_RDWR|O_CREAT|O_EXCL, 0600);\n\n \t\tif (keep_fd < 0) {\n-- \n1.6.2.rc2.1005.gb0117\n"},{"id":"106317","messageId":"e2b179460902260319je8ce6c6qc028d260b9d35b7c@mail.gmail.com","threadId":"17992","inReplyTo":"49A65EBF.10307@viscovery.net","subject":"Re: [PATCH v2] Make sure objects/pack exists before creating a new pack","fromName":"Mike Ralphson","fromEmail":"mike.ralphson@gmail.com","sentAt":"2009-02-26T11:19:02Z","receivedAt":"2009-02-26T11:19:02Z","isPatch":true,"sender":{"key":"mike.ralphson@gmail.com","avatar":"https://avatars.githubusercontent.com/u/21603?v=4"},"body":"2009/2/26 Johannes Sixt \n>\n> Junio C Hamano schrieb:\n> > @@ -795,22 +794,24 @@ static void final(const char *final_pack_name, const char *curr_pack_name,\n> >\n> >       if (keep_msg) {\n> >               int keep_fd, keep_msg_len = strlen(keep_msg);\n> > -             if (!keep_name) {\n> > -                     snprintf(name, sizeof(name), \"%s/pack/pack-%s.keep\",\n> > -                              get_object_directory(), sha1_to_hex(sha1));\n> > -                     keep_name = name;\n>\n> You should not have removed this line...\n\nEven with j6t's patch, I'm seeing failures in t5300--pack-object.sh on AIX 5.3\n\n* FAIL 15: survive missing objects/pack directory\nfatal: Unable to create temporary file (): No such file or directory\n\nFrom a bit of instrumenting (no working gdb here), I see that in\nwrapper.c/odb_mkstemp, template is empty on entry, empty before the\ncall to safe_create_leading_directories, but contains\n.git/objects/pack/tmp_pack_XXXXXX after the second snprintf. TMPDIR is\nnot set here, which I thought might be the root cause, but it doesn't\nseem to be that.\n\nMy naive solution is to:\n\nindex b07cdf2..80fee48 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -268,9 +268,9 @@ int odb_mkstemp(char *template, size_t limit,\nconst char *pattern)\n return fd;\n\n /* slow path */\n- safe_create_leading_directories(template);\n snprintf(template, limit, \"%s/%s\",\n get_object_directory(), pattern);\n+ safe_create_leading_directories(template);\n return xmkstemp(template);\n }\n\nWhich then passes all tests.\n\nBearing in mind that for me, fiddling with C is like trying to order\nroom service in Latin, if this is in any way the correct thing to do,\nI'll post a proper patch with s-o-b if required, or you can just\nsquash into j6t's.\n\nMike\n"},{"id":"106337","messageId":"49A6B5E8.8040306@viscovery.net","threadId":"17992","inReplyTo":"e2b179460902260319je8ce6c6qc028d260b9d35b7c@mail.gmail.com","subject":"Re: [PATCH v2] Make sure objects/pack exists before creating a new pack","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-02-26T15:31:52Z","receivedAt":"2009-02-26T15:31:52Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Mike Ralphson schrieb:\n> 2009/2/26 Johannes Sixt \n>> Junio C Hamano schrieb:\n>>> @@ -795,22 +794,24 @@ static void final(const char *final_pack_name, const char *curr_pack_name,\n>>>\n>>> if (keep_msg) {\n>>> int keep_fd, keep_msg_len = strlen(keep_msg);\n>>> - if (!keep_name) {\n>>> - snprintf(name, sizeof(name), \"%s/pack/pack-%s.keep\",\n>>> - get_object_directory(), sha1_to_hex(sha1));\n>>> - keep_name = name;\n>> You should not have removed this line...\n> \n> Even with j6t's patch, I'm seeing failures in t5300--pack-object.sh on AIX 5.3\n> \n> * FAIL 15: survive missing objects/pack directory\n> fatal: Unable to create temporary file (): No such file or directory\n\nI can confirm this. Your patch is good. I wrapped it up:\n\n-- snip --\nFrom: Mike Ralphson \nSubject: [PATCH] Fix odb_mkstemp() on AIX\n\nThe AIX mkstemp() modifies its template parameter to an empty string if\nthe call fails. The existing code had already recomputed the template,\nbut too late to be good.\n\nSee also 6ff6af62, which fixed this problem in a different spot.\n\nSigned-off-by: Johannes Sixt \n---\n wrapper.c | 3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/wrapper.c b/wrapper.c\nindex b07cdf2..d8efb13 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -268,9 +268,10 @@ int odb_mkstemp(char *template, size_t limit,\n \t\treturn fd;\n\n \t/* slow path */\n-\tsafe_create_leading_directories(template);\n+\t/* some mkstemp implementations erase template on failure */\n \tsnprintf(template, limit, \"%s/%s\",\n \t\t get_object_directory(), pattern);\n+\tsafe_create_leading_directories(template);\n \treturn xmkstemp(template);\n }\n\n-- \n1.6.1.rc1.5.gf691f\n"},{"id":"106340","messageId":"e2b179460902260754t38618c3fsf22a699ec35070cf@mail.gmail.com","threadId":"17992","inReplyTo":"49A6B5E8.8040306@viscovery.net","subject":"Re: [PATCH v2] Make sure objects/pack exists before creating a new pack","fromName":"Mike Ralphson","fromEmail":"mike.ralphson@gmail.com","sentAt":"2009-02-26T15:54:03Z","receivedAt":"2009-02-26T15:54:03Z","isPatch":true,"sender":{"key":"mike.ralphson@gmail.com","avatar":"https://avatars.githubusercontent.com/u/21603?v=4"},"body":"2009/2/26 Johannes Sixt :\n> Mike Ralphson schrieb:\n>> Even with j6t's patch, I'm seeing failures in t5300--pack-object.sh on AIX 5.3\n>>\n>> * FAIL 15: survive missing objects/pack directory\n>> fatal: Unable to create temporary file (): No such file or directory\n>\n> I can confirm this. Your patch is good. I wrapped it up:\n>\n> -- snip --\n> From: Mike Ralphson \n> Subject: [PATCH] Fix odb_mkstemp() on AIX\n>\n> The AIX mkstemp() modifies its template parameter to an empty string if\n> the call fails.  The existing code had already recomputed the template,\n> but too late to be good.\n>\n> See also 6ff6af62, which fixed this problem in a different spot.\n\nAh, I should have remembered that one.\n\nIf my $DAYJOB email address could be used in the From, as per this s-o-b:\n\nSigned-off-by: Mike Ralphson \n\nThanks.\n"},{"id":"106344","messageId":"7vmyc9t9d6.fsf@gitster.siamese.dyndns.org","threadId":"17992","inReplyTo":"e2b179460902260754t38618c3fsf22a699ec35070cf@mail.gmail.com","subject":"Re: [PATCH v2] Make sure objects/pack exists before creating a new pack","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-26T17:07:33Z","receivedAt":"2009-02-26T17:07:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Ralphson writes:\n\n> 2009/2/26 Johannes Sixt :\n>> Mike Ralphson schrieb:\n>>> Even with j6t's patch, I'm seeing failures in t5300--pack-object.sh on AIX 5.3\n>>>\n>>> * FAIL 15: survive missing objects/pack directory\n>>> fatal: Unable to create temporary file (): No such file or directory\n>>\n>> I can confirm this. Your patch is good. I wrapped it up:\n>>\n>> -- snip --\n>> From: Mike Ralphson \n>> Subject: [PATCH] Fix odb_mkstemp() on AIX\n>>\n>> The AIX mkstemp() modifies its template parameter to an empty string if\n>> the call fails.  The existing code had already recomputed the template,\n>> but too late to be good.\n>>\n>> See also 6ff6af62, which fixed this problem in a different spot.\n>\n> Ah, I should have remembered that one.\n>\n> If my $DAYJOB email address could be used in the From, as per this s-o-b:\n>\n> Signed-off-by: Mike Ralphson \n\nThanks.\n"}]}