Linux: Linus And Kernel Patches

Submitted by Jeremy
on May 15, 2002 - 8:37pm

When Linux creator Linus Torvalds began using the BitKeeper (BK) source control tool for managing the 2.5 Linux development kernel [earlier story], one of the big fears people put forward was that all Linux kernel developers would eventually be forced to use this tool. (The BK license is a major source of contention. [earlier story])

David Woodhouse recently asked Linus if he was still accepting BK changesets by mail, as some time back Linus had reported that they frequently do not apply. In reply, Linus explained that when receiving BK changesets, "it's actually more work for me, and about 50% of the BK patches I get don't even apply, because the person who sent them to me didn't send the whole series". The two preferred methods are "BK Pulls", or the old style patches. Linus says, "I don't much care one way or the other: regular email patches are easy for me to integrate, so it's really up to you."

From there, the discussion flowed into how patches should be commented. Linus did make it clear that the comment thoughts were simply his opinion, and have "no impact on patch acceptance, as long as enough changelog information exists _somewhere_".

From: Linus Torvalds
Subject: Re: Changelogs on kernel.org
Date: Wed, 15 May 2002 09:39:30 -0700 (PDT)

On Wed, 15 May 2002, David Woodhouse wrote:
>
> Are you still accepting BK changesets by mail? I heard rumours that you'd
> said you didn't want them any more -- but couldn't find a message in which
> you actually said that.

I try to avoid it as much as possible - it's actually more work for me,
and about 50% of the BK patches I get don't even apply, because the person
who sent them to me didn't send the whole series (ie left out some patch
he didn't like or something like that).

> When making changesets to send to you, I usually start with a fresh clone
> of your latest tree and apply my changes to that -- would you prefer me to
> make the whole tree available somewhere public (or on master.kernel.org)
> for a 'bk pull' instead?

I much prefer a bk pull, if the tree I pull from is clean (ie it doesn't
have random crud in it, and it contains changsets from just one project).

Some people actually use BK but then send me regular patches anyway,
because they find that easier than setting up an exported BK tree and
being careful about cleanliness. I don't much care one way or the other:
regular email patches are easy for me to integrate, so it's really up to
you.

> Having the facility to put per-file changelogs in with BK rather than just
> sending patches is something I quite like, so I'd rather not just revert to
> sending patches.

[ Personal opinion alert! No impact on patch acceptance, as long as
enough changelog information exists _somewhere_ ]

I personally like good changelog comments, and I find per-file comments to
be a mistake.

I have a few simple reasons for this:

 - most changes are related, and per-file comments tend to just repeat.

In fact, for patches, I just repeat the Subject: line in the per-file
comments, so that they have just enough context to get the big picture.

- if your changes _aren't_ related, you should have done multiple
changesets in the first place. Rince and repeat as necessary.

- the per-file comments don't show up in many of the standard changelogs
(not mine, not in "bk changes" etc), so the per-changeset comment
should be "sufficient" in many ways anyway.

But to each his own.

[ End personal opinion ]

Linus

From: David Woodhouse
Subject: Re: Changelogs on kernel.org
Date: Wed, 15 May 2002 19:07:16 +0100

Linus Torvalds said:
> I try to avoid it as much as possible - it's actually more work for
> me, and about 50% of the BK patches I get don't even apply, because
> the person who sent them to me didn't send the whole series (ie left
> out some patch he didn't like or something like that).

Oh Larry, are you listening? :)

> I much prefer a bk pull, if the tree I pull from is clean (ie it
> doesn't have random crud in it, and it contains changsets from just
> one project).

Noted. Thanks.

> I personally like good changelog comments, and I find per-file
> comments to be a mistake.

They can be, but sometimes it can be useful to put a high-level overview of
what you've done suitable for people who aren't familiar with the code into
the changeset comment, and describe exactly _how_ you did it in per-file
comments.

Which in the case of the patch I sent you yesterday would be something like
'fix zisofs breakage with shared zlib' on the changeset and 'set return
value to trv not f in NEEDBYTE' in the lib/zlib_inflate/inflate.c log.

In this case, the latter can obviously be deduced from the diffs because
it's a one-liner, so perhaps it's a bad example -- but you don't always
actually want to have to refer to the diffs.

--
dwmw2


From: Larry McVoy
Subject: Re: Changelogs on kernel.org
Date: Wed, 15 May 2002 12:20:03 -0700

On Wed, May 15, 2002 at 09:39:30AM -0700, Linus Torvalds wrote:
> I try to avoid it as much as possible - it's actually more work for me,
> and about 50% of the BK patches I get don't even apply, because the person
> who sent them to me didn't send the whole series (ie left out some patch
> he didn't like or something like that).

FYI, if they do a

bk send -ubk://linux.bkbits.net/linux-2.5 torvalds@transmeta.com

that problem goes away. The -u stuff does the same sort of handshake
that a pull does to figure out what needs to be sent to fill in the holes.

> I personally like good changelog comments, and I find per-file comments to
> be a mistake.

