{"thread":{"id":"38952","subject":"[PATCH] sha1_file: squelch \"packfile cannot be accessed\" warnings","startedAt":"2015-03-31T00:47:38Z","lastAt":"2015-03-31T00:47:38Z","messageCount":1,"participants":["Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"258712","messageId":"20150331004738.GA24431@peff.net","threadId":"38952","inReplyTo":null,"subject":"[PATCH] sha1_file: squelch \"packfile cannot be accessed\" warnings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-31T00:47:38Z","receivedAt":"2015-03-31T00:47:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we find an object in a packfile index, we make sure we\ncan still open the packfile itself (or that it is already\nopen), as it might have been deleted by a simultaneous\nrepack. If we can't access the packfile, we print a warning\nfor the user and tell the caller that we don't have the\nobject (we can then look in other packfiles, or find a loose\nversion, before giving up).\n\nThe warning we print to the user isn't really accomplishing\nanything, and it is potentially confusing to users. In the\nnormal case, it is complete noise; we find the object\nelsewhere, and the user does not have to care that we racily\nsaw a packfile index that became stale. It didn't affect the\noperation at all.\n\nA possibly more interesting case is when we later can't find\nthe object, and report failure to the user. In this case the\nwarning could be considered a clue toward that ultimate\nfailure. But it's not really a useful clue in practice. We\nwouldn't even print it consistently (since we are racing\nwith another process, we might not even see the .idx file,\nor we might win the race and open the packfile, completing\nthe operation).\n\nThis patch drops the warning entirely (not only from the\nfill_pack_entry site, but also from an identical use in\npack-objects). If we did find the warning interesting in the\nerror case, we could stuff it away and reveal it to the user\nwhen we later die() due to the broken object. But that\ncomplexity just isn't worth it.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/pack-objects.c | 4 +---\n sha1_file.c            | 4 +---\n 2 files changed, 2 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex c3a7516..c067107 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -961,10 +961,8 @@ static int want_object_in_pack(const unsigned char *sha1,\n \t\toff_t offset = find_pack_entry_one(sha1, p);\n \t\tif (offset) {\n \t\t\tif (!*found_pack) {\n-\t\t\t\tif (!is_pack_valid(p)) {\n-\t\t\t\t\twarning(\"packfile %s cannot be accessed\", p->pack_name);\n+\t\t\t\tif (!is_pack_valid(p))\n \t\t\t\t\tcontinue;\n-\t\t\t\t}\n \t\t\t\t*found_offset = offset;\n \t\t\t\t*found_pack = p;\n \t\t\t}\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 88f06ba..0b0d07e 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2473,10 +2473,8 @@ static int fill_pack_entry(const unsigned char *sha1,\n \t * answer, as it may have been deleted since the index was\n \t * loaded!\n \t */\n-\tif (!is_pack_valid(p)) {\n-\t\twarning(\"packfile %s cannot be accessed\", p->pack_name);\n+\tif (!is_pack_valid(p))\n \t\treturn 0;\n-\t}\n \te->offset = offset;\n \te->p = p;\n \thashcpy(e->sha1, sha1);\n-- \n2.4.0.rc0.363.gf9f328b\n"}]}