On Mon, May 19, 2014 at 10:35:56PM +0300, Michael S. Tsirkin wrote:
> I tried to fump the whole history of qemu with format-patch.
> It crashes both with v2.0.0-rc2-21-g6087111
> and with git 1.8.3.1:
>
> ~/opt/libexec/git-core/git-format-patch --follow -o patches/all
> e63c3dc74bfb90e4522d075d0d5a7600c5145745..
You can't use "--follow" without specifying a single pathspec. Running
"git log --follow" notices and blocks this, but format-patch doesn't
(and segfaults later when the follow code assumes there is a pathspec).
I think we need:
-- >8 --
Subject: move "--follow needs one pathspec" rule to diff_setup_done
Because of the way "--follow" is implemented, we must have
exactly one pathspec. "git log" enforces this restriction,
but other users of the revision traversal code do not. For
example, "git format-patch --follow" will segfault during
try_to_follow_renames, as we have no pathspecs at all.
We can push this check down into diff_setup_done, which is
probably a better place anyway. It is the diff code that
introduces this restriction, so other parts of the code
should not need to care themselves.
Reported-by: "Michael S. Tsirkin" <mst@xxxxxxxxxx>
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
I didn't include a test, as I don't think the current behavior is what
we want in the long run. That is, it would be nice if eventually
--follow learned to be more flexible. Any test for this would
necessarily be looking for the opposite of that behavior. But maybe it's
worth it to just have in the meantime, and anyone who works on --follow
can rip out the test.
builtin/log.c | 8 ++------
diff.c | 3 +++
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 39e8836..3b6a6bb 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -158,13 +158,9 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
if (rev->show_notes)
init_display_notes(&rev->notes_opt);
- if (rev->diffopt.pickaxe || rev->diffopt.filter)
+ if (rev->diffopt.pickaxe || rev->diffopt.filter ||
+ DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES))
rev->always_show_header = 0;
- if (DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES)) {
- rev->always_show_header = 0;
- if (rev->diffopt.pathspec.nr != 1)
- usage("git logs can only follow renames on one pathname at a time");
- }
if (source)
rev->show_source = 1;
diff --git a/diff.c b/diff.c
index f72769a..68bb8c5 100644
--- a/diff.c
+++ b/diff.c
@@ -3325,6 +3325,9 @@ void diff_setup_done(struct diff_options *options)
}
options->diff_path_counter = 0;
+
+ if (DIFF_OPT_TST(options, FOLLOW_RENAMES) && options->pathspec.nr != 1)
+ die(_("--follow requires exactly one pathspec"));
}
static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)
--
2.0.0.rc1.436.g03cb729
--
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