On Tue, May 30, 2017 at 10:23:54PM +0900, Junio C Hamano wrote:
> Jeff King <[email protected]> writes:
>
> > Nope, I have those patches directly on your e83352ef23, and it passes. I
> > wonder if there's something funny between our environments. What does
> > the failure look like for you?
>
> Travis seems to be seeing the same failure. Curiously, the topic by
> itself passes for me; iow, pu fails, pu^2 doesn't fail.
>
> git.git/pu$ ./git rev-list -h
> BUG: environment.c:173: setup_git_env called without repository
> Aborted (core dumped)
>
> Hmph...
Ah, OK, I can reproduce when merged with pu. Bisecting it was tricky.
To see the problem, you need both my new test _and_ b1ef400ee
(setup_git_env: avoid blind fall-back to ".git", 2016-10-20). The latter
is only in v2.13, so topics forked from v2.12 need that commit applied.
Anyway, the problem is sk/dash-is-previous, specifically fc5684b47
(revision.c: args starting with "-" might be a revision, 2017-02-25). It
looks like the revision parser used to just bail on "-h", because
revision.c would say "I don't recognize this" and then cmd_rev_list()
would similarly say "I don't recognize this" and call usage(). But now
we actually try to read it as a ref, which obviously requires being
inside a repository.
Normally that's OK, but because of the "-h doesn't set up the repo"
thing from 99caeed05, we may not have setup the repo, and so looking up
refs is forbidden. The fix is probably to have revision.c explicitly
recognize "-h" and bail on it as an unknown option (it can't handle
the flag itself because only the caller knows the full usage()).
I do wonder, though, if there's any other downside to trying to look up
other options as revisions (at least it wastes time doing nonsense
revision lookups on options known only to cmd_rev_list()). I'm not sure
why that commit passes everything starting with a dash as a possible
revision, rather than just "-".
I.e.:
diff --git a/revision.c b/revision.c
index 5470c33ac..1e26c3951 100644
--- a/revision.c
+++ b/revision.c
@@ -2233,7 +2233,14 @@ int setup_revisions(int argc, const char **argv, struct
rev_info *revs, struct s
}
if (opts < 0)
exit(128);
- maybe_opt = 1;
+ if (arg[1]) {
+ /* arg is an unknown option */
+ argv[left++] = arg;
+ continue;
+ } else {
+ /* special token "-" */
+ maybe_opt = 1;
+ }
}
I don't see anything in the commit message, but I didn't dig in the
mailing list.
-Peff
