git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Behavior of git log --diff-filter=d
@ 2025-07-02 20:28 Eric Salem
  2025-07-03 13:42 ` Re " K Jayatheerth
  2025-07-03 15:34 ` Jeff King
  0 siblings, 2 replies; 9+ messages in thread
From: Eric Salem @ 2025-07-02 20:28 UTC (permalink / raw)
  To: git

Hi all. What am I doing wrong here?

The git log --diff-filter documentation[1] for deleted files says:

> Select only files that are Added (A), Copied (C), Deleted (D)...

> Also, these upper-case letters can be downcased to exclude.
> E.g. --diff-filter=ad excludes added and deleted paths.

A simple test:

$ cd $(mktemp -d)
$ git init
Initialized empty Git repository in /tmp/tmp.xnvNav956B/.git/
$ echo test1 > file1.txt
$ git add file1.txt 
$ git commit -m "first"
[master (root-commit) 71288dd00aff] first
 1 file changed, 1 insertion(+)
 create mode 100644 file1.txt
$ git rm file1.txt 
rm 'file1.txt'
$ git commit -m "second"
[master 6ff8f522b744] second
 1 file changed, 1 deletion(-)
 delete mode 100644 file1.txt
$ echo test2 > file2.txt
$ git add file2.txt 
$ git commit -m "third"
[master 41498d26ea5e] third
 1 file changed, 1 insertion(+)
 create mode 100644 file2.txt
$ git log --format="%H"
41498d26ea5ee6820834aa51351e1e7ce6ce8733
6ff8f522b744dfbc7c2201c5bf77cf5acc3028ce
71288dd00afff60f3a6576f93930aeb0130e5cd1
$ git log --format="%H" --diff-filter=D
6ff8f522b744dfbc7c2201c5bf77cf5acc3028ce
$ git log --format="%H" --diff-filter=d
$ git log --format="%H" --diff-filter=d --stat
41498d26ea5ee6820834aa51351e1e7ce6ce8733

 file2.txt | 1 +
 1 file changed, 1 insertion(+)
71288dd00afff60f3a6576f93930aeb0130e5cd1

 file1.txt | 1 +
 1 file changed, 1 insertion(+)
$ git version
git version 2.50.0

--diff-filter=D behaves as expected, but when using "d" instead, I don't
get any output unless I add another option (such as --stat or
--name-only).

Is this expected behavior?

Thanks,

Eric

[1] https://git-scm.com/docs/git-log#Documentation/git-log.txt---diff-filterACDMRTUXB

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re Behavior of git log --diff-filter=d
  2025-07-02 20:28 Behavior of git log --diff-filter=d Eric Salem
@ 2025-07-03 13:42 ` K Jayatheerth
  2025-07-03 15:36   ` Jeff King
  2025-07-03 21:30   ` Eric Salem
  2025-07-03 15:34 ` Jeff King
  1 sibling, 2 replies; 9+ messages in thread
From: K Jayatheerth @ 2025-07-03 13:42 UTC (permalink / raw)
  To: ericsalem; +Cc: git

Hi Eric,

Nice question

Yes, this is expected behavior
You're seeing a difference because

--diff-filter=d tells Git to exclude commits that have deleted files in their diffs.
However, this filter only applies if there is a diff to filter.

In other words, 
if you run git log --format="%H" --diff-filter=d, 
Git doesn't show any output unless 
the diff logic is actually invoked. 
But --format="%H" alone does not invoke diff generation...
so --diff-filter silently does nothing.

When you add --stat or --name-only, 
you're explicitly telling Git: 
"Please compute the diff". 

Now Git has something to filter, 
and it applies --diff-filter=d 
to exclude those diffs that involve deletions.

If you want to exclude deletion commits and get just commit hashes
This is something I think should work
probably set this up with an alias if you use this many times

git log --format="%H" --diff-filter=d --name-only | grep -v '^$'

This should work just fine...

Or perhaps if you wanna tinker more
git log --format="%H" --diff-filter=d --stat | grep -B1 -v "delete mode"

But if you're just trying to filter commits 
by file change type and want to see 
only those hashes where a deleted file is not present, 
you'll need some way to trigger the diff 
even if you discard the output later.

