Hi Jakub, On Tue, Jul 26, 2022 at 3:31 PM Jakub Kicinski <kuba@xxxxxxxxxx> wrote: > > On Tue, 26 Jul 2022 15:05:17 -0700 Luiz Augusto von Dentz wrote: > > > > Ive just fixup the original patch that introduced it, btw how do you > > > > run sparse to capture such errors? > > > > > > We run builds with W=1 C=1 in the CI and then diff the outputs. > > > That's pretty noisy so we have a regex which counts number of > > > warnings per file, that makes it possible to locate the exact new > > > warning. At least most of the time... > > > > Hmm, is there any way to trigger net CI, either that or we need to > > duplicate the same test under our CI to avoid these last minute > > findings when we are attempting to merge something. > > The code is at: > > https://github.com/kuba-moo/nipa > > But it hardcodes net and bpf tree maching in places. You may want > to steal just the build script, its in bash. We can either incorporate it on our own CI or start to consolidate everything in one place since there are quite a few tests that apply tree wide, though we would need to add support for subsystem specific tests as well, anyway I leave for @Tedd Ho-Jeong An to comment on this when he is back from vacation. > > > > So we don't need to rebase? > > > > > > No, not usually. After we pull from you, you should pull back from us > > > (git pull --ff-only $net-or-net-next depending on the tree you > > > targeted), and that's it. The only patches that go into your tree then > > > are bluetooth patches, everything else is fed via pulling back from us. > > > > > > > There were some patches already applied via bluetooth.git so at least > > > > I do it to remove them > > > > > > Normally you'd not apply bluetooth fixes to bluetooth-next, apply > > > them to bluetooth and send us a PR. Then once a week we'll merge > > > net (containing your fixes) into net-next, at which point you can > > > send a bluetooth-next PR and get the fixes into bluetooth-next. > > > FWIW from our perspective there's no limit on how often you send PRs. > > > > Are you saying we should be using merge commits instead of rebase then? > > Not sure what merge commits would mean in this case. > > > > Alternatively you could apply the fixes into bluetooth and then > > > merge bluetooth into bluetooth-next. If you never rebase either tree, > > > git will be able to figure out that it's the same commit hash even if > > > it makes it to the tree twice (once thru direct merge and once via > > > net). That said, I believe Linus does not like cross tree merges, i.e. > > > merges which are not fast forwards to the downstream tree. So it's > > > better to take the long road via bt -> net -> net-next -> bt-next. > > > > Well I got the impression that merge commits shall be avoided, but > > There's many schools of thought, but upstream there's very little > rebasing of "official" branches (i.e. main/master branches, not > testing or other unstable branches) AFAIK. > > > rebase overwrites the committer, so the two option seem to have > > drawbacks, well we can just resign on rebase as well provided git > > doesn't duplicate Signed-off-by if I use something like exec="git > > commit -s --amend". > > Sure, be careful tho because I think it doesn't check the signoff > history, IIRC just the most recent tag. So you may end up with multiple > signoffs from yourself and Marcel. Yep, I thought that was a bug though since I doubt there is any use for duplicated signoffs. > > > > and any possible conflicts if there were > > > > changes introduced to the bluetooth directories that can eventually > > > > come from some other tree. > > > > > > Conflicts are not a worry, just let us know in the PR description how > > > to resolve them. > > > > Not really following, how can we anticipate a merge conflict if we > > don't rebase? > > If your trees are hooked up to linux-next (I presume not 'cause Stephen > would probably scream at you for rebasing?) - Stephen will tell you > there's a conflict within a day or two. > > Obviously sometimes you'll notice right away when applying patches that > two patches touch the same function. > > > With merge strategy it seem that the one pulling needs > > to resolve the conflicts rather than the submitter which I think would > > lead to bad interaction between subsystems, expect if we do a merge > > [-> resolve conflict] -> pull request -> [resolve conflicts ->] merge > > which sounds a little too complicated since we have to resolve > > conflicts in both directions. > > The pulling back should always be a fast-forward so there's no merge > commit or conflicts (git pull --ff-only). Only the actual downstream > tree (netdev) has to resolve conflicts, which is not all that bad > thanks for Stephen's advanced notices. > > > In my opinion rebase strategy is cleaner and is what we recommend for > > possible clones of bluetooth-next and bluetooth trees including CI so > > possible conflicts are fixed in place rather on the time the trees are > > merged. > > No strong preference here as long as we can keep the sign-offs etc in > control. Note that I'm not aware of any other tree we pull rebasing, > tho, so you may run into unique issues. Maybe I need to get in touch with other maintainers to know what they are doing, but how about net-next, how does it gets updated? Is that done via git merge or git pull alone is enough? -- Luiz Augusto von Dentz