Re: [PATCH] diff: stop output garbled message in dry run mode

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 17, 2025, 20:17 UTC
Message-ID: <xmqqa51pz3ih.fsf@gitster.g>
In-Reply-To: <4ff55fc5-7880-b8bf-257f-3186552e9c36@gmx.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> I do see a still-unguarded `fprintf(o->file, ...)` call in
> `run_diff_cmd()`, but as far as I can see, this call is not in any code
> path where `dry_run` is set.

Among the callers of run_diff_cmd(), only the caller that wants to report "this path is unmerged" passes NULL diff_filespec pointers in parameters one and two, in which case run_diff_cmd() would give that message. So if you have an unmerged filepair in queued_diff, this callchain

	diff_flush()
	  loop over diff_queued_diff
          -> diff_flush_patch_quietly()
	     fiddle with dry_run bit
	     -> diff_flush_patch()
		-> run_diff()
		   -> run_diff_cmd() with one&two set to NULL

may hit the fprintf into o->file.

So you are right to worry about that fprintf(). If I make a whitespace-only change to one file, and then make another path unmerged, here is what I would see:

    $ rungit v2.48.0 diff --raw
    :100644 100644 b82c4963e7 0000000000 M  cache-tree.h
    :000000 100644 0000000000 0000000000 U  t/lib-gpg.sh

This is version before that dry-run thing. It operated under the old rule to show "--raw" to report object differences, hence ignoring "-w".

    $ rungit v2.48.0 diff --raw -w
    :100644 100644 b82c4963e7 0000000000 M  cache-tree.h
    :000000 100644 0000000000 0000000000 U  t/lib-gpg.sh

With a version with the dry_run thing, here is what we see:

    $ git diff --raw -w
    * Unmerged path t/lib-gpg.sh
    :000000 100644 0000000000 0000000000 U  t/lib-gpg.sh

As dry_run thing intended, the entry on the whitespace-only path is gone from the output, but the fprintf(o->file) you noticed comes out, which is not what we want to see. Of course, if we omit -w to avoid triggering the dry-run thing, we won't see it.

    $ git diff --raw
    :100644 100644 b82c4963e7 0000000000 M  cache-tree.h
    :000000 100644 0000000000 0000000000 U  t/lib-gpg.sh

As a regression-fix change, I'd feel safer with Peff's version.

Thanks.