{"thread":{"id":"46193","subject":"[PATCH v3 1/4] sha1_file: teach packed_object_info about typename","startedAt":"2017-06-15T20:39:58Z","lastAt":"2017-06-15T20:40:12Z","messageCount":5,"participants":["Jonathan Tan"],"isPatch":true,"patchVersion":3,"patchTotal":4},"messages":[{"id":"322387","messageId":"e7a79c114b7131ba286bf3f76789b2efaeeb1cf1.1497558850.git.jonathantanmy@google.com","threadId":"46193","inReplyTo":"cover.1497558850.git.jonathantanmy@google.com","subject":"[PATCH v3 1/4] sha1_file: teach packed_object_info about typename","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-06-15T20:39:28Z","receivedAt":"2017-06-15T20:39:58Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"In commit 46f0344 (\"sha1_file: support reading from a loose object of\nunknown type\", 2015-05-06), \"struct object_info\" gained a \"typename\"\nfield that could represent a type name from a loose object file, whether\nvalid or invalid, as opposed to the existing \"typep\" which could only\nrepresent valid types. Some relatively complex manipulations were added\nto avoid breaking packed_object_info() without modifying it, but it is\nmuch easier to just teach packed_object_info() about the new field.\nTherefore, teach packed_object_info() as described above.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n sha1_file.c | 29 ++++++++++++-----------------\n 1 file changed, 12 insertions(+), 17 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 59a4ed2ed..a52b27541 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2277,9 +2277,18 @@ int packed_object_info(struct packed_git *p, off_t obj_offset,\n \t\t*oi->disk_sizep = revidx[1].offset - obj_offset;\n \t}\n \n-\tif (oi->typep) {\n-\t\t*oi->typep = packed_to_object_type(p, obj_offset, type, &w_curs, curpos);\n-\t\tif (*oi->typep < 0) {\n+\tif (oi->typep || oi->typename) {\n+\t\tenum object_type ptot;\n+\t\tptot = packed_to_object_type(p, obj_offset, type, &w_curs,\n+\t\t\t\t\t     curpos);\n+\t\tif (oi->typep)\n+\t\t\t*oi->typep = ptot;\n+\t\tif (oi->typename) {\n+\t\t\tconst char *tn = typename(ptot);\n+\t\t\tif (tn)\n+\t\t\t\tstrbuf_addstr(oi->typename, tn);\n+\t\t}\n+\t\tif (ptot < 0) {\n \t\t\ttype = OBJ_BAD;\n \t\t\tgoto out;\n \t\t}\n@@ -2960,7 +2969,6 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n \tstruct cached_object *co;\n \tstruct pack_entry e;\n \tint rtype;\n-\tenum object_type real_type;\n \tconst unsigned char *real = lookup_replace_object_extended(sha1, flags);\n \n \tco = find_cached_object(real);\n@@ -2992,18 +3000,9 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n \t\t\treturn -1;\n \t}\n \n-\t/*\n-\t * packed_object_info() does not follow the delta chain to\n-\t * find out the real type, unless it is given oi->typep.\n-\t */\n-\tif (oi->typename && !oi->typep)\n-\t\toi->typep = &real_type;\n-\n \trtype = packed_object_info(e.p, e.offset, oi);\n \tif (rtype < 0) {\n \t\tmark_bad_packed_object(e.p, real);\n-\t\tif (oi->typep == &real_type)\n-\t\t\toi->typep = NULL;\n \t\treturn sha1_object_info_extended(real, oi, 0);\n \t} else if (in_delta_base_cache(e.p, e.offset)) {\n \t\toi->whence = OI_DBCACHED;\n@@ -3014,10 +3013,6 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n \t\toi->u.packed.is_delta = (rtype == OBJ_REF_DELTA ||\n \t\t\t\t\t rtype == OBJ_OFS_DELTA);\n \t}\n-\tif (oi->typename)\n-\t\tstrbuf_addstr(oi->typename, typename(*oi->typep));\n-\tif (oi->typep == &real_type)\n-\t\toi->typep = NULL;\n \n \treturn 0;\n }\n-- \n2.13.1.518.g3df882009-goog\n\n"},{"id":"322388","messageId":"cover.1497558850.git.jonathantanmy@google.com","threadId":"46193","inReplyTo":"cover.1497035376.git.jonathantanmy@google.com","subject":"[PATCH v3 0/4] Improvements to sha1_file","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-06-15T20:39:27Z","receivedAt":"2017-06-15T20:40:00Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Thanks - this has been updated following Junio's comments.\n\nPatch 1 is unmodified from the previous version.\n\nPatch 2 has been modified to remove the extraneous code pointed out by\nJunio. I previously had an idea of populating those fields in\npacked_object_info(), but later changed my mind, but a rebase went\nwrong.\n\nPatches 3-4 have been updated as I have described in [1] and [2].\n\n[1] https://public-inbox.org/git/20170615111447.1208e02b@twelve2.svl.corp.google.com/\n[2] https://public-inbox.org/git/20170615111447.1208e02b@twelve2.svl.corp.google.com/\n\nAs before, I would like review on patches 1-3 to go into the tree.\n(Patch 4 is a work in progress, and is here just to demonstrate the\neffectiveness of the refactoring.)\n\nJonathan Tan (4):\n  sha1_file: teach packed_object_info about typename\n  sha1_file: move delta base cache code up\n  sha1_file: consolidate storage-agnostic object fns\n  sha1_file, fsck: add missing blob support\n\n Documentation/config.txt |  10 +\n builtin/fsck.c           |   7 +\n cache.h                  |   8 +\n sha1_file.c              | 474 ++++++++++++++++++++++++++++++-----------------\n t/t3907-missing-blob.sh  |  69 +++++++\n 5 files changed, 400 insertions(+), 168 deletions(-)\n create mode 100755 t/t3907-missing-blob.sh\n\n-- \n2.13.1.518.g3df882009-goog\n\n"},{"id":"322389","messageId":"b917a463f0ad4ce0ab115203b3f24894961a2e75.1497558851.git.jonathantanmy@google.com","threadId":"46193","inReplyTo":"cover.1497558850.git.jonathantanmy@google.com","subject":"[PATCH v3 4/4] sha1_file, fsck: add missing blob support","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-06-15T20:39:31Z","receivedAt":"2017-06-15T20:40:06Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Currently, Git does not support repos with very large numbers of blobs\nor repos that wish to minimize manipulation of certain blobs (for\nexample, because they are very large) very well, even if the user\noperates mostly on part of the repo, because Git is designed on the\nassumption that every blob referenced by a tree object is available\nsomewhere in the repo storage.\n\nAs a first step to reducing this problem, add rudimentary support for\nmissing blobs by teaching sha1_file to invoke a hook whenever a blob is\nrequested and unavailable but registered to be missing, and by updating\nfsck to tolerate such blobs.  The hook is a shell command that can be\nconfigured through \"git config\"; this hook takes in a list of hashes and\nwrites (if successful) the corresponding objects to the repo's local\nstorage.\n\nThis commit does not include support for generating such a repo; neither\nhas any command (other than fsck) been modified to either tolerate\nmissing blobs (without invoking the hook) or be more efficient in\ninvoking the missing blob hook. Only a fallback is provided in the form\nof sha1_file invoking the missing blob hook when necessary.\n\nIn order to determine the code changes in sha1_file.c necessary, I\ninvestigated the following:\n (1) functions in sha1_file that take in a hash, without the user\n     regarding how the object is stored (loose or packed)\n (2) functions in sha1_file that operate on packed objects (because I\n     need to check callers that know about the loose/packed distinction\n     and operate on both differently, and ensure that they can handle\n     the concept of objects that are neither loose nor packed)\n\n(1) is handled by the modification to sha1_object_info_extended() and\nhas_sha1_file_with_flags().\n\nFor (2), I looked through the same functions as in (1) and also\nfor_each_packed_object. The ones that are relevant are:\n - parse_pack_index\n   - http - indirectly from http_get_info_packs\n - find_pack_entry_one\n   - this searches a single pack that is provided as an argument; the\n     caller already knows (through other means) that the sought object\n     is in a specific pack\n - find_sha1_pack\n   - fast-import - appears to be an optimization to not store a\n     file if it is already in a pack\n   - http-walker - to search through a struct alt_base\n   - http-push - to search through remote packs\n - has_sha1_pack\n   - builtin/fsck - fixed in this commit\n   - builtin/count-objects - informational purposes only (check if loose\n     object is also packed)\n   - builtin/prune-packed - check if object to be pruned is packed (if\n     not, don't prune it)\n   - revision - used to exclude packed objects if requested by user\n   - diff - just for optimization\n - for_each_packed_object\n   - reachable - only to find recent objects\n   - builtin/fsck - fixed in this commit\n   - builtin/cat-file - see below\n\nAs described in the list above, builtin/fsck has been updated. I have\nleft builtin/cat-file alone; this means that cat-file\n--batch-all-objects will only operate on objects physically in the repo.\n\nAn alternative design that I considered but rejected:\n\n - Adding a hook whenever a packed blob is requested, not on any blob.\n   That is, whenever we attempt to search the packfiles for a blob, if\n   it is missing (from the packfiles and from the loose object storage),\n   to invoke the hook (which must then store it as a packfile), open the\n   packfile the hook generated, and report that the blob is found in\n   that new packfile. This reduces the amount of analysis needed (in\n   that we only need to look at how packed blobs are handled), but\n   requires that the hook generate packfiles (or for sha1_file to pack\n   whatever loose objects are generated), creating one packfile for each\n   missing blob and potentially very many packfiles that must be\n   linearly searched. This may be tolerable now for repos that only have\n   a few missing blobs (for example, repos that only want to exclude\n   large blobs), and might be tolerable in the future if we have\n   batching support for the most commonly used commands, but is not\n   tolerable now for repos that exclude a large amount of blobs.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n Documentation/config.txt |  10 +++\n builtin/fsck.c           |   7 ++\n cache.h                  |   7 ++\n sha1_file.c              | 171 +++++++++++++++++++++++++++++++++++++++++++----\n t/t3907-missing-blob.sh  |  69 +++++++++++++++++++\n 5 files changed, 250 insertions(+), 14 deletions(-)\n create mode 100755 t/t3907-missing-blob.sh\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex dd4beec39..10da5fde1 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -390,6 +390,16 @@ The default is false, except linkgit:git-clone[1] or linkgit:git-init[1]\n will probe and set core.ignoreCase true if appropriate when the repository\n is created.\n \n+core.missingBlobCommand::\n+\tIf set, whenever a blob in the local repo is attempted to be\n+\tread but is missing, invoke this shell command to generate or\n+\tobtain that blob before reporting an error. This shell command\n+\tshould take one or more hashes, each terminated by a newline, as\n+\tstandard input, and (if successful) should write the\n+\tcorresponding objects to the local repo (packed or loose).\n++\n+If set, fsck will not treat a missing blob as an error condition.\n+\n core.precomposeUnicode::\n \tThis option is only used by Mac OS implementation of Git.\n \tWhen core.precomposeUnicode=true, Git reverts the unicode decomposition\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex cb2ba6cd1..b447bd5f9 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -37,6 +37,7 @@ static int verbose;\n static int show_progress = -1;\n static int show_dangling = 1;\n static int name_objects;\n+static int missing_blob_ok;\n #define ERROR_OBJECT 01\n #define ERROR_REACHABLE 02\n #define ERROR_PACK 04\n@@ -93,6 +94,9 @@ static int fsck_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"core.missingblobcommand\"))\n+\t\tmissing_blob_ok = 1;\n+\n \treturn git_default_config(var, value, cb);\n }\n \n@@ -222,6 +226,9 @@ static void check_reachable_object(struct object *obj)\n \tif (!(obj->flags & HAS_OBJ)) {\n \t\tif (has_sha1_pack(obj->oid.hash))\n \t\t\treturn; /* it is in pack - forget about it */\n+\t\tif (missing_blob_ok && obj->type == OBJ_BLOB &&\n+\t\t    in_missing_blob_manifest(obj->oid.hash, NULL))\n+\t\t\treturn;\n \t\tprintf(\"missing %s %s\\n\", printable_type(obj),\n \t\t\tdescribe_object(obj));\n \t\terrors_found |= ERROR_REACHABLE;\ndiff --git a/cache.h b/cache.h\nindex 63a73af17..dd69c75f5 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1870,6 +1870,13 @@ struct object_info {\n extern int sha1_object_info_extended(const unsigned char *, struct object_info *, unsigned flags);\n extern int packed_object_info(struct packed_git *pack, off_t offset, struct object_info *);\n \n+/*\n+ * Returns 1 if sha1 is the hash of a known missing blob. If size is not NULL,\n+ * also returns its size.\n+ */\n+extern int in_missing_blob_manifest(const unsigned char *sha1,\n+\t\t\t\t    unsigned long *size);\n+\n /* Dumb servers support */\n extern int update_server_info(int);\n \ndiff --git a/sha1_file.c b/sha1_file.c\nindex 60b487c70..7ef239907 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -27,6 +27,9 @@\n #include \"list.h\"\n #include \"mergesort.h\"\n #include \"quote.h\"\n+#include \"iterator.h\"\n+#include \"dir-iterator.h\"\n+#include \"sha1-lookup.h\"\n \n #define SZ_FMT PRIuMAX\n static inline uintmax_t sz_fmt(size_t s) { return s; }\n@@ -1624,6 +1627,72 @@ static const struct packed_git *has_packed_and_bad(const unsigned char *sha1)\n \treturn NULL;\n }\n \n+struct missing_blob_manifest {\n+\tstruct missing_blob_manifest *next;\n+\tconst char *data;\n+};\n+static struct missing_blob_manifest *missing_blobs;\n+static int missing_blobs_initialized;\n+\n+static void prepare_missing_blobs(void)\n+{\n+\tint ok;\n+\tchar *dirname;\n+\tstruct dir_iterator *iter;\n+\n+\tif (missing_blobs_initialized)\n+\t\treturn;\n+\n+\tmissing_blobs_initialized = 1;\n+\n+\tdirname = xstrfmt(\"%s/missing\", get_object_directory());\n+\titer = dir_iterator_begin(dirname);\n+\n+\twhile ((ok = dir_iterator_advance(iter)) == ITER_OK) {\n+\t\tint fd;\n+\t\tconst char *data;\n+\t\tstruct missing_blob_manifest *m;\n+\t\tif (!S_ISREG(iter->st.st_mode))\n+\t\t\tcontinue;\n+\t\tfd = git_open(iter->path.buf);\n+\t\tdata = xmmap(NULL, iter->st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);\n+\t\tclose(fd);\n+\n+\t\tm = xmalloc(sizeof(*m));\n+\t\tm->next = missing_blobs;\n+\t\tm->data = data;\n+\t\tmissing_blobs = m;\n+\t}\n+\n+\tif (ok != ITER_DONE) {\n+\t\t/* do something */\n+\t}\n+\n+\tfree(dirname);\n+}\n+\n+int in_missing_blob_manifest(const unsigned char *sha1, unsigned long *size)\n+{\n+\tstruct missing_blob_manifest *m;\n+\tprepare_missing_blobs();\n+\tfor (m = missing_blobs; m; m = m->next) {\n+\t\tuint64_t nr_nbo, nr;\n+\t\tint result;\n+\t\tmemcpy(&nr_nbo, m->data, sizeof(nr_nbo));\n+\t\tnr = htonll(nr_nbo);\n+\t\tresult = sha1_entry_pos(m->data, GIT_SHA1_RAWSZ + 8, 8, 0, nr, nr, sha1);\n+\t\tif (result >= 0) {\n+\t\t\tif (size) {\n+\t\t\t\tuint64_t size_nbo;\n+\t\t\t\tmemcpy(&size_nbo, m->data + 8 + result * (GIT_SHA1_RAWSZ + 8) + GIT_SHA1_RAWSZ, sizeof(size_nbo));\n+\t\t\t\t*size = ntohll(size_nbo);\n+\t\t\t}\n+\t\t\treturn 1;\n+\t\t}\n+\t}\n+\treturn 0;\n+}\n+\n /*\n  * With an in-core object data in \"map\", rehash it to make sure the\n  * object name actually matches \"sha1\" to detect object corruption.\n@@ -2975,12 +3044,57 @@ static int sha1_loose_object_info(const unsigned char *sha1,\n \treturn (status < 0) ? status : 0;\n }\n \n+static char *missing_blob_command;\n+static int missing_blob_config(const char *conf_key, const char *value,\n+\t\t\t       void *cb)\n+{\n+\tif (!strcmp(conf_key, \"core.missingblobcommand\")) {\n+\t\tmissing_blob_command = xstrdup(value);\n+\t}\n+\treturn 0;\n+}\n+\n+static void ensure_missing_blob_configured(void)\n+{\n+\tstatic int configured;\n+\tif (configured)\n+\t\treturn;\n+\n+\tgit_config(missing_blob_config, NULL);\n+\tconfigured = 1;\n+}\n+\n+static void handle_missing_blob(const unsigned char *sha1)\n+{\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tconst char *argv[] = {missing_blob_command, NULL};\n+\tchar input[GIT_MAX_HEXSZ + 1];\n+\n+\tmemcpy(input, sha1_to_hex(sha1), 40);\n+\tinput[40] = '\\n';\n+\n+\tcp.argv = argv;\n+\tcp.env = local_repo_env;\n+\tcp.use_shell = 1;\n+\n+\tif (pipe_command(&cp, input, sizeof(input), NULL, 0, NULL, 0)) {\n+\t\tdie(\"failed to load blob %s\", sha1_to_hex(sha1));\n+\t}\n+\n+\t/*\n+\t * The command above may have updated packfiles, so update our record\n+\t * of them.\n+\t */\n+\treprepare_packed_git();\n+}\n+\n int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi, unsigned flags)\n {\n \tstruct cached_object *co;\n \tstruct pack_entry e;\n \tint rtype;\n \tconst unsigned char *real = lookup_replace_object_extended(sha1, flags);\n+\tint already_retried = 0;\n \n \tco = find_cached_object(real);\n \tif (co) {\n@@ -3000,19 +3114,35 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n \t\treturn 0;\n \t}\n \n-\tif (!find_pack_entry(real, &e)) {\n-\t\t/* Most likely it's a loose object. */\n-\t\tif (!sha1_loose_object_info(real, oi, flags)) {\n-\t\t\toi->whence = OI_LOOSE;\n-\t\t\treturn 0;\n-\t\t}\n+retry:\n+\tif (find_pack_entry(real, &e))\n+\t\tgoto found_packed;\n \n-\t\t/* Not a loose object; someone else may have just packed it. */\n-\t\treprepare_packed_git();\n-\t\tif (!find_pack_entry(real, &e))\n-\t\t\treturn -1;\n+\t/* Most likely it's a loose object. */\n+\tif (!sha1_loose_object_info(real, oi, flags)) {\n+\t\toi->whence = OI_LOOSE;\n+\t\treturn 0;\n+\t}\n+\n+\t/* Not a loose object; someone else may have just packed it. */\n+\treprepare_packed_git();\n+\tif (find_pack_entry(real, &e))\n+\t\tgoto found_packed;\n+\n+\t/* Try the missing blobs */\n+\tif (!already_retried) {\n+\t\tensure_missing_blob_configured();\n+\t\tif (missing_blob_command &&\n+\t\t    in_missing_blob_manifest(real, NULL)) {\n+\t\t\talready_retried = 1;\n+\t\t\thandle_missing_blob(real);\n+\t\t\tgoto retry;\n+\t\t}\n \t}\n \n+\treturn -1;\n+\n+found_packed:\n \trtype = packed_object_info(e.p, e.offset, oi);\n \tif (rtype < 0) {\n \t\tmark_bad_packed_object(e.p, real);\n@@ -3475,17 +3605,30 @@ int has_sha1_pack(const unsigned char *sha1)\n int has_sha1_file_with_flags(const unsigned char *sha1, int flags)\n {\n \tstruct pack_entry e;\n+\tint already_retried = 0;\n \n \tif (!startup_info->have_repository)\n \t\treturn 0;\n+retry:\n \tif (find_pack_entry(sha1, &e))\n \t\treturn 1;\n \tif (has_loose_object(sha1))\n \t\treturn 1;\n-\tif (flags & HAS_SHA1_QUICK)\n-\t\treturn 0;\n-\treprepare_packed_git();\n-\treturn find_pack_entry(sha1, &e);\n+\tif (!(flags & HAS_SHA1_QUICK)) {\n+\t\treprepare_packed_git();\n+\t\tif (find_pack_entry(sha1, &e))\n+\t\t\treturn 1;\n+\t}\n+\tif (!already_retried) {\n+\t\tensure_missing_blob_configured();\n+\t\tif (missing_blob_command &&\n+\t\t    in_missing_blob_manifest(sha1, NULL)) {\n+\t\t\talready_retried = 1;\n+\t\t\thandle_missing_blob(sha1);\n+\t\t\tgoto retry;\n+\t\t}\n+\t}\n+\treturn 0;\n }\n \n int has_object_file(const struct object_id *oid)\ndiff --git a/t/t3907-missing-blob.sh b/t/t3907-missing-blob.sh\nnew file mode 100755\nindex 000000000..7962414cb\n--- /dev/null\n+++ b/t/t3907-missing-blob.sh\n@@ -0,0 +1,69 @@\n+#!/bin/sh\n+\n+test_description='core.missingblobcommand option'\n+\n+. ./test-lib.sh\n+\n+pack () {\n+\tperl -e '$/ = undef; $input = <>; print pack(\"H*\", $input)'\n+}\n+\n+test_expect_success 'sha1_object_info_extended and read_sha1_file (through git cat-file -p)' '\n+\trm -rf server client &&\n+\n+\tgit init server &&\n+\ttest_commit -C server 1 &&\n+\ttest_config -C server uploadpack.allowanysha1inwant 1 &&\n+\tHASH=$(git hash-object server/1.t) &&\n+\n+\tgit init client &&\n+\ttest_config -C client core.missingblobcommand \\\n+\t\t\"git -C \\\"$(pwd)/server\\\" pack-objects --stdout | git unpack-objects\" &&\n+\n+\t# does not work if missing blob is not registered\n+\ttest_must_fail git -C client cat-file -p \"$HASH\" &&\n+\n+\tmkdir -p client/.git/objects/missing &&\n+\tprintf \"%016x%s%016x\" 1 \"$HASH\" \"$(wc -c <server/1.t)\" |\n+\t\tpack >client/.git/objects/missing/x &&\n+\n+\t# works when missing blob is registered\n+\tgit -C client cat-file -p \"$HASH\"\n+'\n+\n+test_expect_success 'has_sha1_file (through git cat-file -e)' '\n+\trm -rf server client &&\n+\n+\tgit init server &&\n+\ttest_commit -C server 1 &&\n+\ttest_config -C server uploadpack.allowanysha1inwant 1 &&\n+\tHASH=$(git hash-object server/1.t) &&\n+\n+\tgit init client &&\n+\ttest_config -C client core.missingblobcommand \\\n+\t\t\"git -C \\\"$(pwd)/server\\\" pack-objects --stdout | git unpack-objects\" &&\n+\tmkdir -p client/.git/objects/missing &&\n+\tprintf \"%016x%s%016x\" 1 \"$HASH\" \"$(wc -c <server/1.t)\" |\n+\t\tpack >client/.git/objects/missing/x &&\n+\tgit -C client cat-file -e \"$HASH\"\n+'\n+\n+test_expect_success 'fsck' '\n+\trm -rf server client &&\n+\n+\tgit init server &&\n+\ttest_commit -C server 1 &&\n+\ttest_config -C server uploadpack.allowanysha1inwant 1 &&\n+\tHASH=$(git hash-object server/1.t) &&\n+\techo hash is $HASH &&\n+\n+\tcp -r server client &&\n+\ttest_config -C client core.missingblobcommand \"this-command-is-not-actually-run\" &&\n+\tmkdir -p client/.git/objects/missing &&\n+\tprintf \"%016x%s%016x\" 1 \"$HASH\" \"$(wc -c <server/1.t)\" |\n+\t\tpack >client/.git/objects/missing/x &&\n+\trm client/.git/objects/$(echo $HASH | cut -c1-2)/$(echo $HASH | cut -c3-40) &&\n+\tgit -C client fsck\n+'\n+\n+test_done\n-- \n2.13.1.518.g3df882009-goog\n\n"},{"id":"322390","messageId":"d78ecc7f35c88b3145ac55c845a3fd756b0fd5ad.1497558850.git.jonathantanmy@google.com","threadId":"46193","inReplyTo":"cover.1497558850.git.jonathantanmy@google.com","subject":"[PATCH v3 3/4] sha1_file: consolidate storage-agnostic object fns","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-06-15T20:39:30Z","receivedAt":"2017-06-15T20:40:08Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"In sha1_file.c, there are a few functions that provide information on an\nobject regardless of its storage (cached, loose, or packed). Looking\nthrough all non-static functions in sha1_file.c that take in an unsigned\nchar * pointer, the relevant ones are:\n - sha1_object_info_extended\n - sha1_object_info (auto-fixed by sha1_object_info_extended)\n - read_sha1_file_extended (uses read_object)\n - read_object_with_reference (auto-fixed by read_sha1_file_extended)\n - has_sha1_file_with_flags\n - assert_sha1_type (auto-fixed by sha1_object_info)\n\nLooking at the 3 primary functions (sha1_object_info_extended,\nread_object, has_sha1_file_with_flags), they independently implement\nmechanisms such as object replacement, retrying the packed store after\nfailing to find the object in the packed store then the loose store, and\nbeing able to mark a packed object as bad and then retrying the whole\nprocess. Consolidating these mechanisms would be a great help to\nmaintainability.\n\nHowever, has_sha1_file_with_flags() does things that the other 2 don't\n(skipping cached storage, allowing a \"quick\" mode that skips retrying\nthe packed storage after trying the loose storage, and refreshing any\nloose files found).\n\nTherefore, consolidate only the other 2 functions by extending\nsha1_object_info_extended() to support the functionality needed, and\nthen modifying read_object() to use sha1_object_info_extended().\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n cache.h     |  1 +\n sha1_file.c | 84 ++++++++++++++++++++++++++++++-------------------------------\n 2 files changed, 43 insertions(+), 42 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 4d92aae0e..63a73af17 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1835,6 +1835,7 @@ struct object_info {\n \toff_t *disk_sizep;\n \tunsigned char *delta_base_sha1;\n \tstruct strbuf *typename;\n+\tvoid **contentp;\n \n \t/* Response */\n \tenum {\ndiff --git a/sha1_file.c b/sha1_file.c\nindex a38319443..60b487c70 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2005,19 +2005,6 @@ int parse_sha1_header(const char *hdr, unsigned long *sizep)\n \treturn parse_sha1_header_extended(hdr, &oi, LOOKUP_REPLACE_OBJECT);\n }\n \n-static void *unpack_sha1_file(void *map, unsigned long mapsize, enum object_type *type, unsigned long *size, const unsigned char *sha1)\n-{\n-\tint ret;\n-\tgit_zstream stream;\n-\tchar hdr[8192];\n-\n-\tret = unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr));\n-\tif (ret < Z_OK || (*type = parse_sha1_header(hdr, size)) < 0)\n-\t\treturn NULL;\n-\n-\treturn unpack_sha1_rest(&stream, hdr, *size, sha1);\n-}\n-\n unsigned long get_size_from_delta(struct packed_git *p,\n \t\t\t\t  struct pack_window **w_curs,\n \t\t\t          off_t curpos)\n@@ -2326,8 +2313,10 @@ static void *cache_or_unpack_entry(struct packed_git *p, off_t base_offset,\n \tif (!ent)\n \t\treturn unpack_entry(p, base_offset, type, base_size);\n \n-\t*type = ent->type;\n-\t*base_size = ent->size;\n+\tif (type)\n+\t\t*type = ent->type;\n+\tif (base_size)\n+\t\t*base_size = ent->size;\n \treturn xmemdupz(ent->data, ent->size);\n }\n \n@@ -2388,9 +2377,16 @@ int packed_object_info(struct packed_git *p, off_t obj_offset,\n \t * We always get the representation type, but only convert it to\n \t * a \"real\" type later if the caller is interested.\n \t */\n-\ttype = unpack_object_header(p, &w_curs, &curpos, &size);\n+\tif (oi->contentp) {\n+\t\t*oi->contentp = cache_or_unpack_entry(p, obj_offset, oi->sizep,\n+\t\t\t\t\t\t      &type);\n+\t\tif (!*oi->contentp)\n+\t\t\ttype = OBJ_BAD;\n+\t} else {\n+\t\ttype = unpack_object_header(p, &w_curs, &curpos, &size);\n+\t}\n \n-\tif (oi->sizep) {\n+\tif (!oi->contentp && oi->sizep) {\n \t\tif (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {\n \t\t\toff_t tmp_pos = curpos;\n \t\t\toff_t base_offset = get_delta_base(p, &w_curs, &tmp_pos,\n@@ -2679,8 +2675,10 @@ void *unpack_entry(struct packed_git *p, off_t obj_offset,\n \t\tfree(external_base);\n \t}\n \n-\t*final_type = type;\n-\t*final_size = size;\n+\tif (final_type)\n+\t\t*final_type = type;\n+\tif (final_size)\n+\t\t*final_size = size;\n \n \tunuse_pack(&w_curs);\n \n@@ -2914,6 +2912,7 @@ static int sha1_loose_object_info(const unsigned char *sha1,\n \tgit_zstream stream;\n \tchar hdr[32];\n \tstruct strbuf hdrbuf = STRBUF_INIT;\n+\tunsigned long size_scratch;\n \n \tif (oi->delta_base_sha1)\n \t\thashclr(oi->delta_base_sha1);\n@@ -2926,7 +2925,7 @@ static int sha1_loose_object_info(const unsigned char *sha1,\n \t * return value implicitly indicates whether the\n \t * object even exists.\n \t */\n-\tif (!oi->typep && !oi->typename && !oi->sizep) {\n+\tif (!oi->typep && !oi->typename && !oi->sizep && !oi->contentp) {\n \t\tconst char *path;\n \t\tstruct stat st;\n \t\tif (stat_sha1_file(sha1, &st, &path) < 0)\n@@ -2939,6 +2938,10 @@ static int sha1_loose_object_info(const unsigned char *sha1,\n \tmap = map_sha1_file(sha1, &mapsize);\n \tif (!map)\n \t\treturn -1;\n+\n+\tif (!oi->sizep)\n+\t\toi->sizep = &size_scratch;\n+\n \tif (oi->disk_sizep)\n \t\t*oi->disk_sizep = mapsize;\n \tif ((flags & LOOKUP_UNKNOWN_OBJECT)) {\n@@ -2956,10 +2959,18 @@ static int sha1_loose_object_info(const unsigned char *sha1,\n \t\t\t\t       sha1_to_hex(sha1));\n \t} else if ((status = parse_sha1_header_extended(hdr, oi, flags)) < 0)\n \t\tstatus = error(\"unable to parse %s header\", sha1_to_hex(sha1));\n-\tgit_inflate_end(&stream);\n+\n+\tif (status >= 0 && oi->contentp)\n+\t\t*oi->contentp = unpack_sha1_rest(&stream, hdr,\n+\t\t\t\t\t\t *oi->sizep, sha1);\n+\telse\n+\t\tgit_inflate_end(&stream);\n+\n \tmunmap(map, mapsize);\n \tif (status && oi->typep)\n \t\t*oi->typep = status;\n+\tif (oi->sizep == &size_scratch)\n+\t\toi->sizep = NULL;\n \tstrbuf_release(&hdrbuf);\n \treturn (status < 0) ? status : 0;\n }\n@@ -2983,6 +2994,8 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n \t\t\thashclr(oi->delta_base_sha1);\n \t\tif (oi->typename)\n \t\t\tstrbuf_addstr(oi->typename, typename(co->type));\n+\t\tif (oi->contentp)\n+\t\t\t*oi->contentp = xmemdupz(co->buf, co->size);\n \t\toi->whence = OI_CACHED;\n \t\treturn 0;\n \t}\n@@ -3075,28 +3088,15 @@ int pretend_sha1_file(void *buf, unsigned long len, enum object_type type,\n static void *read_object(const unsigned char *sha1, enum object_type *type,\n \t\t\t unsigned long *size)\n {\n-\tunsigned long mapsize;\n-\tvoid *map, *buf;\n-\tstruct cached_object *co;\n-\n-\tco = find_cached_object(sha1);\n-\tif (co) {\n-\t\t*type = co->type;\n-\t\t*size = co->size;\n-\t\treturn xmemdupz(co->buf, co->size);\n-\t}\n+\tstruct object_info oi = OBJECT_INFO_INIT;\n+\tvoid *content;\n+\toi.typep = type;\n+\toi.sizep = size;\n+\toi.contentp = &content;\n \n-\tbuf = read_packed_sha1(sha1, type, size);\n-\tif (buf)\n-\t\treturn buf;\n-\tmap = map_sha1_file(sha1, &mapsize);\n-\tif (map) {\n-\t\tbuf = unpack_sha1_file(map, mapsize, type, size, sha1);\n-\t\tmunmap(map, mapsize);\n-\t\treturn buf;\n-\t}\n-\treprepare_packed_git();\n-\treturn read_packed_sha1(sha1, type, size);\n+\tif (sha1_object_info_extended(sha1, &oi, 0))\n+\t\treturn NULL;\n+\treturn content;\n }\n \n /*\n-- \n2.13.1.518.g3df882009-goog\n\n"},{"id":"322391","messageId":"691edc66828b6f377862c8e3c982753dbcf85c0b.1497558850.git.jonathantanmy@google.com","threadId":"46193","inReplyTo":"cover.1497558850.git.jonathantanmy@google.com","subject":"[PATCH v3 2/4] sha1_file: move delta base cache code up","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-06-15T20:39:29Z","receivedAt":"2017-06-15T20:40:12Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"In a subsequent patch, packed_object_info() will be modified to use the\ndelta base cache, so move the relevant code to before\npacked_object_info().\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n sha1_file.c | 220 ++++++++++++++++++++++++++++++------------------------------\n 1 file changed, 110 insertions(+), 110 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex a52b27541..a38319443 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2239,116 +2239,6 @@ static enum object_type packed_to_object_type(struct packed_git *p,\n \tgoto out;\n }\n \n-int packed_object_info(struct packed_git *p, off_t obj_offset,\n-\t\t       struct object_info *oi)\n-{\n-\tstruct pack_window *w_curs = NULL;\n-\tunsigned long size;\n-\toff_t curpos = obj_offset;\n-\tenum object_type type;\n-\n-\t/*\n-\t * We always get the representation type, but only convert it to\n-\t * a \"real\" type later if the caller is interested.\n-\t */\n-\ttype = unpack_object_header(p, &w_curs, &curpos, &size);\n-\n-\tif (oi->sizep) {\n-\t\tif (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {\n-\t\t\toff_t tmp_pos = curpos;\n-\t\t\toff_t base_offset = get_delta_base(p, &w_curs, &tmp_pos,\n-\t\t\t\t\t\t\t   type, obj_offset);\n-\t\t\tif (!base_offset) {\n-\t\t\t\ttype = OBJ_BAD;\n-\t\t\t\tgoto out;\n-\t\t\t}\n-\t\t\t*oi->sizep = get_size_from_delta(p, &w_curs, tmp_pos);\n-\t\t\tif (*oi->sizep == 0) {\n-\t\t\t\ttype = OBJ_BAD;\n-\t\t\t\tgoto out;\n-\t\t\t}\n-\t\t} else {\n-\t\t\t*oi->sizep = size;\n-\t\t}\n-\t}\n-\n-\tif (oi->disk_sizep) {\n-\t\tstruct revindex_entry *revidx = find_pack_revindex(p, obj_offset);\n-\t\t*oi->disk_sizep = revidx[1].offset - obj_offset;\n-\t}\n-\n-\tif (oi->typep || oi->typename) {\n-\t\tenum object_type ptot;\n-\t\tptot = packed_to_object_type(p, obj_offset, type, &w_curs,\n-\t\t\t\t\t     curpos);\n-\t\tif (oi->typep)\n-\t\t\t*oi->typep = ptot;\n-\t\tif (oi->typename) {\n-\t\t\tconst char *tn = typename(ptot);\n-\t\t\tif (tn)\n-\t\t\t\tstrbuf_addstr(oi->typename, tn);\n-\t\t}\n-\t\tif (ptot < 0) {\n-\t\t\ttype = OBJ_BAD;\n-\t\t\tgoto out;\n-\t\t}\n-\t}\n-\n-\tif (oi->delta_base_sha1) {\n-\t\tif (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {\n-\t\t\tconst unsigned char *base;\n-\n-\t\t\tbase = get_delta_base_sha1(p, &w_curs, curpos,\n-\t\t\t\t\t\t   type, obj_offset);\n-\t\t\tif (!base) {\n-\t\t\t\ttype = OBJ_BAD;\n-\t\t\t\tgoto out;\n-\t\t\t}\n-\n-\t\t\thashcpy(oi->delta_base_sha1, base);\n-\t\t} else\n-\t\t\thashclr(oi->delta_base_sha1);\n-\t}\n-\n-out:\n-\tunuse_pack(&w_curs);\n-\treturn type;\n-}\n-\n-static void *unpack_compressed_entry(struct packed_git *p,\n-\t\t\t\t    struct pack_window **w_curs,\n-\t\t\t\t    off_t curpos,\n-\t\t\t\t    unsigned long size)\n-{\n-\tint st;\n-\tgit_zstream stream;\n-\tunsigned char *buffer, *in;\n-\n-\tbuffer = xmallocz_gently(size);\n-\tif (!buffer)\n-\t\treturn NULL;\n-\tmemset(&stream, 0, sizeof(stream));\n-\tstream.next_out = buffer;\n-\tstream.avail_out = size + 1;\n-\n-\tgit_inflate_init(&stream);\n-\tdo {\n-\t\tin = use_pack(p, w_curs, curpos, &stream.avail_in);\n-\t\tstream.next_in = in;\n-\t\tst = git_inflate(&stream, Z_FINISH);\n-\t\tif (!stream.avail_out)\n-\t\t\tbreak; /* the payload is larger than it should be */\n-\t\tcurpos += stream.next_in - in;\n-\t} while (st == Z_OK || st == Z_BUF_ERROR);\n-\tgit_inflate_end(&stream);\n-\tif ((st != Z_STREAM_END) || stream.total_out != size) {\n-\t\tfree(buffer);\n-\t\treturn NULL;\n-\t}\n-\n-\treturn buffer;\n-}\n-\n static struct hashmap delta_base_cache;\n static size_t delta_base_cached;\n \n@@ -2486,6 +2376,116 @@ static void add_delta_base_cache(struct packed_git *p, off_t base_offset,\n \thashmap_add(&delta_base_cache, ent);\n }\n \n+int packed_object_info(struct packed_git *p, off_t obj_offset,\n+\t\t       struct object_info *oi)\n+{\n+\tstruct pack_window *w_curs = NULL;\n+\tunsigned long size;\n+\toff_t curpos = obj_offset;\n+\tenum object_type type;\n+\n+\t/*\n+\t * We always get the representation type, but only convert it to\n+\t * a \"real\" type later if the caller is interested.\n+\t */\n+\ttype = unpack_object_header(p, &w_curs, &curpos, &size);\n+\n+\tif (oi->sizep) {\n+\t\tif (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {\n+\t\t\toff_t tmp_pos = curpos;\n+\t\t\toff_t base_offset = get_delta_base(p, &w_curs, &tmp_pos,\n+\t\t\t\t\t\t\t   type, obj_offset);\n+\t\t\tif (!base_offset) {\n+\t\t\t\ttype = OBJ_BAD;\n+\t\t\t\tgoto out;\n+\t\t\t}\n+\t\t\t*oi->sizep = get_size_from_delta(p, &w_curs, tmp_pos);\n+\t\t\tif (*oi->sizep == 0) {\n+\t\t\t\ttype = OBJ_BAD;\n+\t\t\t\tgoto out;\n+\t\t\t}\n+\t\t} else {\n+\t\t\t*oi->sizep = size;\n+\t\t}\n+\t}\n+\n+\tif (oi->disk_sizep) {\n+\t\tstruct revindex_entry *revidx = find_pack_revindex(p, obj_offset);\n+\t\t*oi->disk_sizep = revidx[1].offset - obj_offset;\n+\t}\n+\n+\tif (oi->typep || oi->typename) {\n+\t\tenum object_type ptot;\n+\t\tptot = packed_to_object_type(p, obj_offset, type, &w_curs,\n+\t\t\t\t\t     curpos);\n+\t\tif (oi->typep)\n+\t\t\t*oi->typep = ptot;\n+\t\tif (oi->typename) {\n+\t\t\tconst char *tn = typename(ptot);\n+\t\t\tif (tn)\n+\t\t\t\tstrbuf_addstr(oi->typename, tn);\n+\t\t}\n+\t\tif (ptot < 0) {\n+\t\t\ttype = OBJ_BAD;\n+\t\t\tgoto out;\n+\t\t}\n+\t}\n+\n+\tif (oi->delta_base_sha1) {\n+\t\tif (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {\n+\t\t\tconst unsigned char *base;\n+\n+\t\t\tbase = get_delta_base_sha1(p, &w_curs, curpos,\n+\t\t\t\t\t\t   type, obj_offset);\n+\t\t\tif (!base) {\n+\t\t\t\ttype = OBJ_BAD;\n+\t\t\t\tgoto out;\n+\t\t\t}\n+\n+\t\t\thashcpy(oi->delta_base_sha1, base);\n+\t\t} else\n+\t\t\thashclr(oi->delta_base_sha1);\n+\t}\n+\n+out:\n+\tunuse_pack(&w_curs);\n+\treturn type;\n+}\n+\n+static void *unpack_compressed_entry(struct packed_git *p,\n+\t\t\t\t    struct pack_window **w_curs,\n+\t\t\t\t    off_t curpos,\n+\t\t\t\t    unsigned long size)\n+{\n+\tint st;\n+\tgit_zstream stream;\n+\tunsigned char *buffer, *in;\n+\n+\tbuffer = xmallocz_gently(size);\n+\tif (!buffer)\n+\t\treturn NULL;\n+\tmemset(&stream, 0, sizeof(stream));\n+\tstream.next_out = buffer;\n+\tstream.avail_out = size + 1;\n+\n+\tgit_inflate_init(&stream);\n+\tdo {\n+\t\tin = use_pack(p, w_curs, curpos, &stream.avail_in);\n+\t\tstream.next_in = in;\n+\t\tst = git_inflate(&stream, Z_FINISH);\n+\t\tif (!stream.avail_out)\n+\t\t\tbreak; /* the payload is larger than it should be */\n+\t\tcurpos += stream.next_in - in;\n+\t} while (st == Z_OK || st == Z_BUF_ERROR);\n+\tgit_inflate_end(&stream);\n+\tif ((st != Z_STREAM_END) || stream.total_out != size) {\n+\t\tfree(buffer);\n+\t\treturn NULL;\n+\t}\n+\n+\treturn buffer;\n+}\n+\n static void *read_object(const unsigned char *sha1, enum object_type *type,\n \t\t\t unsigned long *size);\n \n-- \n2.13.1.518.g3df882009-goog\n\n"}]}