Re: [patch 16/17] memremap: move from kernel/ to mm/

Re: [patch 16/17] memremap: move from kernel/ to mm/

[Linus Torvalds]

[ Adding git people due to

On Sat, Aug 3, 2019 at 10:24 AM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> Aside.  I guess renames are just a wee bit dangerous.  There's no
> guarantee that the file which you rename is exactly the same as the one
> which I renamed, which could result in breakage.  Such as, ummm, if
> some intervening patch had added a #include "foo.h".  Maybe the rename
> instructions should include a hash.

It turns out that renames that have changes _do_ include the object ID
for both sides, but exact renames do not. I guess that's technically a
deficiency in git.

A pure rename with no changes looks like this:

  diff --git a/kernel/memremap.c b/mm/memremap.c
  similarity index 100%
  rename from kernel/memremap.c
  rename to mm/memremap.c

so you see that it's a rename, but there's no way to actually see what
the *contents* at the point of the rename were.

In contrast, a rename with changes might look like this:

  diff --git a/.../intel,ixp4xx-queue-manager.yaml
b/.../intel,ixp4xx-ahb-queue-manager.yaml
  similarity index 95%
  rename from .../intel,ixp4xx-queue-manager.yaml
  rename to .../intel,ixp4xx-ahb-queue-manager.yaml
  index d2313b1d9405..0ea21a6f70b4 100644
  --- a/.../intel,ixp4xx-queue-manager.yaml
  +++ b/.../intel,ixp4xx-ahb-queue-manager.yaml
  @@ -2,7 +2,7 @@
   # Copyright 2019 Linaro Ltd.
   %YAML 1.2
   ---
  -$id: "http://devicetree.org/schemas/misc/intel-ixp4xx-ahb-queue-manager.yaml#"
  +$id: "http://devicetree.org/schemas/misc/intel,ixp4xx-ahb-queue-manager.yaml#"
   $schema: "http://devicetree.org/meta-schemas/core.yaml#"

   title: Intel IXP4xx AHB Queue Manager

so when the rename wasn't exact, we do show the index line so that you
can verify the exact content of the file as it was renamed.

Of course, I doubt your quilt scripts look at the index anyway, and
this is (I think) the first time this has come up as a potential
issue.

But you are correct that it would probably be a good idea to have an
index line for the identical file case too.

Adding Junio and the git list to the cc, in case anybody has ideas and
cares.. I don't think this is a big issue, this is more of a "let's cc
people so that they know it got mentioned".

>     But I can see that the present
> "rename it while ignoring other stuff which happened" is kinda neat.

It's very convenient indeed. That said, in an all-git environment, a
rename is basically never really treated as a patch, so when you do
*merges* with renames, it does take changes into account and give you
a "conflict".

Although generally it will then resolve those conflicts for you
automatically (and if the renaming side made no changes that will be
visible as the same object ID, the end result of the automatic merge
resolution is that the rename is just done with the change from the
other side in place).

Honestly, I very seldom see any issues with pure renames. The issues
we _do_ see is when somebody renames a file and then does a lot of
changes (often because they also then rename the variables in the file
because the rename was some bigger "coneptual" rename) and then any
conflicts with other changes can be very nasty indeed to sort out.

Plain "just move a file" situations tend to be very simple. Yes, it
_can_ cause subtle issues, but in practice it never really does. No
more than any other "two people change a file differently", at least.

The big advantage of renames in diffs is that it makes it _so_ much
easier for humans to see what is going on. It's almost impossible for
a human to see that it's a pure rename when you see multiple hundreds
of lines being deleted and added, while a rename patch is really easy
for humans to understand when they look at a patch like this.

             Linus