Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:
> For some inexplicable reason, "git diff" would call "diff_setup_done()"
> iff we hadn't given an explicit output format.
>
> That makes no sense, since much of what diff_setup_done() does is exactly
> about checking the output format!
We did not even have _any_ setup_done() call in "git diff"
itself in the original, because setup_revisions() has its own
call to setup_done(), and it always called setup_revisions()
before you reached that part of the code you have in your patch.
Commit 047fbe906b375e8a3a7564ad0e4443f62dd528a2 ("builtin-diff:
turn recursive on when defaulting to --patch format.") added the
call to setup_done() only when we are defaulting to output
format, to re-validate the consistency of output format options.
The screw-up was commit fcfa33ec905fcde1c16e7cbbe00d7147b89f1f01
(diff: make more cases implicit --no-index) that made the call
to setup_revisions() conditional if setup_diff_no_index()
decides to take over, but it forgot that setup_diff_no_index()
does not call setup_done().
So I tend to think the attached is a better fix.
---
diff-lib.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/diff-lib.c b/diff-lib.c
index f5568c3..da55713 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -298,6 +298,8 @@ int setup_diff_no_index(struct rev_info *revs,
revs->diffopt.nr_paths = 2;
revs->diffopt.no_index = 1;
revs->max_count = -2;
+ if (diff_setup_done(&revs->diffopt) < 0)
+ die("diff_setup_done failed");
return 0;
}
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
