All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Toon Claes <toon@iotcl.com>
Cc: git@vger.kernel.org,  Patrick Steinhardt <ps@pks.im>
Subject: Re: [PATCH] combine-diff: don't override recursive flag
Date: Thu, 04 Sep 2025 09:54:09 -0700	[thread overview]
Message-ID: <xmqqecsm9n0u.fsf@gitster.g> (raw)
In-Reply-To: <20250904-toon-fix-last-modified-v1-1-91bf87ddf62b@iotcl.com> (Toon Claes's message of "Thu, 04 Sep 2025 13:53:40 +0200")

Toon Claes <toon@iotcl.com> writes:

> Because internally diff-tree no longer runs recursive, this results in a
> nice speedup when running `git last-modified` on git.git:

And users of "git diff-tree -c" now needs to pass "-r" themselves to
obtain the result they expect to see?

Do we need to retian backward compatibility there?  As things like this ...

> diff --git a/t/t4013/diff.diff-tree_-c_master b/t/t4013/diff.diff-tree_-c_master
> index e2d2bb26114ac886fbf2467dc7a33ec8cfab5daf..1ecc2cfc6c50e923a671fba29a6a395f4240e0ff 100644
> --- a/t/t4013/diff.diff-tree_-c_master
> +++ b/t/t4013/diff.diff-tree_-c_master
> @@ -1,5 +1,5 @@
>  $ git diff-tree -c master
>  59d314ad6f356dd08601a4cd5e530381da3e3c64
> -::100644 100644 100644 cead32e925b1420c84c14cbf7cf755e7e45af8ad 7289e35bff32727c08dda207511bec138fdb9ea5 992913c5aa0a5476d10c49ed0f21fc0c6d1aedf3 MM	dir/sub
> +::040000 040000 040000 65f5c9dd60ce3b2b3324b618ac7accf8d912c113 f977ed46ae6873c1c30ab878e15a4accedc3618b 0564e026437809817a64fff393079714b6dd4628 MM	dir
>  ::100644 100644 100644 b414108e81e5091fe0974a1858b4d0d22b107f70 f4615da674c09df322d6ba8d6b21ecfb1b1ba510 10a8a9f3657f91a156b9f0184ed79a20adef9f7f MM	file0
>  $

... is a clear regression of age old expectation.

I do not mind too much if you did want to make a breaking change and
be honest about it, but then I would expect you would be adding "-r"
on the command line without changing the expected output in a test
like the above one, i.e. I would expect that the above should look
more like this:

> -$ git diff-tree -c master
> +$ git diff-tree -r -c master
>  59d314ad6f356dd08601a4cd5e530381da3e3c64
>  ::100644 100644 100644 cead32e925b1420c84c14cbf7cf755e7e45af8ad 7289e35bff32727c08dda207511bec138fdb9ea5 992913c5aa0a5476d10c49ed0f21fc0c6d1aedf3 MM	dir/sub
>  ::100644 100644 100644 b414108e81e5091fe0974a1858b4d0d22b107f70 f4615da674c09df322d6ba8d6b21ecfb1b1ba510 10a8a9f3657f91a156b9f0184ed79a20adef9f7f MM	file0


      reply	other threads:[~2025-09-04 16:54 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-04 11:53 [PATCH] combine-diff: don't override recursive flag Toon Claes
2025-09-04 16:54 ` Junio C Hamano [this message]

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

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

  git send-email \
    --in-reply-to=xmqqecsm9n0u.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=ps@pks.im \
    --cc=toon@iotcl.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.