Thanks for your review. > > Note that commits and tags may still be lazy-fetched. I limited myself > > to objects that could be trees or blobs here because Git does not > > support creating such commit- and tag-excluding clones yet, and even if > > such a clone were manually created, Git does not have good support for > > fetching a single commit (when fetching a commit, it and all its > > ancestors would be sent). > > Is there a place we could put a NEEDSWORK comment to avoid confusion > when debugging if this gets introduced later? > > Even if not, this seems like a sensible choice. Done in the test. > > I've verified that this also solves the bug explained in: > > https://public-inbox.org/git/20191007181825.13463-1-jonathantanmy@xxxxxxxxxx/ > > Might be worth mentioning the example from there in the commit message > as well, to help explain the context behind the change. > > I would still be in favor of applying that more conservative change to > "master", even this late in the -rc cycle. If we're applying that change first, then this no longer fixes any bug, but is just a code cleanup. (If we don't apply that change, then I'll include that example in the commit message.) > Should we make OBJECT_INFO_QUICK always imply > OBJECT_INFO_SKIP_FETCH_OBJECT? I would suspect that if we are willing to > avoid checking thoroughly locally, checking remotely would be even more > undesirable. As I wrote in [1], the implication does not really go both ways. I think it's better to keep them separate. (Or, at least, we don't need to make the decision in this patch.) [1] https://public-inbox.org/git/20191011220822.154063-1-jonathantanmy@xxxxxxxxxx/ > > +test_expect_success 'fetch lazy-fetches only to resolve deltas' ' > > + setup_triangle && > > + > > + # Exercise to make sure it works. Git will not fetch anything from the > > + # promisor remote other than for the big blob (because it needs to > > + # resolve the delta). > > + GIT_TRACE_PACKET="$(pwd)/trace" git -C client \ > > + fetch "file://$(pwd)/server" master && > > + > > + # Verify the assumption that the client needed to fetch the delta base > > + # to resolve the delta. > > + git hash-object big-blob.txt >hash && > > + grep "want $(cat hash)" trace > > nit: can avoid using cat: > > hash=$(git hash-object big-blob.txt) && > grep "want $hash" trace I think it's less error-prone if we always have a "git" command on its own on a line, to avoid losing its error code. When piped into another invocation, or when command-substituted into an argument (e.g. "echo $(git hash-object foo)"), we lose its error code.
