Re: [PATCH] t6018-rev-list-glob: fix 'empty stdin' test

[Junio C Hamano <gitster@pobox.com>]

SZEDER Gábor <szeder.dev@gmail.com> writes:

> Redirect 'git rev-list's stdin explicitly from /dev/null to provide
> empty input.  (Strictly speaking we don't need this redirection,
> because the test script's stdin is already redirected from /dev/null
> anyway, but I think it's better to be explicit about it.)

Yes.

>  test_expect_failure 'rev-list should succeed with empty output on empty stdin' '
> -	git rev-list --stdin <expect >actual &&
> +	git rev-list --stdin </dev/null >actual &&
>  	test_must_be_empty actual
>  '

By the way, it may be about time to turn that expect-failure into
expect-success.  It is somewhat unfortunate that 0c5dc743 ("t6018:
flesh out empty input/output rev-list tests", 2017-08-02) removed
the comment that said "we _might_ want to change the behaviour in
these cases" and explained the tests as reminders, anticipating that
the series will change the behaviour for three cases where the
pending list ends up empty to make the discussion moot, but it
changed the behaviour of only two of them, leaving the "--stdin
reads empty" case behind.

It may be just the matter of doing something like the attached
patch.  I won't be committing such a behaviour change during the
pre-release feature freeze, but we may want to consider doing this
early in the next cycle.

 revision.c               | 13 +++++++++++++
 t/t6018-rev-list-glob.sh |  4 ++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/revision.c b/revision.c
index d12e6d8a4a..21fb413511 100644
--- a/revision.c
+++ b/revision.c
@@ -2441,6 +2441,19 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 		object = get_reference(revs, revs->def, &oid, 0);
 		add_pending_object_with_mode(revs, object, revs->def, oc.mode);
 	}
+	/* 
+	 * Even if revs->pending is empty after all the above, if we
+	 * handled "--stdin", then the caller really meant to give us
+	 * an empty commit range.  Just let the traversal give an
+	 * empty result without causing a "no input?  do you know how
+	 * to use this command?" failure.
+	 *
+	 * NOTE!!!  Because "--stdin </dev/null --default HEAD" should
+	 * default to HEAD, this must come _after_ the above block
+	 * that deals with revs->ref fallback.
+	 */
+	if (read_from_stdin)
+		revs->rev_input_given = 1;
 
 	/* Did the user ask for any diff output? Run the diff! */
 	if (revs->diffopt.output_format & ~DIFF_FORMAT_NO_OUTPUT)
diff --git a/t/t6018-rev-list-glob.sh b/t/t6018-rev-list-glob.sh
index 02936c2f24..db8a7834d8 100755
--- a/t/t6018-rev-list-glob.sh
+++ b/t/t6018-rev-list-glob.sh
@@ -255,8 +255,8 @@ test_expect_success 'rev-list accumulates multiple --exclude' '
 	compare rev-list "--exclude=refs/remotes/* --exclude=refs/tags/* --all" --branches
 '
 
-test_expect_failure 'rev-list should succeed with empty output on empty stdin' '
+test_expect_success 'rev-list should succeed with empty output on empty stdin' '
 	git rev-list --stdin </dev/null >actual &&
 	test_must_be_empty actual
 '