From: Linus Torvalds <email@example.com> To: Andrew Morton <firstname.lastname@example.org>, Junio Hamano C <email@example.com> Cc: Dan Williams <firstname.lastname@example.org>, email@example.com, Christoph Hellwig <firstname.lastname@example.org>, Git List Mailing <email@example.com> Subject: Re: [patch 16/17] memremap: move from kernel/ to mm/ Date: Sat, 3 Aug 2019 11:21:59 -0700 [thread overview] Message-ID: <CAHk-=whUHq7NGM6e7pCkMOaqKhzv9mqE8+JGJPbY=PPATV9mbA@mail.gmail.com> (raw) In-Reply-To: <firstname.lastname@example.org> [ Adding git people due to On Sat, Aug 3, 2019 at 10:24 AM Andrew Morton <email@example.com> 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
parent reply other threads:[~2019-08-03 18:22 UTC|newest] Thread overview: expand[flat|nested] mbox.gz Atom feed [parent not found: <firstname.lastname@example.org>]
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAHk-=whUHq7NGM6e7pCkMOaqKhzv9mqE8+JGJPbY=PPATV9mbA@mail.gmail.com' \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [patch 16/17] memremap: move from kernel/ to mm/' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Code repositories for project(s) associated with this inbox: https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).