Re: [PATCH] object-name: explain why <ref>~N fails in a shallow clone

"Harald Nordgren via GitGitGadget" [off-list ref] writes:

From: Harald Nordgren <redacted>

Asking for a commit's ancestor with <ref>~N or <ref>^N in a shallow
clone that does not have N commits of history locally fails with a
bare "is not a commit" error, with no indication that the repository
being shallow is the reason, or what to do about it.

I am not sure if bringing up '^N' (the N-th parent of a merge) in an
attempt to be more complete helps readers or confuses them.  Unlike
'<rev>~N', where increasing N raises the required depth of a shallow
clone to make the target revision available, both '<rev>^1' and
'<rev>^43' of '<rev>' share the same depth.  If '<rev>' exists
locally and its first parent '<rev>^1' also does, it is likely that
'<rev>^2' is also available, as they are at the same depth from
'<rev>'.

The title of the commit does not share the problem, which is a good
thing ;-).

Add a hint, shown when the walk runs out of parents exactly at a
recorded shallow boundary, explaining that history was intentionally
truncated there. When <ref> looks like <remote>/<branch> and <remote>
is configured, the suggested command names that remote and branch

Good thinking.


As branch 'B' of remote 'R' is not necessarily stored locally at
'refs/remotes/R/B', implementing the semantics correctly and showing
the correct remote name and their branch name by reverse mapping R/B
back requires a bit of care, but it should not be impossibly hard.

directly. For <ref>~N it suggests the exact --deepen needed,
accounting for any history already present instead of just N. For
<ref>^N the suggestion is always --deepen=1, regardless of N: a
shallow boundary commit has no parents recorded locally at all, so
deepening by one generation fetches its complete real parent list in
one step, whether that commit turns out to have one parent or several.
The hint only fires when the search stops at an actual shallow
boundary, not merely because the repository happens to be shallow
elsewhere, so it does not misfire on a short history that is not
shallow-truncated.

I think Ben also mentioned this, but <ref> is probably better
written as <rev> in the above.  A ref(erence) like "master",
"origin/next", or "refs/remotes/origin/topic" are all rev(ision)s,
and this new advice feature is not limited to requests that are
made using references.

    When the revision <rev> is given as a remote-tracking branch,
    the remote and branch are exactly named in the suggested
    command.  For <rev>~N, it suggests ...

The advice is threaded through GET_OID_QUIETLY so it is not shown
during the internal re-resolution some commands do while building a
better error message, which would otherwise print it twice for the
same failing argument.

Nice.

Signed-off-by: Harald Nordgren <redacted>
---
    object-name: explain why ~N fails in a shallow clone
    
    Asking for a commit's ancestor with <ref>~N in a shallow clone that
    doesn't have N commits of history locally fails with a "is not a commit"
    error, with no indication that the repository being shallow is the
    reason.

It may not be intuitive to new users that in a shallow clone "git
log" stops in the middle, instead of going down to the beginning of
the history, downloading necessary objects on demand.  But fixing it
by adding such a feature is totally unrelated and outside the scope
of this topic ;-).

It is obvious that users would see such a message when they say

    $ git show HEAD~20
    $ git log HEAD~20..HEAD

but would they see the same when

    $ git log -20 HEAD
    $ git log --since=2.months HEAD

and internally HEAD~20 fails to resolve?  Should they see the same
hint?

+	test_must_fail git -C shallow-advice rev-parse origin/main~1 2>err &&
+	check_shallow_history_advice origin/main "$oid" \
+		"git fetch --deepen=1 origin main"

This is very straight-forward.

+		test_must_fail git rev-parse origin/main~5 2>err &&
+		check_shallow_history_advice origin/main "$oid" \
+			"git fetch --deepen=3 origin main" &&

Again, very straight-forward.

+	test_must_fail git -C shallow-advice-caret rev-parse origin/main^1 2>err &&
+	check_shallow_history_advice origin/main "$oid" \
+		"git fetch --deepen=1 origin main"

Ditto.

+		test_must_fail git rev-parse origin/main^2 2>err &&
+		check_shallow_history_advice origin/main "$oid" \
+			"git fetch --deepen=1 origin main" &&

Ditto.

+test_expect_success 'shallowHistory advice not shown for a non-shallow repository' '
+	test_must_fail git rev-parse HEAD~100000 2>err &&
+	test_grep ! "^hint:" err
+'

OK.

+test_expect_success 'shallowHistory advice not shown when resolution succeeds' '
+	test_commit shallow_ok_1 &&
+	test_commit shallow_ok_2 &&
+	test_commit shallow_ok_3 &&
+	git clone --no-local --depth=3 --branch main --single-branch \
+		.git shallow-advice-ok &&
+	test_when_finished "rm -rf shallow-advice-ok" &&
+	git -C shallow-advice-ok rev-parse origin/main~1 >actual 2>err &&
+	test_grep ! "^hint:" err
+'

OK.

I guess the answer to my earlier "does internally failing to resolve
due to graft point count?" is "no"?

Thanks.