{"thread":{"id":"46475","subject":"[RFC PATCH 0/4] Some patches for fsck for missing objects","startedAt":"2017-07-26T23:30:11Z","lastAt":"2017-08-08T20:20:48Z","messageCount":40,"participants":["Jonathan Tan","brian m. carlson","Stefan Beller","Junio C Hamano","Ben Peart","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"325163","messageId":"cover.1501111615.git.jonathantanmy@google.com","threadId":"46475","inReplyTo":null,"subject":"[RFC PATCH 0/4] Some patches for fsck for missing objects","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-07-26T23:29:58Z","receivedAt":"2017-07-26T23:30:11Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"This is an updated version of my previous patch [1], but without the\nlist of promises. It is somewhat different now because fsck cannot just\nmark all promised objects as HAS_OBJ.\n\nThis could be adapted and incorporated into patch sets that have objects\nmissing from the local repo.\n\nI split this up into 4 patches so that you can see how the changes in\nfsck are being tested, but eventually these should probably be combined\ninto 1 patch.\n\n[1] https://public-inbox.org/git/3420d9ae9ef86b78af1abe721891233e3f5865a2.1500508695.git.jonathantanmy@google.com/\n\nJonathan Tan (4):\n environment, fsck: introduce lazyobject extension\n fsck: support refs pointing to lazy objects\n fsck: support referenced lazy objects\n fsck: support lazy objects as CLI argument\n\n Documentation/technical/repository-version.txt | 7 ++\n builtin/fsck.c | 23 ++++++-\n cache.h | 2 +\n environment.c | 1 +\n setup.c | 7 +-\n t/t0410-lazy-object.sh | 95 ++++++++++++++++++++++++++\n 6 files changed, 133 insertions(+), 2 deletions(-)\n create mode 100755 t/t0410-lazy-object.sh\n\n-- \n2.14.0.rc0.400.g1c36432dff-goog\n\n"},{"id":"325164","messageId":"75766ee26264e50b7fcb3c7a8cc5808907586799.1501111615.git.jonathantanmy@google.com","threadId":"46475","inReplyTo":"cover.1501111615.git.jonathantanmy@google.com","subject":"[RFC PATCH 1/4] environment, fsck: introduce lazyobject extension","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-07-26T23:29:59Z","receivedAt":"2017-07-26T23:30:14Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Currently, Git does not support repos with very large numbers of objects\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 referenced object is available somewhere in the\nrepo storage.\n\nIntroduce a new repository extension option \"extensions.lazyobject\", of\ndata type string. If this is set in a repository, Git will tolerate\nobjects being missing in that repository. When Git needs those objects,\nit will invoke the command in that option.\n\nTeach fsck about the new state of affairs. In this commit, teach fsck\nthat missing objects referenced from the reflog are not an error case;\nin future commits, fsck will be taught about other cases.\n\nSigned-off-by: Jonathan Tan \n---\n Documentation/technical/repository-version.txt | 7 ++++++\n builtin/fsck.c | 2 +-\n cache.h | 2 ++\n environment.c | 1 +\n setup.c | 7 +++++-\n t/t0410-lazy-object.sh | 32 ++++++++++++++++++++++++++\n 6 files changed, 49 insertions(+), 2 deletions(-)\n create mode 100755 t/t0410-lazy-object.sh\n\ndiff --git a/Documentation/technical/repository-version.txt b/Documentation/technical/repository-version.txt\nindex 00ad37986..39e362543 100644\n--- a/Documentation/technical/repository-version.txt\n+++ b/Documentation/technical/repository-version.txt\n@@ -86,3 +86,10 @@ for testing format-1 compatibility.\n When the config key `extensions.preciousObjects` is set to `true`,\n objects in the repository MUST NOT be deleted (e.g., by `git-prune` or\n `git repack -d`).\n+\n+`lazyObject`\n+~~~~~~~~~~~~~~~~~\n+\n+When the config key `extensions.lazyObject` is set to a string, Git\n+tolerates objects being missing in the repository. This string contains\n+the command to be run whenever a missing object is needed.\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex fb0947009..1cfb8d98c 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -402,7 +402,7 @@ static void fsck_handle_reflog_oid(const char *refname, struct object_id *oid,\n \t\t\t\t\txstrfmt(\"%s@{%\"PRItime\"}\", refname, timestamp));\n \t\t\tobj->flags |= USED;\n \t\t\tmark_object_reachable(obj);\n-\t\t} else {\n+\t\t} else if (!repository_format_lazy_object) {\n \t\t\terror(\"%s: invalid reflog entry %s\", refname, oid_to_hex(oid));\n \t\t\terrors_found |= ERROR_REACHABLE;\n \t\t}\ndiff --git a/cache.h b/cache.h\nindex 6c8242340..9e6bc0a21 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -853,10 +853,12 @@ extern int grafts_replace_parents;\n #define GIT_REPO_VERSION 0\n #define GIT_REPO_VERSION_READ 1\n extern int repository_format_precious_objects;\n+extern char *repository_format_lazy_object;\n \n struct repository_format {\n \tint version;\n \tint precious_objects;\n+\tchar *lazy_object;\n \tint is_bare;\n \tchar *work_tree;\n \tstruct string_list unknown_extensions;\ndiff --git a/environment.c b/environment.c\nindex 3fd4b1084..cd8ef2897 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -27,6 +27,7 @@ int warn_ambiguous_refs = 1;\n int warn_on_object_refname_ambiguity = 1;\n int ref_paranoia = -1;\n int repository_format_precious_objects;\n+char *repository_format_lazy_object;\n const char *git_commit_encoding;\n const char *git_log_output_encoding;\n const char *apply_default_whitespace;\ndiff --git a/setup.c b/setup.c\nindex 860507e1f..94cfde3cc 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -425,7 +425,11 @@ static int check_repo_format(const char *var, const char *value, void *vdata)\n \t\t\t;\n \t\telse if (!strcmp(ext, \"preciousobjects\"))\n \t\t\tdata->precious_objects = git_config_bool(var, value);\n-\t\telse\n+\t\telse if (!strcmp(ext, \"lazyobject\")) {\n+\t\t\tif (!value)\n+\t\t\t\treturn config_error_nonbool(var);\n+\t\t\tdata->lazy_object = xstrdup(value);\n+\t\t} else\n \t\t\tstring_list_append(&data->unknown_extensions, ext);\n \t} else if (strcmp(var, \"core.bare\") == 0) {\n \t\tdata->is_bare = git_config_bool(var, value);\n@@ -468,6 +472,7 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok)\n \t}\n \n \trepository_format_precious_objects = candidate.precious_objects;\n+\trepository_format_lazy_object = candidate.lazy_object;\n \tstring_list_clear(&candidate.unknown_extensions, 0);\n \tif (!has_common) {\n \t\tif (candidate.is_bare != -1) {\ndiff --git a/t/t0410-lazy-object.sh b/t/t0410-lazy-object.sh\nnew file mode 100755\nindex 000000000..36442531f\n--- /dev/null\n+++ b/t/t0410-lazy-object.sh\n@@ -0,0 +1,32 @@\n+#!/bin/sh\n+\n+test_description='lazy object'\n+\n+. ./test-lib.sh\n+\n+delete_object () {\n+\trm $1/.git/objects/$(echo $2 | cut -c1-2)/$(echo $2 | cut -c3-40)\n+}\n+\n+test_expect_success 'fsck fails on lazy object in reflog' '\n+\ttest_create_repo repo &&\n+\ttest_commit -C repo 1 &&\n+\n+\tA=$(git -C repo commit-tree -m a HEAD^{tree}) &&\n+\tB=$(git -C repo commit-tree -m b HEAD^{tree}) &&\n+\n+\t# Reference $A only from reflog, and delete it\n+\tgit -C repo branch mybranch \"$A\" &&\n+\tgit -C repo branch -f mybranch \"$B\" &&\n+\tdelete_object repo \"$A\" &&\n+\n+\ttest_must_fail git -C repo fsck\n+'\n+\n+test_expect_success '...but succeeds if lazyobject is set' '\n+\tgit -C repo config core.repositoryformatversion 1 &&\n+\tgit -C repo config extensions.lazyobject \"arbitrary string\" &&\n+\tgit -C repo fsck\n+'\n+\n+test_done\n-- \n2.14.0.rc0.400.g1c36432dff-goog\n\n"},{"id":"325165","messageId":"4c8bc7ab543d8d0d60719f65bd8a150c66123056.1501111615.git.jonathantanmy@google.com","threadId":"46475","inReplyTo":"cover.1501111615.git.jonathantanmy@google.com","subject":"[RFC PATCH 2/4] fsck: support refs pointing to lazy objects","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-07-26T23:30:00Z","receivedAt":"2017-07-26T23:30:15Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Teach fsck to not treat refs with missing targets as an error when\nextensions.lazyobject is set.\n\nFor the purposes of warning about no default refs, such refs are still\ntreated as legitimate refs.\n\nSigned-off-by: Jonathan Tan \n---\n builtin/fsck.c | 8 ++++++++\n t/t0410-lazy-object.sh | 20 ++++++++++++++++++++\n 2 files changed, 28 insertions(+)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 1cfb8d98c..e29ff760b 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -438,6 +438,14 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid,\n \n \tobj = parse_object(oid);\n \tif (!obj) {\n+\t\tif (repository_format_lazy_object) {\n+\t\t\t/*\n+\t\t\t * Increment default_refs anyway, because this is a\n+\t\t\t * valid ref.\n+\t\t\t */\n+\t\t\tdefault_refs++;\n+\t\t\treturn 0;\n+\t\t}\n \t\terror(\"%s: invalid sha1 pointer %s\", refname, oid_to_hex(oid));\n \t\terrors_found |= ERROR_REACHABLE;\n \t\t/* We'll continue with the rest despite the error.. */\ndiff --git a/t/t0410-lazy-object.sh b/t/t0410-lazy-object.sh\nindex 36442531f..00e1b4a88 100755\n--- a/t/t0410-lazy-object.sh\n+++ b/t/t0410-lazy-object.sh\n@@ -29,4 +29,24 @@ test_expect_success '...but succeeds if lazyobject is set' '\n \tgit -C repo fsck\n '\n \n+test_expect_success 'fsck fails on lazy object pointed to by ref' '\n+\trm -rf repo &&\n+\ttest_create_repo repo &&\n+\ttest_commit -C repo 1 &&\n+\n+\tA=$(git -C repo commit-tree -m a HEAD^{tree}) &&\n+\n+\t# Reference $A only from ref, and delete it\n+\tgit -C repo branch mybranch \"$A\" &&\n+\tdelete_object repo \"$A\" &&\n+\n+\ttest_must_fail git -C repo fsck\n+'\n+\n+test_expect_success '...but succeeds if lazyobject is set' '\n+\tgit -C repo config core.repositoryformatversion 1 &&\n+\tgit -C repo config extensions.lazyobject \"arbitrary string\" &&\n+\tgit -C repo fsck\n+'\n+\n test_done\n-- \n2.14.0.rc0.400.g1c36432dff-goog\n\n"},{"id":"325166","messageId":"1d841f0e4188f0ae9fa3c238c58d3e1b5fb232e8.1501111615.git.jonathantanmy@google.com","threadId":"46475","inReplyTo":"cover.1501111615.git.jonathantanmy@google.com","subject":"[RFC PATCH 3/4] fsck: support referenced lazy objects","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-07-26T23:30:01Z","receivedAt":"2017-07-26T23:30:19Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Teach fsck to not treat missing objects indirectly pointed to by refs as\nan error when extensions.lazyobject is set.\n\nSigned-off-by: Jonathan Tan \n---\n builtin/fsck.c | 11 +++++++++++\n t/t0410-lazy-object.sh | 27 +++++++++++++++++++++++++++\n 2 files changed, 38 insertions(+)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex e29ff760b..238532cc2 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -149,6 +149,15 @@ static int mark_object(struct object *obj, int type, void *data, struct fsck_opt\n \t\treturn 0;\n \tobj->flags |= REACHABLE;\n \tif (!(obj->flags & HAS_OBJ)) {\n+\t\tif (repository_format_lazy_object)\n+\t\t\t/*\n+\t\t\t * Return immediately; this is not an error, and further\n+\t\t\t * recursion does not need to be performed on this\n+\t\t\t * object since it is missing (so it does not need to be\n+\t\t\t * added to \"pending\").\n+\t\t\t */\n+\t\t\treturn 0;\n+\n \t\tif (parent && !has_object_file(&obj->oid)) {\n \t\t\tprintf(\"broken link from %7s %s\\n\",\n \t\t\t\t printable_type(parent), describe_object(parent));\n@@ -212,6 +221,8 @@ static void check_reachable_object(struct object *obj)\n \t * do a full fsck\n \t */\n \tif (!(obj->flags & HAS_OBJ)) {\n+\t\tif (repository_format_lazy_object)\n+\t\t\treturn;\n \t\tif (has_sha1_pack(obj->oid.hash))\n \t\t\treturn; /* it is in pack - forget about it */\n \t\tprintf(\"missing %s %s\\n\", printable_type(obj),\ndiff --git a/t/t0410-lazy-object.sh b/t/t0410-lazy-object.sh\nindex 00e1b4a88..45f665a15 100755\n--- a/t/t0410-lazy-object.sh\n+++ b/t/t0410-lazy-object.sh\n@@ -49,4 +49,31 @@ test_expect_success '...but succeeds if lazyobject is set' '\n \tgit -C repo fsck\n '\n \n+test_expect_success 'fsck fails on lazy object indirectly pointed to by ref' '\n+\trm -rf repo &&\n+\ttest_create_repo repo &&\n+\ttest_commit -C repo 1 &&\n+\ttest_commit -C repo 2 &&\n+\ttest_commit -C repo 3 &&\n+\tgit -C repo tag -a annotated_tag -m \"annotated tag\" &&\n+\n+\tC=$(git -C repo rev-parse 1) &&\n+\tT=$(git -C repo rev-parse 2^{tree}) &&\n+\tB=$(git hash-object repo/3.t) &&\n+\tAT=$(git -C repo rev-parse annotated_tag) &&\n+\n+\t# missing commit, tree, blob, and tag\n+\tdelete_object repo \"$C\" &&\n+\tdelete_object repo \"$T\" &&\n+\tdelete_object repo \"$B\" &&\n+\tdelete_object repo \"$AT\" &&\n+\ttest_must_fail git -C repo fsck\n+'\n+\n+test_expect_success '...but succeeds if lazyobject is set' '\n+\tgit -C repo config core.repositoryformatversion 1 &&\n+\tgit -C repo config extensions.lazyobject \"arbitrary string\" &&\n+\tgit -C repo fsck\n+'\n+\n test_done\n-- \n2.14.0.rc0.400.g1c36432dff-goog\n\n"},{"id":"325167","messageId":"926ff80cb9b5dc9d0d75257f261ddc27c1d250b7.1501111615.git.jonathantanmy@google.com","threadId":"46475","inReplyTo":"cover.1501111615.git.jonathantanmy@google.com","subject":"[RFC PATCH 4/4] fsck: support lazy objects as CLI argument","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-07-26T23:30:02Z","receivedAt":"2017-07-26T23:30:22Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Teach fsck to not treat missing objects provided on the CLI as an error\nwhen extensions.lazyobject is set.\n\nSigned-off-by: Jonathan Tan \n---\n builtin/fsck.c | 2 ++\n t/t0410-lazy-object.sh | 16 ++++++++++++++++\n 2 files changed, 18 insertions(+)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 238532cc2..592e64172 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -755,6 +755,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)\n \t\t\tstruct object *obj = lookup_object(oid.hash);\n \n \t\t\tif (!obj || !(obj->flags & HAS_OBJ)) {\n+\t\t\t\tif (repository_format_lazy_object)\n+\t\t\t\t\tcontinue;\n \t\t\t\terror(\"%s: object missing\", oid_to_hex(&oid));\n \t\t\t\terrors_found |= ERROR_OBJECT;\n \t\t\t\tcontinue;\ndiff --git a/t/t0410-lazy-object.sh b/t/t0410-lazy-object.sh\nindex 45f665a15..3ac61c1c5 100755\n--- a/t/t0410-lazy-object.sh\n+++ b/t/t0410-lazy-object.sh\n@@ -76,4 +76,20 @@ test_expect_success '...but succeeds if lazyobject is set' '\n \tgit -C repo fsck\n '\n \n+test_expect_success 'fsck fails on lazy object directly given in command-line' '\n+\trm -rf repo &&\n+\ttest_create_repo repo &&\n+\ttest_commit -C repo 1 &&\n+\tHASH=$(git hash-object repo/1.t) &&\n+\tdelete_object repo \"$HASH\" &&\n+\n+\ttest_must_fail git -C repo fsck \"$HASH\"\n+'\n+\n+test_expect_success '...but succeeds if lazyobject is set' '\n+\tgit -C repo config core.repositoryformatversion 1 &&\n+\tgit -C repo config extensions.lazyobject \"arbitrary string\" &&\n+\tgit -C repo fsck \"$HASH\"\n+'\n+\n test_done\n-- \n2.14.0.rc0.400.g1c36432dff-goog\n\n"},{"id":"325169","messageId":"20170726234238.2osugw2lheubmetw@genre.crustytoothpaste.net","threadId":"46475","inReplyTo":"cover.1501111615.git.jonathantanmy@google.com","subject":"Re: [RFC PATCH 0/4] Some patches for fsck for missing objects","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2017-07-26T23:42:38Z","receivedAt":"2017-07-26T23:42:48Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Wed, Jul 26, 2017 at 04:29:58PM -0700, Jonathan Tan wrote:\n> This is an updated version of my previous patch [1], but without the\n> list of promises. It is somewhat different now because fsck cannot just\n> mark all promised objects as HAS_OBJ.\n> \n> This could be adapted and incorporated into patch sets that have objects\n> missing from the local repo.\n> \n> I split this up into 4 patches so that you can see how the changes in\n> fsck are being tested, but eventually these should probably be combined\n> into 1 patch.\n\nI looked at this and I like the direction it's going. It's pretty\nsimple and straightforward, which I also like.\n\nWhat I'd recommend is that instead of making lazyObject a string, we\nmake it an integer representing a protocol version. We then add a\ndifferent config setting that is the actual program to invoke, using the\ngiven protocol version. This lets us change the way we speak to the\ntool without breaking backwards compatibility, and it also allows us to\nsimply check the lazyObject script for supported protocols up front.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\nhttps://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"325175","messageId":"CAGZ79kbLVor39Hs+OnhiFwXOpJtTGMiENvunCVv_Q-PB-Mfczw@mail.gmail.com","threadId":"46475","inReplyTo":"20170726234238.2osugw2lheubmetw@genre.crustytoothpaste.net","subject":"Re: [RFC PATCH 0/4] Some patches for fsck for missing objects","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-27T00:24:50Z","receivedAt":"2017-07-27T00:24:55Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Jul 26, 2017 at 4:42 PM, brian m. carlson\n wrote:\n> On Wed, Jul 26, 2017 at 04:29:58PM -0700, Jonathan Tan wrote:\n>> This is an updated version of my previous patch [1], but without the\n>> list of promises. It is somewhat different now because fsck cannot just\n>> mark all promised objects as HAS_OBJ.\n>>\n>> This could be adapted and incorporated into patch sets that have objects\n>> missing from the local repo.\n>>\n>> I split this up into 4 patches so that you can see how the changes in\n>> fsck are being tested, but eventually these should probably be combined\n>> into 1 patch.\n>\n> I looked at this and I like the direction it's going. It's pretty\n> simple and straightforward, which I also like.\n\nI agree.\n"},{"id":"325195","messageId":"20170727102547.0c45a8db@twelve2.svl.corp.google.com","threadId":"46475","inReplyTo":"20170726234238.2osugw2lheubmetw@genre.crustytoothpaste.net","subject":"Re: [RFC PATCH 0/4] Some patches for fsck for missing objects","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-07-27T17:25:47Z","receivedAt":"2017-07-27T17:25:57Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Wed, 26 Jul 2017 23:42:38 +0000\n\"brian m. carlson\" wrote:\n\n> I looked at this and I like the direction it's going. It's pretty\n> simple and straightforward, which I also like.\n\nThanks.\n\n> What I'd recommend is that instead of making lazyObject a string, we\n> make it an integer representing a protocol version. We then add a\n> different config setting that is the actual program to invoke, using the\n> given protocol version. This lets us change the way we speak to the\n> tool without breaking backwards compatibility, and it also allows us to\n> simply check the lazyObject script for supported protocols up front.\n\nThat's possible too. As for version negotiation, I think we'll end up\nusing a protocol similar to the clean/smudge long-running process\nprotocol (as documented as gitattributes), so that does not need to be\ntaken care of here, but making lazyObject be the version integer is fine\ntoo.\n"},{"id":"325202","messageId":"xmqqzibpn1zh.fsf@gitster.mtv.corp.google.com","threadId":"46475","inReplyTo":"75766ee26264e50b7fcb3c7a8cc5808907586799.1501111615.git.jonathantanmy@google.com","subject":"Re: [RFC PATCH 1/4] environment, fsck: introduce lazyobject extension","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-27T18:55:46Z","receivedAt":"2017-07-27T18:56:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> Currently, Git does not support repos with very large numbers of objects\n> or repos that wish to minimize manipulation of certain blobs (for\n> example, because they are very large) very well, even if the user\n> operates mostly on part of the repo, because Git is designed on the\n> assumption that every referenced object is available somewhere in the\n> repo storage.\n>\n> Introduce a new repository extension option \"extensions.lazyobject\", of\n> data type string. If this is set in a repository, Git will tolerate\n> objects being missing in that repository. When Git needs those objects,\n> it will invoke the command in that option.\n\nMy reading hiccupped after the first sentence, as the problem\ndescription made it sound like this was a boolean (\"are we using\nlazy object feature?\"), after reading \"data type string\". And then\n\"the command in that option\" made me hiccup one more time, as it did\nnot \"click\" that \"in that option\" was trying to say that the string\nis used as the command name (or is it a whole command line? The\nleading part of the command line to which some arguments are\nappended before it gets invoked as a command? or what?).\n\nLogically, I think it is more like\n\n - extensions.lazyobject can be set to tell Git to consider missing\n objects in certain cases are not errors;\n\n - the value of extensions.lazyobject variable must be a string,\n which is used to name the command to lazily make the object\n \"appear\" in the repository on demand.\n\n> Teach fsck about the new state of affairs. In this commit, teach fsck\n> that missing objects referenced from the reflog are not an error case;\n> in future commits, fsck will be taught about other cases.\n\nIn any case, sounds like a small and good first step.\n\n> +\n> +`lazyObject`\n> +~~~~~~~~~~~~~~~~~\n> +\n> +When the config key `extensions.lazyObject` is set to a string, Git\n> +tolerates objects being missing in the repository. This string contains\n> +the command to be run whenever a missing object is needed.\n\nThis has the same issue but to a lessor degree as there is \"string\ncontains\". What the command will do (e.g. \"makes the object\nmagically appear in the object store\" or \"emits the contents of the\nobject to its standard output, so that Git can hash it to make it\nappear in the object store\"), and how it is used (e.g. \"this is a\nsingle command name and nothing else\", \"this is a leading part of\ncommand line and arguments are appended at the end, before the whole\nthing is passed to the shell to be executed\", etc.) will need to be\nclarified in the final version of the series (not necessarily at\nthis step---the series can elaborate in the later patches).\n\n> diff --git a/builtin/fsck.c b/builtin/fsck.c\n> index fb0947009..1cfb8d98c 100644\n> --- a/builtin/fsck.c\n> +++ b/builtin/fsck.c\n> @@ -402,7 +402,7 @@ static void fsck_handle_reflog_oid(const char *refname, struct object_id *oid,\n> \t\t\t\t\txstrfmt(\"%s@{%\"PRItime\"}\", refname, timestamp));\n> \t\t\tobj->flags |= USED;\n> \t\t\tmark_object_reachable(obj);\n> -\t\t} else {\n> +\t\t} else if (!repository_format_lazy_object) {\n> \t\t\terror(\"%s: invalid reflog entry %s\", refname, oid_to_hex(oid));\n> \t\t\terrors_found |= ERROR_REACHABLE;\n> \t\t}\n> diff --git a/cache.h b/cache.h\n> index 6c8242340..9e6bc0a21 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -853,10 +853,12 @@ extern int grafts_replace_parents;\n> #define GIT_REPO_VERSION 0\n> #define GIT_REPO_VERSION_READ 1\n> extern int repository_format_precious_objects;\n> +extern char *repository_format_lazy_object;\n\nThis is not a new problem, but I think these two should be\ncalled repository_extension_$NAME not repository_format_$NAME.\n\n> diff --git a/t/t0410-lazy-object.sh b/t/t0410-lazy-object.sh\n> new file mode 100755\n> index 000000000..36442531f\n> --- /dev/null\n> +++ b/t/t0410-lazy-object.sh\n> @@ -0,0 +1,32 @@\n> +#!/bin/sh\n> +\n> +test_description='lazy object'\n> +\n> +. ./test-lib.sh\n> +\n> +delete_object () {\n> +\trm $1/.git/objects/$(echo $2 | cut -c1-2)/$(echo $2 | cut -c3-40)\n> +}\n> +\n> +test_expect_success 'fsck fails on lazy object in reflog' '\n> +\ttest_create_repo repo &&\n> +\ttest_commit -C repo 1 &&\n> +\n> +\tA=$(git -C repo commit-tree -m a HEAD^{tree}) &&\n> +\tB=$(git -C repo commit-tree -m b HEAD^{tree}) &&\n> +\n> +\t# Reference $A only from reflog, and delete it\n> +\tgit -C repo branch mybranch \"$A\" &&\n> +\tgit -C repo branch -f mybranch \"$B\" &&\n> +\tdelete_object repo \"$A\" &&\n> +\n> +\ttest_must_fail git -C repo fsck\n> +'\n> +test_expect_success '...but succeeds if lazyobject is set' '\n> +\tgit -C repo config core.repositoryformatversion 1 &&\n> +\tgit -C repo config extensions.lazyobject \"arbitrary string\" &&\n> +\tgit -C repo fsck\n> +'\n> +\n> +test_done\n"},{"id":"325204","messageId":"xmqqvamdn1ss.fsf@gitster.mtv.corp.google.com","threadId":"46475","inReplyTo":"4c8bc7ab543d8d0d60719f65bd8a150c66123056.1501111615.git.jonathantanmy@google.com","subject":"Re: [RFC PATCH 2/4] fsck: support refs pointing to lazy objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-27T18:59:47Z","receivedAt":"2017-07-27T19:00:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> Teach fsck to not treat refs with missing targets as an error when\n> extensions.lazyobject is set.\n>\n> For the purposes of warning about no default refs, such refs are still\n> treated as legitimate refs.\n>\n> Signed-off-by: Jonathan Tan \n> ---\n> builtin/fsck.c | 8 ++++++++\n> t/t0410-lazy-object.sh | 20 ++++++++++++++++++++\n> 2 files changed, 28 insertions(+)\n>\n> diff --git a/builtin/fsck.c b/builtin/fsck.c\n> index 1cfb8d98c..e29ff760b 100644\n> --- a/builtin/fsck.c\n> +++ b/builtin/fsck.c\n> @@ -438,6 +438,14 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid,\n> \n> \tobj = parse_object(oid);\n> \tif (!obj) {\n> +\t\tif (repository_format_lazy_object) {\n> +\t\t\t/*\n> +\t\t\t * Increment default_refs anyway, because this is a\n> +\t\t\t * valid ref.\n> +\t\t\t */\n> +\t\t\tdefault_refs++;\n> +\t\t\treturn 0;\n> +\t\t}\n> \t\terror(\"%s: invalid sha1 pointer %s\", refname, oid_to_hex(oid));\n> \t\terrors_found |= ERROR_REACHABLE;\n\nAt this point, do we know (or can we tell) if this is a missing\nobject or a file exists as a loose object but is corrupt? If we\ncould, it would be nice to do this only for the former to avoid\nsweeping a real corruption that is unrelated to the lazy fetch under\nthe rug.\n\n> +test_expect_success 'fsck fails on lazy object pointed to by ref' '\n> +\trm -rf repo &&\n> +\ttest_create_repo repo &&\n> +\ttest_commit -C repo 1 &&\n> +\n> +\tA=$(git -C repo commit-tree -m a HEAD^{tree}) &&\n> +\n> +\t# Reference $A only from ref, and delete it\n> +\tgit -C repo branch mybranch \"$A\" &&\n> +\tdelete_object repo \"$A\" &&\n> +\n> +\ttest_must_fail git -C repo fsck\n> +'\n\nAnd a new test that uses a helper different from delete_object\n(perhaps call it corrupt_object?) can be used to make sure that we\ncomplain in that case here.\n\n> +test_expect_success '...but succeeds if lazyobject is set' '\n> +\tgit -C repo config core.repositoryformatversion 1 &&\n> +\tgit -C repo config extensions.lazyobject \"arbitrary string\" &&\n> +\tgit -C repo fsck\n> +'\n> +\n> test_done\n"},{"id":"325206","messageId":"xmqqpocln0z2.fsf@gitster.mtv.corp.google.com","threadId":"46475","inReplyTo":"1d841f0e4188f0ae9fa3c238c58d3e1b5fb232e8.1501111615.git.jonathantanmy@google.com","subject":"Re: [RFC PATCH 3/4] fsck: support referenced lazy objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-27T19:17:37Z","receivedAt":"2017-07-27T19:17:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> Teach fsck to not treat missing objects indirectly pointed to by refs as\n> an error when extensions.lazyobject is set.\n>\n> Signed-off-by: Jonathan Tan \n> ---\n> builtin/fsck.c | 11 +++++++++++\n> t/t0410-lazy-object.sh | 27 +++++++++++++++++++++++++++\n> 2 files changed, 38 insertions(+)\n>\n> diff --git a/builtin/fsck.c b/builtin/fsck.c\n> index e29ff760b..238532cc2 100644\n> --- a/builtin/fsck.c\n> +++ b/builtin/fsck.c\n> @@ -149,6 +149,15 @@ static int mark_object(struct object *obj, int type, void *data, struct fsck_opt\n> \t\treturn 0;\n> \tobj->flags |= REACHABLE;\n> \tif (!(obj->flags & HAS_OBJ)) {\n> +\t\tif (repository_format_lazy_object)\n> +\t\t\t/*\n> +\t\t\t * Return immediately; this is not an error, and further\n> +\t\t\t * recursion does not need to be performed on this\n> +\t\t\t * object since it is missing (so it does not need to be\n> +\t\t\t * added to \"pending\").\n> +\t\t\t */\n> +\t\t\treturn 0;\n> +\n\nThe same comment as 2/4 applies here.\n\n> @@ -212,6 +221,8 @@ static void check_reachable_object(struct object *obj)\n> \t * do a full fsck\n> \t */\n> \tif (!(obj->flags & HAS_OBJ)) {\n> +\t\tif (repository_format_lazy_object)\n> +\t\t\treturn;\n> \t\tif (has_sha1_pack(obj->oid.hash))\n> \t\t\treturn; /* it is in pack - forget about it */\n> \t\tprintf(\"missing %s %s\\n\", printable_type(obj),\n\nAlso this reminds as a related issue. Imagine:\n\n - An object X was once retrieved, perhaps but not necessarily\n lazily, together with another object Y that is referred to by X\n (e.g. X is a tree, Y is a blob in the directory at path D, which\n is represented by X).\n\n - The same blob Y is added to the index in a different directory at\n path E.\n\n - The user decides to make this a slimmed-down \"narrow clone\" style\n repository and tells Git that path D is not interesting. We lose\n X, but not Y because Y is still referenced from the index.\n\n - \"git reset --hard\" happens, and there no longer is any reference\n to Y.\n\nNow, when we run fsck, should we diagnose Y as \"unreachable and/or\ndangling\"?\n"},{"id":"325211","messageId":"20170727165045.6b8da3aa@twelve2.svl.corp.google.com","threadId":"46475","inReplyTo":"xmqqpocln0z2.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC PATCH 3/4] fsck: support referenced lazy objects","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-07-27T23:50:45Z","receivedAt":"2017-07-27T23:50:55Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Thu, 27 Jul 2017 12:17:37 -0700\nJunio C Hamano wrote:\n\n> The same comment as 2/4 applies here.\n\nNoted - whatever the resolution is, I'll apply it to all the patches.\n> \n> > @@ -212,6 +221,8 @@ static void check_reachable_object(struct object *obj)\n> > \t * do a full fsck\n> > \t */\n> > \tif (!(obj->flags & HAS_OBJ)) {\n> > +\t\tif (repository_format_lazy_object)\n> > +\t\t\treturn;\n> > \t\tif (has_sha1_pack(obj->oid.hash))\n> > \t\t\treturn; /* it is in pack - forget about it */\n> > \t\tprintf(\"missing %s %s\\n\", printable_type(obj),\n> \n> Also this reminds as a related issue. Imagine:\n> \n> - An object X was once retrieved, perhaps but not necessarily\n> lazily, together with another object Y that is referred to by X\n> (e.g. X is a tree, Y is a blob in the directory at path D, which\n> is represented by X).\n> \n> - The same blob Y is added to the index in a different directory at\n> path E.\n> \n> - The user decides to make this a slimmed-down \"narrow clone\" style\n> repository and tells Git that path D is not interesting. We lose\n> X, but not Y because Y is still referenced from the index.\n> \n> - \"git reset --hard\" happens, and there no longer is any reference\n> to Y.\n> \n> Now, when we run fsck, should we diagnose Y as \"unreachable and/or\n> dangling\"?\n\nI would say yes (or whatever happens in the case where we re-fetch into\na shallow clone).\n\nCome to think of it...\"git reset --hard\" always has the potential to\ncreate unreachable objects, right (regardless of whether it's a \"shallow\nclone\" or \"narrow clone\" or ordinary clone)?\n"},{"id":"325212","messageId":"20170727165058.4eda505e@twelve2.svl.corp.google.com","threadId":"46475","inReplyTo":"xmqqvamdn1ss.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC PATCH 2/4] fsck: support refs pointing to lazy objects","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-07-27T23:50:58Z","receivedAt":"2017-07-27T23:51:07Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Thu, 27 Jul 2017 11:59:47 -0700\nJunio C Hamano wrote:\n\n> > @@ -438,6 +438,14 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid,\n> > \n> > \tobj = parse_object(oid);\n> > \tif (!obj) {\n> > +\t\tif (repository_format_lazy_object) {\n> > +\t\t\t/*\n> > +\t\t\t * Increment default_refs anyway, because this is a\n> > +\t\t\t * valid ref.\n> > +\t\t\t */\n> > +\t\t\tdefault_refs++;\n> > +\t\t\treturn 0;\n> > +\t\t}\n> > \t\terror(\"%s: invalid sha1 pointer %s\", refname, oid_to_hex(oid));\n> > \t\terrors_found |= ERROR_REACHABLE;\n> \n> At this point, do we know (or can we tell) if this is a missing\n> object or a file exists as a loose object but is corrupt? If we\n> could, it would be nice to do this only for the former to avoid\n> sweeping a real corruption that is unrelated to the lazy fetch under\n> the rug.\n\nBefore all this is run, there is a check over all loose and packed\nobjects and I've verified that this check reports failure in\ncorrupt-object situations (see test below).\n\nIt is true that parse_object() cannot report the difference, but since\nfsck has already verified non-corruptness, I don't think it needs to\nknow the difference at this point.\n\n> > +test_expect_success 'fsck fails on lazy object pointed to by ref' '\n> > +\trm -rf repo &&\n> > +\ttest_create_repo repo &&\n> > +\ttest_commit -C repo 1 &&\n> > +\n> > +\tA=$(git -C repo commit-tree -m a HEAD^{tree}) &&\n> > +\n> > +\t# Reference $A only from ref, and delete it\n> > +\tgit -C repo branch mybranch \"$A\" &&\n> > +\tdelete_object repo \"$A\" &&\n> > +\n> > +\ttest_must_fail git -C repo fsck\n> > +'\n> \n> And a new test that uses a helper different from delete_object\n> (perhaps call it corrupt_object?) can be used to make sure that we\n> complain in that case here.\n\nI agree that object corruption can cause this specific part of the\nproduction code to falsely work. But I think that this specific part of\nthe code can and should rely on object corruption being checked\nelsewhere. (I usually don't like to assume that other components work\nand will continue to work, but in this case, I think that fsck checking\nfor object corruption is very foundational and should be relied upon.)\n\nBut if we think that defense \"in depth\" is a good idea, I have no\nproblem adding such tests (like the one below).\n\n---\ndelete_object () {\n\trm $1/.git/objects/$(echo $2 | cut -c1-2)/$(echo $2 | cut -c3-40)\n}\n\ncorrupt_object () {\n\tchmod a+w $1/.git/objects/$(echo $2 | cut -c1-2)/$(echo $2 | cut -c3-40) &&\n\techo CORRUPT >$1/.git/objects/$(echo $2 | cut -c1-2)/$(echo $2 | cut -c3-40)\n}\n\nsetup_object_in_reflog () {\n\trm -rf repo &&\n\ttest_create_repo repo &&\n\ttest_commit -C repo 1 &&\n\n\tA=$(git -C repo commit-tree -m a HEAD^{tree}) &&\n\tB=$(git -C repo commit-tree -m b HEAD^{tree}) &&\n\n\t# Reference $A only from reflog\n\tgit -C repo branch mybranch \"$A\" &&\n\tgit -C repo branch -f mybranch \"$B\"\n}\n\ntest_expect_success 'lazy object in reflog' '\n\tsetup_object_in_reflog &&\n\tdelete_object repo \"$A\" &&\n\ttest_must_fail git -C repo fsck &&\n\tgit -C repo config core.repositoryformatversion 1 &&\n\tgit -C repo config extensions.lazyobject \"arbitrary string\" &&\n\tgit -C repo fsck\n'\n\ntest_expect_success 'corrupt loose object in reflog' '\n\tsetup_object_in_reflog &&\n\tcorrupt_object repo \"$A\" &&\n\ttest_must_fail git -C repo fsck &&\n\tgit -C repo config core.repositoryformatversion 1 &&\n\tgit -C repo config extensions.lazyobject \"arbitrary string\" &&\n\ttest_must_fail git -C repo fsck\n'\n\ntest_expect_success 'missing packed object in reflog' '\n\tsetup_object_in_reflog &&\n\tgit -C repo repack -a &&\n\tdelete_object repo \"$A\" &&\n\tchmod a+w repo/.git/objects/pack/*.pack &&\n\techo CORRUPT >\"$(echo repo/.git/objects/pack/*.pack)\" &&\n\ttest_must_fail git -C repo fsck &&\n\tgit -C repo config core.repositoryformatversion 1 &&\n\tgit -C repo config extensions.lazyobject \"arbitrary string\" &&\n\ttest_must_fail git -C repo fsck\n'\n"},{"id":"325216","messageId":"df0402d4-860a-762a-a993-5b125369bf50@gmail.com","threadId":"46475","inReplyTo":"xmqqzibpn1zh.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC PATCH 1/4] environment, fsck: introduce lazyobject extension","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-07-28T13:20:17Z","receivedAt":"2017-07-28T13:20:27Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 7/27/2017 2:55 PM, Junio C Hamano wrote:\n> Jonathan Tan writes:\n> \n>> Currently, Git does not support repos with very large numbers of objects\n>> or repos that wish to minimize manipulation of certain blobs (for\n>> example, because they are very large) very well, even if the user\n>> operates mostly on part of the repo, because Git is designed on the\n>> assumption that every referenced object is available somewhere in the\n>> repo storage.\n>>\n\nI very much like the direction this is taking. Handling missing objects \ngracefully is an important part of the overall partial clone support.\n\n>> Introduce a new repository extension option \"extensions.lazyobject\", of\n>> data type string. If this is set in a repository, Git will tolerate\n>> objects being missing in that repository. When Git needs those objects,\n>> it will invoke the command in that option.\n> \n\nI'm very glad you are doing this. Having never used precious objects I \nwas unaware of the extensions concept but it looks like a great way to \ndeal with this repo specific option.\n\n> My reading hiccupped after the first sentence, as the problem\n> description made it sound like this was a boolean (\"are we using\n> lazy object feature?\"), after reading \"data type string\". And then\n> \"the command in that option\" made me hiccup one more time, as it did\n> not \"click\" that \"in that option\" was trying to say that the string\n> is used as the command name (or is it a whole command line? The\n> leading part of the command line to which some arguments are\n> appended before it gets invoked as a command? or what?).\n> \n> Logically, I think it is more like\n> \n> - extensions.lazyobject can be set to tell Git to consider missing\n> objects in certain cases are not errors;\n> \n> - the value of extensions.lazyobject variable must be a string,\n> which is used to name the command to lazily make the object\n> \"appear\" in the repository on demand.\n> \n>> Teach fsck about the new state of affairs. In this commit, teach fsck\n>> that missing objects referenced from the reflog are not an error case;\n>> in future commits, fsck will be taught about other cases.\n> \n> In any case, sounds like a small and good first step.\n> \n\nI agree completely with the feedback on the description. That is quite \nthe run on sentence. :)\n\n>> +\n>> +`lazyObject`\n>> +~~~~~~~~~~~~~~~~~\n>> +\n>> +When the config key `extensions.lazyObject` is set to a string, Git\n>> +tolerates objects being missing in the repository. This string contains\n>> +the command to be run whenever a missing object is needed.\n> \n> This has the same issue but to a lessor degree as there is \"string\n> contains\". What the command will do (e.g. \"makes the object\n> magically appear in the object store\" or \"emits the contents of the\n> object to its standard output, so that Git can hash it to make it\n> appear in the object store\"), and how it is used (e.g. \"this is a\n> single command name and nothing else\", \"this is a leading part of\n> command line and arguments are appended at the end, before the whole\n> thing is passed to the shell to be executed\", etc.) will need to be\n> clarified in the final version of the series (not necessarily at\n> this step---the series can elaborate in the later patches).\n> \n>> diff --git a/builtin/fsck.c b/builtin/fsck.c\n>> index fb0947009..1cfb8d98c 100644\n>> --- a/builtin/fsck.c\n>> +++ b/builtin/fsck.c\n>> @@ -402,7 +402,7 @@ static void fsck_handle_reflog_oid(const char *refname, struct object_id *oid,\n>> \t\t\t\t\txstrfmt(\"%s@{%\"PRItime\"}\", refname, timestamp));\n>> \t\t\tobj->flags |= USED;\n>> \t\t\tmark_object_reachable(obj);\n>> -\t\t} else {\n>> +\t\t} else if (!repository_format_lazy_object) {\n>> \t\t\terror(\"%s: invalid reflog entry %s\", refname, oid_to_hex(oid));\n>> \t\t\terrors_found |= ERROR_REACHABLE;\n>> \t\t}\n>> diff --git a/cache.h b/cache.h\n>> index 6c8242340..9e6bc0a21 100644\n>> --- a/cache.h\n>> +++ b/cache.h\n>> @@ -853,10 +853,12 @@ extern int grafts_replace_parents;\n>> #define GIT_REPO_VERSION 0\n>> #define GIT_REPO_VERSION_READ 1\n>> extern int repository_format_precious_objects;\n>> +extern char *repository_format_lazy_object;\n> \n> This is not a new problem, but I think these two should be\n> called repository_extension_$NAME not repository_format_$NAME.\n> \n>> diff --git a/t/t0410-lazy-object.sh b/t/t0410-lazy-object.sh\n>> new file mode 100755\n>> index 000000000..36442531f\n>> --- /dev/null\n>> +++ b/t/t0410-lazy-object.sh\n>> @@ -0,0 +1,32 @@\n>> +#!/bin/sh\n>> +\n>> +test_description='lazy object'\n>> +\n>> +. ./test-lib.sh\n>> +\n>> +delete_object () {\n>> +\trm $1/.git/objects/$(echo $2 | cut -c1-2)/$(echo $2 | cut -c3-40)\n>> +}\n>> +\n>> +test_expect_success 'fsck fails on lazy object in reflog' '\n>> +\ttest_create_repo repo &&\n>> +\ttest_commit -C repo 1 &&\n>> +\n>> +\tA=$(git -C repo commit-tree -m a HEAD^{tree}) &&\n>> +\tB=$(git -C repo commit-tree -m b HEAD^{tree}) &&\n>> +\n>> +\t# Reference $A only from reflog, and delete it\n>> +\tgit -C repo branch mybranch \"$A\" &&\n>> +\tgit -C repo branch -f mybranch \"$B\" &&\n>> +\tdelete_object repo \"$A\" &&\n>> +\n>> +\ttest_must_fail git -C repo fsck\n>> +'\n>> +test_expect_success '...but succeeds if lazyobject is set' '\n>> +\tgit -C repo config core.repositoryformatversion 1 &&\n>> +\tgit -C repo config extensions.lazyobject \"arbitrary string\" &&\n>> +\tgit -C repo fsck\n>> +'\n>> +\n>> +test_done\n"},{"id":"325217","messageId":"dc3c12fb-38e1-7c62-0d51-cca2f9883927@gmail.com","threadId":"46475","inReplyTo":"20170727165058.4eda505e@twelve2.svl.corp.google.com","subject":"Re: [RFC PATCH 2/4] fsck: support refs pointing to lazy objects","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-07-28T13:29:31Z","receivedAt":"2017-07-28T13:29:39Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 7/27/2017 7:50 PM, Jonathan Tan wrote:\n> On Thu, 27 Jul 2017 11:59:47 -0700\n> Junio C Hamano wrote:\n> \n>>> @@ -438,6 +438,14 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid,\n>>> \n>>> \tobj = parse_object(oid);\n>>> \tif (!obj) {\n>>> +\t\tif (repository_format_lazy_object) {\n>>> +\t\t\t/*\n>>> +\t\t\t * Increment default_refs anyway, because this is a\n>>> +\t\t\t * valid ref.\n>>> +\t\t\t */\n>>> +\t\t\tdefault_refs++;\n>>> +\t\t\treturn 0;\n>>> +\t\t}\n>>> \t\terror(\"%s: invalid sha1 pointer %s\", refname, oid_to_hex(oid));\n>>> \t\terrors_found |= ERROR_REACHABLE;\n>>\n>> At this point, do we know (or can we tell) if this is a missing\n>> object or a file exists as a loose object but is corrupt? If we\n>> could, it would be nice to do this only for the former to avoid\n>> sweeping a real corruption that is unrelated to the lazy fetch under\n>> the rug.\n> \n> Before all this is run, there is a check over all loose and packed\n> objects and I've verified that this check reports failure in\n> corrupt-object situations (see test below).\n> \n> It is true that parse_object() cannot report the difference, but since\n> fsck has already verified non-corruptness, I don't think it needs to\n> know the difference at this point.\n> \n>>> +test_expect_success 'fsck fails on lazy object pointed to by ref' '\n>>> +\trm -rf repo &&\n>>> +\ttest_create_repo repo &&\n>>> +\ttest_commit -C repo 1 &&\n>>> +\n>>> +\tA=$(git -C repo commit-tree -m a HEAD^{tree}) &&\n>>> +\n>>> +\t# Reference $A only from ref, and delete it\n>>> +\tgit -C repo branch mybranch \"$A\" &&\n>>> +\tdelete_object repo \"$A\" &&\n>>> +\n>>> +\ttest_must_fail git -C repo fsck\n>>> +'\n>>\n>> And a new test that uses a helper different from delete_object\n>> (perhaps call it corrupt_object?) can be used to make sure that we\n>> complain in that case here.\n> \n> I agree that object corruption can cause this specific part of the\n> production code to falsely work. But I think that this specific part of\n> the code can and should rely on object corruption being checked\n> elsewhere. (I usually don't like to assume that other components work\n> and will continue to work, but in this case, I think that fsck checking\n> for object corruption is very foundational and should be relied upon.)\n> \n> But if we think that defense \"in depth\" is a good idea, I have no\n> problem adding such tests (like the one below).\n> \n\nIt's good to know that object corruption is already being checked \nelsewhere in fsck. I agree that you should be able to rely that as long \nas it is guaranteed to run. I'd rather see a single location in the \ncode that has the logic to detect corrupted objects rather than two \ncopies (or worse, two different versions).\n\nAre there also tests for validating the object corruption detection \ncode? If not, adding some (like below) would be great.\n\n\n> ---\n> delete_object () {\n> \trm $1/.git/objects/$(echo $2 | cut -c1-2)/$(echo $2 | cut -c3-40)\n> }\n> \n> corrupt_object () {\n> \tchmod a+w $1/.git/objects/$(echo $2 | cut -c1-2)/$(echo $2 | cut -c3-40) &&\n> \techo CORRUPT >$1/.git/objects/$(echo $2 | cut -c1-2)/$(echo $2 | cut -c3-40)\n> }\n> \n> setup_object_in_reflog () {\n> \trm -rf repo &&\n> \ttest_create_repo repo &&\n> \ttest_commit -C repo 1 &&\n> \n> \tA=$(git -C repo commit-tree -m a HEAD^{tree}) &&\n> \tB=$(git -C repo commit-tree -m b HEAD^{tree}) &&\n> \n> \t# Reference $A only from reflog\n> \tgit -C repo branch mybranch \"$A\" &&\n> \tgit -C repo branch -f mybranch \"$B\"\n> }\n> \n> test_expect_success 'lazy object in reflog' '\n> \tsetup_object_in_reflog &&\n> \tdelete_object repo \"$A\" &&\n> \ttest_must_fail git -C repo fsck &&\n> \tgit -C repo config core.repositoryformatversion 1 &&\n> \tgit -C repo config extensions.lazyobject \"arbitrary string\" &&\n> \tgit -C repo fsck\n> '\n> \n> test_expect_success 'corrupt loose object in reflog' '\n> \tsetup_object_in_reflog &&\n> \tcorrupt_object repo \"$A\" &&\n> \ttest_must_fail git -C repo fsck &&\n> \tgit -C repo config core.repositoryformatversion 1 &&\n> \tgit -C repo config extensions.lazyobject \"arbitrary string\" &&\n> \ttest_must_fail git -C repo fsck\n> '\n> \n> test_expect_success 'missing packed object in reflog' '\n> \tsetup_object_in_reflog &&\n> \tgit -C repo repack -a &&\n> \tdelete_object repo \"$A\" &&\n> \tchmod a+w repo/.git/objects/pack/*.pack &&\n> \techo CORRUPT >\"$(echo repo/.git/objects/pack/*.pack)\" &&\n> \ttest_must_fail git -C repo fsck &&\n> \tgit -C repo config core.repositoryformatversion 1 &&\n> \tgit -C repo config extensions.lazyobject \"arbitrary string\" &&\n> \ttest_must_fail git -C repo fsck\n> '\n> \n"},{"id":"325218","messageId":"8c2266ad-586b-109e-1f99-a2315aa94c75@gmail.com","threadId":"46475","inReplyTo":"20170727102547.0c45a8db@twelve2.svl.corp.google.com","subject":"Re: [RFC PATCH 0/4] Some patches for fsck for missing objects","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-07-28T13:40:50Z","receivedAt":"2017-07-28T13:40:58Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 7/27/2017 1:25 PM, Jonathan Tan wrote:\n> On Wed, 26 Jul 2017 23:42:38 +0000\n> \"brian m. carlson\" wrote:\n> \n>> I looked at this and I like the direction it's going. It's pretty\n>> simple and straightforward, which I also like.\n> \n\nditto, simple is good!\n\n> Thanks.\n> \n>> What I'd recommend is that instead of making lazyObject a string, we\n>> make it an integer representing a protocol version. We then add a\n>> different config setting that is the actual program to invoke, using the\n>> given protocol version. This lets us change the way we speak to the\n>> tool without breaking backwards compatibility, and it also allows us to\n>> simply check the lazyObject script for supported protocols up front.\n> \n> That's possible too. As for version negotiation, I think we'll end up\n> using a protocol similar to the clean/smudge long-running process\n> protocol (as documented as gitattributes), so that does not need to be\n> taken care of here, but making lazyObject be the version integer is fine\n> too.\n> \n\nI was also thinking the way to retrieve the missing objects be a \nversioned negotiation via the long-running process protocol.\n\nThat said, I'm a fan of including versions to make our life easier later \nif we decide to adopt an entirely different model for retrieving the \nmissing objects.\n"},{"id":"325227","messageId":"20170728200802.12496-1-jonathantanmy@google.com","threadId":"46475","inReplyTo":"dc3c12fb-38e1-7c62-0d51-cca2f9883927@gmail.com","subject":"[PATCH] tests: ensure fsck fails on corrupt packfiles","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-07-28T20:08:02Z","receivedAt":"2017-07-28T20:08:15Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"t1450-fsck.sh does not have a test that checks fsck's behavior when a\npackfile is invalid. It does have a test for when an object in a\npackfile is invalid, but in that test, the packfile itself is valid.\n\nAdd such a test.\n\nSigned-off-by: Jonathan Tan \n---\nI think the existing packfile corruption test (t5303) is good enough\nthat we would notice such things, but just in case we want to test fsck\nspecifically, here's a patch.\n---\n t/t1450-fsck.sh | 16 ++++++++++++++++\n 1 file changed, 16 insertions(+)\n\ndiff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\nindex bb89e1a5d..4087150db 100755\n--- a/t/t1450-fsck.sh\n+++ b/t/t1450-fsck.sh\n@@ -608,6 +608,22 @@ test_expect_success 'fsck errors in packed objects' '\n \t! grep corrupt out\n '\n \n+test_expect_success 'fsck fails on corrupt packfile' '\n+\thsh=$(git commit-tree -m mycommit HEAD^{tree}) &&\n+\tpack=$(echo $hsh | git pack-objects .git/objects/pack/pack) &&\n+\n+\t# Corrupt the first byte of the first object. (It contains 3 type bits,\n+\t# at least one of which is not zero, so setting the first byte to 0 is\n+\t# sufficient.)\n+\tchmod a+w .git/objects/pack/pack-$pack.pack &&\n+\tprintf '\\0' | dd of=.git/objects/pack/pack-$pack.pack bs=1 conv=notrunc seek=12 &&\n+\n+\ttest_when_finished \"rm -f .git/objects/pack/pack-$pack.*\" &&\n+\tremove_object $hsh &&\n+\ttest_must_fail git fsck 2>out &&\n+\ttest_i18ngrep \"checksum mismatch\" out\n+'\n+\n test_expect_success 'fsck finds problems in duplicate loose objects' '\n \trm -rf broken-duplicate &&\n \tgit init broken-duplicate &&\n-- \n2.14.0.rc0.400.g1c36432dff-goog\n\n"},{"id":"325233","messageId":"20170728165035.2be61cec@twelve2.svl.corp.google.com","threadId":"46475","inReplyTo":"xmqqzibpn1zh.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC PATCH 1/4] environment, fsck: introduce lazyobject extension","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-07-28T23:50:35Z","receivedAt":"2017-07-28T23:50:46Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Thu, 27 Jul 2017 11:55:46 -0700\nJunio C Hamano wrote:\n\n> My reading hiccupped after the first sentence, as the problem\n> description made it sound like this was a boolean (\"are we using\n> lazy object feature?\"), after reading \"data type string\". And then\n> \"the command in that option\" made me hiccup one more time, as it did\n> not \"click\" that \"in that option\" was trying to say that the string\n> is used as the command name (or is it a whole command line? The\n> leading part of the command line to which some arguments are\n> appended before it gets invoked as a command? or what?).\n> \n> Logically, I think it is more like\n> \n> - extensions.lazyobject can be set to tell Git to consider missing\n> objects in certain cases are not errors;\n> \n> - the value of extensions.lazyobject variable must be a string,\n> which is used to name the command to lazily make the object\n> \"appear\" in the repository on demand.\n\nOK, I'll update the commit message in the next reroll.\n\n> > extern int repository_format_precious_objects;\n> > +extern char *repository_format_lazy_object;\n> \n> This is not a new problem, but I think these two should be\n> called repository_extension_$NAME not repository_format_$NAME.\n\nLooking at the original commit 067fbd4 (\"introduce \"preciousObjects\"\nrepository extension\", 2015-06-24), it seems that this was so named to\nbe analogous to the existing \"struct repository_format { int version;\n...}\" => \"int repository_format_version;\". The existing\nrepository_format_$NAME thus seems reasonable to me.\n"},{"id":"325234","messageId":"xmqqinicks93.fsf@gitster.mtv.corp.google.com","threadId":"46475","inReplyTo":"20170728165035.2be61cec@twelve2.svl.corp.google.com","subject":"Re: [RFC PATCH 1/4] environment, fsck: introduce lazyobject extension","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-29T00:21:12Z","receivedAt":"2017-07-29T00:21:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n>> > extern int repository_format_precious_objects;\n>> > +extern char *repository_format_lazy_object;\n>> \n>> This is not a new problem, but I think these two should be\n>> called repository_extension_$NAME not repository_format_$NAME.\n>\n> Looking at the original commit 067fbd4 (\"introduce \"preciousObjects\"\n> repository extension\", 2015-06-24), it seems that this was so named to\n> be analogous to the existing \"struct repository_format { int version;\n> ...}\" => \"int repository_format_version;\". The existing\n> repository_format_$NAME thus seems reasonable to me.\n\nOK. They smell like \"repository extension\" to me, but probably the\nfully spelled name of the concept is \"repository format extension\",\nso using the word \"format\" out of that phrase sounds OK to me.\n\nThanks.\n"},{"id":"325246","messageId":"xmqq379fkz4x.fsf@gitster.mtv.corp.google.com","threadId":"46475","inReplyTo":"1d841f0e4188f0ae9fa3c238c58d3e1b5fb232e8.1501111615.git.jonathantanmy@google.com","subject":"Re: [RFC PATCH 3/4] fsck: support referenced lazy objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-29T16:04:46Z","receivedAt":"2017-07-29T16:05:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> Teach fsck to not treat missing objects indirectly pointed to by refs as\n> an error when extensions.lazyobject is set.\n\nI forgot to mention a potential flaw in this approach in my previous\nmessage.\n\nIf you are a pure sightseer, then this is perfectly fine. The\nobject store in your local Git client working in that mode is purely\na cache, lazily populated while browsing the object store backed by\nthe source of what lazy-object \"hook\" talks with. As long as that\ncache does not give us a corrupt object, we are OK, because missing\nobjects do not matter.\n\nBut once you start using the repository as more than a sightseer,\nyou will have objects that only exist in your local \"cache\" and are\nnot yet in that backing store behind the lazy-object \"hook\". You\nneed to notice it when any of them goes corrupt or missing, or your\nnext \"git push\" to send them over to a remote location will fail by\ndefinition because you are the only one with these objects.\n\nIf we had the \"promise\" thing, then we could say that it is OK if\ntraversal terminated at a \"promised but not fetched yet\" boundary,\nbut we cannot afford the \"promise\", and more importantly, I do not\nthink \"promise\" has to be the only approach to ensure that the\nobjects that exist only in the local repository are all connected.\n\nFor example, if we know that the remote 'origin' is the actual\nbacking store lazy-object \"hook\" talks with, a validation rule to\nensure that we haven't lost any local commit is to ensure that a\ntraversal from our local branch tips down to remote-tracking\nbranches taken from 'origin' must not hit _any_ missing commit.\n\nThat covers only the commit objects. I do not know offhand if we\ncan and how we extend this concept to protect the tags, trees and\nblobs we have locally generated and haven't pushed out, but you and\nBen hopefully can come up with ways to cover them.\n\n\n\n"},{"id":"325335","messageId":"cover.1501532294.git.jonathantanmy@google.com","threadId":"46475","inReplyTo":"cover.1501111615.git.jonathantanmy@google.com","subject":"[PATCH v2 0/5] Fsck for lazy objects, and (now) actual invocation of loader","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-07-31T21:02:42Z","receivedAt":"2017-07-31T21:03:00Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Besides review changes, this patch set now includes my rewritten\nlazy-loading sha1_file patch, so you can now do this (excerpted from one\nof the tests):\n\n test_create_repo server\n test_commit -C server 1 1.t abcdefgh\n HASH=$(git hash-object server/1.t)\n \n test_create_repo client\n test_must_fail git -C client cat-file -p \"$HASH\"\n git -C client config core.repositoryformatversion 1\n git -C client config extensions.lazyobject \\\n \"\\\"$TEST_DIRECTORY/t0410/lazy-object\\\" \\\"$(pwd)/server/.git\\\"\"\n git -C client cat-file -p \"$HASH\"\n\nwith fsck still working. Also, there is no need for a list of promised\nblobs, and the long-running process protocol is being used.\n\nChanges from v1:\n - added last patch that supports lazy loading\n - clarified documentation in \"introduce lazyobject extension\" patch\n (following Junio's comments [1])\n\nAs listed in the changes above, I have rewritten my lazy-loading\nsha1_file patch to no longer use the list of promises. Also, I have\nadded documentation about the protocol used to (hopefully) the\nappropriate places.\n\nThis is a minimal implementation, hopefully enough of a foundation to be\nbuilt upon. In particular, I haven't added the environment variable to\nsuppress lazy loading, and the lazy loading protocol only supports one\nobject at a time.\n\nOther work\n----------\n\nThis differs slightly from Ben Peart's patch [2] in that the\nlazy-loading functionality is provided through a configured shell\ncommand instead of a hook shell script. I envision commands like \"git\nclone\", in the future, needing to pre-configure lazy loading, and I\nthink that it will be less surprising to the user if \"git clone\" wrote a\ndefault configuration instead of a default hook.\n\nThis also differs from Christian Couder's patch set [3] that implement a\nlarger-scale object database, in that (i) my patch set does not support\nputting objects into external databases, and (ii) my patch set requires\nthe lazy loader to make the objects available in the local repo, instead\nof allowing the objects to only be stored in the external database.\n\n[1] https://public-inbox.org/git/xmqqzibpn1zh.fsf@gitster.mtv.corp.google.com/\n[2] https://public-inbox.org/git/20170714132651.170708-2-benpeart@microsoft.com/\n[3] https://public-inbox.org/git/20170620075523.26961-1-chriscool@tuxfamily.org/\n\nJonathan Tan (5):\n environment, fsck: introduce lazyobject extension\n fsck: support refs pointing to lazy objects\n fsck: support referenced lazy objects\n fsck: support lazy objects as CLI argument\n sha1_file: support loading lazy objects\n\n Documentation/Makefile | 1 +\n Documentation/gitattributes.txt | 54 ++--------\n Documentation/gitrepository-layout.txt | 3 +\n .../technical/long-running-process-protocol.txt | 50 +++++++++\n Documentation/technical/repository-version.txt | 23 +++++\n Makefile | 1 +\n builtin/cat-file.c | 2 +\n builtin/fsck.c | 25 ++++-\n cache.h | 4 +\n environment.c | 1 +\n lazy-object.c | 80 +++++++++++++++\n lazy-object.h | 12 +++\n object.c | 7 ++\n object.h | 13 +++\n setup.c | 7 +-\n sha1_file.c | 44 +++++---\n t/t0410-lazy-object.sh | 113 +++++++++++++++++++++\n t/t0410/lazy-object | 102 +++++++++++++++++++\n 18 files changed, 478 insertions(+), 64 deletions(-)\n create mode 100644 Documentation/technical/long-running-process-protocol.txt\n create mode 100644 lazy-object.c\n create mode 100644 lazy-object.h\n create mode 100755 t/t0410-lazy-object.sh\n create mode 100755 t/t0410/lazy-object\n\n-- \n2.14.0.rc1.383.gd1ce394fe2-goog\n\n"},{"id":"325336","messageId":"ffb734d277132802bcc25baa13e8ede3490af62a.1501532294.git.jonathantanmy@google.com","threadId":"46475","inReplyTo":"cover.1501532294.git.jonathantanmy@google.com","subject":"[PATCH v2 1/5] environment, fsck: introduce lazyobject extension","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-07-31T21:02:43Z","receivedAt":"2017-07-31T21:03:02Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Currently, Git does not support repos with very large numbers of objects\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 referenced object is available somewhere in the\nrepo storage.\n\nIntroduce a new repository extension option `extensions.lazyObject`.\nWhen it is set, Git does not treat missing objects as errors. The value\nof `extensions.lazyObject` must be a string, naming the command used to\nmake a missing object appear whenever it is needed.\n\nTeach fsck about the new state of affairs. In this commit, teach fsck\nthat missing objects referenced from the reflog are not an error case;\nin future commits, fsck will be taught about other cases.\n\nSigned-off-by: Jonathan Tan \n---\n Documentation/technical/repository-version.txt | 7 ++++++\n builtin/fsck.c | 2 +-\n cache.h | 2 ++\n environment.c | 1 +\n setup.c | 7 +++++-\n t/t0410-lazy-object.sh | 32 ++++++++++++++++++++++++++\n 6 files changed, 49 insertions(+), 2 deletions(-)\n create mode 100755 t/t0410-lazy-object.sh\n\ndiff --git a/Documentation/technical/repository-version.txt b/Documentation/technical/repository-version.txt\nindex 00ad37986..f0482cae4 100644\n--- a/Documentation/technical/repository-version.txt\n+++ b/Documentation/technical/repository-version.txt\n@@ -86,3 +86,10 @@ for testing format-1 compatibility.\n When the config key `extensions.preciousObjects` is set to `true`,\n objects in the repository MUST NOT be deleted (e.g., by `git-prune` or\n `git repack -d`).\n+\n+`lazyObject`\n+~~~~~~~~~~~~~~~~~\n+\n+When the config key `extensions.lazyObject` is set, Git does not treat missing\n+objects as errors. The value of `extensions.lazyObject` must be a string,\n+naming the command used to make a missing object appear whenever it is needed.\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex a92f44818..b7e245654 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -402,7 +402,7 @@ static void fsck_handle_reflog_oid(const char *refname, struct object_id *oid,\n \t\t\t\t\txstrfmt(\"%s@{%\"PRItime\"}\", refname, timestamp));\n \t\t\tobj->flags |= USED;\n \t\t\tmark_object_reachable(obj);\n-\t\t} else {\n+\t\t} else if (!repository_format_lazy_object) {\n \t\t\terror(\"%s: invalid reflog entry %s\", refname, oid_to_hex(oid));\n \t\t\terrors_found |= ERROR_REACHABLE;\n \t\t}\ndiff --git a/cache.h b/cache.h\nindex 6c8242340..9e6bc0a21 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -853,10 +853,12 @@ extern int grafts_replace_parents;\n #define GIT_REPO_VERSION 0\n #define GIT_REPO_VERSION_READ 1\n extern int repository_format_precious_objects;\n+extern char *repository_format_lazy_object;\n \n struct repository_format {\n \tint version;\n \tint precious_objects;\n+\tchar *lazy_object;\n \tint is_bare;\n \tchar *work_tree;\n \tstruct string_list unknown_extensions;\ndiff --git a/environment.c b/environment.c\nindex 3fd4b1084..cd8ef2897 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -27,6 +27,7 @@ int warn_ambiguous_refs = 1;\n int warn_on_object_refname_ambiguity = 1;\n int ref_paranoia = -1;\n int repository_format_precious_objects;\n+char *repository_format_lazy_object;\n const char *git_commit_encoding;\n const char *git_log_output_encoding;\n const char *apply_default_whitespace;\ndiff --git a/setup.c b/setup.c\nindex 860507e1f..94cfde3cc 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -425,7 +425,11 @@ static int check_repo_format(const char *var, const char *value, void *vdata)\n \t\t\t;\n \t\telse if (!strcmp(ext, \"preciousobjects\"))\n \t\t\tdata->precious_objects = git_config_bool(var, value);\n-\t\telse\n+\t\telse if (!strcmp(ext, \"lazyobject\")) {\n+\t\t\tif (!value)\n+\t\t\t\treturn config_error_nonbool(var);\n+\t\t\tdata->lazy_object = xstrdup(value);\n+\t\t} else\n \t\t\tstring_list_append(&data->unknown_extensions, ext);\n \t} else if (strcmp(var, \"core.bare\") == 0) {\n \t\tdata->is_bare = git_config_bool(var, value);\n@@ -468,6 +472,7 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok)\n \t}\n \n \trepository_format_precious_objects = candidate.precious_objects;\n+\trepository_format_lazy_object = candidate.lazy_object;\n \tstring_list_clear(&candidate.unknown_extensions, 0);\n \tif (!has_common) {\n \t\tif (candidate.is_bare != -1) {\ndiff --git a/t/t0410-lazy-object.sh b/t/t0410-lazy-object.sh\nnew file mode 100755\nindex 000000000..36442531f\n--- /dev/null\n+++ b/t/t0410-lazy-object.sh\n@@ -0,0 +1,32 @@\n+#!/bin/sh\n+\n+test_description='lazy object'\n+\n+. ./test-lib.sh\n+\n+delete_object () {\n+\trm $1/.git/objects/$(echo $2 | cut -c1-2)/$(echo $2 | cut -c3-40)\n+}\n+\n+test_expect_success 'fsck fails on lazy object in reflog' '\n+\ttest_create_repo repo &&\n+\ttest_commit -C repo 1 &&\n+\n+\tA=$(git -C repo commit-tree -m a HEAD^{tree}) &&\n+\tB=$(git -C repo commit-tree -m b HEAD^{tree}) &&\n+\n+\t# Reference $A only from reflog, and delete it\n+\tgit -C repo branch mybranch \"$A\" &&\n+\tgit -C repo branch -f mybranch \"$B\" &&\n+\tdelete_object repo \"$A\" &&\n+\n+\ttest_must_fail git -C repo fsck\n+'\n+\n+test_expect_success '...but succeeds if lazyobject is set' '\n+\tgit -C repo config core.repositoryformatversion 1 &&\n+\tgit -C repo config extensions.lazyobject \"arbitrary string\" &&\n+\tgit -C repo fsck\n+'\n+\n+test_done\n-- \n2.14.0.rc1.383.gd1ce394fe2-goog\n\n"},{"id":"325337","messageId":"089edcd78c7ed6f3d0a6b0cdd107a20f1f9b583e.1501532294.git.jonathantanmy@google.com","threadId":"46475","inReplyTo":"cover.1501532294.git.jonathantanmy@google.com","subject":"[PATCH v2 2/5] fsck: support refs pointing to lazy objects","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-07-31T21:02:44Z","receivedAt":"2017-07-31T21:03:06Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Teach fsck to not treat refs with missing targets as an error when\nextensions.lazyobject is set.\n\nFor the purposes of warning about no default refs, such refs are still\ntreated as legitimate refs.\n\nSigned-off-by: Jonathan Tan \n---\n builtin/fsck.c | 8 ++++++++\n t/t0410-lazy-object.sh | 20 ++++++++++++++++++++\n 2 files changed, 28 insertions(+)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex b7e245654..38ed630d8 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -438,6 +438,14 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid,\n \n \tobj = parse_object(oid);\n \tif (!obj) {\n+\t\tif (repository_format_lazy_object) {\n+\t\t\t/*\n+\t\t\t * Increment default_refs anyway, because this is a\n+\t\t\t * valid ref.\n+\t\t\t */\n+\t\t\tdefault_refs++;\n+\t\t\treturn 0;\n+\t\t}\n \t\terror(\"%s: invalid sha1 pointer %s\", refname, oid_to_hex(oid));\n \t\terrors_found |= ERROR_REACHABLE;\n \t\t/* We'll continue with the rest despite the error.. */\ndiff --git a/t/t0410-lazy-object.sh b/t/t0410-lazy-object.sh\nindex 36442531f..00e1b4a88 100755\n--- a/t/t0410-lazy-object.sh\n+++ b/t/t0410-lazy-object.sh\n@@ -29,4 +29,24 @@ test_expect_success '...but succeeds if lazyobject is set' '\n \tgit -C repo fsck\n '\n \n+test_expect_success 'fsck fails on lazy object pointed to by ref' '\n+\trm -rf repo &&\n+\ttest_create_repo repo &&\n+\ttest_commit -C repo 1 &&\n+\n+\tA=$(git -C repo commit-tree -m a HEAD^{tree}) &&\n+\n+\t# Reference $A only from ref, and delete it\n+\tgit -C repo branch mybranch \"$A\" &&\n+\tdelete_object repo \"$A\" &&\n+\n+\ttest_must_fail git -C repo fsck\n+'\n+\n+test_expect_success '...but succeeds if lazyobject is set' '\n+\tgit -C repo config core.repositoryformatversion 1 &&\n+\tgit -C repo config extensions.lazyobject \"arbitrary string\" &&\n+\tgit -C repo fsck\n+'\n+\n test_done\n-- \n2.14.0.rc1.383.gd1ce394fe2-goog\n\n"},{"id":"325338","messageId":"c02229e54e5d8d4626ad6d39f223871dfa1df255.1501532294.git.jonathantanmy@google.com","threadId":"46475","inReplyTo":"cover.1501532294.git.jonathantanmy@google.com","subject":"[PATCH v2 4/5] fsck: support lazy objects as CLI argument","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-07-31T21:02:46Z","receivedAt":"2017-07-31T21:03:07Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Teach fsck to not treat missing objects provided on the CLI as an error\nwhen extensions.lazyobject is set.\n\nSigned-off-by: Jonathan Tan \n---\n builtin/fsck.c | 2 ++\n t/t0410-lazy-object.sh | 16 ++++++++++++++++\n 2 files changed, 18 insertions(+)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 19681c5b3..20415902f 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -754,6 +754,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)\n \t\t\tstruct object *obj = lookup_object(oid.hash);\n \n \t\t\tif (!obj || !(obj->flags & HAS_OBJ)) {\n+\t\t\t\tif (repository_format_lazy_object)\n+\t\t\t\t\tcontinue;\n \t\t\t\terror(\"%s: object missing\", oid_to_hex(&oid));\n \t\t\t\terrors_found |= ERROR_OBJECT;\n \t\t\t\tcontinue;\ndiff --git a/t/t0410-lazy-object.sh b/t/t0410-lazy-object.sh\nindex 45f665a15..3ac61c1c5 100755\n--- a/t/t0410-lazy-object.sh\n+++ b/t/t0410-lazy-object.sh\n@@ -76,4 +76,20 @@ test_expect_success '...but succeeds if lazyobject is set' '\n \tgit -C repo fsck\n '\n \n+test_expect_success 'fsck fails on lazy object directly given in command-line' '\n+\trm -rf repo &&\n+\ttest_create_repo repo &&\n+\ttest_commit -C repo 1 &&\n+\tHASH=$(git hash-object repo/1.t) &&\n+\tdelete_object repo \"$HASH\" &&\n+\n+\ttest_must_fail git -C repo fsck \"$HASH\"\n+'\n+\n+test_expect_success '...but succeeds if lazyobject is set' '\n+\tgit -C repo config core.repositoryformatversion 1 &&\n+\tgit -C repo config extensions.lazyobject \"arbitrary string\" &&\n+\tgit -C repo fsck \"$HASH\"\n+'\n+\n test_done\n-- \n2.14.0.rc1.383.gd1ce394fe2-goog\n\n"},{"id":"325339","messageId":"9228491818d5983fd11a53977487b97b677755e4.1501532294.git.jonathantanmy@google.com","threadId":"46475","inReplyTo":"cover.1501532294.git.jonathantanmy@google.com","subject":"[PATCH v2 3/5] fsck: support referenced lazy objects","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-07-31T21:02:45Z","receivedAt":"2017-07-31T21:03:11Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Teach fsck to not treat missing objects indirectly pointed to by refs as\nan error when extensions.lazyobject is set.\n\nSigned-off-by: Jonathan Tan \n---\n builtin/fsck.c | 11 +++++++++++\n t/t0410-lazy-object.sh | 27 +++++++++++++++++++++++++++\n 2 files changed, 38 insertions(+)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 38ed630d8..19681c5b3 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -149,6 +149,15 @@ static int mark_object(struct object *obj, int type, void *data, struct fsck_opt\n \t\treturn 0;\n \tobj->flags |= REACHABLE;\n \tif (!(obj->flags & HAS_OBJ)) {\n+\t\tif (repository_format_lazy_object)\n+\t\t\t/*\n+\t\t\t * Return immediately; this is not an error, and further\n+\t\t\t * recursion does not need to be performed on this\n+\t\t\t * object since it is missing (so it does not need to be\n+\t\t\t * added to \"pending\").\n+\t\t\t */\n+\t\t\treturn 0;\n+\n \t\tif (parent && !has_object_file(&obj->oid)) {\n \t\t\tprintf(\"broken link from %7s %s\\n\",\n \t\t\t\t printable_type(parent), describe_object(parent));\n@@ -212,6 +221,8 @@ static void check_reachable_object(struct object *obj)\n \t * do a full fsck\n \t */\n \tif (!(obj->flags & HAS_OBJ)) {\n+\t\tif (repository_format_lazy_object)\n+\t\t\treturn;\n \t\tif (has_sha1_pack(obj->oid.hash))\n \t\t\treturn; /* it is in pack - forget about it */\n \t\tprintf(\"missing %s %s\\n\", printable_type(obj),\ndiff --git a/t/t0410-lazy-object.sh b/t/t0410-lazy-object.sh\nindex 00e1b4a88..45f665a15 100755\n--- a/t/t0410-lazy-object.sh\n+++ b/t/t0410-lazy-object.sh\n@@ -49,4 +49,31 @@ test_expect_success '...but succeeds if lazyobject is set' '\n \tgit -C repo fsck\n '\n \n+test_expect_success 'fsck fails on lazy object indirectly pointed to by ref' '\n+\trm -rf repo &&\n+\ttest_create_repo repo &&\n+\ttest_commit -C repo 1 &&\n+\ttest_commit -C repo 2 &&\n+\ttest_commit -C repo 3 &&\n+\tgit -C repo tag -a annotated_tag -m \"annotated tag\" &&\n+\n+\tC=$(git -C repo rev-parse 1) &&\n+\tT=$(git -C repo rev-parse 2^{tree}) &&\n+\tB=$(git hash-object repo/3.t) &&\n+\tAT=$(git -C repo rev-parse annotated_tag) &&\n+\n+\t# missing commit, tree, blob, and tag\n+\tdelete_object repo \"$C\" &&\n+\tdelete_object repo \"$T\" &&\n+\tdelete_object repo \"$B\" &&\n+\tdelete_object repo \"$AT\" &&\n+\ttest_must_fail git -C repo fsck\n+'\n+\n+test_expect_success '...but succeeds if lazyobject is set' '\n+\tgit -C repo config core.repositoryformatversion 1 &&\n+\tgit -C repo config extensions.lazyobject \"arbitrary string\" &&\n+\tgit -C repo fsck\n+'\n+\n test_done\n-- \n2.14.0.rc1.383.gd1ce394fe2-goog\n\n"},{"id":"325340","messageId":"eadce97b6a1e80345a2621e71ce187e9e6bc05bf.1501532294.git.jonathantanmy@google.com","threadId":"46475","inReplyTo":"cover.1501532294.git.jonathantanmy@google.com","subject":"[PATCH v2 5/5] sha1_file: support loading lazy objects","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-07-31T21:02:47Z","receivedAt":"2017-07-31T21:03:15Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Teach sha1_file to invoke the command configured in\nextensions.lazyObject whenever an object is requested and unavailable.\n\nThe usage of the hook can be suppressed through a flag when invoking\nhas_object_file_with_flags() and other similar functions.\n\nThis is meant as a temporary measure to ensure that all Git commands\nwork in such a situation. Future patches will update some commands to\neither tolerate missing objects (without invoking the command) or be\nmore efficient in invoking this command.\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().\n\nFor (2), I looked at for_each_packed_object and at the packed-related\nfunctions that take in a hash. For for_each_packed_object, the callers\neither already work or are fixed in this patch:\n - reachable - only to find recent objects\n - builtin/fsck - already knows about missing objects\n - builtin/cat-file - warning message added in this commit\n\nCallers of the other functions do not need to be changed:\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 - already knows about promised objects\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\nAn alternative design that I considered but rejected:\n\n - Adding a hook whenever a packed object is requested, not on any\n object. That is, whenever we attempt to search the packfiles for an\n object, if it is missing (from the packfiles and from the loose\n object storage), to invoke the hook (which must then store it as a\n packfile), open the packfile the hook generated, and report that the\n object is found in that new packfile. This reduces the amount of\n analysis needed (in that we only need to look at how packed objects\n are handled), but requires that the hook generate packfiles (or for\n sha1_file to pack whatever loose objects are generated), creating one\n packfile for each missing object and potentially very many packfiles\n that must be linearly searched. This may be tolerable now for repos\n that only have a few missing objects (for example, repos that only\n want to exclude large blobs), and might be tolerable in the future if\n we have batching support for the most commonly used commands, but is\n not tolerable now for repos that exclude a large amount of objects.\n\nHelped-by: Ben Peart \nSigned-off-by: Jonathan Tan \n---\n Documentation/Makefile | 1 +\n Documentation/gitattributes.txt | 54 ++---------\n Documentation/gitrepository-layout.txt | 3 +\n .../technical/long-running-process-protocol.txt | 50 ++++++++++\n Documentation/technical/repository-version.txt | 16 ++++\n Makefile | 1 +\n builtin/cat-file.c | 2 +\n builtin/fsck.c | 2 +-\n cache.h | 2 +\n lazy-object.c | 80 ++++++++++++++++\n lazy-object.h | 12 +++\n object.c | 7 ++\n object.h | 13 +++\n sha1_file.c | 44 ++++++---\n t/t0410-lazy-object.sh | 18 ++++\n t/t0410/lazy-object | 102 +++++++++++++++++++++\n 16 files changed, 345 insertions(+), 62 deletions(-)\n create mode 100644 Documentation/technical/long-running-process-protocol.txt\n create mode 100644 lazy-object.c\n create mode 100644 lazy-object.h\n create mode 100755 t/t0410/lazy-object\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 2415e0d65..5657d49c2 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -69,6 +69,7 @@ SP_ARTICLES += $(API_DOCS)\n \n TECH_DOCS += technical/http-protocol\n TECH_DOCS += technical/index-format\n+TECH_DOCS += technical/long-running-process-protocol\n TECH_DOCS += technical/pack-format\n TECH_DOCS += technical/pack-heuristics\n TECH_DOCS += technical/pack-protocol\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex c4f2be254..ed5e8204e 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -387,46 +387,14 @@ Long Running Filter Process\n If the filter command (a string value) is defined via\n `filter..process` then Git can process all blobs with a\n single filter invocation for the entire life of a single Git\n-command. This is achieved by using a packet format (pkt-line,\n-see technical/protocol-common.txt) based protocol over standard\n-input and standard output as follows. All packets, except for the\n-\"*CONTENT\" packets and the \"0000\" flush packet, are considered\n-text and therefore are terminated by a LF.\n-\n-Git starts the filter when it encounters the first file\n-that needs to be cleaned or smudged. After the filter started\n-Git sends a welcome message (\"git-filter-client\"), a list of supported\n-protocol version numbers, and a flush packet. Git expects to read a welcome\n-response message (\"git-filter-server\"), exactly one protocol version number\n-from the previously sent list, and a flush packet. All further\n-communication will be based on the selected version. The remaining\n-protocol description below documents \"version=2\". Please note that\n-\"version=42\" in the example below does not exist and is only there\n-to illustrate how the protocol would look like with more than one\n-version.\n-\n-After the version negotiation Git sends a list of all capabilities that\n-it supports and a flush packet. Git expects to read a list of desired\n-capabilities, which must be a subset of the supported capabilities list,\n-and a flush packet as response:\n-------------------------\n-packet: git> git-filter-client\n-packet: git> version=2\n-packet: git> version=42\n-packet: git> 0000\n-packet: git< git-filter-server\n-packet: git< version=2\n-packet: git< 0000\n-packet: git> capability=clean\n-packet: git> capability=smudge\n-packet: git> capability=not-yet-invented\n-packet: git> 0000\n-packet: git< capability=clean\n-packet: git< capability=smudge\n-packet: git< 0000\n-------------------------\n-Supported filter capabilities in version 2 are \"clean\", \"smudge\",\n-and \"delay\".\n+command. This is achieved by using the long-running process protocol\n+(described in technical/long-running-process-protocol.txt).\n+\n+When Git encounters the first file that needs to be cleaned or smudged,\n+it starts the filter and performs the handshake. In the handshake, the\n+welcome message sent by Git is \"git-filter-client\", only version 2 is\n+suppported, and the supported capabilities are \"clean\", \"smudge\", and\n+\"delay\".\n \n Afterwards Git sends a list of \"key=value\" pairs terminated with\n a flush packet. The list will contain at least the filter command\n@@ -512,12 +480,6 @@ the protocol then Git will stop the filter process and restart it\n with the next file that needs to be processed. Depending on the\n `filter..required` flag Git will interpret that as error.\n \n-After the filter has processed a command it is expected to wait for\n-a \"key=value\" list containing the next command. Git will close\n-the command pipe on exit. The filter is expected to detect EOF\n-and exit gracefully on its own. Git will wait until the filter\n-process has stopped.\n-\n Delay\n ^^^^^\n \ndiff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt\nindex f51ed4e37..a690fdcd5 100644\n--- a/Documentation/gitrepository-layout.txt\n+++ b/Documentation/gitrepository-layout.txt\n@@ -47,6 +47,9 @@ use with dumb transports but otherwise is OK as long as\n `objects/info/alternates` points at the object stores it\n borrows from.\n +\n+. If `extensions.lazyObject` is set, some objects could be missing, to\n+be lazily loaded by Git when required.\n++\n This directory is ignored if $GIT_COMMON_DIR is set and\n \"$GIT_COMMON_DIR/objects\" will be used instead.\n \ndiff --git a/Documentation/technical/long-running-process-protocol.txt b/Documentation/technical/long-running-process-protocol.txt\nnew file mode 100644\nindex 000000000..aa0aa9af1\n--- /dev/null\n+++ b/Documentation/technical/long-running-process-protocol.txt\n@@ -0,0 +1,50 @@\n+Long-running process protocol\n+=============================\n+\n+This protocol is used when Git needs to communicate with an external\n+process throughout the entire life of a single Git command. All\n+communication is in pkt-line format (see technical/protocol-common.txt)\n+over standard input and standard output.\n+\n+Handshake\n+---------\n+\n+Git starts by sending a welcome message (for example,\n+\"git-filter-client\"), a list of supported protocol version numbers, and\n+a flush packet. Git expects to read the welcome message with \"server\"\n+instead of \"client\" (for example, \"git-filter-server\"), exactly one\n+protocol version number from the previously sent list, and a flush\n+packet. All further communication will be based on the selected version.\n+The remaining protocol description below documents \"version=2\". Please\n+note that \"version=42\" in the example below does not exist and is only\n+there to illustrate how the protocol would look like with more than one\n+version.\n+\n+After the version negotiation Git sends a list of all capabilities that\n+it supports and a flush packet. Git expects to read a list of desired\n+capabilities, which must be a subset of the supported capabilities list,\n+and a flush packet as response:\n+------------------------\n+packet: git> git-filter-client\n+packet: git> version=2\n+packet: git> version=42\n+packet: git> 0000\n+packet: git< git-filter-server\n+packet: git< version=2\n+packet: git< 0000\n+packet: git> capability=clean\n+packet: git> capability=smudge\n+packet: git> capability=not-yet-invented\n+packet: git> 0000\n+packet: git< capability=clean\n+packet: git< capability=smudge\n+packet: git< 0000\n+------------------------\n+\n+Shutdown\n+--------\n+\n+Git will close\n+the command pipe on exit. The filter is expected to detect EOF\n+and exit gracefully on its own. Git will wait until the filter\n+process has stopped.\ndiff --git a/Documentation/technical/repository-version.txt b/Documentation/technical/repository-version.txt\nindex f0482cae4..38c8f966e 100644\n--- a/Documentation/technical/repository-version.txt\n+++ b/Documentation/technical/repository-version.txt\n@@ -93,3 +93,19 @@ objects in the repository MUST NOT be deleted (e.g., by `git-prune` or\n When the config key `extensions.lazyObject` is set, Git does not treat missing\n objects as errors. The value of `extensions.lazyObject` must be a string,\n naming the command used to make a missing object appear whenever it is needed.\n+\n+Git communicates with this command using the long-running process\n+protocol described in technical/long-running-process-protocol.txt.\n+During the handshake, the welcome message used is \"git-lazy-object\", the\n+only supported version is 1, and the only supported capability is \"get\".\n+After the handshake, for each object to be loaded, Git will send\n+\"command=get\\n\", \"sha1=\\n\", and a flush packet, and will expect\n+\"status=success\\n\" or \"status=error\\n\" and a flush packet. For example:\n+\n+------------------------\n+packet: git> command=get\n+packet: git> sha1=1234567890123456789012345678901234567890\n+packet: git> 0000\n+packet: git< status=success\n+packet: git< 0000\n+------------------------\ndiff --git a/Makefile b/Makefile\nindex cd7d36c7d..3cac24775 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -800,6 +800,7 @@ LIB_OBJS += help.o\n LIB_OBJS += hex.o\n LIB_OBJS += ident.o\n LIB_OBJS += kwset.o\n+LIB_OBJS += lazy-object.o\n LIB_OBJS += levenshtein.o\n LIB_OBJS += line-log.o\n LIB_OBJS += line-range.o\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 62c8cf0eb..ef016b84e 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -473,6 +473,8 @@ static int batch_objects(struct batch_options *opt)\n \n \t\tfor_each_loose_object(batch_loose_object, &sa, 0);\n \t\tfor_each_packed_object(batch_packed_object, &sa, 0);\n+\t\tif (repository_format_lazy_object)\n+\t\t\twarning(\"This repository has extensions.lazyObject set. Some objects may not be loaded.\");\n \n \t\tcb.opt = opt;\n \t\tcb.expand = &data;\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 20415902f..327b087c2 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -447,7 +447,7 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid,\n {\n \tstruct object *obj;\n \n-\tobj = parse_object(oid);\n+\tobj = parse_maybe_missing_object(oid);\n \tif (!obj) {\n \t\tif (repository_format_lazy_object) {\n \t\t\t/*\ndiff --git a/cache.h b/cache.h\nindex 9e6bc0a21..375f5b700 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1837,6 +1837,8 @@ struct object_info {\n #define OBJECT_INFO_SKIP_CACHED 4\n /* Do not retry packed storage after checking packed and loose storage */\n #define OBJECT_INFO_QUICK 8\n+/* Never invoke extensions.lazyObject, even if object is missing */\n+#define OBJECT_INFO_NO_LAZY 16\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 \ndiff --git a/lazy-object.c b/lazy-object.c\nnew file mode 100644\nindex 000000000..d4b271a4e\n--- /dev/null\n+++ b/lazy-object.c\n@@ -0,0 +1,80 @@\n+#include \"cache.h\"\n+#include \"lazy-object.h\"\n+#include \"sha1-lookup.h\"\n+#include \"strbuf.h\"\n+#include \"run-command.h\"\n+#include \"sha1-array.h\"\n+#include \"config.h\"\n+#include \"sigchain.h\"\n+#include \"sub-process.h\"\n+#include \"pkt-line.h\"\n+\n+#define CAP_GET (1u<<0)\n+\n+static struct hashmap subprocess_map;\n+\n+int start_lazy_object_fn(struct subprocess_entry *subprocess)\n+{\n+\tstatic int versions[] = {1, 0};\n+\tstatic struct subprocess_capability capabilities[] = {\n+\t\t{ \"get\", CAP_GET },\n+\t\t{ NULL, 0 }\n+\t};\n+\n+\tunsigned int supported_capabilities = 0;\n+\tint ret = subprocess_handshake(subprocess, \"git-lazy-object\", versions,\n+\t\t\t\t NULL, capabilities,\n+\t\t\t\t &supported_capabilities);\n+\tif (ret)\n+\t\treturn ret;\n+\treturn supported_capabilities != CAP_GET;\n+}\n+\n+void load_lazy_object(const unsigned char *sha1)\n+{\n+\tstruct subprocess_entry *entry;\n+\tstruct child_process *process;\n+\tstruct strbuf status = STRBUF_INIT;\n+\n+\tif (!repository_format_lazy_object)\n+\t\tdie(\"BUG: extensions.lazyObject is not set\");\n+\n+\tif (!subprocess_map.tablesize) {\n+\t\thashmap_init(&subprocess_map,\n+\t\t\t (hashmap_cmp_fn) cmd2process_cmp, NULL, 0);\n+\t\tentry = NULL;\n+\t} else {\n+\t\tentry = subprocess_find_entry(&subprocess_map,\n+\t\t\t\t\t repository_format_lazy_object);\n+\t}\n+\tif (!entry) {\n+\t\tentry = xmalloc(sizeof(*entry));\n+\n+\t\tif (subprocess_start(&subprocess_map, entry,\n+\t\t\t\t repository_format_lazy_object,\n+\t\t\t\t start_lazy_object_fn)) {\n+\t\t\tfree(entry);\n+\t\t\tdie(\"could not start '%s'\", repository_format_lazy_object);\n+\t\t}\n+\t}\n+\tprocess = subprocess_get_child_process(entry);\n+\n+\tpacket_write_fmt(process->in, \"command=get\\n\");\n+\tpacket_write_fmt(process->in, \"sha1=%s\\n\", sha1_to_hex(sha1));\n+\tpacket_flush(process->in);\n+\tif (subprocess_read_status(process->out, &status))\n+\t\tdie(\"Could not read status from '%s'\", repository_format_lazy_object);\n+\t\n+\tif (!strcmp(status.buf, \"success\"))\n+\t\treturn;\n+\tif (!strcmp(status.buf, \"error\"))\n+\t\tdie(\"external process '%s' returned error\",\n+\t\t repository_format_lazy_object);\n+\tdie(\"external process '%s' failed\", repository_format_lazy_object);\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+}\ndiff --git a/lazy-object.h b/lazy-object.h\nnew file mode 100644\nindex 000000000..1ec6ff203\n--- /dev/null\n+++ b/lazy-object.h\n@@ -0,0 +1,12 @@\n+#ifndef LAZY_OBJECT_H\n+#define LAZY_OBJECT_H\n+\n+#include \"cache.h\"\n+#include \"sha1-array.h\"\n+\n+/*\n+ * Invokes repository_format_lazy_object to load the given missing object.\n+ */\n+void load_lazy_object(const unsigned char *sha1);\n+\n+#endif\ndiff --git a/object.c b/object.c\nindex 321d7e920..e3e734f67 100644\n--- a/object.c\n+++ b/object.c\n@@ -279,6 +279,13 @@ struct object *parse_object(const struct object_id *oid)\n \treturn NULL;\n }\n \n+struct object *parse_maybe_missing_object(const struct object_id *oid)\n+{\n+\tif (has_object_file_with_flags(oid, OBJECT_INFO_NO_LAZY))\n+\t\treturn parse_object(oid);\n+\treturn NULL;\n+}\n+\n struct object_list *object_list_insert(struct object *item,\n \t\t\t\t struct object_list **list_p)\n {\ndiff --git a/object.h b/object.h\nindex 0a419ba8d..b98e73572 100644\n--- a/object.h\n+++ b/object.h\n@@ -87,10 +87,23 @@ void *object_as_type(struct object *obj, enum object_type type, int quiet);\n /*\n * Returns the object, having parsed it to find out what it is.\n *\n+ * This function automatically invokes the command in extensions.lazyObject if\n+ * the object is missing and extensions.lazyObject is set.\n+ *\n * Returns NULL if the object is missing or corrupt.\n */\n struct object *parse_object(const struct object_id *oid);\n \n+/*\n+ * Returns the object, having parsed it to find out what it is.\n+ *\n+ * This function never invokes the command in extensions.lazyObject, unlike\n+ * parse_object().\n+ *\n+ * Returns NULL if the object is missing or corrupt.\n+ */\n+struct object *parse_maybe_missing_object(const struct object_id *oid);\n+\n /*\n * Like parse_object, but will die() instead of returning NULL. If the\n * \"name\" parameter is not NULL, it is included in the error message\ndiff --git a/sha1_file.c b/sha1_file.c\nindex b60ae15f7..1785c61d8 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -28,6 +28,11 @@\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+#include \"lazy-object.h\"\n+#include \"sha1-array.h\"\n \n #define SZ_FMT PRIuMAX\n static inline uintmax_t sz_fmt(size_t s) { return s; }\n@@ -2984,6 +2989,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n \tconst unsigned char *real = (flags & OBJECT_INFO_LOOKUP_REPLACE) ?\n \t\t\t\t lookup_replace_object(sha1) :\n \t\t\t\t sha1;\n+\tint already_retried = 0;\n \n \tif (!oi)\n \t\toi = &blank_oi;\n@@ -3008,30 +3014,38 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n \t\t}\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\tif (flags & OBJECT_INFO_QUICK) {\n-\t\t\treturn -1;\n-\t\t} else {\n-\t\t\treprepare_packed_git();\n-\t\t\tif (!find_pack_entry(real, &e))\n-\t\t\t\treturn -1;\n-\t\t}\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/* Check if it is a missing object */\n+\tif (repository_format_lazy_object && !already_retried &&\n+\t !(flags & OBJECT_INFO_NO_LAZY)) {\n+\t\tload_lazy_object(real);\n+\t\talready_retried = 1;\n+\t\tgoto retry;\n+\t}\n+\n+\treturn -1;\n+\n+found_packed:\n \tif (oi == &blank_oi)\n \t\t/*\n \t\t * We know that the caller doesn't actually need the\n \t\t * information below, so return early.\n \t\t */\n \t\treturn 0;\n-\n \trtype = packed_object_info(e.p, e.offset, oi);\n \tif (rtype < 0) {\n \t\tmark_bad_packed_object(e.p, real);\ndiff --git a/t/t0410-lazy-object.sh b/t/t0410-lazy-object.sh\nindex 3ac61c1c5..d8888baa0 100755\n--- a/t/t0410-lazy-object.sh\n+++ b/t/t0410-lazy-object.sh\n@@ -92,4 +92,22 @@ test_expect_success '...but succeeds if lazyobject is set' '\n \tgit -C repo fsck \"$HASH\"\n '\n \n+test_expect_success 'sha1_object_info_extended (through git cat-file)' '\n+\trm -rf repo client &&\n+\n+\ttest_create_repo server &&\n+\ttest_commit -C server 1 1.t abcdefgh &&\n+\tHASH=$(git hash-object server/1.t) &&\n+\n+\ttest_create_repo client &&\n+\ttest_must_fail git -C client cat-file -p \"$HASH\"\n+'\n+\n+test_expect_success '...but succeeds if lazyobject is set' '\n+\tgit -C client config core.repositoryformatversion 1 &&\n+\tgit -C client config extensions.lazyobject \\\n+\t\t\"\\\"$TEST_DIRECTORY/t0410/lazy-object\\\" \\\"$(pwd)/server/.git\\\"\" &&\n+\tgit -C client cat-file -p \"$HASH\"\n+'\n+\n test_done\ndiff --git a/t/t0410/lazy-object b/t/t0410/lazy-object\nnew file mode 100755\nindex 000000000..4f4a9c38a\n--- /dev/null\n+++ b/t/t0410/lazy-object\n@@ -0,0 +1,102 @@\n+#!/usr/bin/perl\n+#\n+# Example implementation for the Git lazyObject protocol version 1. See\n+# the documentation for extensions.lazyObject in\n+# Documentation/technical/repository-version.txt\n+#\n+# Allows you to test the ability for blobs to be pulled from a host git repo\n+# \"on demand.\" Called when git needs a blob it couldn't find locally due to\n+# a lazy clone that only cloned the commits and trees.\n+#\n+# Please note, this sample is a minimal skeleton. No proper error handling\n+# was implemented.\n+\n+use strict;\n+use warnings;\n+\n+#\n+# Point $DIR to the folder where your host git repo is located so we can pull\n+# missing objects from it\n+#\n+my $DIR = $ARGV[0];\n+\n+sub packet_bin_read {\n+\tmy $buffer;\n+\tmy $bytes_read = read STDIN, $buffer, 4;\n+\tif ( $bytes_read == 0 ) {\n+\n+\t\t# EOF - Git stopped talking to us!\n+\t\texit();\n+\t}\n+\telsif ( $bytes_read != 4 ) {\n+\t\tdie \"invalid packet: '$buffer'\";\n+\t}\n+\tmy $pkt_size = hex($buffer);\n+\tif ( $pkt_size == 0 ) {\n+\t\treturn ( 1, \"\" );\n+\t}\n+\telsif ( $pkt_size > 4 ) {\n+\t\tmy $content_size = $pkt_size - 4;\n+\t\t$bytes_read = read STDIN, $buffer, $content_size;\n+\t\tif ( $bytes_read != $content_size ) {\n+\t\t\tdie \"invalid packet ($content_size bytes expected; $bytes_read bytes read)\";\n+\t\t}\n+\t\treturn ( 0, $buffer );\n+\t}\n+\telse {\n+\t\tdie \"invalid packet size: $pkt_size\";\n+\t}\n+}\n+\n+sub packet_txt_read {\n+\tmy ( $res, $buf ) = packet_bin_read();\n+\tunless ( $buf =~ s/\\n$// ) {\n+\t\tdie \"A non-binary line MUST be terminated by an LF.\";\n+\t}\n+\treturn ( $res, $buf );\n+}\n+\n+sub packet_bin_write {\n+\tmy $buf = shift;\n+\tprint STDOUT sprintf( \"%04x\", length($buf) + 4 );\n+\tprint STDOUT $buf;\n+\tSTDOUT->flush();\n+}\n+\n+sub packet_txt_write {\n+\tpacket_bin_write( $_[0] . \"\\n\" );\n+}\n+\n+sub packet_flush {\n+\tprint STDOUT sprintf( \"%04x\", 0 );\n+\tSTDOUT->flush();\n+}\n+\n+( packet_txt_read() eq ( 0, \"git-lazy-object-client\" ) ) || die \"bad initialize\";\n+( packet_txt_read() eq ( 0, \"version=1\" ) )\t\t || die \"bad version\";\n+( packet_bin_read() eq ( 1, \"\" ) ) || die \"bad version end\";\n+\n+packet_txt_write(\"git-lazy-object-server\");\n+packet_txt_write(\"version=1\");\n+packet_flush();\n+\n+( packet_txt_read() eq ( 0, \"capability=get\" ) ) || die \"bad capability\";\n+( packet_bin_read() eq ( 1, \"\" ) ) || die \"bad capability end\";\n+\n+packet_txt_write(\"capability=get\");\n+packet_flush();\n+\n+while (1) {\n+\tmy ($command) = packet_txt_read() =~ /^command=([^=]+)$/;\n+\n+\tif ( $command eq \"get\" ) {\n+\t\tmy ($sha1) = packet_txt_read() =~ /^sha1=([0-9a-f]{40})$/;\n+\t\tpacket_bin_read();\n+\n+\t\tsystem ('git --git-dir=\"' . $DIR . '\" cat-file blob ' . $sha1 . ' | git -c extensions.lazyobject=false hash-object -w --stdin >/dev/null 2>&1');\n+\t\tpacket_txt_write(($?) ? \"status=error\" : \"status=success\");\n+\t\tpacket_flush();\n+\t} else {\n+\t\tdie \"bad command '$command'\";\n+\t}\n+}\n-- \n2.14.0.rc1.383.gd1ce394fe2-goog\n\n"},{"id":"325343","messageId":"xmqq4ltsi9or.fsf@gitster.mtv.corp.google.com","threadId":"46475","inReplyTo":"cover.1501532294.git.jonathantanmy@google.com","subject":"Re: [PATCH v2 0/5] Fsck for lazy objects, and (now) actual invocation of loader","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-31T21:21:56Z","receivedAt":"2017-07-31T21:22:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> Besides review changes, this patch set now includes my rewritten\n> lazy-loading sha1_file patch, so you can now do this (excerpted from one\n> of the tests):\n>\n> test_create_repo server\n> test_commit -C server 1 1.t abcdefgh\n> HASH=$(git hash-object server/1.t)\n> \n> test_create_repo client\n> test_must_fail git -C client cat-file -p \"$HASH\"\n> git -C client config core.repositoryformatversion 1\n> git -C client config extensions.lazyobject \\\n> \"\\\"$TEST_DIRECTORY/t0410/lazy-object\\\" \\\"$(pwd)/server/.git\\\"\"\n> git -C client cat-file -p \"$HASH\"\n>\n> with fsck still working. Also, there is no need for a list of promised\n> blobs, and the long-running process protocol is being used.\n\nI do not think I read your response to my last comment on this\nseries, so I could be missing something large, but I am afraid that\nthe resulting fsck is only half as useful as the normal fsck. I do\nnot see it any better than a hypothetical castrated version of fsck\nthat only checks the integrity of objects that appear in the local\nobject store, without doing any connectivity checks.\n\nDon't get me wrong. The integrity check on local objects you still\ndo is important---that is what allows us to make sure that the local\n\"cache\" does not prevent us from going to the real source of the\nremote object store by having a corrupt copy. \n\nBut not being able to tell if a missing object is OK to be missing\n(because we can get them if/as needed from elsewhere) or we lost the\nsole copy of an object that we created and have not pushed out\n(hence we are in deep yogurt) makes it pretty much pointless to run\n\"fsck\", doesn't it? It does not give us any guarantee that our\nrepository plus perfect network connectivity gives us an environment\nto build further work on.\n\nOr am I missing something fundamental?\n"},{"id":"325346","messageId":"xmqqzibkgury.fsf@gitster.mtv.corp.google.com","threadId":"46475","inReplyTo":"eadce97b6a1e80345a2621e71ce187e9e6bc05bf.1501532294.git.jonathantanmy@google.com","subject":"Re: [PATCH v2 5/5] sha1_file: support loading lazy objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-31T21:29:21Z","receivedAt":"2017-07-31T21:29:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> Teach sha1_file to invoke the command configured in\n> extensions.lazyObject whenever an object is requested and unavailable.\n>\n> The usage of the hook can be suppressed through a flag when invoking\n> has_object_file_with_flags() and other similar functions.\n>\n> This is meant as a temporary measure to ensure that all Git commands\n> work in such a situation. Future patches will update some commands to\n> either tolerate missing objects (without invoking the command) or be\n> more efficient in invoking this command.\n>\n> In order to determine the code changes in sha1_file.c necessary, I\n> investigated 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().\n>\n> For (2), I looked at for_each_packed_object and at the packed-related\n> functions that take in a hash. For for_each_packed_object, the callers\n> either already work or are fixed in this patch:\n> - reachable - only to find recent objects\n> - builtin/fsck - already knows about missing objects\n> - builtin/cat-file - warning message added in this commit\n>\n> Callers of the other functions do not need to be changed:\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 - already knows about promised objects\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>\n> An alternative design that I considered but rejected:\n>\n> - Adding a hook whenever a packed object is requested, not on any\n> object. That is, whenever we attempt to search the packfiles for an\n> object, if it is missing (from the packfiles and from the loose\n> object storage), to invoke the hook (which must then store it as a\n> packfile), open the packfile the hook generated, and report that the\n> object is found in that new packfile. This reduces the amount of\n> analysis needed (in that we only need to look at how packed objects\n> are handled), but requires that the hook generate packfiles (or for\n> sha1_file to pack whatever loose objects are generated), creating one\n> packfile for each missing object and potentially very many packfiles\n> that must be linearly searched. This may be tolerable now for repos\n> that only have a few missing objects (for example, repos that only\n> want to exclude large blobs), and might be tolerable in the future if\n> we have batching support for the most commonly used commands, but is\n> not tolerable now for repos that exclude a large amount of objects.\n>\n> Helped-by: Ben Peart \n> Signed-off-by: Jonathan Tan \n> ---\n\nEven though I said a hugely negative thing about the \"missing\nobjects are always OK\" butchering of fsck, I do like what this patch\ndoes. The interface is reasonably well isolated, and moving of the\nlong-running-process documentation to a standalone file is very\nsensible.\n\n"},{"id":"325354","messageId":"20170731160533.0a446244@twelve2.svl.corp.google.com","threadId":"46475","inReplyTo":"xmqq4ltsi9or.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 0/5] Fsck for lazy objects, and (now) actual invocation of loader","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-07-31T23:05:33Z","receivedAt":"2017-07-31T23:05:45Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Mon, 31 Jul 2017 14:21:56 -0700\nJunio C Hamano wrote:\n\n> Jonathan Tan writes:\n> \n> > Besides review changes, this patch set now includes my rewritten\n> > lazy-loading sha1_file patch, so you can now do this (excerpted from one\n> > of the tests):\n> >\n> > test_create_repo server\n> > test_commit -C server 1 1.t abcdefgh\n> > HASH=$(git hash-object server/1.t)\n> > \n> > test_create_repo client\n> > test_must_fail git -C client cat-file -p \"$HASH\"\n> > git -C client config core.repositoryformatversion 1\n> > git -C client config extensions.lazyobject \\\n> > \"\\\"$TEST_DIRECTORY/t0410/lazy-object\\\" \\\"$(pwd)/server/.git\\\"\"\n> > git -C client cat-file -p \"$HASH\"\n> >\n> > with fsck still working. Also, there is no need for a list of promised\n> > blobs, and the long-running process protocol is being used.\n> \n> I do not think I read your response to my last comment on this\n> series, so I could be missing something large, but I am afraid that\n> the resulting fsck is only half as useful as the normal fsck. I do\n> not see it any better than a hypothetical castrated version of fsck\n> that only checks the integrity of objects that appear in the local\n> object store, without doing any connectivity checks.\n\nSorry, I haven't replied to your last response [1]. That does sound like\na good idea, though, and probably can be extended to trees and blobs in\nthat we need to make sure that any object referenced from local-only\ncommits (calculated as you describe in [1]) can be obtained through an\nobject walk from a remote-tracking branch.\n\nI haven't fully thought of the implications of things like building\ncommits on top of an arbitrary upstream commit (so since our upstream\ncommit is not a tip, the object walk from all remote-tracking branches\nmight not reach our upstream commit).\n\nTo try to solve that, we could use an alternate object store to store\nremote objects in order to be able to find remote objects quickly\nwithout doing a traversal, but that does not fully solve the problem,\nbecause some information about remote object possession lies only in\ntheir parents (for example, if we don't have a remote blob, sometimes\nthe only way to know that the remote has it is by having a tree\ncontaining that blob).\n\nIn addition, this also couples the lazy object loading with either a\nremote ref (or all remote refs, if we decide to consider objects from\nall remote refs as potentially loadable).\n\nI'll think about this further.\n\n[1] https://public-inbox.org/git/xmqq379fkz4x.fsf@gitster.mtv.corp.google.com/\n\n> Don't get me wrong. The integrity check on local objects you still\n> do is important---that is what allows us to make sure that the local\n> \"cache\" does not prevent us from going to the real source of the\n> remote object store by having a corrupt copy. \n> \n> But not being able to tell if a missing object is OK to be missing\n> (because we can get them if/as needed from elsewhere) or we lost the\n> sole copy of an object that we created and have not pushed out\n> (hence we are in deep yogurt) makes it pretty much pointless to run\n> \"fsck\", doesn't it? It does not give us any guarantee that our\n> repository plus perfect network connectivity gives us an environment\n> to build further work on.\n> \n> Or am I missing something fundamental?\n\nWell, the fsck can still detect issues like corrupt objects (as you\nmention above) and dangling heads, which might be real issues. But it is\ntrue that it does not give you the guarantee you describe.\n\nFrom a user standpoint, this might be able to be worked around by\nproviding a network-requiring object connectivity checking tool or by\njust having the user running a build to ensure that all necessary files\nare present.\n\nHaving said that, this feature will be very nice to have.\n"},{"id":"325380","messageId":"xmqq379bgqlx.fsf@gitster.mtv.corp.google.com","threadId":"46475","inReplyTo":"20170731160533.0a446244@twelve2.svl.corp.google.com","subject":"Re: [PATCH v2 0/5] Fsck for lazy objects, and (now) actual invocation of loader","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-01T17:11:38Z","receivedAt":"2017-08-01T17:11:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> Well, the fsck can still detect issues like corrupt objects (as you\n> mention above) and dangling heads, which might be real issues. But it is\n> true that it does not give you the guarantee you describe.\n\nWhich makes it pretty much useless. The whole point of running\n\"fsck\" is to make sure that we won't waste work by not finding\na corruption long after it was introduced and spent a lot of effort\nbuilding on top of a state that nobody can reproduce.\n\n> From a user standpoint, this might be able to be worked around by\n> providing a network-requiring object connectivity checking tool or by\n> just having the user running a build to ensure that all necessary files\n> are present.\n\nI actually was hoping that you do not have to go to the network for\nthe checking. And I have to say that \"only the tip matters\" is a\nhorrible cop-out that is not even a workaround. Your users would be\nserved better if you honestly admit that your fsck will not be\nuseful when this feature is used---at least they won't be harmed by\na false expectation that \"fsck\" would give them some assurance,\nwhich is not the case.\n\nLet's step back a bit and think what already happens in the pre-\nlazy-object world. We record cut-off commits when a depth limited\nclone is created in \"shallow\". These essentially are promises,\nsaying something like:\n\n Rest assured that everything in the history behind these commits\n are on the other side and you can retrieve them by unshallowing.\n\n If you traverse from your local tips and find no missing objects\n before reaching one of these commits, then you do not have any\n local corruption you need to worry about.\n\nthe other end made to us, when the shallow clone was made. And we\ntake this promise and build more commits on top, and then we adjust\nthese cut-off commits incrementally as we deepen our clone or make\nit even shallower. For this assurance to work, we of course need to\nassume a bit more than what we assume for a complete clone, namely,\nthe \"other side\" will hold onto the history behind these, i.e. does\nnot remind the tips it already has shown to us, or even if it does,\nthe objects that are reachable from these cut-off points will\nsomehow always be available to us on demand.\n\nCan we do something similar, i.e. maintain minimum set of cut-off\npoints and adjust that set incrementally, just sufficient to ensure\nthe integrity of objects locally created and not yet safely stored\naway by pushing them the \"other side\"?\n\nI haven't thought things through (and I know you, Ben and others\nhave thought much longer and harder), but I would imagine if we have\na commit object [*1*], some of whose parent commits, trees and blobs\nare locally missing, and know that the commit exists on the \"other\nside\", we know that all of these \"missing\" objects that are\nreferenced by the commit are also available from the \"other side\".\nIOW, I suspect that the same principle \"shallow\" uses to give us the\nintegrity guarantee can be naturally extended to allow us to see if\na broken connectivity is OK.\n\n\n[Footnote]\n\n*1* The same can be said for a tag or a tree object that we know\n exist on the \"other side\"; they may refer, directly or\n indirectly through objects we locally have, to objects that that\n are missing locally, and as long as the starting point object\n are known to be available on the \"other side\", it is OK for them\n to be missing locally.\n\n"},{"id":"325383","messageId":"20170801174502.GR13924@aiede.mtv.corp.google.com","threadId":"46475","inReplyTo":"xmqq379bgqlx.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 0/5] Fsck for lazy objects, and (now) actual invocation of loader","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-08-01T17:45:02Z","receivedAt":"2017-08-01T17:45:11Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJunio C Hamano wrote:\n\n> Can we do something similar, i.e. maintain minimum set of cut-off\n> points and adjust that set incrementally, just sufficient to ensure\n> the integrity of objects locally created and not yet safely stored\n> away by pushing them the \"other side\"?\n\nThis sounds like a variant on the \"promises\" idea (maintaining a list\nof objects at the frontier) described before. Instead of listing\nblobs that the server promised, you are proposing listing trees that\nthe server has promised to handle all references from.\n\n> I haven't thought things through (and I know you, Ben and others\n> have thought much longer and harder), but I would imagine if we have\n> a commit object [*1*], some of whose parent commits, trees and blobs\n> are locally missing, and know that the commit exists on the \"other\n> side\", we know that all of these \"missing\" objects that are\n> referenced by the commit are also available from the \"other side\".\n> IOW, I suspect that the same principle \"shallow\" uses to give us the\n> integrity guarantee can be naturally extended to allow us to see if\n> a broken connectivity is OK.\n\nIf we are deeply worried about this kind of broken connectivity, there\nis another case to care about: the server can \"promise\" to serve\nrequests for some object (e.g., the tree pointed to by the server's\n\"master\") and then decide it does not want to fulfill that promise\n(e.g., that tree pointed to private key material and \"master\" was\nrewound to avoid it). In the promises model, how we do we get a fresh\nunderstanding of what the server wants to promise now?\n\nEarlier in this discussion of fsck, I thought you were proposing a\nslightly different idea. The idea I heard is that you want to check\nconnectivity for whatever you have built locally, while accepting a\nrelaxed guarantee for objects from upstream. If an object is missing,\nthe idea would be that at least this way you know whose fault it is.\n:) (Not that there's much to do with that knowledge.) Implementing\nthat by treating everything reachable from a remote-tracking branch as\n\"from upstream\" seems natural. But that implementation suffers from\nthe same problems: not all objects from upstream need be reachable\nfrom a remote-tracking branch (e.g. after a fetch-by-object-id, or\nbecause a remote branch can be rewound).\n\nBoth variants proposed of the promises idea also hold some promise,\nbut my understanding was that the cost of maintaining the promises\nfile (getting data to fill it, locking on update, merging in new\nobjects into it on update), for little benefit wasn't palatable to\nMicrosoft, who has been coping fine without such a file.\n\nJonathan\n"},{"id":"325399","messageId":"xmqqfudbf3jd.fsf@gitster.mtv.corp.google.com","threadId":"46475","inReplyTo":"20170801174502.GR13924@aiede.mtv.corp.google.com","subject":"Re: [PATCH v2 0/5] Fsck for lazy objects, and (now) actual invocation of loader","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-01T20:15:18Z","receivedAt":"2017-08-01T20:15:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> If we are deeply worried about this kind of broken connectivity, there\n> is another case to care about: the server can \"promise\" to serve\n> requests for some object (e.g., the tree pointed to by the server's\n> \"master\") and then decide it does not want to fulfill that promise\n> (e.g., that tree pointed to private key material and \"master\" was\n> rewound to avoid it). \n\nI think I've already covered that in my message (i.e. \"we need to\nassume more than the normal Git\"). In short, it is not our problem,\nbut the \"lazy-object\" service's problem. If developers cannot trust\nthe \"central server\", most likely owned by the organization that\nemploys them and forces them to offload the access to these objects\nto the \"central server\", I think there is much larger problem there.\n\n> In the promises model, how we do we get a fresh\n> understanding of what the server wants to promise now?\n\nYes, that is one of the things that needs to be designed if we want\nto officially support lazy-objects like structure. We need a way to\nincrementally adjust the cut-off point, below which it is the\nresponsibility of the \"other side\" to ensure that necessary objects\nare available (on demand), and above which it is a local\nrepository's responsibility to notice corrupted and/or missing\nobjects.\n\n"},{"id":"325413","messageId":"20170801171944.7690a63f@twelve2.svl.corp.google.com","threadId":"46475","inReplyTo":"xmqq379bgqlx.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 0/5] Fsck for lazy objects, and (now) actual invocation of loader","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-08-02T00:19:44Z","receivedAt":"2017-08-02T00:19:57Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Tue, 01 Aug 2017 10:11:38 -0700\nJunio C Hamano wrote:\n\n> Let's step back a bit and think what already happens in the pre-\n> lazy-object world. We record cut-off commits when a depth limited\n> clone is created in \"shallow\". These essentially are promises,\n> saying something like:\n> \n> Rest assured that everything in the history behind these commits\n> are on the other side and you can retrieve them by unshallowing.\n> \n> If you traverse from your local tips and find no missing objects\n> before reaching one of these commits, then you do not have any\n> local corruption you need to worry about.\n> \n> the other end made to us, when the shallow clone was made. And we\n> take this promise and build more commits on top, and then we adjust\n> these cut-off commits incrementally as we deepen our clone or make\n> it even shallower. For this assurance to work, we of course need to\n> assume a bit more than what we assume for a complete clone, namely,\n> the \"other side\" will hold onto the history behind these, i.e. does\n> not remind the tips it already has shown to us, or even if it does,\n> the objects that are reachable from these cut-off points will\n> somehow always be available to us on demand.\n> \n> Can we do something similar, i.e. maintain minimum set of cut-off\n> points and adjust that set incrementally, just sufficient to ensure\n> the integrity of objects locally created and not yet safely stored\n> away by pushing them the \"other side\"?\n\nThis suggestion (the \"frontier\" of what we have) does seem to incur less\noverhead than the original promise suggestion (the \"frontier\" of what we\ndon't have), but after some in-office discussion, I'm convinced that it\nmight not be the case - for example, one tree (that we have) might\nreference many blobs (that we don't have), but at the same time, many\ntrees (that we have) might have the same blob (that we don't have). And\nthe promise overhead was already decided to be too much - which is why\nwe moved away from it.\n\nOne possibility to conceptually have the same thing without the overhead\nof the list is to put the obtained-from-elsewhere objects into its own\nalternate object store, so that we can distinguish the two. I mentioned\nthis in my e-mail but rejected it, but after some more thought, this\nmight be sufficient - we might still need to iterate through every\nobject to know exactly what we can assume the remote to have, but the\n\"frontier\" solution also needs this iteration, so we are no worse off.\n\nGoing back to the original use cases that motivated this (the monorepo\nlike Microsoft's repo and the large-blob repo like Android's repo), it\nmight be better just to disable the connectivity check when\nextensions.lazyObject is set (as you mentioned). This does change the\nmeaning of fsck, but it may be fine since the \"meaning\" of the repo (a\nview of another repo, and no longer a full repo) has changed too. Then\nthis patch set will be more about ensuring that the lazy object loader\nis not inadvertently run. As future work, we could add diagnostics that,\nfor example, attempt a walk anyway and print a list of missing SHA-1s.\n\n(I suspect that we will also need to disable the connectivity check for\nthings like \"git fetch\", which means that we won't be able to tell\nlocally if the server sent us all the objects that we requested for.\nThis might not be a problem, though, since the local repo already has\nsome measure of trust for the server.)\n"},{"id":"325437","messageId":"xmqqefsudjqk.fsf@gitster.mtv.corp.google.com","threadId":"46475","inReplyTo":"20170801171944.7690a63f@twelve2.svl.corp.google.com","subject":"Re: [PATCH v2 0/5] Fsck for lazy objects, and (now) actual invocation of loader","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-02T16:20:35Z","receivedAt":"2017-08-02T16:20:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> One possibility to conceptually have the same thing without the overhead\n> of the list is to put the obtained-from-elsewhere objects into its own\n> alternate object store, so that we can distinguish the two.\n\nNow you are talking. Either a separate object store, or a packfile\nthat is specially marked as such, would work. \"Maintaining a list\nof object names in a flat file is too costly\" is not a valid excuse\nto discard the integrity of locally created objects, without which\nGit will no longer be a version control system, and your \"We have to\ntrust the sender of objects on the other side anyway when operating\nin lazy-objects mode, so devise a way that we can use to tell which\nobjects we _know_ the other side has\" that leads to the above idea\nis a good line of thought.\n\n> I mentioned\n> this in my e-mail but rejected it, but after some more thought, this\n> might be sufficient - we might still need to iterate through every\n> object to know exactly what we can assume the remote to have, but the\n> \"frontier\" solution also needs this iteration, so we are no worse off.\n\nMost importantly, this is allowed to be costly---we are doing this\nnot at runtime all the time, but when the user says \"make sure that\nI haven't lost objects and it is safe for me to build further on\nwhat I created locally so far\" by running \"git fsck\".\n"},{"id":"325447","messageId":"20170802173857.GU13924@aiede.mtv.corp.google.com","threadId":"46475","inReplyTo":"xmqqefsudjqk.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 0/5] Fsck for lazy objects, and (now) actual invocation of loader","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-08-02T17:38:57Z","receivedAt":"2017-08-02T17:39:06Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJunio C Hamano wrote:\n> Jonathan Tan writes:\n\n>> One possibility to conceptually have the same thing without the overhead\n>> of the list is to put the obtained-from-elsewhere objects into its own\n>> alternate object store, so that we can distinguish the two.\n>\n> Now you are talking. Either a separate object store, or a packfile\n> that is specially marked as such, would work.\n\nJonathan's not in today, so let me say a few more words about this\napproach.\n\nThis approach implies a relaxed connectivity guarantee, by creating\ntwo classes of objects:\n\n 1. Objects that I made should satisfy the connectivity check. They\n can point to other objects I made, objects I fetched, or (*) objects\n pointed to directly by objects I fetched. More on (*) below.\n\n 2. Objects that I fetched do not need to satisfy a connectivity\n check. I can trust the server to provide objects that they point\n to when I ask for them, except in extraordinary cases like a\n credit card number that was accidentally pushed to the server and\n prompted a rewriting of history to remove it (**).\n\nThe guarantee (1) looks like it should be easy to satisfy (just like\nthe current connectivity guarantee where all objects are in class (1)).\nI have to know about an object to point to it --- that means the\npointed-to object has to be in the object store or pointed to by\nsomething in the object store.\n\nThe complication is in the \"git gc\" operation for the case (*).\nToday, \"git gc\" uses a reachability walk to decide which objects to\nremove --- an object referenced by no other object is fair game to\nremove. With (*), there is another kind of object that must not be\nremoved: if an object that I made, M, points to a missing/promised\nobject, O, pointed to by a an object I fetched, F, then I cannot prune\nF unless there is another fetched object present to anchor O.\n\nFor example: suppose I have a sparse checkout and run\n\n\tgit fetch origin refs/pulls/x\n\tgit checkout -b topic FETCH_HEAD\n\techo \"Some great modification\" >> README\n\tgit add README\n\tgit commit --amend\n\nWhen I run \"git gc\", there is nothing pointing to the commit that was\npointed to by the remote ref refs/pulls/x, so it can be pruned. I\nwould naively also expect that the tree pointed to by that commit\ncould be pruned. But pruning it means pruning the promise that made\nit permissible to lack various blobs that my topic branch refers to\nthat are outside the sparse checkout area. So \"git gc\" must notice\nthat it is not safe to prune that tree.\n\nThis feels hacky. I prefer the promised object list over this\napproach.\n\n> \"Maintaining a list\n> of object names in a flat file is too costly\" is not a valid excuse\n> to discard the integrity of locally created objects, without which\n> Git will no longer be a version control system,\n\nI am confused by this: I think that Git without a \"git fsck\" command\nat all would still be a version control system, just not as good of\none.\n\nCan you spell this out more? To be clear, are you speaking as a\nreviewer or as the project maintainer? In other words, if other\nreviewers are able to settle on a design that involves a relaxed\nguarantee for fsck in this mode that they can agree on, does this\nrepresent a veto meaning the patch can still not go through?\n\nOn one hand I'm grateful for the help exploring the design space, and\nI think it has helped get a better understanding of the issues\ninvolved.\n\nOn the other hand, if this is a veto then it feels very black and\nwhite and a hard starting point to build a consensus from. I am\nworried.\n\n[...]\n>> I mentioned\n>> this in my e-mail but rejected it, but after some more thought, this\n>> might be sufficient - we might still need to iterate through every\n>> object to know exactly what we can assume the remote to have, but the\n>> \"frontier\" solution also needs this iteration, so we are no worse off.\n>\n> Most importantly, this is allowed to be costly---we are doing this\n> not at runtime all the time, but when the user says \"make sure that\n> I haven't lost objects and it is safe for me to build further on\n> what I created locally so far\" by running \"git fsck\".\n\ncheck_everything_connected is also used in some other circumstances:\ne.g. when running a fetch, and when receiving a push in git\nreceive-pack.\n\nThanks,\nJonathan\n"},{"id":"325479","messageId":"xmqq8tj1snfq.fsf@gitster.mtv.corp.google.com","threadId":"46475","inReplyTo":"20170802173857.GU13924@aiede.mtv.corp.google.com","subject":"Re: [PATCH v2 0/5] Fsck for lazy objects, and (now) actual invocation of loader","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-02T20:51:37Z","receivedAt":"2017-08-02T20:51:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Junio C Hamano wrote:\n>> Jonathan Tan writes:\n>\n>>> One possibility to conceptually have the same thing without the overhead\n>>> of the list is to put the obtained-from-elsewhere objects into its own\n>>> alternate object store, so that we can distinguish the two.\n>>\n>> Now you are talking. Either a separate object store, or a packfile\n>> that is specially marked as such, would work.\n>\n> Jonathan's not in today, so let me say a few more words about this\n> approach.\n>\n> This approach implies a relaxed connectivity guarantee, by creating\n> two classes of objects:\n>\n> 1. Objects that I made should satisfy the connectivity check. They\n> can point to other objects I made, objects I fetched, or (*) objects\n> pointed to directly by objects I fetched. More on (*) below.\n\nOr objects that are referred to by objects I fetched. If you\nnarrowly clone while omitting a subdirectory, updated a file\nthat is outside the subdirectory, and created a new commit, while\nrecording the same tree object name for the directory you do not\nknow its contents (becaues you didn't fetch), then it is OK for the\ntop-level tree of the resulting commit you created to be pointing\nat the tree that represents the subdirectory you never touched.\n\n> The complication is in the \"git gc\" operation for the case (*).\n> Today, \"git gc\" uses a reachability walk to decide which objects to\n> remove --- an object referenced by no other object is fair game to\n> remove. With (*), there is another kind of object that must not be\n> removed: if an object that I made, M, points to a missing/promised\n> object, O, pointed to by a an object I fetched, F, then I cannot prune\n> F unless there is another fetched object present to anchor O.\n\nAbsolutely. Lazy-objects support comes with certain cost and this\nis one of them. \n\nBut I do not think it is realistic to expect that you can prune\nanything you fetched from the \"other place\" (i.e. the source\n'lazy-objects' hook reads from). After all, once they give out\nobjects to their clients (like us in this case), they cannot prune\nit, if we take the \"implicit promise\" approach to avoid the cost to\ntransmit and maintain a separate \"object list\".\n\n> For example: suppose I have a sparse checkout and run\n>\n> \tgit fetch origin refs/pulls/x\n> \tgit checkout -b topic FETCH_HEAD\n> \techo \"Some great modification\" >> README\n> \tgit add README\n> \tgit commit --amend\n>\n> When I run \"git gc\", there is nothing pointing to the commit that was\n> pointed to by the remote ref refs/pulls/x, so it can be pruned. I\n> would naively also expect that the tree pointed to by that commit\n> could be pruned. But pruning it means pruning the promise that made\n> it permissible to lack various blobs that my topic branch refers to\n> that are outside the sparse checkout area. So \"git gc\" must notice\n> that it is not safe to prune that tree.\n>\n> This feels hacky. I prefer the promised object list over this\n> approach.\n\nI think they are moral equivalents implemented differently with\ndifferent assumptions. The example we are discussing makes an extra\nassumption: In order to reduce the cost of transferring and\nmaintaining the list, we assume that all objects that came during\nthat transfer are implicitly \"promised\", i.e. everything behind each\nof these objects will later be available on demand. How these\nobjects are marked is up to the exact mechanism (my preference is to\nmark the resulting packfile as special; Jon Tan's message to which\nmy message was a resopnse alluded to using an alternate object\nstore). If you choose to maintain a separate \"object list\" and have\nthe \"other side\" explicitly give it, perhaps you can lift that\nassumption and replace it with some other assumption that assumes\nless.\n\n> Can you spell this out more? To be clear, are you speaking as a\n> reviewer or as the project maintainer? In other words, if other\n> reviewers are able to settle on a design that involves a relaxed\n> guarantee for fsck in this mode that they can agree on, does this\n> represent a veto meaning the patch can still not go through?\n\nConsider it a veto over punting without making sure that we can\nlater come up with a solution to give such a guarantee. I am not\ngetting a feeling that \"other reviewers\" are even seeking a \"relaxed\nguarantee\"---all I've seen in the thread is to give up any guarantee\nand to hope for the best.\n\n"},{"id":"325484","messageId":"20170802221334.GV13924@aiede.mtv.corp.google.com","threadId":"46475","inReplyTo":"xmqq8tj1snfq.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 0/5] Fsck for lazy objects, and (now) actual invocation of loader","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-08-02T22:13:34Z","receivedAt":"2017-08-02T22:13:42Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder writes:\n\n>> Can you spell this out more? To be clear, are you speaking as a\n>> reviewer or as the project maintainer? In other words, if other\n>> reviewers are able to settle on a design that involves a relaxed\n>> guarantee for fsck in this mode that they can agree on, does this\n>> represent a veto meaning the patch can still not go through?\n>\n> Consider it a veto over punting without making sure that we can\n> later come up with a solution to give such a guarantee. I am not\n> getting a feeling that \"other reviewers\" are even seeking a \"relaxed\n> guarantee\"---all I've seen in the thread is to give up any guarantee\n> and to hope for the best.\n\nThank you. That makes sense.\n\nIn my defense, one reason I had for being okay with dropping the\nconnectivity check in the \"lazy object\" setup (at a higher level than\nthis patch currently does it, to avoid wasted work) is that this patch\nseries does not include the required components to do it more properly\nand previous discussions on list had pointed to some of those\ncomponents that will arrive later (the \"object size cache\", which\ndoubles as an incomplete list of promises). But that doesn't put the\nproject in a good position because it isn't an explicitly spelled out\nplan.\n\nThe set of other reviewers that I was hoping will weigh in at some\npoint is the GVFS team at Microsoft.\n\nI'll write up a summary of the ideas discussed so far to try to get\nthis unblocked.\n\nSincerely,\nJonathan\n"},{"id":"325575","messageId":"20170803120849.5f7382d3@twelve2.svl.corp.google.com","threadId":"46475","inReplyTo":"xmqq8tj1snfq.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 0/5] Fsck for lazy objects, and (now) actual invocation of loader","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-08-03T19:08:49Z","receivedAt":"2017-08-03T19:09:00Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Wed, 02 Aug 2017 13:51:37 -0700\nJunio C Hamano wrote:\n\n> > The complication is in the \"git gc\" operation for the case (*).\n> > Today, \"git gc\" uses a reachability walk to decide which objects to\n> > remove --- an object referenced by no other object is fair game to\n> > remove. With (*), there is another kind of object that must not be\n> > removed: if an object that I made, M, points to a missing/promised\n> > object, O, pointed to by a an object I fetched, F, then I cannot prune\n> > F unless there is another fetched object present to anchor O.\n> \n> Absolutely. Lazy-objects support comes with certain cost and this\n> is one of them. \n> \n> But I do not think it is realistic to expect that you can prune\n> anything you fetched from the \"other place\" (i.e. the source\n> 'lazy-objects' hook reads from). After all, once they give out\n> objects to their clients (like us in this case), they cannot prune\n> it, if we take the \"implicit promise\" approach to avoid the cost to\n> transmit and maintain a separate \"object list\".\n\nBy this, do you mean that the client cannot prune anything lazily loaded\nfrom the server? If yes, I understand that the server cannot prune\nanything (for the reasons that you describe), but I don't see how that\napplies to the client.\n\n> > For example: suppose I have a sparse checkout and run\n> >\n> > \tgit fetch origin refs/pulls/x\n> > \tgit checkout -b topic FETCH_HEAD\n> > \techo \"Some great modification\" >> README\n> > \tgit add README\n> > \tgit commit --amend\n> >\n> > When I run \"git gc\", there is nothing pointing to the commit that was\n> > pointed to by the remote ref refs/pulls/x, so it can be pruned. I\n> > would naively also expect that the tree pointed to by that commit\n> > could be pruned. But pruning it means pruning the promise that made\n> > it permissible to lack various blobs that my topic branch refers to\n> > that are outside the sparse checkout area. So \"git gc\" must notice\n> > that it is not safe to prune that tree.\n> >\n> > This feels hacky. I prefer the promised object list over this\n> > approach.\n> \n> I think they are moral equivalents implemented differently with\n> different assumptions. The example we are discussing makes an extra\n> assumption: In order to reduce the cost of transferring and\n> maintaining the list, we assume that all objects that came during\n> that transfer are implicitly \"promised\", i.e. everything behind each\n> of these objects will later be available on demand. How these\n> objects are marked is up to the exact mechanism (my preference is to\n> mark the resulting packfile as special; Jon Tan's message to which\n> my message was a resopnse alluded to using an alternate object\n> store). If you choose to maintain a separate \"object list\" and have\n> the \"other side\" explicitly give it, perhaps you can lift that\n> assumption and replace it with some other assumption that assumes\n> less.\n\nMarking might be an issue if we expect the lazy loader to emit an object\nafter every hash, like in the current design. There would thus be one\nmark per object, with overhead similar to the promise list. (Having said\nthat, batching is possible - I plan to optimize common cases like\ncheckout, and have such a patch online for an older \"promised blob\"\ndesign [1].)\n\nOverhead could be reduced by embedding the mark in both the packed and\nloose objects, requiring a different format (instead of having a\nseparate \"catalog\" of marked files). This seems more complicated than\nusing an alternate object store, hence my suggestion.\n\n[1] https://github.com/jonathantanmy/git/commit/14f07d3f06bc3a1a2c9bca85adc8c42b230b9143\n"},{"id":"325819","messageId":"0bcbd46c-8b40-269d-f8d9-934fadd407dd@gmail.com","threadId":"46475","inReplyTo":"cover.1501532294.git.jonathantanmy@google.com","subject":"Re: [PATCH v2 0/5] Fsck for lazy objects, and (now) actual invocation of loader","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-08-08T17:13:57Z","receivedAt":"2017-08-08T17:14:04Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 7/31/2017 5:02 PM, Jonathan Tan wrote:\n> Besides review changes, this patch set now includes my rewritten\n> lazy-loading sha1_file patch, so you can now do this (excerpted from one\n> of the tests):\n> \n> test_create_repo server\n> test_commit -C server 1 1.t abcdefgh\n> HASH=$(git hash-object server/1.t)\n> \n> test_create_repo client\n> test_must_fail git -C client cat-file -p \"$HASH\"\n> git -C client config core.repositoryformatversion 1\n> git -C client config extensions.lazyobject \\\n> \"\\\"$TEST_DIRECTORY/t0410/lazy-object\\\" \\\"$(pwd)/server/.git\\\"\"\n> git -C client cat-file -p \"$HASH\"\n> \n> with fsck still working. Also, there is no need for a list of promised\n> blobs, and the long-running process protocol is being used.\n> \n> Changes from v1:\n> - added last patch that supports lazy loading\n> - clarified documentation in \"introduce lazyobject extension\" patch\n> (following Junio's comments [1])\n> \n> As listed in the changes above, I have rewritten my lazy-loading\n> sha1_file patch to no longer use the list of promises. Also, I have\n> added documentation about the protocol used to (hopefully) the\n> appropriate places.\n\nGlad to see the removal of the promises. Given the ongoing \nconversation, I'm interested to see how you are detecting locally create \nobjects vs those downloaded from a server.\n\n> \n> This is a minimal implementation, hopefully enough of a foundation to be\n> built upon. In particular, I haven't added the environment variable to\n> suppress lazy loading, and the lazy loading protocol only supports one\n> object at a time.\n\nWe can add multiple object support to the protocol when we get to the \npoint that we have code that will utilize it.\n\n> \n> Other work\n> ----------\n> \n> This differs slightly from Ben Peart's patch [2] in that the\n> lazy-loading functionality is provided through a configured shell\n> command instead of a hook shell script. I envision commands like \"git\n> clone\", in the future, needing to pre-configure lazy loading, and I\n> think that it will be less surprising to the user if \"git clone\" wrote a\n> default configuration instead of a default hook.\n\nThis was on my \"todo\" list to investigate as I've been told it can \nenable people to use taskset to set CPU affinity and get some \nsignificant performance wins. I'd be interested to see if it actually \nhelps here at all.\n\n> \n> This also differs from Christian Couder's patch set [3] that implement a\n> larger-scale object database, in that (i) my patch set does not support\n> putting objects into external databases, and (ii) my patch set requires\n> the lazy loader to make the objects available in the local repo, instead\n> of allowing the objects to only be stored in the external database.\n\nThis is the model we're using today so I'm confident it will meet our \nrequirements.\n\n> \n> [1] https://public-inbox.org/git/xmqqzibpn1zh.fsf@gitster.mtv.corp.google.com/\n> [2] https://public-inbox.org/git/20170714132651.170708-2-benpeart@microsoft.com/\n> [3] https://public-inbox.org/git/20170620075523.26961-1-chriscool@tuxfamily.org/\n> \n> Jonathan Tan (5):\n> environment, fsck: introduce lazyobject extension\n> fsck: support refs pointing to lazy objects\n> fsck: support referenced lazy objects\n> fsck: support lazy objects as CLI argument\n> sha1_file: support loading lazy objects\n> \n> Documentation/Makefile | 1 +\n> Documentation/gitattributes.txt | 54 ++--------\n> Documentation/gitrepository-layout.txt | 3 +\n> .../technical/long-running-process-protocol.txt | 50 +++++++++\n> Documentation/technical/repository-version.txt | 23 +++++\n> Makefile | 1 +\n> builtin/cat-file.c | 2 +\n> builtin/fsck.c | 25 ++++-\n> cache.h | 4 +\n> environment.c | 1 +\n> lazy-object.c | 80 +++++++++++++++\n> lazy-object.h | 12 +++\n> object.c | 7 ++\n> object.h | 13 +++\n> setup.c | 7 +-\n> sha1_file.c | 44 +++++---\n> t/t0410-lazy-object.sh | 113 +++++++++++++++++++++\n> t/t0410/lazy-object | 102 +++++++++++++++++++\n> 18 files changed, 478 insertions(+), 64 deletions(-)\n> create mode 100644 Documentation/technical/long-running-process-protocol.txt\n> create mode 100644 lazy-object.c\n> create mode 100644 lazy-object.h\n> create mode 100755 t/t0410-lazy-object.sh\n> create mode 100755 t/t0410/lazy-object\n> \n"},{"id":"325853","messageId":"0c2a1339-0645-bb0c-216f-e093edf85994@gmail.com","threadId":"46475","inReplyTo":"eadce97b6a1e80345a2621e71ce187e9e6bc05bf.1501532294.git.jonathantanmy@google.com","subject":"Re: [PATCH v2 5/5] sha1_file: support loading lazy objects","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-08-08T20:20:40Z","receivedAt":"2017-08-08T20:20:48Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 7/31/2017 5:02 PM, Jonathan Tan wrote:\n> Teach sha1_file to invoke the command configured in\n> extensions.lazyObject whenever an object is requested and unavailable.\n> \n> The usage of the hook can be suppressed through a flag when invoking\n> has_object_file_with_flags() and other similar functions.\n> \n> This is meant as a temporary measure to ensure that all Git commands\n> work in such a situation. Future patches will update some commands to\n> either tolerate missing objects (without invoking the command) or be\n> more efficient in invoking this command.\n\nTo prevent fetch from downloading all missing objects, you will also \nneed to add logic in check_connected. The simplest model is to simply \nreturn 0 if repository_format_lazy_object is set.\n\n/*\n * Running a with lazy_objects there will be objects that are\n * missing locally and we don't want to download a bunch of\n * commits, trees, and blobs just to make sure everything is\n * reachable locally so this option will skip reachablility\n * checks below that use rev-list. This will stop the check\n * before uploadpack runs to determine if there is anything to\n * fetch. Returning zero for the first check will also prevent the\n * uploadpack from happening. It will also skip the check after\n * the fetch is finished to make sure all the objects where\n * downloaded in the pack file. This will allow the fetch to\n * run and get all the latest tip commit ids for all the branches\n * in the fetch but not pull down commits, trees, or blobs via\n * upload pack.\n */\nif (repository_format_lazy_object)\n\treturn 0;\n\n[...]\n> diff --git a/sha1_file.c b/sha1_file.c\n> index b60ae15f7..1785c61d8 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -28,6 +28,11 @@\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> +#include \"lazy-object.h\"\n> +#include \"sha1-array.h\"\n> \n> #define SZ_FMT PRIuMAX\n> static inline uintmax_t sz_fmt(size_t s) { return s; }\n> @@ -2984,6 +2989,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n> \tconst unsigned char *real = (flags & OBJECT_INFO_LOOKUP_REPLACE) ?\n> \t\t\t\t lookup_replace_object(sha1) :\n> \t\t\t\t sha1;\n> +\tint already_retried = 0;\n> \n> \tif (!oi)\n> \t\toi = &blank_oi;\n> @@ -3008,30 +3014,38 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n> \t\t}\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\tif (flags & OBJECT_INFO_QUICK) {\n> -\t\t\treturn -1;\n> -\t\t} else {\n> -\t\t\treprepare_packed_git();\n> -\t\t\tif (!find_pack_entry(real, &e))\n> -\t\t\t\treturn -1;\n> -\t\t}\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\nSame feedback as before. I like to avoid using goto's as flow control \nother than in error handling.\n\nAlso, this patch looses the OBJECT_INFO_QUICK logic which could be restored.\n\n[...]\n\n> diff --git a/t/t0410/lazy-object b/t/t0410/lazy-object\n> new file mode 100755\n> index 000000000..4f4a9c38a\n> --- /dev/null\n> +++ b/t/t0410/lazy-object\n> @@ -0,0 +1,102 @@\n> +#!/usr/bin/perl\n> +#\n> +# Example implementation for the Git lazyObject protocol version 1. See\n> +# the documentation for extensions.lazyObject in\n> +# Documentation/technical/repository-version.txt\n> +#\n> +# Allows you to test the ability for blobs to be pulled from a host git repo\n> +# \"on demand.\" Called when git needs a blob it couldn't find locally due to\n> +# a lazy clone that only cloned the commits and trees.\n> +#\n> +# Please note, this sample is a minimal skeleton. No proper error handling\n> +# was implemented.\n> +\n> +use strict;\n> +use warnings;\n> +\n> +#\n> +# Point $DIR to the folder where your host git repo is located so we can pull\n> +# missing objects from it\n> +#\n> +my $DIR = $ARGV[0];\n> +\n\nAt some point, this should be based on the refactored pkt_* functions \ncurrently contained in the ObjectDB patch series.\n\n> +sub packet_bin_read {\n> +\tmy $buffer;\n> +\tmy $bytes_read = read STDIN, $buffer, 4;\n> +\tif ( $bytes_read == 0 ) {\n> +\n> +\t\t# EOF - Git stopped talking to us!\n> +\t\texit();\n> +\t}\n> +\telsif ( $bytes_read != 4 ) {\n> +\t\tdie \"invalid packet: '$buffer'\";\n> +\t}\n> +\tmy $pkt_size = hex($buffer);\n> +\tif ( $pkt_size == 0 ) {\n> +\t\treturn ( 1, \"\" );\n> +\t}\n> +\telsif ( $pkt_size > 4 ) {\n> +\t\tmy $content_size = $pkt_size - 4;\n> +\t\t$bytes_read = read STDIN, $buffer, $content_size;\n> +\t\tif ( $bytes_read != $content_size ) {\n> +\t\t\tdie \"invalid packet ($content_size bytes expected; $bytes_read bytes read)\";\n> +\t\t}\n> +\t\treturn ( 0, $buffer );\n> +\t}\n> +\telse {\n> +\t\tdie \"invalid packet size: $pkt_size\";\n> +\t}\n> +}\n> +\n> +sub packet_txt_read {\n> +\tmy ( $res, $buf ) = packet_bin_read();\n> +\tunless ( $buf =~ s/\\n$// ) {\n> +\t\tdie \"A non-binary line MUST be terminated by an LF.\";\n> +\t}\n> +\treturn ( $res, $buf );\n> +}\n> +\n> +sub packet_bin_write {\n> +\tmy $buf = shift;\n> +\tprint STDOUT sprintf( \"%04x\", length($buf) + 4 );\n> +\tprint STDOUT $buf;\n> +\tSTDOUT->flush();\n> +}\n> +\n> +sub packet_txt_write {\n> +\tpacket_bin_write( $_[0] . \"\\n\" );\n> +}\n> +\n> +sub packet_flush {\n> +\tprint STDOUT sprintf( \"%04x\", 0 );\n> +\tSTDOUT->flush();\n> +}\n> +\n> +( packet_txt_read() eq ( 0, \"git-lazy-object-client\" ) ) || die \"bad initialize\";\n> +( packet_txt_read() eq ( 0, \"version=1\" ) )\t\t || die \"bad version\";\n> +( packet_bin_read() eq ( 1, \"\" ) ) || die \"bad version end\";\n> +\n> +packet_txt_write(\"git-lazy-object-server\");\n> +packet_txt_write(\"version=1\");\n> +packet_flush();\n> +\n> +( packet_txt_read() eq ( 0, \"capability=get\" ) ) || die \"bad capability\";\n> +( packet_bin_read() eq ( 1, \"\" ) ) || die \"bad capability end\";\n> +\n> +packet_txt_write(\"capability=get\");\n> +packet_flush();\n> +\n> +while (1) {\n> +\tmy ($command) = packet_txt_read() =~ /^command=([^=]+)$/;\n> +\n> +\tif ( $command eq \"get\" ) {\n> +\t\tmy ($sha1) = packet_txt_read() =~ /^sha1=([0-9a-f]{40})$/;\n> +\t\tpacket_bin_read();\n> +\n> +\t\tsystem ('git --git-dir=\"' . $DIR . '\" cat-file blob ' . $sha1 . ' | git -c extensions.lazyobject=false hash-object -w --stdin >/dev/null 2>&1');\n> +\t\tpacket_txt_write(($?) ? \"status=error\" : \"status=success\");\n> +\t\tpacket_flush();\n> +\t} else {\n> +\t\tdie \"bad command '$command'\";\n> +\t}\n> +}\n> \n"}]}