1*4882a593Smuzhiyun.. SPDX-License-Identifier: GPL-2.0 2*4882a593Smuzhiyun 3*4882a593Smuzhiyun==================== 4*4882a593SmuzhiyunRebasing and merging 5*4882a593Smuzhiyun==================== 6*4882a593Smuzhiyun 7*4882a593SmuzhiyunMaintaining a subsystem, as a general rule, requires a familiarity with the 8*4882a593SmuzhiyunGit source-code management system. Git is a powerful tool with a lot of 9*4882a593Smuzhiyunfeatures; as is often the case with such tools, there are right and wrong 10*4882a593Smuzhiyunways to use those features. This document looks in particular at the use 11*4882a593Smuzhiyunof rebasing and merging. Maintainers often get in trouble when they use 12*4882a593Smuzhiyunthose tools incorrectly, but avoiding problems is not actually all that 13*4882a593Smuzhiyunhard. 14*4882a593Smuzhiyun 15*4882a593SmuzhiyunOne thing to be aware of in general is that, unlike many other projects, 16*4882a593Smuzhiyunthe kernel community is not scared by seeing merge commits in its 17*4882a593Smuzhiyundevelopment history. Indeed, given the scale of the project, avoiding 18*4882a593Smuzhiyunmerges would be nearly impossible. Some problems encountered by 19*4882a593Smuzhiyunmaintainers result from a desire to avoid merges, while others come from 20*4882a593Smuzhiyunmerging a little too often. 21*4882a593Smuzhiyun 22*4882a593SmuzhiyunRebasing 23*4882a593Smuzhiyun======== 24*4882a593Smuzhiyun 25*4882a593Smuzhiyun"Rebasing" is the process of changing the history of a series of commits 26*4882a593Smuzhiyunwithin a repository. There are two different types of operations that are 27*4882a593Smuzhiyunreferred to as rebasing since both are done with the ``git rebase`` 28*4882a593Smuzhiyuncommand, but there are significant differences between them: 29*4882a593Smuzhiyun 30*4882a593Smuzhiyun - Changing the parent (starting) commit upon which a series of patches is 31*4882a593Smuzhiyun built. For example, a rebase operation could take a patch set built on 32*4882a593Smuzhiyun the previous kernel release and base it, instead, on the current 33*4882a593Smuzhiyun release. We'll call this operation "reparenting" in the discussion 34*4882a593Smuzhiyun below. 35*4882a593Smuzhiyun 36*4882a593Smuzhiyun - Changing the history of a set of patches by fixing (or deleting) broken 37*4882a593Smuzhiyun commits, adding patches, adding tags to commit changelogs, or changing 38*4882a593Smuzhiyun the order in which commits are applied. In the following text, this 39*4882a593Smuzhiyun type of operation will be referred to as "history modification" 40*4882a593Smuzhiyun 41*4882a593SmuzhiyunThe term "rebasing" will be used to refer to both of the above operations. 42*4882a593SmuzhiyunUsed properly, rebasing can yield a cleaner and clearer development 43*4882a593Smuzhiyunhistory; used improperly, it can obscure that history and introduce bugs. 44*4882a593Smuzhiyun 45*4882a593SmuzhiyunThere are a few rules of thumb that can help developers to avoid the worst 46*4882a593Smuzhiyunperils of rebasing: 47*4882a593Smuzhiyun 48*4882a593Smuzhiyun - History that has been exposed to the world beyond your private system 49*4882a593Smuzhiyun should usually not be changed. Others may have pulled a copy of your 50*4882a593Smuzhiyun tree and built on it; modifying your tree will create pain for them. If 51*4882a593Smuzhiyun work is in need of rebasing, that is usually a sign that it is not yet 52*4882a593Smuzhiyun ready to be committed to a public repository. 53*4882a593Smuzhiyun 54*4882a593Smuzhiyun That said, there are always exceptions. Some trees (linux-next being 55*4882a593Smuzhiyun a significant example) are frequently rebased by their nature, and 56*4882a593Smuzhiyun developers know not to base work on them. Developers will sometimes 57*4882a593Smuzhiyun expose an unstable branch for others to test with or for automated 58*4882a593Smuzhiyun testing services. If you do expose a branch that may be unstable in 59*4882a593Smuzhiyun this way, be sure that prospective users know not to base work on it. 60*4882a593Smuzhiyun 61*4882a593Smuzhiyun - Do not rebase a branch that contains history created by others. If you 62*4882a593Smuzhiyun have pulled changes from another developer's repository, you are now a 63*4882a593Smuzhiyun custodian of their history. You should not change it. With few 64*4882a593Smuzhiyun exceptions, for example, a broken commit in a tree like this should be 65*4882a593Smuzhiyun explicitly reverted rather than disappeared via history modification. 66*4882a593Smuzhiyun 67*4882a593Smuzhiyun - Do not reparent a tree without a good reason to do so. Just being on a 68*4882a593Smuzhiyun newer base or avoiding a merge with an upstream repository is not 69*4882a593Smuzhiyun generally a good reason. 70*4882a593Smuzhiyun 71*4882a593Smuzhiyun - If you must reparent a repository, do not pick some random kernel commit 72*4882a593Smuzhiyun as the new base. The kernel is often in a relatively unstable state 73*4882a593Smuzhiyun between release points; basing development on one of those points 74*4882a593Smuzhiyun increases the chances of running into surprising bugs. When a patch 75*4882a593Smuzhiyun series must move to a new base, pick a stable point (such as one of 76*4882a593Smuzhiyun the -rc releases) to move to. 77*4882a593Smuzhiyun 78*4882a593Smuzhiyun - Realize that reparenting a patch series (or making significant history 79*4882a593Smuzhiyun modifications) changes the environment in which it was developed and, 80*4882a593Smuzhiyun likely, invalidates much of the testing that was done. A reparented 81*4882a593Smuzhiyun patch series should, as a general rule, be treated like new code and 82*4882a593Smuzhiyun retested from the beginning. 83*4882a593Smuzhiyun 84*4882a593SmuzhiyunA frequent cause of merge-window trouble is when Linus is presented with a 85*4882a593Smuzhiyunpatch series that has clearly been reparented, often to a random commit, 86*4882a593Smuzhiyunshortly before the pull request was sent. The chances of such a series 87*4882a593Smuzhiyunhaving been adequately tested are relatively low - as are the chances of 88*4882a593Smuzhiyunthe pull request being acted upon. 89*4882a593Smuzhiyun 90*4882a593SmuzhiyunIf, instead, rebasing is limited to private trees, commits are based on a 91*4882a593Smuzhiyunwell-known starting point, and they are well tested, the potential for 92*4882a593Smuzhiyuntrouble is low. 93*4882a593Smuzhiyun 94*4882a593SmuzhiyunMerging 95*4882a593Smuzhiyun======= 96*4882a593Smuzhiyun 97*4882a593SmuzhiyunMerging is a common operation in the kernel development process; the 5.1 98*4882a593Smuzhiyundevelopment cycle included 1,126 merge commits - nearly 9% of the total. 99*4882a593SmuzhiyunKernel work is accumulated in over 100 different subsystem trees, each of 100*4882a593Smuzhiyunwhich may contain multiple topic branches; each branch is usually developed 101*4882a593Smuzhiyunindependently of the others. So naturally, at least one merge will be 102*4882a593Smuzhiyunrequired before any given branch finds its way into an upstream repository. 103*4882a593Smuzhiyun 104*4882a593SmuzhiyunMany projects require that branches in pull requests be based on the 105*4882a593Smuzhiyuncurrent trunk so that no merge commits appear in the history. The kernel 106*4882a593Smuzhiyunis not such a project; any rebasing of branches to avoid merges will, most 107*4882a593Smuzhiyunlikely, lead to trouble. 108*4882a593Smuzhiyun 109*4882a593SmuzhiyunSubsystem maintainers find themselves having to do two types of merges: 110*4882a593Smuzhiyunfrom lower-level subsystem trees and from others, either sibling trees or 111*4882a593Smuzhiyunthe mainline. The best practices to follow differ in those two situations. 112*4882a593Smuzhiyun 113*4882a593SmuzhiyunMerging from lower-level trees 114*4882a593Smuzhiyun------------------------------ 115*4882a593Smuzhiyun 116*4882a593SmuzhiyunLarger subsystems tend to have multiple levels of maintainers, with the 117*4882a593Smuzhiyunlower-level maintainers sending pull requests to the higher levels. Acting 118*4882a593Smuzhiyunon such a pull request will almost certainly generate a merge commit; that 119*4882a593Smuzhiyunis as it should be. In fact, subsystem maintainers may want to use 120*4882a593Smuzhiyunthe --no-ff flag to force the addition of a merge commit in the rare cases 121*4882a593Smuzhiyunwhere one would not normally be created so that the reasons for the merge 122*4882a593Smuzhiyuncan be recorded. The changelog for the merge should, for any kind of 123*4882a593Smuzhiyunmerge, say *why* the merge is being done. For a lower-level tree, "why" is 124*4882a593Smuzhiyunusually a summary of the changes that will come with that pull. 125*4882a593Smuzhiyun 126*4882a593SmuzhiyunMaintainers at all levels should be using signed tags on their pull 127*4882a593Smuzhiyunrequests, and upstream maintainers should verify the tags when pulling 128*4882a593Smuzhiyunbranches. Failure to do so threatens the security of the development 129*4882a593Smuzhiyunprocess as a whole. 130*4882a593Smuzhiyun 131*4882a593SmuzhiyunAs per the rules outlined above, once you have merged somebody else's 132*4882a593Smuzhiyunhistory into your tree, you cannot rebase that branch, even if you 133*4882a593Smuzhiyunotherwise would be able to. 134*4882a593Smuzhiyun 135*4882a593SmuzhiyunMerging from sibling or upstream trees 136*4882a593Smuzhiyun-------------------------------------- 137*4882a593Smuzhiyun 138*4882a593SmuzhiyunWhile merges from downstream are common and unremarkable, merges from other 139*4882a593Smuzhiyuntrees tend to be a red flag when it comes time to push a branch upstream. 140*4882a593SmuzhiyunSuch merges need to be carefully thought about and well justified, or 141*4882a593Smuzhiyunthere's a good chance that a subsequent pull request will be rejected. 142*4882a593Smuzhiyun 143*4882a593SmuzhiyunIt is natural to want to merge the master branch into a repository; this 144*4882a593Smuzhiyuntype of merge is often called a "back merge". Back merges can help to make 145*4882a593Smuzhiyunsure that there are no conflicts with parallel development and generally 146*4882a593Smuzhiyungives a warm, fuzzy feeling of being up-to-date. But this temptation 147*4882a593Smuzhiyunshould be avoided almost all of the time. 148*4882a593Smuzhiyun 149*4882a593SmuzhiyunWhy is that? Back merges will muddy the development history of your own 150*4882a593Smuzhiyunbranch. They will significantly increase your chances of encountering bugs 151*4882a593Smuzhiyunfrom elsewhere in the community and make it hard to ensure that the work 152*4882a593Smuzhiyunyou are managing is stable and ready for upstream. Frequent merges can 153*4882a593Smuzhiyunalso obscure problems with the development process in your tree; they can 154*4882a593Smuzhiyunhide interactions with other trees that should not be happening (often) in 155*4882a593Smuzhiyuna well-managed branch. 156*4882a593Smuzhiyun 157*4882a593SmuzhiyunThat said, back merges are occasionally required; when that happens, be 158*4882a593Smuzhiyunsure to document *why* it was required in the commit message. As always, 159*4882a593Smuzhiyunmerge to a well-known stable point, rather than to some random commit. 160*4882a593SmuzhiyunEven then, you should not back merge a tree above your immediate upstream 161*4882a593Smuzhiyuntree; if a higher-level back merge is really required, the upstream tree 162*4882a593Smuzhiyunshould do it first. 163*4882a593Smuzhiyun 164*4882a593SmuzhiyunOne of the most frequent causes of merge-related trouble is when a 165*4882a593Smuzhiyunmaintainer merges with the upstream in order to resolve merge conflicts 166*4882a593Smuzhiyunbefore sending a pull request. Again, this temptation is easy enough to 167*4882a593Smuzhiyununderstand, but it should absolutely be avoided. This is especially true 168*4882a593Smuzhiyunfor the final pull request: Linus is adamant that he would much rather see 169*4882a593Smuzhiyunmerge conflicts than unnecessary back merges. Seeing the conflicts lets 170*4882a593Smuzhiyunhim know where potential problem areas are. He does a lot of merges (382 171*4882a593Smuzhiyunin the 5.1 development cycle) and has gotten quite good at conflict 172*4882a593Smuzhiyunresolution - often better than the developers involved. 173*4882a593Smuzhiyun 174*4882a593SmuzhiyunSo what should a maintainer do when there is a conflict between their 175*4882a593Smuzhiyunsubsystem branch and the mainline? The most important step is to warn 176*4882a593SmuzhiyunLinus in the pull request that the conflict will happen; if nothing else, 177*4882a593Smuzhiyunthat demonstrates an awareness of how your branch fits into the whole. For 178*4882a593Smuzhiyunespecially difficult conflicts, create and push a *separate* branch to show 179*4882a593Smuzhiyunhow you would resolve things. Mention that branch in your pull request, 180*4882a593Smuzhiyunbut the pull request itself should be for the unmerged branch. 181*4882a593Smuzhiyun 182*4882a593SmuzhiyunEven in the absence of known conflicts, doing a test merge before sending a 183*4882a593Smuzhiyunpull request is a good idea. It may alert you to problems that you somehow 184*4882a593Smuzhiyundidn't see from linux-next and helps to understand exactly what you are 185*4882a593Smuzhiyunasking upstream to do. 186*4882a593Smuzhiyun 187*4882a593SmuzhiyunAnother reason for doing merges of upstream or another subsystem tree is to 188*4882a593Smuzhiyunresolve dependencies. These dependency issues do happen at times, and 189*4882a593Smuzhiyunsometimes a cross-merge with another tree is the best way to resolve them; 190*4882a593Smuzhiyunas always, in such situations, the merge commit should explain why the 191*4882a593Smuzhiyunmerge has been done. Take a moment to do it right; people will read those 192*4882a593Smuzhiyunchangelogs. 193*4882a593Smuzhiyun 194*4882a593SmuzhiyunOften, though, dependency issues indicate that a change of approach is 195*4882a593Smuzhiyunneeded. Merging another subsystem tree to resolve a dependency risks 196*4882a593Smuzhiyunbringing in other bugs and should almost never be done. If that subsystem 197*4882a593Smuzhiyuntree fails to be pulled upstream, whatever problems it had will block the 198*4882a593Smuzhiyunmerging of your tree as well. Preferable alternatives include agreeing 199*4882a593Smuzhiyunwith the maintainer to carry both sets of changes in one of the trees or 200*4882a593Smuzhiyuncreating a topic branch dedicated to the prerequisite commits that can be 201*4882a593Smuzhiyunmerged into both trees. If the dependency is related to major 202*4882a593Smuzhiyuninfrastructural changes, the right solution might be to hold the dependent 203*4882a593Smuzhiyuncommits for one development cycle so that those changes have time to 204*4882a593Smuzhiyunstabilize in the mainline. 205*4882a593Smuzhiyun 206*4882a593SmuzhiyunFinally 207*4882a593Smuzhiyun======= 208*4882a593Smuzhiyun 209*4882a593SmuzhiyunIt is relatively common to merge with the mainline toward the beginning of 210*4882a593Smuzhiyunthe development cycle in order to pick up changes and fixes done elsewhere 211*4882a593Smuzhiyunin the tree. As always, such a merge should pick a well-known release 212*4882a593Smuzhiyunpoint rather than some random spot. If your upstream-bound branch has 213*4882a593Smuzhiyunemptied entirely into the mainline during the merge window, you can pull it 214*4882a593Smuzhiyunforward with a command like:: 215*4882a593Smuzhiyun 216*4882a593Smuzhiyun git merge v5.2-rc1^0 217*4882a593Smuzhiyun 218*4882a593SmuzhiyunThe "^0" will cause Git to do a fast-forward merge (which should be 219*4882a593Smuzhiyunpossible in this situation), thus avoiding the addition of a spurious merge 220*4882a593Smuzhiyuncommit. 221*4882a593Smuzhiyun 222*4882a593SmuzhiyunThe guidelines laid out above are just that: guidelines. There will always 223*4882a593Smuzhiyunbe situations that call out for a different solution, and these guidelines 224*4882a593Smuzhiyunshould not prevent developers from doing the right thing when the need 225*4882a593Smuzhiyunarises. But one should always think about whether the need has truly 226*4882a593Smuzhiyunarisen and be prepared to explain why something abnormal needs to be done. 227