Hope that helps :)

Thank you,

- Jayatheerth

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Behavior of git log --diff-filter=d
  2025-07-02 20:28 Behavior of git log --diff-filter=d Eric Salem
  2025-07-03 13:42 ` Re " K Jayatheerth
@ 2025-07-03 15:34 ` Jeff King
  2025-07-03 22:44   ` [PATCH] setup_revisions(): turn on diffs for all-negative diff filter Jeff King
  1 sibling, 1 reply; 9+ messages in thread
From: Jeff King @ 2025-07-03 15:34 UTC (permalink / raw)
  To: Eric Salem; +Cc: git

On Wed, Jul 02, 2025 at 03:28:43PM -0500, Eric Salem wrote:

> The git log --diff-filter documentation[1] for deleted files says:
> 
> > Select only files that are Added (A), Copied (C), Deleted (D)...
> 
> > Also, these upper-case letters can be downcased to exclude.
> > E.g. --diff-filter=ad excludes added and deleted paths.
> 
> A simple test:
> [...]
> --diff-filter=D behaves as expected, but when using "d" instead, I don't
> get any output unless I add another option (such as --stat or
> --name-only).

Looks like a bug. This used to produce the output I'd expect (i.e.,
commits "first" and "third", which do not have deletions), but that
changed in 75408ca949 (diff-filter: be more careful when looking for
negative bits, 2022-01-28).

I don't have time to dig into it now, but I've cc'd the author (and left
your whole reproduction recipe quoted below).

-Peff

> $ cd $(mktemp -d)
> $ git init
> Initialized empty Git repository in /tmp/tmp.xnvNav956B/.git/
> $ echo test1 > file1.txt
> $ git add file1.txt 
> $ git commit -m "first"
> [master (root-commit) 71288dd00aff] first
>  1 file changed, 1 insertion(+)
>  create mode 100644 file1.txt
> $ git rm file1.txt 
> rm 'file1.txt'
> $ git commit -m "second"
> [master 6ff8f522b744] second
>  1 file changed, 1 deletion(-)
>  delete mode 100644 file1.txt
> $ echo test2 > file2.txt
> $ git add file2.txt 
> $ git commit -m "third"
> [master 41498d26ea5e] third
>  1 file changed, 1 insertion(+)
>  create mode 100644 file2.txt
> $ git log --format="%H"
> 41498d26ea5ee6820834aa51351e1e7ce6ce8733
> 6ff8f522b744dfbc7c2201c5bf77cf5acc3028ce
> 71288dd00afff60f3a6576f93930aeb0130e5cd1
> $ git log --format="%H" --diff-filter=D
> 6ff8f522b744dfbc7c2201c5bf77cf5acc3028ce
> $ git log --format="%H" --diff-filter=d
> $ git log --format="%H" --diff-filter=d --stat
> 41498d26ea5ee6820834aa51351e1e7ce6ce8733
> 
>  file2.txt | 1 +
>  1 file changed, 1 insertion(+)
> 71288dd00afff60f3a6576f93930aeb0130e5cd1
> 
>  file1.txt | 1 +
>  1 file changed, 1 insertion(+)
> $ git version
> git version 2.50.0

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Re Behavior of git log --diff-filter=d
  2025-07-03 13:42 ` Re " K Jayatheerth
@ 2025-07-03 15:36   ` Jeff King
  2025-07-03 21:30   ` Eric Salem
  1 sibling, 0 replies; 9+ messages in thread
From: Jeff King @ 2025-07-03 15:36 UTC (permalink / raw)
  To: K Jayatheerth; +Cc: ericsalem, git

On Thu, Jul 03, 2025 at 07:12:20PM +0530, K Jayatheerth wrote:

> Yes, this is expected behavior
> You're seeing a difference because
> 
> --diff-filter=d tells Git to exclude commits that have deleted files in their diffs.
> However, this filter only applies if there is a diff to filter.

I don't think that is true. Even if we are not showing a diff, we'll
select which commits to show based on diff options (and show only those
that still touch something). That's why "--diff-filter=D" shows only the
commit with the deletion. With the "d" filter we should omit deletions,
leaving the second commit with no changes (and thus not shown).

