Re: [RFC PATCH] git-apply: Permit change of file mode when filename does not change

Jeff King [off-list ref] writes:

On Tue, Mar 24, 2020 at 09:00:54AM -0700, Daniel Sommermann wrote:

quoted

The documentation for git's diff format does not expressly disallow
changing the mode of a file without splitting it into a delete and
create. Mercurial's `hg diff --git` in fact produces git diffs with such
format. When applying such patches in Git, this assert can be hit. The check
preventing this type of diff has been around since 2005 in
3cca928d4aae691572ef9a73dcc29a04f66900a1.

This description confused me for a moment, because in Git we generally
refer to "mode changes" as flipping the executable bit. And anything
further is a "type change" (and this isn't just academic; options like
--diff-filter distinguish the two).

I too found that file "mode" made me confused and thought we reject
a patch that flips executable bits of files with or without touching
their contents.  You are talking about a patch that changes file
"type" between regular files and symbolic links, or worse yet,
regular files or symbolic links and submodules.

It sounds more like a documentation bug, or a bug in the reader of
the documentation.  It's not like what is not explicitly disallowed
are all allowed.

And we do indeed allow a simple mode change like:

  $ git show c9d4999155700651a37f4eb577cec1f4b5b8d6be --format=
  diff --git a/t/perf/p0004-lazy-init-name-hash.sh b/t/perf/p0004-lazy-init-name-hash.sh
  old mode 100644
  new mode 100755

But you're talking about typechanges here, and we do always represent
those as a deletion/addition pair:

While I, like you said below, do not offhand think of a reason why
it may hurt if we allowed to express a replacement of a symbolic
link with a regular file that holds the result of readlink(2) as
pure mode bits change without (or even with) content change at the
mechanical level, I strongly suspect that we chose not to emit such
a patch on the "diff" side because it would be easily missed by
human readers, and that is why such a change is always shown as a
deletion followed by a creation.

And this safety was there in the oldest "still not yet fully but
started working barely" version, so we can safely say that we never
allowed such a patch.

So, I am not sure if we want to lose it.  As you found out, the
removed segment is not only about regular file becoming symlink,
and I do not think we would want to allow other typechanges by
just simply removing the check like the proposed patch did.

Thanks.