Commit 4cf67869b2 ("list-objects.c: don't segfault for missing cmdline objects", 2018-12-06) prevented some segmentation faults from occurring by tightening handling of missing objects provided through the CLI: if --ignore-missing is set, then it is OK (and the missing object ignored, just like one would if encountered in traversal).
However, in the case that --ignore-missing is not set but --exclude-promisor-objects is set, there is still no distinction between the case wherein the missing object is a promisor object and the case wherein it is not. This is unnecessarily restrictive, since if a missing promisor object is encountered in traversal, it is ignored; likewise it should be ignored if provided through the CLI. Therefore, distinguish between these 2 cases. (As a bonus, the code is now simpler.)
(Note that this only affects handling of missing promisor objects. Handling of non-missing promisor objects is already done by setting all of them to UNINTERESTING in prepare_revision_walk().)
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
For those curious, I discovered this while trying to extend the
check-targets-only optimization done in dfa33a298d ("clone: do faster
object check for partial clones", 2019-04-21) to fetches as well. While
investigating my previous work of adding check-target functionality in
addition to the usual connectivity check (for correctness, not for
performance) in 35f9e3e5e7 ("fetch: in partial clone, check presence of
targets", 2018-09-21), I discovered that at current master, the check
was somehow no longer needed because rev-list dies on missing objects on
CLI. But I don't think that the current behavior is obvious, hence this
commit (which also restores the need for the check-target
functionality).
---
revision.c | 10 +++++++++-
t/t0410-partial-clone.sh | 10 ++--------
2 files changed, 11 insertions(+), 9 deletions(-)Show changes to 2 files +11 −9
revision.c, t/t0410-partial-clone.sh
diff --git a/revision.c b/revision.c index 8136929e23..345615e300 100644 --- a/revision.c +++ b/revision.c @@ -1907,7 +1907,15 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi verify_non_filename(revs->prefix, arg); object = get_reference(revs, arg, &oid, flags ^ local_flags); if (!object) - return revs->ignore_missing ? 0 : -1; + /* + * Either this object is missing and ignore_missing is true, or + * this object is a (missing) promisor object and + * exclude_promisor_objects is true. In any case, we are + * allowed to skip processing of this object; this object will + * not appear in output and cannot be used as a source of + * UNINTERESTING ancestors (since it is missing). + */ + return 0; add_rev_cmdline(revs, object, arg_, REV_CMD_REV, flags ^ local_flags); add_pending_object_with_path(revs, object, arg, oc.mode, oc.path); free(oc.path); diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh index a3988bd4b8..fd28f5402a 100755 --- a/t/t0410-partial-clone.sh +++ b/t/t0410-partial-clone.sh @@ -416,15 +416,9 @@ test_expect_success 'rev-list dies for missing objects on cmd line' ' git -C repo config extensions.partialclone "arbitrary string" && for OBJ in "$COMMIT" "$TREE" "$BLOB"; do - test_must_fail git -C repo rev-list --objects \ + git -C repo rev-list --objects \ --exclude-promisor-objects "$OBJ" && - test_must_fail git -C repo rev-list --objects-edge-aggressive \ - --exclude-promisor-objects "$OBJ" && - - # Do not die or crash when --ignore-missing is passed. - git -C repo rev-list --ignore-missing --objects \ - --exclude-promisor-objects "$OBJ" && - git -C repo rev-list --ignore-missing --objects-edge-aggressive \ + git -C repo rev-list --objects-edge-aggressive \ --exclude-promisor-objects "$OBJ" done '
-- 2.24.1.735.g03f4e72817-goog