-Peff

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Re Behavior of git log --diff-filter=d
  2025-07-03 13:42 ` Re " K Jayatheerth
  2025-07-03 15:36   ` Jeff King
@ 2025-07-03 21:30   ` Eric Salem
  1 sibling, 0 replies; 9+ messages in thread
From: Eric Salem @ 2025-07-03 21:30 UTC (permalink / raw)
  To: K Jayatheerth; +Cc: git

Thank you, Jayatheerth. That was very helpful. Now it makes sense why
Git is behaving like this. If I use --dirstat=<param> and then filter
out the blank lines, I get the output I want.

Thanks again for your help.

Eric

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] setup_revisions(): turn on diffs for all-negative diff filter
  2025-07-03 15:34 ` Jeff King
@ 2025-07-03 22:44   ` Jeff King
  2025-07-03 23:42     ` Eric Salem
                       ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Jeff King @ 2025-07-03 22:44 UTC (permalink / raw)
  To: Eric Salem; +Cc: Johannes Schindelin, git

On Thu, Jul 03, 2025 at 11:34:38AM -0400, Jeff King wrote:

> On Wed, Jul 02, 2025 at 03:28:43PM -0500, Eric Salem wrote:
> 
> > The git log --diff-filter documentation[1] for deleted files says:
> > 
> > > Select only files that are Added (A), Copied (C), Deleted (D)...
> > 
> > > Also, these upper-case letters can be downcased to exclude.
> > > E.g. --diff-filter=ad excludes added and deleted paths.
> > 
> > A simple test:
> > [...]
> > --diff-filter=D behaves as expected, but when using "d" instead, I don't
> > get any output unless I add another option (such as --stat or
> > --name-only).
> 
> Looks like a bug. This used to produce the output I'd expect (i.e.,
> commits "first" and "third", which do not have deletions), but that
> changed in 75408ca949 (diff-filter: be more careful when looking for
> negative bits, 2022-01-28).
> 
> I don't have time to dig into it now, but I've cc'd the author (and left
> your whole reproduction recipe quoted below).

Argh, I forgot to add Johannes to the cc. Fortunately since then I had a
moment to look at this, and the solution is pretty simple. So here it is
as a patch with a test.

-- >8 --
Subject: setup_revisions(): turn on diffs for all-negative diff filter

When the user gives us a diff filter like --diff-filter=D, we need to do
a tree diff even if we're not planning to show the diff result itself,
in order to decide whether to show the commit at all. So there's an
explicit check of revs->diffopt.filter in setup_revisions(), and we set
revs->diff if any bits are set.

Originally that "filter" field covered both positive capital-letter
filters (like "D") and also negative lowercase filters (like "d"), so it
was sufficient for both cases. But later, 75408ca949 (diff-filter: be
more careful when looking for negative bits, 2022-01-28) split the
negative bits out into a "filter_not" field.

We eventually fold those into "filter", but not until diff_setup_done()
is called, which happens after our explicit check. As a result, a purely
negative filter like:

  git log --diff-filter=d

failed to turn on diffs at all. But rather than fail to filter by diff,
because the filter variable is eventually set, we mistakenly show no
commits at all, thinking that the empty diffs were cases where nothing
passed through the filter.

The smallest fix here is to just have our check look for any bits in
either "filter" or "filter_not". I suspect it would also be OK to
reorder the function a bit to call diff_setup_done() earlier, but that
risks violating some other subtle ordering dependency. So I went with
the simple and safe solution here.

Signed-off-by: Jeff King <peff@peff.net>
---
 revision.c     | 2 +-
 t/t4202-log.sh | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/revision.c b/revision.c
index afee111196..9892d08748 100644
--- a/revision.c
+++ b/revision.c
@@ -3112,7 +3112,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 
 	/* Pickaxe, diff-filter and rename following need diffs */
 	if ((revs->diffopt.pickaxe_opts & DIFF_PICKAXE_KINDS_MASK) ||
-	    revs->diffopt.filter ||
+	    revs->diffopt.filter || revs->diffopt.filter_not ||
 	    revs->diffopt.flags.follow_renames)
 		revs->diff = 1;
 
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 4a6c4dfbf4..05cee9e41b 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -134,6 +134,12 @@ test_expect_success 'diff-filter=D' '
 
 '
 
