Message-ID: <CABBYNZJoAe+XDp_Zq4bfepizxpmUiB_Vo-ix1A2TyJXjzQVe+Q@mail.gmail.com>
Date:   Tue, 26 Jul 2022 18:06:47 -0700
From:   Luiz Augusto von Dentz <luiz.dentz@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>,
        Tedd Ho-Jeong An <hj.tedd.an@...il.com>
Cc:     David Miller <davem@...emloft.net>,
        "linux-bluetooth@...r.kernel.org" <linux-bluetooth@...r.kernel.org>,
        "open list:NETWORKING [GENERAL]" <netdev@...r.kernel.org>
Subject: Re: pull request: bluetooth-next 2022-07-22

Hi Jakub,

On Tue, Jul 26, 2022 at 3:31 PM Jakub Kicinski <kuba@...nel.org> 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

Powered by blists - more mailing lists