Jeff King <peff@xxxxxxxx> writes: > On Tue, Mar 24, 2020 at 09:00:54AM -0700, Daniel Sommermann wrote: > >> 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.