+test_expect_success 'all-negative filter' '
+	git log --no-renames --format=%s --diff-filter=d HEAD >actual &&
+	printf "%s\n" fifth fourth third second initial >expect &&
+	test_cmp expect actual
+'
+
 test_expect_success 'diff-filter=R' '
 
 	git log -M --pretty="format:%s" --diff-filter=R HEAD >actual &&
-- 
2.50.0.438.g3b3bebd3e8



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] setup_revisions(): turn on diffs for all-negative diff filter
  2025-07-03 22:44   ` [PATCH] setup_revisions(): turn on diffs for all-negative diff filter Jeff King
@ 2025-07-03 23:42     ` Eric Salem
  2025-07-04 13:30     ` Karthik Nayak
  2025-07-07  5:31     ` Junio C Hamano
  2 siblings, 0 replies; 9+ messages in thread
From: Eric Salem @ 2025-07-03 23:42 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Schindelin, git

On 7/3/25 5:44 PM, Jeff King wrote:
> Argh, I forgot to add Johannes to the cc. Fortunately since then I had a
> moment to look at this, and the solution is pretty simple. So here it is
> as a patch with a test.

Patch worked great for me. Thanks for fixing it so quickly, Jeff!

Eric

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] setup_revisions(): turn on diffs for all-negative diff filter
  2025-07-03 22:44   ` [PATCH] setup_revisions(): turn on diffs for all-negative diff filter Jeff King
  2025-07-03 23:42     ` Eric Salem
@ 2025-07-04 13:30     ` Karthik Nayak
  2025-07-07  5:31     ` Junio C Hamano
  2 siblings, 0 replies; 9+ messages in thread
From: Karthik Nayak @ 2025-07-04 13:30 UTC (permalink / raw)
  To: Jeff King, Eric Salem; +Cc: Johannes Schindelin, git

[-- Attachment #1: Type: text/plain, Size: 4158 bytes --]

Jeff King <peff@peff.net> writes:

> On Thu, Jul 03, 2025 at 11:34:38AM -0400, Jeff King wrote:
>
>> On Wed, Jul 02, 2025 at 03:28:43PM -0500, Eric Salem wrote:
>>
>> > The git log --diff-filter documentation[1] for deleted files says:
>> >
>> > > Select only files that are Added (A), Copied (C), Deleted (D)...
>> >
>> > > Also, these upper-case letters can be downcased to exclude.
>> > > E.g. --diff-filter=ad excludes added and deleted paths.
>> >
>> > A simple test:
>> > [...]
>> > --diff-filter=D behaves as expected, but when using "d" instead, I don't
>> > get any output unless I add another option (such as --stat or
>> > --name-only).
>>
>> Looks like a bug. This used to produce the output I'd expect (i.e.,
>> commits "first" and "third", which do not have deletions), but that
>> changed in 75408ca949 (diff-filter: be more careful when looking for
>> negative bits, 2022-01-28).
>>
>> I don't have time to dig into it now, but I've cc'd the author (and left
>> your whole reproduction recipe quoted below).
>
> Argh, I forgot to add Johannes to the cc. Fortunately since then I had a
> moment to look at this, and the solution is pretty simple. So here it is
> as a patch with a test.
>
> -- >8 --
> Subject: setup_revisions(): turn on diffs for all-negative diff filter
>
> When the user gives us a diff filter like --diff-filter=D, we need to do
> a tree diff even if we're not planning to show the diff result itself,
> in order to decide whether to show the commit at all. So there's an
> explicit check of revs->diffopt.filter in setup_revisions(), and we set
> revs->diff if any bits are set.

So if `revs->diff` is set, then we compute the tree diff for the given
commit.

