rev-list: make empty --stdin not an error

When we originally did the series that contains 7ba826290a (revision: add rev_input_given flag, 2017-08-02) the intent was that "git rev-list --stdin </dev/null" would similarly become a successful noop. However, an attempt at the time to do that did not work[1]. The problem is that rev_input_given serves two roles: - it tells rev-list.c that it should not error out - it tells revision.c that it should not have the "default" ref kick (e.g., "HEAD" in "git log") We want to trigger the former, but not the latter. This is technically possible with a single flag, if we set the flag only after revision.c's revs->def check. But this introduces a rather subtle ordering dependency. Instead, let's keep two flags: one to denote when we got actual input (which triggers both roles) and one for when we read stdin (which triggers only the first). This does mean a caller interested in the first role has to check both flags, but there's only one such caller. And any future callers might want to make the distinction anyway (e.g., if they care less about erroring out, and more about whether revision.c soaked up our stdin). In fact, we already keep such a flag internally in revision.c for this purpose, so this is really just exposing that to the caller (and the old function-local flag can go away in favor of our new one). [1] https://public-inbox.org/git/20170802223416.gwiezhbuxbdmbjzx@sigill.intra.peff.net/ Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Aug 22, 2018 at 17:37 UTC a12cbe23ef765e89ba5b0872f6330a693ea391d2
4 files changed +9 -5
builtin/rev-list.c
+1 -1
@@ -493,7 +493,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
493 if ((!revs.commits && reflog_walk_empty(revs.reflog_info) &&
494 (!(revs.tag_objects || revs.tree_objects || revs.blob_objects) &&
495 !revs.pending.nr) &&
496 - !revs.rev_input_given) ||
496 + !revs.rev_input_given && !revs.read_from_stdin) ||
497 revs.diff)
498 usage(rev_list_usage);
499
revision.c
+2 -3
@@ -2315,7 +2315,7 @@ static void NORETURN diagnose_missing_default(const char *def)
2315 */
2316 int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct setup_revision_opt *opt)
2317 {
2318 - int i, flags, left, seen_dashdash, read_from_stdin, got_rev_arg = 0, revarg_opt;
2318 + int i, flags, left, seen_dashdash, got_rev_arg = 0, revarg_opt;
2319 struct argv_array prune_data = ARGV_ARRAY_INIT;
2320 const char *submodule = NULL;
2321
@@ -2345,7 +2345,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
2345 revarg_opt = opt ? opt->revarg_opt : 0;
2346 if (seen_dashdash)
2347 revarg_opt |= REVARG_CANNOT_BE_FILENAME;
2348 - read_from_stdin = 0;
2348 for (left = i = 1; i < argc; i++) {
2349 const char *arg = argv[i];
2350 if (*arg == '-') {
@@ -2364,7 +2363,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
2363 argv[left++] = arg;
2364 continue;
2365 }
2367 - if (read_from_stdin++)
2366 + if (revs->read_from_stdin++)
2367 die("--stdin given twice?");
2368 read_revisions_from_stdin(revs, &prune_data);
2369 continue;
revision.h
+5
@@ -81,6 +81,11 @@ struct rev_info {
81 */
82 int rev_input_given;
83
84 + /*
85 + * Whether we read from stdin due to the --stdin option.
86 + */
87 + int read_from_stdin;
88 +
89 /* topo-sort */
90 enum rev_sort_order sort_order;
91
t/t6018-rev-list-glob.sh
+1 -1
@@ -255,7 +255,7 @@ test_expect_success 'rev-list accumulates multiple --exclude' '
255 compare rev-list "--exclude=refs/remotes/* --exclude=refs/tags/* --all" --branches
256 '
257
258 -test_expect_failure 'rev-list should succeed with empty output on empty stdin' '
258 +test_expect_success 'rev-list should succeed with empty output on empty stdin' '
259 git rev-list --stdin </dev/null >actual &&
260 test_must_be_empty actual
261 '