From: Ben Peart Date: Fri, 28 Jul 2017 13:20:17 GMT Subject: Re: [RFC PATCH 1/4] environment, fsck: introduce lazyobject extension Message-ID: In-Reply-To: On 7/27/2017 2:55 PM, Junio C Hamano wrote: > Jonathan Tan writes: > >> Currently, Git does not support repos with very large numbers of objects >> or repos that wish to minimize manipulation of certain blobs (for >> example, because they are very large) very well, even if the user >> operates mostly on part of the repo, because Git is designed on the >> assumption that every referenced object is available somewhere in the >> repo storage. >> I very much like the direction this is taking. Handling missing objects gracefully is an important part of the overall partial clone support. >> Introduce a new repository extension option "extensions.lazyobject", of >> data type string. If this is set in a repository, Git will tolerate >> objects being missing in that repository. When Git needs those objects, >> it will invoke the command in that option. > I'm very glad you are doing this. Having never used precious objects I was unaware of the extensions concept but it looks like a great way to deal with this repo specific option. > My reading hiccupped after the first sentence, as the problem > description made it sound like this was a boolean ("are we using > lazy object feature?"), after reading "data type string". And then > "the command in that option" made me hiccup one more time, as it did > not "click" that "in that option" was trying to say that the string > is used as the command name (or is it a whole command line? The > leading part of the command line to which some arguments are > appended before it gets invoked as a command? or what?). > > Logically, I think it is more like > > - extensions.lazyobject can be set to tell Git to consider missing > objects in certain cases are not errors; > > - the value of extensions.lazyobject variable must be a string, > which is used to name the command to lazily make the object > "appear" in the repository on demand. > >> Teach fsck about the new state of affairs. In this commit, teach fsck >> that missing objects referenced from the reflog are not an error case; >> in future commits, fsck will be taught about other cases. > > In any case, sounds like a small and good first step. > I agree completely with the feedback on the description. That is quite the run on sentence. :) >> + >> +`lazyObject` >> +~~~~~~~~~~~~~~~~~ >> + >> +When the config key `extensions.lazyObject` is set to a string, Git >> +tolerates objects being missing in the repository. This string contains >> +the command to be run whenever a missing object is needed. > > This has the same issue but to a lessor degree as there is "string > contains". What the command will do (e.g. "makes the object > magically appear in the object store" or "emits the contents of the > object to its standard output, so that Git can hash it to make it > appear in the object store"), and how it is used (e.g. "this is a > single command name and nothing else", "this is a leading part of > command line and arguments are appended at the end, before the whole > thing is passed to the shell to be executed", etc.) will need to be > clarified in the final version of the series (not necessarily at > this step---the series can elaborate in the later patches). > >> diff --git a/builtin/fsck.c b/builtin/fsck.c >> index fb0947009..1cfb8d98c 100644 >> --- a/builtin/fsck.c >> +++ b/builtin/fsck.c >> @@ -402,7 +402,7 @@ static void fsck_handle_reflog_oid(const char *refname, struct object_id *oid, >> xstrfmt("%s@{%"PRItime"}", refname, timestamp)); >> obj->flags |= USED; >> mark_object_reachable(obj); >> - } else { >> + } else if (!repository_format_lazy_object) { >> error("%s: invalid reflog entry %s", refname, oid_to_hex(oid)); >> errors_found |= ERROR_REACHABLE; >> } >> diff --git a/cache.h b/cache.h >> index 6c8242340..9e6bc0a21 100644 >> --- a/cache.h >> +++ b/cache.h >> @@ -853,10 +853,12 @@ extern int grafts_replace_parents; >> #define GIT_REPO_VERSION 0 >> #define GIT_REPO_VERSION_READ 1 >> extern int repository_format_precious_objects; >> +extern char *repository_format_lazy_object; > > This is not a new problem, but I think these two should be > called repository_extension_$NAME not repository_format_$NAME. > >> diff --git a/t/t0410-lazy-object.sh b/t/t0410-lazy-object.sh >> new file mode 100755 >> index 000000000..36442531f >> --- /dev/null >> +++ b/t/t0410-lazy-object.sh >> @@ -0,0 +1,32 @@ >> +#!/bin/sh >> + >> +test_description='lazy object' >> + >> +. ./test-lib.sh >> + >> +delete_object () { >> + rm $1/.git/objects/$(echo $2 | cut -c1-2)/$(echo $2 | cut -c3-40) >> +} >> + >> +test_expect_success 'fsck fails on lazy object in reflog' ' >> + test_create_repo repo && >> + test_commit -C repo 1 && >> + >> + A=$(git -C repo commit-tree -m a HEAD^{tree}) && >> + B=$(git -C repo commit-tree -m b HEAD^{tree}) && >> + >> + # Reference $A only from reflog, and delete it >> + git -C repo branch mybranch "$A" && >> + git -C repo branch -f mybranch "$B" && >> + delete_object repo "$A" && >> + >> + test_must_fail git -C repo fsck >> +' >> +test_expect_success '...but succeeds if lazyobject is set' ' >> + git -C repo config core.repositoryformatversion 1 && >> + git -C repo config extensions.lazyobject "arbitrary string" && >> + git -C repo fsck >> +' >> + >> +test_done