Re: [GIT PULL] isci: sas controller driver for 3.0

James Bottomley
2011-07-03T05:16:29Z

On Sat, 2011-07-02 at 19:43 -0700, Dan Williams wrote:
> On 07/02/2011 03:45 PM, James Bottomley wrote:
> > Look, this tree is unacceptable because
> >
> > A. It's not bisectable. This is what happens when I try a bisection:
> [..]
> > make[1]: *** [drivers/scsi/isci/init.o] Error 1
>
> This was inadvertent, but breakage is breakage.

That's fine for a development tree. When you submit a series of
patches, whether directly or via a git tree, they have to be bisectable.
This isn't negotiable because any old user has to be able to perform a
bisection.

This is why most submissions tend to be patches rather than git trees.
To submit a git tree, you really have to prove that you knew the rules
when you created and ran it (like no bisectability breaks etc.)

> > And B. It's got contaminated history like this:
>
> This was on purpose. We were in the position of stabilizing /
> validating the driver while carrying out the cleanups. To minimize back
> merges I duplicated upstream fixes/enabling in these instances. If this
> is de-facto unacceptable I will adjust my tree management going forward.

Yes, it is.

> But I don't believe it is given occasions where two maintainers take
> the same patch through their respective trees.

These aren't the same patches applied by different maintainers; it's a
cherry pick backport which shouldn't be there.

> > I need clean history and a bisectable tree ... By Sunday, since this has
> > to be in linux-next before the next kernel release, which will likely be
> > Monday.
>
> I certainly can rebase this to address the compile breakage. Although
> this does throw away old commit ids which have seen validation both
> inside and outside of Intel, but that's unavoidable at this point.
>
> The value I see in maintaining the history (even with some re-written
> ids) is:
>
> 1/ The history is available in mainline and not a side tree. Care went
> into the commit changelogs. They provide driver documentation and
> contain notes about future cleanups and improvements.
>
> 2/ Code reviewers are credited with Reported-by tags for the commits
> they generated.
>
> 3/ The history is instructive of the dangers of OS abstraction drivers.

The internal intel history is completely irrelevant. I can see value to
maintaining the commit history after publication because several
non-intel people worked on it and it's nice to credit them.

> > Since the tree is huge, I don't think this is fixable in the timescale,
> > so just a single patch will do ... I can construct that, but I need the
> > change log from you, please.
>
> I can go either way, but my preference is a rebase to clean up the
> bisection.

I just stopped at the first failure ... please ensure that there aren't
any more ... as in test every bisection point in the tree, don't just
squash the commit I've flagged as failing.

The fact that there is a bisection failure tends to indicate you haven't
been careful to check each commit in the tree ... which does make it
suspect and will mean every commit in the reconstituted tree needs
checking for bisectability.

James