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@xxxxxxxxxx>
---
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(-)
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
