Re: pull request: bluetooth-next 2022-07-22

Jakub Kicinski
2022-07-26T22:31:45Z

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.

> > > 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.

> > > 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.