git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: "SZEDER Gábor" <szeder.dev@gmail.com>,
	"Sathyajith Bhat" <sathya@sathyasays.com>,
	git@vger.kernel.org
Subject: Re*: Segfault in git when using git logs
Date: Wed, 04 Nov 2020 09:54:01 -0800	[thread overview]
Message-ID: <xmqq7dr1nh3a.fsf_-_@gitster.c.googlers.com> (raw)
In-Reply-To: <xmqqimamqjhl.fsf@gitster.c.googlers.com> (Junio C. Hamano's message of "Tue, 03 Nov 2020 12:21:42 -0800")

Junio C Hamano <gitster@pobox.com> writes:

>>> +	if (rev->line_level_traverse) {
>>> +		if (rev->diffopt.filter)
>>> +			die(_("-L<range>:<file> cannot be used with pathspec"));
>>
>> Should this be checking rev->diffopt.pathspec.nr?
>
> Embarrassing but yes ;-).

I wonder if rev->prune_data.nr is what matters here, though.

The prune_data is often identical to diffopt.pathspec, but the
former affects the paths that participate in history simplification,
while the latter is used when deciding which paths to show
differences between the commit and its parent(s) and used to widen
the set independently from prune_data for the "--full-diff" option.

-- >8 --
Subject: [PATCH] log: diagnose -L used with pathspec as an error

The -L option is documented to accept no pathspec, but the
command line option parser has allowed the combination without
checking so far.  Ensure that there is no pathspec when the -L
option is in effect to fix this.

Incidentally, this change fixes another bug in the command line
option parser, which has allowed the -L option used together
with the --follow option.  Because the latter requires exactly
one path given, but the former takes no pathspec, they become
mutually incompatible automatically.  Because the -L option
follows renames on its own, there is no reason to give --follow
at the same time.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/log.c       |  3 +++
 t/t4211-line-log.sh | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/builtin/log.c b/builtin/log.c
index 0a7ed4bef9..9d70f3e60b 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -206,6 +206,9 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 	if (argc > 1)
 		die(_("unrecognized argument: %s"), argv[1]);
 
+	if (rev->line_level_traverse && rev->prune_data.nr)
+		die(_("-L<range>:<file> cannot be used with pathspec"));
+
 	memset(&w, 0, sizeof(w));
 	userformat_find_requirements(NULL, &w);
 
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index 2d1d7b5d19..3d1bd6ed80 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -8,6 +8,24 @@ test_expect_success 'setup (import history)' '
 	git reset --hard
 '
 
+# Basic command line option parsing
+test_expect_success '-L is incompatible with pathspec' '
+	# This may fail due to "no such path a.c in commit",
+	# or "-L is incompatible with pathspec". Either is acceptable.
+	test_must_fail git log -L1,1:a.c -- a.c &&
+
+	# This must fail due to "-L is incompatible with pathspec".
+	test_must_fail git log -L1,1:b.c -- b.c &&
+
+	# These must fail due to "follow requires one pathspec".
+	test_must_fail git log -L1,1:b.c --follow &&
+	test_must_fail git log --follow -L1,1:b.c &&
+
+	# This may fail due to "-L is incompatible with pathspec",
+	# or "-L is incompatible with pathspec". Either is acceptable.
+	test_must_fail git log --follow -L1,1:b.c -- b.c
+'
+
 canned_test_1 () {
 	test_expect_$1 "$2" "
 		git log $2 >actual &&
-- 
2.29.2-287-gba574db674


  parent reply	other threads:[~2020-11-04 17:54 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-02 13:59 Sathyajith Bhat
2020-11-02 14:43 ` Jeff King
2020-11-02 18:31   ` Junio C Hamano
2020-11-03 10:15   ` SZEDER Gábor
2020-11-03 11:21     ` Christian Couder
2020-11-03 16:10       ` Elijah Newren
2020-11-03 18:21     ` Jeff King
2020-11-03 18:34       ` Junio C Hamano
2020-11-03 18:57         ` Jeff King
2020-11-03 20:21           ` Junio C Hamano
2020-11-04 13:31             ` Jeff King
2020-11-04 16:26               ` Junio C Hamano
2020-11-04 17:54             ` Junio C Hamano [this message]
2020-11-04 19:41               ` Re*: " Jeff King
2020-11-04 20:16                 ` Junio C Hamano
2020-11-04 20:35                   ` [PATCH] log: diagnose -L used with pathspec as an error Junio C Hamano
2020-11-04 21:03                     ` Jeff King
2020-11-03 18:46 ` Segfault in git when using git logs Derrick Stolee
2020-11-03 18:55   ` Sathyajith Bhat
2020-11-03 19:23     ` Jeff King
2020-11-03 20:07       ` Derrick Stolee
2020-11-03 21:04         ` Derrick Stolee
2020-11-04 15:49           ` Sathyajith Bhat

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq7dr1nh3a.fsf_-_@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=sathya@sathyasays.com \
    --cc=szeder.dev@gmail.com \
    --subject='Re: Re*: Segfault in git when using git logs' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).