git diff-tree do not honor diff.orderfile config

git diff-tree do not honor diff.orderfile config

[Tiago de Bem Natel de Moura]

What did you do before the bug happened? (Steps to reproduce your issue)
The config `diff.orderfile` is not being honored in the `git diff-tree` command
as stated by the documentation.

What did you expect to happen? (Expected behavior)
The output ordered by the pattern file.

What happened instead? (Actual behavior)
Output has normal order.

What's different between what you expected and what actually happened?

The order.

Anything else you want to add:

The complete command that I'm running is:
git diff-tree -r --relative --name-only HEAD main

if I provide -O ~/ordefile it works but if I add it to my config it does not.

Please review the rest of the bug report below.
You can delete any lines you don't wish to share.


[System Info]
git version:
git version 2.44.0
cpu: arm64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon
uname: Darwin 23.5.0 Darwin Kernel Version 23.5.0: Wed May  1 20:14:38
PDT 2024; root:xnu-10063.121.3~5/RELEASE_ARM64_T6020 arm64
compiler info: clang: 15.0.0 (clang-1500.1.0.2.5)
libc info: no libc information available
$SHELL (typically, interactive shell): /bin/zsh


[Enabled Hooks]

Re: git diff-tree do not honor diff.orderfile config

[Taylor Blau]

On Fri, Jul 05, 2024 at 11:28:23AM +0100, Tiago de Bem Natel de Moura wrote:
> What did you do before the bug happened? (Steps to reproduce your issue)
> The config `diff.orderfile` is not being honored in the `git diff-tree` command
> as stated by the documentation.

Makes sense... the diff.orderFile configuration is part of the "UI" set
of diff.* configuration options, which are honored by porcelain commands
like commit, diff, log, etc., but not by plumbing commands like
diff-tree.

I think from 6d8940b562 (diff: add diff.orderfile configuration
variable, 2013-12-18) adding it to the UI-only configuration set was
intentional, but it is somewhat awkward that we respect -O but not the
configuration it falls back on.

So I suppose the question is whether supporting -O from diff-tree is
sensible. If it is, then reading the diff.orderFile configuration option
is a no-brainer. But if it isn't, then we should probably not make a bad
situation worse by adding support for it.

Thanks,
Taylor

Re: git diff-tree do not honor diff.orderfile config

[Jeff King]

On Fri, Jul 05, 2024 at 05:04:44PM -0400, Taylor Blau wrote:

> On Fri, Jul 05, 2024 at 11:28:23AM +0100, Tiago de Bem Natel de Moura wrote:
> > What did you do before the bug happened? (Steps to reproduce your issue)
> > The config `diff.orderfile` is not being honored in the `git diff-tree` command
> > as stated by the documentation.
> 
> Makes sense... the diff.orderFile configuration is part of the "UI" set
> of diff.* configuration options, which are honored by porcelain commands
> like commit, diff, log, etc., but not by plumbing commands like
> diff-tree.
> 
> I think from 6d8940b562 (diff: add diff.orderfile configuration
> variable, 2013-12-18) adding it to the UI-only configuration set was
> intentional, but it is somewhat awkward that we respect -O but not the
> configuration it falls back on.

I don't think it's awkward at all. The plumbing commands should provide
most every option that the porcelain ones support, but should do so
based only on the options given by the caller. I.e., imagine you were
writing "git diff" (or a similar command) as a script and you wanted to
support something like diff.orderfile. You'd do it by parsing
diff.orderfile yourself and passing "-O" where it made sense to do so.

> So I suppose the question is whether supporting -O from diff-tree is
> sensible. If it is, then reading the diff.orderFile configuration option
> is a no-brainer. But if it isn't, then we should probably not make a bad
> situation worse by adding support for it.

No, I don't think we ever want diff-tree (or other plumbing commands) to
support diff.orderFile. It will cause unexpected results for consumers
of the plumbing output. E.g., it would probably at least screw up
patch-id.

So this all just looks like the usual "plumbing does not respect some
config" situation.

-Peff

Re: git diff-tree do not honor diff.orderfile config

[Junio C Hamano]

Taylor Blau <me@ttaylorr.com> writes:

> So I suppose the question is whether supporting -O from diff-tree is
> sensible. If it is, then reading the diff.orderFile configuration option
> is a no-brainer. But if it isn't, then we should probably not make a bad
> situation worse by adding support for it.

If plumbing commands paid attention to the end-user configuration,
they will not serve as a reliable building blocks for script
writers.  The would instead change their behaviour in a way not
expected by script writers.

On the other hand, we do want to give script writers an option to
utilize power of the underlying machinery.  If they want to specify
a particular order of paths in the output, they are welcome to feed
an orderfile via the command line and that is why we have an option.

Supporting "-O <orderfile>" and ignoring "diff.orderfile" are both
very sensible thing for the plumbing commands and it is the result
of deliberate design.

Thanks.

Re: git diff-tree do not honor diff.orderfile config

[Tiago de Bem Natel de Moura]

I agree with pretty much everything said here but then the
documentation must be fixed here:
https://git-scm.com/docs/git-diff-tree#Documentation/git-diff-tree.txt--Oltorderfilegt

I use diff-tree in a Go program[1] and I expected it to be plumbing
but I got an error report that was hard to reproduce then I read in
the documentation that diff-tree also relies on the configuration but
when I tried it didn't work, hence this report.
If you folks say it does not depend on the config and never will, then all good.

Thank you.

1: https://github.com/terramate-io/terramate/blob/af73c2b688fae94360169da6ab8de49ea5b07649/git/git.go#L466-L482

Re: git diff-tree do not honor diff.orderfile config

[Junio C Hamano]

Tiago de Bem Natel de Moura <t.nateldemoura@gmail.com> writes:

> What did you do before the bug happened? (Steps to reproduce your issue)
> The config `diff.orderfile` is not being honored in the `git diff-tree` command
> as stated by the documentation.
>
> What did you expect to happen? (Expected behavior)
> The output ordered by the pattern file.
>
> What happened instead? (Actual behavior)
> Output has normal order.

The diff-files, diff-index, and diff-tree ignore most (if not all)
of the end-user configuration variables in order to give a stable
output, which writers of scripts would expect to see out of these
plumbing commands.

Those who use these plumbing commands should still be able to use
the equivalent command line option to afffect their behaviour.  In
this case, something like

	orderfile=$(git config diff.orderfile)
	git diff-tree ${orderfile:+"-O$orderfile"} ...

would be what script writers would do when the output is not for
machine consumption inside the script, but is to be shown to the
end-user directly and they want to pretend as if they used the 'git
diff' Porcelain command, which pays attention to the configuration
variable".

But when the command is the final output phase (as opposed to a step
in the multi-step logic your script implements, e.g. diff-files
produces a list of changed files to be read by other commands and
processed by your other commands in the downstream on the same
pipeline) and that is why it is a good idea to pay attention to the
configuration, using "git diff" not "diff-tree" may be more natural
thing to do.

Thanks.