>
> Originally that "filter" field covered both positive capital-letter
> filters (like "D") and also negative lowercase filters (like "d"), so it
> was sufficient for both cases. But later, 75408ca949 (diff-filter: be
> more careful when looking for negative bits, 2022-01-28) split the
> negative bits out into a "filter_not" field.
>
> We eventually fold those into "filter", but not until diff_setup_done()
> is called, which happens after our explicit check. As a result, a purely
> negative filter like:
>
>   git log --diff-filter=d
>
> failed to turn on diffs at all. But rather than fail to filter by diff,
> because the filter variable is eventually set, we mistakenly show no
> commits at all, thinking that the empty diffs were cases where nothing
> passed through the filter.
>
> The smallest fix here is to just have our check look for any bits in
> either "filter" or "filter_not". I suspect it would also be OK to
> reorder the function a bit to call diff_setup_done() earlier, but that
> risks violating some other subtle ordering dependency. So I went with
> the simple and safe solution here.
>

The explanation here was really nice to read and explained the problem
well.

> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  revision.c     | 2 +-
>  t/t4202-log.sh | 6 ++++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/revision.c b/revision.c
> index afee111196..9892d08748 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -3112,7 +3112,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
>
>  	/* Pickaxe, diff-filter and rename following need diffs */
>  	if ((revs->diffopt.pickaxe_opts & DIFF_PICKAXE_KINDS_MASK) ||
> -	    revs->diffopt.filter ||
> +	    revs->diffopt.filter || revs->diffopt.filter_not ||
>  	    revs->diffopt.flags.follow_renames)
>  		revs->diff = 1;
>

Makes sense.

> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> index 4a6c4dfbf4..05cee9e41b 100755
> --- a/t/t4202-log.sh
> +++ b/t/t4202-log.sh
> @@ -134,6 +134,12 @@ test_expect_success 'diff-filter=D' '
>
>  '
>
> +test_expect_success 'all-negative filter' '
> +	git log --no-renames --format=%s --diff-filter=d HEAD >actual &&
> +	printf "%s\n" fifth fourth third second initial >expect &&
> +	test_cmp expect actual
> +'
> +
>  test_expect_success 'diff-filter=R' '
>
>  	git log -M --pretty="format:%s" --diff-filter=R HEAD >actual &&
> --
> 2.50.0.438.g3b3bebd3e8

The fix looks great. Thanks!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] setup_revisions(): turn on diffs for all-negative diff filter
  2025-07-03 22:44   ` [PATCH] setup_revisions(): turn on diffs for all-negative diff filter Jeff King
  2025-07-03 23:42     ` Eric Salem
  2025-07-04 13:30     ` Karthik Nayak
@ 2025-07-07  5:31     ` Junio C Hamano
  2 siblings, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2025-07-07  5:31 UTC (permalink / raw)
  To: Jeff King; +Cc: Eric Salem, Johannes Schindelin, git

Jeff King <peff@peff.net> writes:

> Argh, I forgot to add Johannes to the cc. Fortunately since then I had a
> moment to look at this, and the solution is pretty simple. So here it is
> as a patch with a test.
>
> -- >8 --
> Subject: setup_revisions(): turn on diffs for all-negative diff filter
>
> When the user gives us a diff filter like --diff-filter=D, we need to do
> a tree diff even if we're not planning to show the diff result itself,
> in order to decide whether to show the commit at all. So there's an
> explicit check of revs->diffopt.filter in setup_revisions(), and we set
> revs->diff if any bits are set.
>
> Originally that "filter" field covered both positive capital-letter
> filters (like "D") and also negative lowercase filters (like "d"), so it
> was sufficient for both cases. But later, 75408ca949 (diff-filter: be
> more careful when looking for negative bits, 2022-01-28) split the
> negative bits out into a "filter_not" field.

Ah, ouch, so the patch somehow ended up to be less careful about
negative bits after all ;-)

Thanks for noticing the breakage and fixing it.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-07-07  5:31 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-02 20:28 Behavior of git log --diff-filter=d Eric Salem
2025-07-03 13:42 ` Re " K Jayatheerth
2025-07-03 15:36   ` Jeff King
2025-07-03 21:30   ` Eric Salem
2025-07-03 15:34 ` Jeff King
2025-07-03 22:44   ` [PATCH] setup_revisions(): turn on diffs for all-negative diff filter Jeff King
2025-07-03 23:42     ` Eric Salem
2025-07-04 13:30     ` Karthik Nayak
2025-07-07  5:31     ` Junio C Hamano

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 NNTP newsgroup(s).