On Thu, Aug 03, 2017 at 08:50:06AM -0700, Darren Hart wrote:
> On Wed, Aug 02, 2017 at 06:06:20PM -0700, Linus Torvalds wrote:
> > On Wed, Aug 2, 2017 at 5:28 PM, Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx> wrote:
> > >
> > > I would say that if you rebase someone's commit(s), then you are on the
> > > "patch's delivery path" and so should add a Signed-off-by tag.
> >
> > Yeah, I agree. Rebasing really is pretty much the exact same thing as
> > applying a patch.
I will be away for a few days, but will follow up on this when I return.
In the meantime, my plan is to leave the current for-next branch alone
rather than rebasing it to fix the previous rebase which resulted in the
mixed committer/signoff issue Stephen's new test identified.
I just want it to be clear I'm not ignoring the issue, but rather
planning on addressing it in commits going forward - based on the
results of the discussion below.
Thanks,
> >
> > > "git rebase" does have a "--signoff" option.
> >
> > I think you end up signing off twice using that. I don't think it's
> > smart enough to say "oh, you already did it once".
> >
> > But I didn't check. Sometimes git is a lot smarter than I remember it
> > being, simply because I don't worry about it. Junio does a good job.
> >
> > And in general, you simply should never rebase commits that have
> > already been publicized. And the fact that you didn't commit them in
> > the first place definitely means that they've been public somewhere.
>
> For the platform driver x86 subsystem, Andy I have defined our "testing"
> branch as mutable. It's the place where our CI pulls from, as well as
> the first place 0day pulls from, and where we stage things prior to
> going to the publication branches ("for-next" and then sometimes
> "fixes"). We find it valuable to let the robots have a chance to catch
> issues we may have missed before pushing patches to a publication
> branch, but to do that, we need the testing branch to be accessible to
> them.
>
> The usual case that would land us in the situation here is we discover a
> bug in a patch and revert it before going to a publication branch.
> Generally, this will involve one file (most patches here are isolated),
> which we drop via rebase, and the rest are entirely unaffected in terms
> of code, but as the tree changed under them, they get "re-committed".
>
> This seems like a reasonable way to handle a tree with more than one
> maintainer and take advantage of some automation. Andy and I do need a
> common tree to work from, and I prefer to sync with him as early in the
> process as possible, rather than have him and I work with two private
> testing branches and have to negotiate who takes which patches. It would
> slow us down and wouldn't improve quality in any measurable way. Even if
> we did this work in an access controlled repository, we would still have
> this problem.
>
> With more and more maintainer teams, I think we need to distinguish
> between "published" branches and "collaboration" branches. I suspect
> maintainer teams will expose this rebasing behavior, but I don't believe
> it is new or unique to us. To collaborate, we need a common branch,
> which a lone maintainer doesn't need, and the committer/sign-off delta
> makes this discoverable, whereas it was invisible with a lone
> maintainer.
>
> Note: A guiding principle behind our process is that of not introducing
> bugs into mainline. Rather than reverting bad patches in testing, we
> drop them, and replace them with a fixed version. The idea being we
> don't want to introduce git bisect breakage, and we don't want to open
> the window for stable/distro maintainers to pull a bad patch and forget
> the revert or the fixup. If we can correct it before it goes to Linus,
> we do.
>
> > So I would definitely suggest against the "git rebase --signoff"
> > model, even if git were to do the "right thing". It's simply
> > fundamentally the wrong thing to do. Either you already committed them
> > (and hopefully signed off correctly the first time), or you didn't
> > (and you shouldn't be rebasing). So in neither case is "git rebase
> > --signoff" sensible.
>
> So in light of the above, we can:
>
> a) Keep doing what we're doing
> b) Sign off whenever we rebase
> c) Add our signoff whenever we move patches from testing to for-next
> (I hadn't considered this until now... this might be the most
> compatible with maintainer teams while strictly tracking the
> "patches" delivery path")
> d) Redefine testing as immutable and revert patches rather than drop
> them, introducing bugs into mainline.
> e) Make each maintainer work from a private set of branches (this just
> masks the problem by making the rebase invisible)
>
> Whatever we decide, I'd like to add this to some documentation for
> maintainer teams (which I'm happy to prepare and submit).
--
Darren Hart
VMware Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-next" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
