@matheusbsbs / matheus-bagliano / commits / f97c8b1e00

revision: make pseudo-opt flags read via stdin behave consistently

When reading revisions from stdin via git-rev-list(1)'s `--stdin` option then these revisions never honor flags like `--not` which have been passed on the command line. Thus, an invocation like e.g. `git rev-list --all --not --stdin` will not treat all revisions read from stdin as uninteresting. While this behaviour may be surprising to a user, it's been this way ever since it has been introduced via 42cabc341c4 (Teach rev-list an option to read revs from the standard input., 2006-09-05). With that said, in c40f0b7877 (revision: handle pseudo-opts in `--stdin` mode, 2023-06-15) we have introduced a new mode to read pseudo opts from standard input where this behaviour is a lot more confusing. If you pass `--not` via stdin, it will: - Influence subsequent revisions or pseudo-options passed on the command line. - Influence pseudo-options passed via standard input. - _Not_ influence normal revisions passed via standard input. This behaviour is extremely inconsistent and bound to cause confusion. While it would be nice to retroactively change the behaviour for how `--not` and `--stdin` behave together, chances are quite high that this would break existing scripts that expect the current behaviour that has been around for many years by now. This is thus not really a viable option to explore to fix the inconsistency. Instead, we change the behaviour of how pseudo-opts read via standard input influence the flags such that the effect is fully localized. With this change, when reading `--not` via standard input, it will: - _Not_ influence subsequent revisions or pseudo-options passed on the command line, which is a change in behaviour. - Influence pseudo-options passed via standard input. - Influence normal revisions passed via standard input, which is a change in behaviour. Thus, all flags read via standard input are fully self-contained to that standard input, only. While this is a breaking change as well, the behaviour has only been recently introduced with Git v2.42.0. Furthermore, the current behaviour can be regarded as a simple bug. With that in mind it feels like the right thing to retroactively change it and make the behaviour sane. Signed-off-by: Patrick Steinhardt <ps@pks.im> Reported-by: Christian Couder <christian.couder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Sep 25, 2023 at 15:02 UTC f97c8b1e00297570a390789be84513f435d5ea52

3 files changed +33 -6

Documentation/rev-list-options.txt

+7 -1

@@ -151,6 +151,10 @@ endif::git-log[]

151 --not::

152 Reverses the meaning of the '{caret}' prefix (or lack thereof)

153 for all following revision specifiers, up to the next `--not`.

154 + When used on the command line before --stdin, the revisions passed

155 + through stdin will not be affected by it. Conversely, when passed

156 + via standard input, the revisions passed on the command line will

157 + not be affected by it.

158

159 --all::

160 Pretend as if all the refs in `refs/`, along with `HEAD`, are

@@ -240,7 +244,9 @@ endif::git-rev-list[]

244 them from standard input as well. This accepts commits and

245 pseudo-options like `--all` and `--glob=`. When a `--` separator

246 is seen, the following input is treated as paths and used to

243 - limit the result.

247 + limit the result. Flags like `--not` which are read via standard input

248 + are only respected for arguments passed in the same way and will not

249 + influence any subsequent command line arguments.

250

251 ifdef::git-rev-list[]

252 --quiet::

revision.c

+5 -5

@@ -2788,13 +2788,13 @@ static int handle_revision_pseudo_opt(struct rev_info *revs,

2788 }

2789

2790 static void read_revisions_from_stdin(struct rev_info *revs,

2791 - struct strvec *prune,

2792 - int *flags)

2791 + struct strvec *prune)

2792 {

2793 struct strbuf sb;

2794 int seen_dashdash = 0;

2795 int seen_end_of_options = 0;

2796 int save_warning;

2797 + int flags = 0;

2798

2799 save_warning = warn_on_object_refname_ambiguity;

2800 warn_on_object_refname_ambiguity = 0;

@@ -2817,13 +2817,13 @@ static void read_revisions_from_stdin(struct rev_info *revs,

2817 continue;

2818 }

2819

2820 - if (handle_revision_pseudo_opt(revs, argv, flags) > 0)

2820 + if (handle_revision_pseudo_opt(revs, argv, &flags) > 0)

2821 continue;

2822

2823 die(_("invalid option '%s' in --stdin mode"), sb.buf);

2824 }

2825

2826 - if (handle_revision_arg(sb.buf, revs, 0,

2826 + if (handle_revision_arg(sb.buf, revs, flags,

2827 REVARG_CANNOT_BE_FILENAME))

2828 die("bad revision '%s'", sb.buf);

2829 }

@@ -2906,7 +2906,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s

2906 }

2907 if (revs->read_from_stdin++)

2908 die("--stdin given twice?");

2909 - read_revisions_from_stdin(revs, &prune_data, &flags);

2909 + read_revisions_from_stdin(revs, &prune_data);

2910 continue;

2911 }

2912

t/t6017-rev-list-stdin.sh

+21

@@ -68,6 +68,7 @@ check --glob=refs/heads

68 check --glob=refs/heads --

69 check --glob=refs/heads -- file-1

70 check --end-of-options -dashed-branch

71 +check --all --not refs/heads/main

73 test_expect_success 'not only --stdin' '

74 cat >expect <<-EOF &&

@@ -127,4 +128,24 @@ test_expect_success 'unknown option without --end-of-options' '

128 test_cmp expect error

129 '

130

131 +test_expect_success '--not on command line does not influence revisions read via --stdin' '

132 + cat >input <<-EOF &&

133 + refs/heads/main

134 + EOF

135 + git rev-list refs/heads/main >expect &&

136 +

137 + git rev-list refs/heads/main --not --stdin <input >actual &&

138 + test_cmp expect actual

139 +'

140 +

141 +test_expect_success '--not via stdin does not influence revisions from command line' '

142 + cat >input <<-EOF &&

143 + --not

144 + EOF

145 + git rev-list refs/heads/main >expect &&

146 +

147 + git rev-list refs/heads/main --stdin refs/heads/main <input >actual &&

148 + test_cmp expect actual

149 +'

150 +

151 test_done