On Tue, Mar 07, 2023 at 12:43:42PM +0100, Vegard Nossum wrote: > This whole document is meant for the developer doing the backport. > > git-format-patch --base= is already covered here: > > https://docs.kernel.org/process/submitting-patches.html#providing-base-tree-information > > I don't think we need to repeat it in this document. OK. > > > > "In most cases, you will likely want to cherry-pick with ``-x`` option > > to record upstream commit in the resulting backport commit description, > > which looks like:: > > > > (cherry picked from commit <upstream commit>) > > > > However, for backporting to stable, you need to edit the description > > above to either:: > > > > commit <upstream commit> upstream > > > > or > > [ Upstream commit <upstream commit> ] > > > > " > > Good point -- the original blog post where this came from was meant to > be more general than just stable backports, but this document in > particular is partly also meant to aid stable contributors we might as > well include it. Nice. > > > > +For backports, what likely happened was that your older branch is > > > +missing a patch compared to the branch you are backporting from -- > > > +however, it is also possible that your older branch has some commit that > > > +doesn't exist in the newer branch. In any case, the result is a conflict > > > +that needs to be resolved. > > > > Another conflict culprit that there are non-prerequisite commits that > > change the context line. > > I think that's already covered by "missing a patch", or at least that > was my intention. I guess we can change it to something like: > > +For backports, what likely happened was that the branch you are > +backporting from contains patches not in the branch you are backporting > +to. However, it is also possible that your older branch has some commit > +that doesn't exist in the newer branch. In any case, the result is a > +conflict that needs to be resolved. What I mean is "hey, we have changes that make context lines conflicted". By "patches not in the branch", I interpret that as "we have possible non-prereqs that cause this (messy) conflict". > > I'll fiddle a bit more with the exact phrasing. > > > > +git log > > > +^^^^^^^ > > > + > > > +A good first step is to look at ``git log`` for the file that has the > > > +conflict -- this is usually sufficient when there aren't a lot of > > > +patches to the file, but may get confusing if the file is big and > > > +frequently patched. You should run ``git log`` on the range of commits > > > +between your currently checked-out branch (``HEAD``) and the parent of > > > +the patch you are picking (``COMMIT``), i.e.:: > > > + > > > + git log HEAD..COMMIT^ -- PATH > > > > HEAD and <commit> swapped, giving empty log. The correct way is: > > > > ``` > > git log <commit>^..HEAD -- <path> > > ``` > > Hrrm, I've double checked this and I think the original text is correct. > > HEAD..<commit>^ gives you commits reachable from <commit>^ (parent of > the commit we are backporting), excluding all commits that are reachable > from HEAD (the branch we are backporting to). > > <commit>^..HEAD, on the other hand, would give you commits reachable > from HEAD excluding all commits that are reachable from the parent of > the commit we are backporting. > > With a diagram like this: > > o--o--x--y--<commit> > \ > \--u--v--HEAD > > HEAD..<commit>^ would give you x and y while > <commit>^..HEAD would give you u and v. In any case, the HEAD you mentioned is at target branch (linux-x.y), right? > > > +Sometimes the right thing to do will be to also backport the patch that > > > +did the rename, but that's definitely not the most common case. Instead, > > > +what you can do is to temporarily rename the file in the branch you're > > > +backporting to (using ``git mv`` and committing the result), restart the > > > +attempt to cherry-pick the patch, rename the file back (``git mv`` and > > > +committing again), and finally squash the result using ``git rebase -i`` > > > +(`tutorial <https://medium.com/@slamflipstrom/a-beginners-guide-to-squashing-commits-with-git-rebase-8185cf6e62ec>`__) > > > +so it appears as a single commit when you are done. > > > > I'm kinda confused with above. Did you mean that after renaming file, I > > have to abort cherry-picking (``git cherry-pick --abort``) first and > > then redo cherry-picking? > > Yes, the idea is that instead of trying to resolve it as a conflict, you > rename the file first, do a (clean) cherry-pick, and then rename it back. > > What caused the confusion, specifically? I thought that the sequence was: ``` $ git checkout -b my-backport linux-x.y $ git cherry-pick <upstream commit> # we get mv/modified content conflict $ git mv <original path> <intended path> && git commit $ git cherry-pick --abort $ git cherry-pick <upstream commit> # resolve content conflicts $ git add <conflicted path>... && git commit $ git rebase -i linux-x.y ``` > > > > +Build testing > > > +~~~~~~~~~~~~~ > > > + > > > +We won't cover runtime testing here, but it can be a good idea to build > > Runtime testing is described in the next section. > > > +just the files touched by the patch as a quick sanity check. For the > > > +Linux kernel you can build single files like this, assuming you have the > > > +``.config`` and build environment set up correctly:: > > > + > > > + make path/to/file.o > > > + > > > +Note that this won't discover linker errors, so you should still do a > > > +full build after verifying that the single file compiles. By compiling > > > +the single file first you can avoid having to wait for a full build *in > > > +case* there are compiler errors in any of the files you've changed. > > > + > > > > plain ``make``? > > Yes, but I don't think we need to spell that out as it's the common case > (in other words, it is presupposed that you know this). > > > > +One concrete example of this was where a patch to the system call entry > > > +code saved/restored a register and a later patch made use of the saved > > > +register somewhere in the middle -- since there was no conflict, one > > > +could backport the second patch and believe that everything was fine, > > > +but in fact the code was now scribbling over an unsaved register. > > > > Did you mean the later patch is the backported syscall patch? > > Yes. I'll fiddle a bit with this paragraph to make it clearer. So, in that case, what would the correct resoultion be regarding to registers? > > For the external link targets, I'd like to separate them from > > corresponding link texts (see > > https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#external-links > > for details). > > We can probably do that, but it doesn't seem to be used much in existing > kernel documentation, I find no existing instances of it: > > $ git grep '\.\._.*: http' Documentation/ > $ I recently worked on Documentation/bpf/bpf_devel_QA.rst, where I mention this linking syntax. > > I know that lots of people really prefer to minimize the amount of > markup in these files (as they consume them in source form), so I'd > really like an ack from others before doing this. > OK. For now I'm OK with either separating targets or including them. Thanks for reply! -- An old man doll... just what I always wanted! - Clara
Attachment:
signature.asc
Description: PGP signature
