git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] revision: allow missing promisor objects on CLI
@ 2019-12-28  0:34 Jonathan Tan
  2019-12-28  3:50 ` Junio C Hamano
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Jonathan Tan @ 2019-12-28  0:34 UTC (permalink / raw)
  To: git; +Cc: Jonathan Tan, matvore

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

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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-03-25 20:50 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-28  0:34 [PATCH] revision: allow missing promisor objects on CLI Jonathan Tan
2019-12-28  3:50 ` Junio C Hamano
2019-12-30 18:38   ` Jonathan Tan
2019-12-30 20:33     ` Junio C Hamano
2019-12-30 23:44 ` [PATCH v2] " Jonathan Tan
2019-12-31  0:09   ` Jonathan Nieder
2020-01-02 20:49     ` Jonathan Tan
2020-01-11 22:34 ` [PATCH v3 0/2] Un-regress rev-list --exclude-promisor-objects Jonathan Tan
2020-01-11 22:34   ` [PATCH v3 1/2] revision: document get_reference() Jonathan Tan
2020-03-25 20:46     ` Emily Shaffer
2020-01-11 22:34   ` [PATCH v3 2/2] revision: un-regress --exclude-promisor-objects Jonathan Tan
2020-03-25 20:50     ` Emily Shaffer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).