--diff-filter=T does not list x changes

the execute bit, but it does not. I hear on #git that this is intended, though I still do not know how to filter on the execute bit. Is it impossible?

Testcase:

  mkdir t && cd t && git init
  touch a && git add -A && git commit -m1
  chmod +x a && git add -A && git commit -m2
  git log --diff-filter=T        # <--- shows nothing
  rm -f a && ln -s b a && git add -A && git commit -m3
  git log --diff-filter=T

Anders

On Wed, Oct 15, 2008 at 08:42:35PM +0200, Anders Melchiorsen wrote:

Looking at the code, I think it's impossible, and one would have to add a new --diff-filter letter. However, at the very least, the documentation should clarify this situation. The --diff-filter explanation says:

  Select only files [...] have their type (mode) changed (T) [...]

which to me indicates that your test case should work. 

-Peff

Jeff King <peff@peff.net> writes:

That documentation is quite loosely written. Typechange diff is what T has always meant, and it never was about the executable bit. The word "mode" in that sentence only means the upper bits S_IFREG/S_IFLNK (iow, masked by S_IFMT).

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

I hope you agree that this reading is not obvious from the documentation, so I will send a patch later fixing up the prose (if nobody beats me to it).

How about adding a diff-filter=X for the executable bit? I could probably look at that during the weekend.

Anders.

Anders Melchiorsen <mail@cup.kalibalik.dk> writes:

Yup, didn't I already say that the documentation is buggy?

I do not think it is a good idea for two reasons. Backward compatibility and sane design.

For one thing, "diff --name-status" never shows X, so you would introduce an unnecessary inconsistency. If you change "--name-status" to avoid that, you would be breaking people's existing scripts that expect to see "M" for such a change.

Even if you were forgiven by these people whose scripts are broken by your change, you need to decide between "M" and "X" when both contents and executable bit are changed. The least surprising logic would probably be to show "X" when _only_ executable bit is changed and show "M" when contents changed (even when executable bit also did), but that feels quite arbitrary. And the other way around isn't any better.

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

Possibly, though not in this thread.

(I noticed that X is already used in diff-filter, but will keep it for this discussion)

I was thinking that X could be a subset of M. So only if you specifically ask for diff-filter=X (and not M) would you get this new functionality. That should keep it compatible. It would then pick files that have had their x flipped, regardless of their change in content. With diff-filter=M, it would work as it does today.

If name-status output must be consistent, it could even output M for these changes. That would still be unambiguous (but probably confusing).

...

As you say that this is an unnecessary inconsistency, I wonder whether you have a different way to pick out the commits that toggle the x bit? That is a problem that I am facing, with no solution shown so far ...

Anders.

Anders Melchiorsen <anders@kalibalik.dk> writes:

Are you interested in executable-bit only change, or any change that contains changes to the executable-bit? If I were looking for the latter, probably finding "^:100664 100775 " (or the other way around) in log --raw (or whatchanged) output would be what I would do --- the mode changes are rare enough in a sane project, so I wouldn't mind having to do such scripting as needed.

There are other "commit pickers" such as -S<strting> and --diff-filter that do not absolutely have to exist (iow, they could also be scripted), but what they pick earned easy shortcuts because the need is very common. Once you can demonstrate that the need to pick executable-bit changes is also very common, _and_ if you can come up with a clean solution, we might add a commit picker that looks for changes in executable-ness in the future. I dunno.

With the previous text, one could get the understanding that diff-filter=T also tested for changes in the executable bit.

Signed-off-by: Anders Melchiorsen <mail@cup.kalibalik.dk>
---

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

You are right that this should be a rare need.

I didn't mean to push for this feature. I just offered to implement it, as I needed it myself and din't see other ways. A script will work fine for me, I should have thought of that.

The documentation patch that I promised is here.

Thanks, Anders.

 Documentation/diff-options.txt |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Quoting Anders Melchiorsen <mail@cup.kalibalik.dk>:

Are symlinks and regular files the only kind of object you can see in diff? What happens when a file or directory changes to a submodule?

Nanako Shiraishi <nanako3@lavabit.com> writes:

Oops. I've already applied Anders's patch, but you are right. A change from a blob to submodule also shows up as a typechange event.

Perhaps we should just remove the parenthesised comment from there instead. I'll rewind and rebuild, as I haven't pushed the results out yet (lucky me).

Thanks.

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

I see that you pushed out this change already, and you changed your mind and described them all.  I think the result reads better.

Nanako Shiraishi <nanako3@lavabit.com> writes:

While we are fixing up that paragraph, this part could also use some elaboration:

  Unknown (`X`), or have had their pairing Broken (`B`).

I would do it, but I have no idea what these two mean.

Regards, Anders