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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 26 Jul 2022 15:05:17 -0700
From:   Luiz Augusto von Dentz <luiz.dentz@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>
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 Fri, Jul 22, 2022 at 5:50 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Fri, 22 Jul 2022 17:25:57 -0700 Luiz Augusto von Dentz wrote:
> > > > Crap, let me fix them.
> > >
> > > Do you mean i should hold off with pushing or you'll follow up?
> >
> > 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.

> > > > Yep, that happens when I rebase on top of net-next so I would have to
> > > > redo all the Signed-off-by lines if the patches were originally
> > > > applied by Marcel, at least I don't know of any option to keep the
> > > > original committer while rebasing?
> > >
> > > I think the most common way is to avoid rebasing. Do you rebase to get
> > > rid of revised patches or such?
> >
> > 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?

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

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

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.

-- 
Luiz Augusto von Dentz