Sometimes yes, sometimes no. Certainly the high order bit is to capture
the logical change in the changeset comment. One should only have to read
the change{set,log} comment to see if that change is interesting or not,
it should not be necessary to go read the file comments.

The file comments are more about the details of the implementation, not
the idea. I think the reason that Linus doesn't care about file comments
is that he always reads the diffs and that's better than any comment, in
general. That's more or less true, but the file comment is a place to
give yourself or others a hint as to what was in your mind when you made
that change. Those hints can really save your butt when the pressure is
on to turn around a bugfix fast for a customer/whatever.

One of the engineers here said "Changeset comments are for managers,
file comments are for engineers", which is another way to look at it.

Anyway, I would agree 100% that the changeset comments are the most
important in general, so if those are gotten right then we're ahead
of the game.

> - the per-file comments don't show up in many of the standard changelogs
> (not mine, not in "bk changes" etc), so the per-changeset comment

bk changes -v

will list the file comments as well. There is a minor sorting bug in there
when the timestamps are screwed up, but it tries pretty hard to make it be

	ChangeSet comments

File 1 comments

File 2 comments

Etc.

--
---
Larry McVoy lm at bitmover.com http://www.bitmover.com/lm

From: David Woodhouse
Subject: Re: Changelogs on kernel.org
Date: Wed, 15 May 2002 21:03:40 +0100

Larry McVoy said:
> FYI, if they do a
> bk send -ubk://linux.bkbits.net/linux-2.5 torvalds@transmeta.com
> that problem goes away. The -u stuff does the same sort of
> handshake that a pull does to figure out what needs to be sent to fill
> in the holes.

Not quite. The sender usually omits changesets for a _reason_. You'll often
find that one of the changesets in the middle wasn't necessary and didn't
touch any of the same files -- in which case patches would have applied
just fine.

--
dwmw2

From: Larry McVoy
Subject: Re: Changelogs on kernel.org
Date: Wed, 15 May 2002 13:08:31 -0700

On Wed, May 15, 2002 at 09:03:40PM +0100, David Woodhouse wrote:
>
> Not quite. The sender usually omits changesets for a _reason_. You'll often
> find that one of the changesets in the middle wasn't necessary and didn't
> touch any of the same files -- in which case patches would have applied
> just fine.

Understood. BK doesn't work that way for multiple reasons, some which have
to do with how it synchronizes replicas, and some which have to do with
being able to reproduce a tree exactly.

It's probably best if you simply view this as a BK limitation which isn't
going away any time soon and don't put junk changesets in the middle of
your stream of changes. It's easy enough to export the change you want
as a patch, export the comments in the form that bk comments wants,
undo the junk changeset, import the patch, and set the comments. Yeah,
it's awkward; consider that a feedback loop which encourages you to
think a bit more about what you put in the tree.
--
---
Larry McVoy lm at bitmover.com http://www.bitmover.com/lm

From: David Woodhouse
Subject: Re: Changelogs on kernel.org
Date: Wed, 15 May 2002 21:15:37 +0100

What it actually encourages is for people to have multiple throwaway trees.
(Which isn't quite so much of a BK turnoff once you discover compilercache.)

If the tree's going to be thrown away anyway, it doesn't matter if it gets
confused -- how about making it a little easier to backport changesets --
surely it should be possible to make BK import a changeset iff all the
affected files are identical in both trees before the changeset?

--
dwmw2

From: Larry McVoy
Subject: Re: Changelogs on kernel.org
Date: Wed, 15 May 2002 13:34:32 -0700

On Wed, May 15, 2002 at 09:15:37PM +0100, David Woodhouse wrote:
> What it actually encourages is for people to have multiple throwaway trees.
> (Which isn't quite so much of a BK turnoff once you discover compilercache.)

Yup, we use that here, works great.

> If the tree's going to be thrown away anyway, it doesn't matter if it gets
> confused -- how about making it a little easier to backport changesets --
> surely it should be possible to make BK import a changeset iff all the
> affected files are identical in both trees before the changeset?

We've already built all the interfaces you need to do this, so would
you be interested in writing the shell script that does it? The
interfaces you will want:

	bk export -tpatch
bk import -tpatch
bk comments
bk changes -v

You'll want this, I believe that you can take the output of this command
and feed it into bk comments and have that be a noop. If that works, this
is what you need to save as the comments part of the patch, and now it's
pretty trivial to move the patch backwards.

bk changes -r+ -vd'### Comments for :GFILE:|+n$each(:C:){(:C:)n}'

If you do write this, let me know if you want it included as a "bk backport"
command.

Also note that there is a `bk bin`/bk.script which is a set of shell script
functions. Many BK commands start out their life as

# bk backport - cherry pick a change out of tree A and put it in tree B
# usage: bk backport -r from to
_backport() {
}

If you start the command with a underscore, then BK will autotranslate that
into a command that you can use like

	bk backport ....

If you prefer a standalone command (for licensing reasons, perhaps), then
make it a standalone shell script, put it in `bk bin`, and bk will find it.
--
---
Larry McVoy lm at bitmover.com http://www.bitmover.com/lm