This is an email attempt to move a thread from users.kernel.org to LKML
where it belongs.
I've tried to Cc all those that were on the original Cc (sorry if you
don't want to be, but just send this to /dev/null in that case).
Here's the basic gist, some people believe that linux-next is used as a
dumping ground for their repos that get rebased all the time. They use
linux-next for early testing, and mostly to make sure their repo will
not collide with other developers repos.
Some of the reasons for the constant rebasing are:
1) the patches are held in quilt, which just by nature leads to
rebasing. These developers find that quilt is the best tool for the
job.
2) after collisions with other repos, the developers find other ways to
to solve the issue, and rebase it instead of having a bunch of "merged"
conflicts go off to Linus.
3) They want acks and reviewed-by labels added. Which would cause a
rebase because the commit must change to add these.
4) Major bugs are found and bisectablity is broken. Rebasing would keep
the git history working for bisecting.
I'm sure there are other reasons, please feel free to add your own, or
to refute the ones I listed.
Other developers feel that there's too much rebasing going on in
linux-next and that there should be a cleaner work-flow. Perhaps have
maintainers test their work a bit more before passing it off to
linux-next?
This is not a complete description of what is going on, but it gets the
idea across. Now those of you that want to argue this, go ahead. But use
this email as the starting point and keep it off of users.kernel.org
Thanks,
-- Steve
* Steven Rostedt <[email protected]> wrote:
> This is an email attempt to move a thread from users.kernel.org to
> LKML where it belongs.
[ i fixed the typo in the subject line - replies might want to do that
too ;-) ]
Ingo
* Steven Rostedt <[email protected]> wrote:
> Here's the basic gist, some people believe that linux-next is used as
> a dumping ground for their repos that get rebased all the time. They
> use linux-next for early testing, and mostly to make sure their repo
> will not collide with other developers repos.
I see signs of such an attitude, and i think it's somewhat harmful.
As far as using linux-next for a test-and-rebase workflow - IMO
maintainer trees should lead with a good example and should not push
'avoidable crap that might need rebasing' into linux-next (knowingly at
least - there's enough unintentional damage) that they wouldnt push
upstream to Linus to begin with.
The pure act of integration testing (the stated primary purpose of
linux-next) is a large enough of a job in itself IMHO.
Maintainer trees pushed towards linux-next should strive to be Git
based, append-mostly, 'nice', 'intended for upstream' and defendable
as-is IMO, and rebasing a _maintainer tree_ should really be a rare act
of last resort. [ Developers OTOH can (and will and perhaps should)
rebase frequently until a feature becomes pushable. ]
Anyway - just my two cents - YMMV.
Ingo
On Fri, Oct 23, 2009 at 10:54:00PM +0200, Ingo Molnar wrote:
>
> Maintainer trees pushed towards linux-next should strive to be Git
> based, append-mostly, 'nice', 'intended for upstream' and defendable
> as-is IMO, and rebasing a _maintainer tree_ should really be a rare act
> of last resort.
As maintainer I try to put some effort in crediting people
where credit belongs.
In other words collecting "Acked-by:", "Tested-by", "Reviewed-by".
Adding this require a rebase as soon as said patch hits git.
One could use topic brances and I do not know what.
But frankly - working with this on and off and
in limited spare time makes it sometimes hard enough
to do the basic steps correct.
Trying to fool around with several topics branches and
such does simply not fit.
I try to say with the above that rebasing is sometimes
a way to get the job done without making things too complicated.
And -next has btw caught a lot of integration issues
for kbuild in the past.
Both for varoious architectures but sometimes also
other ways to do the same thing.
And sometimes I'm in the situation that I have to decide:
1) wait another 10 days before I have ~1 hour that I can
dedicate to test stuff
2) do some rudimentary testing and drop in in -next.
It depends but sometimes I go for option 2) knowing
that it is risky.
Sam
On Fri, Oct 23, 2009 at 10:54:00PM +0200, Ingo Molnar wrote:
>
> * Steven Rostedt <[email protected]> wrote:
>
> > Here's the basic gist, some people believe that linux-next is used as
> > a dumping ground for their repos that get rebased all the time. They
> > use linux-next for early testing, and mostly to make sure their repo
> > will not collide with other developers repos.
>
> I see signs of such an attitude, and i think it's somewhat harmful.
>
> As far as using linux-next for a test-and-rebase workflow - IMO
> maintainer trees should lead with a good example and should not push
> 'avoidable crap that might need rebasing' into linux-next (knowingly at
> least - there's enough unintentional damage) that they wouldnt push
> upstream to Linus to begin with.
I think we need to be a bit careful in this discussion. There are two
things that cause a particular git tree to be one which can't be used
as a the basis for subtrees. One is "rebasing", where a series of
commits is dropped onto a new version, or base, hence "rebasing". The
other is where one or more commits are *modified* --- perhaps to add
ack-ed by, or tested-by comment lines, or to improve comments, or to
fix outright bugs in the the patch series. Perhaps it's better to
call this "rewinding", since in most cases this doesn't actually cause
a change in the "base" of the patch series.
The reason why it's important to make this distinction is that some of
the arguments about why constantly changing the base of a patch series
don't apply when we are just fixing up patches in the patch series or
git tree.
So given that, why do I think "rewinding" has a place as a development
methodology for patch sources that feed into linux-next.
1) Linux-next is by definition a constantly rewinding branch. It is
thrown away and recreated every day, based on the tip of Linus's
mainline tree, and so the date of the merge commits means that you can
never base anything on top of linux-next. This has always been the
case, and so trying to impose a straightjacket on all of the sources
of linux-next doesn't actually buy anything as far as the properties
of linux-next.
2) There are many legitimate reasons for "rewinding". In addition to
being able to add credit for tested-by and acked-by lines, sometimes
patches are subtle. More than once, patches have been sitting in the
ext4 tree that have passed the XFSQA test, and thus have been "unit
tested", but they still have bugs; in some cases, subtle bugs. In
some cases, bugs that cause data corruption. In the case where the
patches have hit linux-next, but the merge window hasn't opened yet, I
prefer to fix the patch by mutating it, and rewinding the ext4 tree,
instead of adding a fix later. It makes it easier to cherry pick
patches to the stable tree later, and it keeps the ext4 tree clean,
and it has no downside in terms of linux-next --- see (1) above.
3) I don't have the same access to vast amounts of hardware and
platforms that Ingo does. As a result, while I make a practice of
testing every single patch against the XFS test suite (yes, it's slow
and painful, but I think it's worth it; I'm very paranoid about patch
quality), every once in a while the patch has warnings or doesn't
compile on some platform for which I don't have build/test machines.
Today, this gets tested in linux-next, and when it does, if it the
merge window hasn't opened yet, I will fix it the patch instead of
creating an extra patch. This helps git bisectability for platforms I
don't have access to.
4) The practice of rewinding and updating patches has a long and
honored history. It's the way Andrew Morton has worked with
linux-mm/mmotm, for example. And many patch series sources for
linux-next. One argument which Ingo has advanced is perhaps those
subsystems shouldn't use git at all, and just use patch series. I
think this is nonsense. First of all, it's more convenient to deliver
patches to linux-next and Linus via git. Secondly, sending a git pull
request *does* provide more information than just a patch series. The
git pull request includes one critical bit of information --- the base
version that the maintainer has tested against, and that is preserved
when patches are delivered via a git pull request. If the ext4 tree
was delievered via a patch series, then by definition when the patches
are applied, they will be rebased, in the proper sense of the term,
and you get all of the evils about *rebasing* (as opposed to rewinding
and fixing up patches) that people have complained about.
Ultimately, one of the things that Linus has said recently, both at
LinuxCon and again at Kernel Summit, is on the whole he's very happy
with how the development process has been moving, and one the things
that he has pointed at is linux-next. So I'd argue, let's not break
something that is working quite well. Whether or not a particular
subsystem uses patch series, a rewinding git tree (using tools like
guilt or stgit), or an append-only git tree, is currently up to the
maintainer and the circumstances of how many developers participate in
a particular subsystem, and whether subtrees would actually be helpful
for that particular subsystem. I don't think we should be imposing
straightjackets on maintainers. What works well for one subsystem may
not work well for others. And again, on the whole, Linus has said
that he's very pleased with how things are working today. So maybe we
should leave well enough alone, and not try to impose a procrustean
bed on all maintainers.
Regards,
- Ted
Theodore Tso wrote:
> I think we need to be a bit careful in this discussion. There are two
> things that cause a particular git tree to be one which can't be used
> as a the basis for subtrees. One is "rebasing", where a series of
> commits is dropped onto a new version, or base, hence "rebasing". The
> other is where one or more commits are *modified* --- perhaps to add
> ack-ed by, or tested-by comment lines, or to improve comments, or to
> fix outright bugs in the the patch series. Perhaps it's better to
> call this "rewinding", since in most cases this doesn't actually cause
> a change in the "base" of the patch series.
That's helpful terminology.
> The reason why it's important to make this distinction is that some of
> the arguments about why constantly changing the base of a patch series
> don't apply when we are just fixing up patches in the patch series or
> git tree.
>
> So given that, why do I think "rewinding" has a place as a development
> methodology for patch sources that feed into linux-next.
Though per definition of what is expected to be submitted into
linux-next, both rebasing and rewinding should occur rather rarely.
Instead of a process rule that for-next branches should not be rebased/
rewound, I would suggest that
If a for-next branch is rarely rewound, let alone rebased, it is an
indicator that development and maintenance of a tree are going well.
And vice versa.
stands as a /rule of thumb/. Actually, that's all very obvious because
a for-next branch is pretty much a /release/ branch.
> 1) Linux-next is by definition a constantly rewinding branch. It is
> thrown away and recreated every day, based on the tip of Linus's
[It is not entirely thrown away, see
http://git.kernel.org/?p=linux/kernel/git/sfr/linux-next.git;a=tags. But
it is indeed recreated daily, i.e. next-N does not include the end
result of next-N-1.]
> mainline tree, and so the date of the merge commits means that you can
> never base anything on top of linux-next. This has always been the
> case, and so trying to impose a straightjacket on all of the sources
> of linux-next doesn't actually buy anything as far as the properties
> of linux-next.
>
> 2) There are many legitimate reasons for "rewinding". In addition to
> being able to add credit for tested-by and acked-by lines, sometimes
Per linux-next submission rules, all /essential/ credits are already
present. But I agree that it is worth rewinding a for-next branch in
order to add (non-essential) credits later. linux-next's exact history
is of interest for days or months at most, while mainline's history is
of interest for many years to come.
> patches are subtle. More than once, patches have been sitting in the
> ext4 tree that have passed the XFSQA test, and thus have been "unit
> tested", but they still have bugs; in some cases, subtle bugs. In
> some cases, bugs that cause data corruption. In the case where the
> patches have hit linux-next, but the merge window hasn't opened yet, I
> prefer to fix the patch by mutating it, and rewinding the ext4 tree,
> instead of adding a fix later. It makes it easier to cherry pick
> patches to the stable tree later, and it keeps the ext4 tree clean,
> and it has no downside in terms of linux-next --- see (1) above.
>
> 3) I don't have the same access to vast amounts of hardware and
> platforms that Ingo does. As a result, while I make a practice of
> testing every single patch against the XFS test suite (yes, it's slow
> and painful, but I think it's worth it; I'm very paranoid about patch
> quality), every once in a while the patch has warnings or doesn't
> compile on some platform for which I don't have build/test machines.
> Today, this gets tested in linux-next, and when it does, if it the
> merge window hasn't opened yet, I will fix it the patch instead of
> creating an extra patch. This helps git bisectability for platforms I
> don't have access to.
We should rely less on linux-next as a cross-compile farm; that's not
its purpose. We can cross-compile ourselves. I think the documentation
and toolchain can be found somewhere. In fact, we are supposed to do so
per item 3 in Documentation/SubmitChecklist. This text was added about
two years before linux-next opened for business.
That said, I admit that I don't test more than x86-64 myself (x86-32
too, but decreasingly frequently now). But that's mostly because I only
deal with code where the danger of architecture-dependent build breakage
is very low. (drivers/ieee1394 is frozen, and drivers/firewire is
small, modern, sparse-clean, and uses clean interfaces to the rest of
the kernel. There is some more danger of /runtime/ regression of these
drivers on other architectures, but those would only be exposed in
mainline or distributions, not in linux-next already.) I guess on the
unlikely day that I get notice of a linux-next build bug due to my tree
on one of those platforms, I will reorder my never ending to-do list and
set up local cross compilation.
--
Stefan Richter
-=====-==--= =-=- ==---
http://arcgraph.de/sr/
Steven Rostedt wrote:
> Some of the reasons for the constant rebasing are:
>
> 1) the patches are held in quilt, which just by nature leads to
> rebasing. These developers find that quilt is the best tool for the
> job.
It shouldn't be difficult to implement a git-quiltimport porcelain which
- remembers a previous import of a quilt tree,
- compares that one ( = an existing branch) with a new quilt import
( = a new branch),
- reuses all commits of the old import which have identical diffs/
changelog/ authorship/ parent changeset as ones in the new import,
- discards (or reverts?) commits of the old import that don't match
the new import in this way,
- finally rebases all remaining commits from the new import onto the
kept old ones.
[OTOH the whole discussion is not about stable SHA1s in linux-next per
se, AFAIU, but about stability of (the history of) the code which is
released into linux-next, or about misuse of linux-next for more than
final integration-testing...?]
--
Stefan Richter
-=====-==--= =-=- ==---
http://arcgraph.de/sr/
>
> We should rely less on linux-next as a cross-compile farm; that's not
> its purpose. We can cross-compile ourselves.
Hmm, last time I tried to build binutils/gcc I only got:
alpha arm sparc64 i386 x86_64 um m68k
working.
I recall that I managed to build sh and avr32 alas no luck.
So please do not expect people that touches code impacting
lots of architectures to provide the same level of coverage
that -next does.
Hay - and sometimes I even fail to buil for all the architectures
I have build tool for.
Sam
> 2) There are many legitimate reasons for "rewinding". In addition to
> being able to add credit for tested-by and acked-by lines, sometimes
> patches are subtle. More than once, patches have been sitting in the
> ext4 tree that have passed the XFSQA test, and thus have been "unit
> tested", but they still have bugs; in some cases, subtle bugs. In
> some cases, bugs that cause data corruption. In the case where the
> patches have hit linux-next, but the merge window hasn't opened yet, I
> prefer to fix the patch by mutating it, and rewinding the ext4 tree,
> instead of adding a fix later. It makes it easier to cherry pick
> patches to the stable tree later, and it keeps the ext4 tree clean,
> and it has no downside in terms of linux-next --- see (1) above.
If the "rewind" is simply to add "signed-off-by" notations, update
commit comments (or code comments) ... then it does seem useful to
keep the commit chain anchored to the original commit, as the testing
that has been done is all still valid.
But as soon as you talk about fixing bugs ... then you ought to
just do a "rebase". The code you are adding has changed, so it is
incorrect to preserve the illusion that these changes have had
extensive testing against the old commit base. The code has changed,
so the testing clock gets reset to zero.
-Tony
On Sun, Oct 25, 2009 at 09:53:41PM -0700, Luck, Tony wrote:
>
> If the "rewind" is simply to add "signed-off-by" notations, update
> commit comments (or code comments) ... then it does seem useful to
> keep the commit chain anchored to the original commit, as the testing
> that has been done is all still valid.
>
> But as soon as you talk about fixing bugs ... then you ought to
> just do a "rebase". The code you are adding has changed, so it is
> incorrect to preserve the illusion that these changes have had
> extensive testing against the old commit base. The code has changed,
> so the testing clock gets reset to zero.
I don't think anyone should (or does?) use the base version of a patch
series as an indication of how much testing a patch series has
received. It doesn't make much sense.
Suppose I update the 40th patch of a 50th patch series to add check
for kmalloc() returning NULL that had been inadvertently left out, or
some other error checking is added. Or suppose I add a new tracepoint
definition to a 50 patch series. Sorry, I'm not going to rewind the
entire patch series because someone thinks the base version of the
patch series somehow is a magic "test clock" indicator....
- Ted
Theodore Tso wrote:
> On Sun, Oct 25, 2009 at 09:53:41PM -0700, Luck, Tony wrote:
>> If the "rewind" is simply to add "signed-off-by" notations, update
>> commit comments (or code comments) ... then it does seem useful to
>> keep the commit chain anchored to the original commit, as the testing
>> that has been done is all still valid.
>>
>> But as soon as you talk about fixing bugs ... then you ought to
>> just do a "rebase". The code you are adding has changed, so it is
>> incorrect to preserve the illusion that these changes have had
>> extensive testing against the old commit base. The code has changed,
>> so the testing clock gets reset to zero.
>
> I don't think anyone should (or does?) use the base version of a patch
> series as an indication of how much testing a patch series has
> received. It doesn't make much sense.
FWIW, I agree to the above. But the below...
> Suppose I update the 40th patch of a 50th patch series to add check
> for kmalloc() returning NULL that had been inadvertently left out, or
> some other error checking is added. Or suppose I add a new tracepoint
> definition to a 50 patch series.
...are bad examples in the context of linux-next, IMO. A missing
allocation failure check or a missing tracepoint don't break
bisectability. So why discard this history? (It was already published
in a release preview.)
> Sorry, I'm not going to rewind the
> entire patch series because someone thinks the base version of the
> patch series somehow is a magic "test clock" indicator....
Indeed. Not even the commit date of individual patches says something
about how extensive they were tested. Besides, testers might never have
tested that particular head; it's more likely that they ran a merge
result which was never published anywhere, or they tested a patch
stand-alone on top of whatever kernel they happened to have at hand
(e.g. a distro kernel) when the patch author asked them to test some fix
or feature.
--
Stefan Richter
-=====-==--= =-=- ==-=-
http://arcgraph.de/sr/
On Fri, 2009-10-23 at 23:59 +0200, Sam Ravnborg wrote:
> On Fri, Oct 23, 2009 at 10:54:00PM +0200, Ingo Molnar wrote:
> >
> > Maintainer trees pushed towards linux-next should strive to be Git
> > based, append-mostly, 'nice', 'intended for upstream' and defendable
> > as-is IMO, and rebasing a _maintainer tree_ should really be a rare act
> > of last resort.
>
> As maintainer I try to put some effort in crediting people
> where credit belongs.
> In other words collecting "Acked-by:", "Tested-by", "Reviewed-by".
>
> Adding this require a rebase as soon as said patch hits git.
I've been saying for a while that git really needs a way to "annotate" a
commit. And have git log show those annotations by default.
Signed-off-by must be in the original commit. But Acked-by, Tested-by
and Reviewed-by almost always come after it hits some git repo.
Thus, if we could add an annotation to a commit later, that would be
very helpful. We could add these other labels on.
For Acked-by, when needed (touching a Maintainers code) I usually send
out an RFC patch set first to get these. But for Reviewed-by and
Tested-by which usually come after I have it in my git tree and I push
it off to Ingo, those come later. And unfortunately, I seldom get to add
those tags.
I think this is more of a failure in git than in the work flow we
present.
-- Steve
From: Steven Rostedt <[email protected]>
Date: Mon, 26 Oct 2009 19:26:28 -0400
> I think this is more of a failure in git than in the work flow we
> present.
Others (like me) will say you should have let that patch cook for a
while on the mailing list or in your tree before publishing, in order
to let those acks and tested-by replies come in.
That's how I handle this.
And I really don't buy the argument that you have to publish the
change in a GIT tree to get those ACKs and tested-by replies.
On Mon, 2009-10-26 at 16:30 -0700, David Miller wrote:
> From: Steven Rostedt <[email protected]>
> Date: Mon, 26 Oct 2009 19:26:28 -0400
>
> > I think this is more of a failure in git than in the work flow we
> > present.
>
> Others (like me) will say you should have let that patch cook for a
> while on the mailing list or in your tree before publishing, in order
> to let those acks and tested-by replies come in.
>
> That's how I handle this.
>
> And I really don't buy the argument that you have to publish the
> change in a GIT tree to get those ACKs and tested-by replies.
The thing is, I do my changes with git. I get something working and then
commit it. Then I do more changes and commit that. I don't use quilt
anymore for this.
Then I have an automated process to send out those patches. Heck pushing
it to git makes it _easier_ to do testing. Instead of having someone
pull out 5 or so patches from LKML, they can just pull my git repo and
start testing it.
Yes, I could let it cook on LKML, but I don't have the test suite that
Ingo has either. I do my own set of tests, then I push it out and let
Ingo run it through his test suite.
Some patches get tested by others, some don't. I don't formally ask
anyone to test them besides Ingo. But it is nice to know that someone
tested it, and sometimes I even get people to review it.
So what's the time to leave it out on LKML before pulling it into git?
(which it is already in git for me anyway). I could wait a week, two?
And after pulling it in after this time, I may still get a late
"tested-by" or "reviewed-by".
I think if people knew that they could test or review patches that are
already in git and get acknowledged for doing so, there may be more
people who will test and review patches. Heck, if someone was board,
they could go and review patches already in Linus's tree. I don't think
it would become a new haven for those that want fame, since placing a
tag on a commit is also placing your reputation there too.
-- Steve
From: Steven Rostedt <[email protected]>
Date: Mon, 26 Oct 2009 19:51:01 -0400
> On Mon, 2009-10-26 at 16:30 -0700, David Miller wrote:
>> From: Steven Rostedt <[email protected]>
>> Date: Mon, 26 Oct 2009 19:26:28 -0400
>>
>> > I think this is more of a failure in git than in the work flow we
>> > present.
>>
>> Others (like me) will say you should have let that patch cook for a
>> while on the mailing list or in your tree before publishing, in order
>> to let those acks and tested-by replies come in.
>>
>> That's how I handle this.
>>
>> And I really don't buy the argument that you have to publish the
>> change in a GIT tree to get those ACKs and tested-by replies.
>
> The thing is, I do my changes with git.
So do I.
> I get something working and then commit it. Then I do more changes
> and commit that. I don't use quilt anymore for this.
And you can do this all day long if you like.
What you can't do is _PUBLISH_ this anywhere to a tree that people
also do development against _UNTIL_ you get those acks and tested-by
tags back from people.
Once your acks etc. come in, you can pop all of those pending patches
out of your tree, add the ack tags to the commit messages, then
reapply them.
Then you can push to your public tree, but no sooner.
It really is that simple.
On Mon, 2009-10-26 at 17:15 -0700, David Miller wrote:
> What you can't do is _PUBLISH_ this anywhere to a tree that people
> also do development against _UNTIL_ you get those acks and tested-by
> tags back from people.
>
> Once your acks etc. come in, you can pop all of those pending patches
> out of your tree, add the ack tags to the commit messages, then
> reapply them.
>
> Then you can push to your public tree, but no sooner.
>
> It really is that simple.
But you never answered my question? How long do you let it cook? I don't
have regular people that test my patches and pass tested by, whatever. I
get them here and there, sometimes I even a reviewed by. Most of the
time I just get silence.
My testing is mostly done in Ingo's test suite, and that happens after I
do my push to him. This works best for me.
Perhaps this doesn't bother your work flow, but it does mine. 90% (or
more) of the bugs in my code is found in Ingo's test suites. This means
I want to get it to him ASAP, and I do so by pushing it to him and
Cc'ing LKML. Then I can work on my next set of patches without worrying
about the last set I sent.
If I had to publish and let cook on LKML, then I would also need to keep
better accounting of what I have pushed out and what I have left to do.
My wife does the bills because I can't do accounting for crap. If I need
to account for patches that have been to LKML, and time them to know
when to push into some tree, I'm just destine to let a few patches slip
through the cracks.
-- Steve
From: Steven Rostedt <[email protected]>
Date: Mon, 26 Oct 2009 20:30:03 -0400
> On Mon, 2009-10-26 at 17:15 -0700, David Miller wrote:
>
>> What you can't do is _PUBLISH_ this anywhere to a tree that people
>> also do development against _UNTIL_ you get those acks and tested-by
>> tags back from people.
>>
>> Once your acks etc. come in, you can pop all of those pending patches
>> out of your tree, add the ack tags to the commit messages, then
>> reapply them.
>>
>> Then you can push to your public tree, but no sooner.
>>
>> It really is that simple.
>
> But you never answered my question? How long do you let it cook? I don't
> have regular people that test my patches and pass tested by, whatever. I
> get them here and there, sometimes I even a reviewed by. Most of the
> time I just get silence.
1 day, maybe 2 or even 3 depending upon the type of change and the
travel schedules of core developers. On the networking lists things
get tested and reviewed quite quickly.
> My testing is mostly done in Ingo's test suite, and that happens after I
> do my push to him. This works best for me.
So you both should keep your trees private until it's all sorted out
and you have the results in hand.
Nobody forces you guys to use public trees just to run Ingo's personal
test suite, you have decided to work that way. And that is therefore
something you guys can change without effecting other people.
> If I had to publish and let cook on LKML, then I would also need to keep
> better accounting of what I have pushed out and what I have left to do.
The problem isn't that you have to push patches out and only work
with patches, the problem is that you want to use publicly visible
GIT trees to do your testing at all times.
And sorry, that is not how you're supposed to do things.
On Mon, 2009-10-26 at 18:34 -0700, David Miller wrote:
> > My testing is mostly done in Ingo's test suite, and that happens after I
> > do my push to him. This works best for me.
>
> So you both should keep your trees private until it's all sorted out
> and you have the results in hand.
>
> Nobody forces you guys to use public trees just to run Ingo's personal
> test suite, you have decided to work that way. And that is therefore
> something you guys can change without effecting other people.
I guess we have a different point of view on this. We like to keep
things public as much as possible. As stated earlier, it's easier to get
people to test the patches if the git trees in question are available.
90% of the bugs are found by Ingo's tests, the other 10% is found by
people pulling one of our git trees and testing themselves.
>
> > If I had to publish and let cook on LKML, then I would also need to keep
> > better accounting of what I have pushed out and what I have left to do.
>
> The problem isn't that you have to push patches out and only work
> with patches, the problem is that you want to use publicly visible
> GIT trees to do your testing at all times.
>
> And sorry, that is not how you're supposed to do things.
That is a matter of opinion. We prefer to keep things as public as
possible. No patches back and forth privately doing our own internal
test suites, then come out with some "production release". If someone
found something wrong with it then, we would need to start the cycle all
over again.
I do not like to send patches to anyone privately, unless it is a hack
that is to help someone debug something and not for inclusion. Any time
I send out a patch to be included for mainline, I start it out public. I
also prefer to have a public git repo available to be tested by anyone.
Whether I'm sending a patch to Ingo, Linus, Andrew, or even you, I would
do it publicly and have a git repo to pull from for simplicity.
-- Steve
On Mon, Oct 26, 2009 at 11:02:38PM -0400, Steven Rostedt wrote:
> > The problem isn't that you have to push patches out and only work
> > with patches, the problem is that you want to use publicly visible
> > GIT trees to do your testing at all times.
> >
> > And sorry, that is not how you're supposed to do things.
>
> That is a matter of opinion. We prefer to keep things as public as
> possible. No patches back and forth privately doing our own internal
> test suites, then come out with some "production release". If someone
> found something wrong with it then, we would need to start the cycle all
> over again.
There's nothing wrong with public branches that happen to be regularly
rewound, and they do exist in nature. Exhibit one: The 'pu' branch in
git. Exihibit two: linux-next.
It's strange to see people arguing for non-transparency, just because
we happen to be using git. Given that linux-mm uses quilt, and
linux-next accepts quilt patches, I really don't see anything wrong
with linux-next taking git branches that are occasionally rewound when
doing things like adding tested-by:, or when I want to clarify or
rewrite rewrite the patch commit description or even in-code comments
into proper English.
Maybe we need better ways of advertising that a particular branch is
unstable, so people can be adequately warned they base work on that
branch at their own risk. But fundamentally, saying that we should
keep git branches sekrit just because they might be rewound doesn't
seem to make sense.
- Ted
From: Steven Rostedt <[email protected]>
Date: Mon, 26 Oct 2009 23:02:38 -0400
> Whether I'm sending a patch to Ingo, Linus, Andrew, or even you, I would
> do it publicly and have a git repo to pull from for simplicity.
Sure, but just don't do it with a GIT repo that's going to
make it's way to Linus.
When you commit to any such GIT repo, you're making a permanent
irrevocable change whose history cannot be changed.
And given what you get in return for that, it's a pretty reasonable
constraint.
* Steven Rostedt <[email protected]> wrote:
> On Fri, 2009-10-23 at 23:59 +0200, Sam Ravnborg wrote:
> > On Fri, Oct 23, 2009 at 10:54:00PM +0200, Ingo Molnar wrote:
> > >
> > > Maintainer trees pushed towards linux-next should strive to be Git
> > > based, append-mostly, 'nice', 'intended for upstream' and
> > > defendable as-is IMO, and rebasing a _maintainer tree_ should
> > > really be a rare act of last resort.
> >
> > As maintainer I try to put some effort in crediting people where
> > credit belongs. In other words collecting "Acked-by:", "Tested-by",
> > "Reviewed-by".
> >
> > Adding this require a rebase as soon as said patch hits git.
>
> I've been saying for a while that git really needs a way to "annotate"
> a commit. And have git log show those annotations by default.
> Signed-off-by must be in the original commit. But Acked-by, Tested-by
> and Reviewed-by almost always come after it hits some git repo.
For any reasonably complex change you simply need to wait a bit anyway
to gather feedback. And for trivial/obvious/small patches it makes
little sense to upset the Git history just to add the tags.
(And if there's frequent problems with small changes that were supposed
to be easy you need to revisit the quality process.)
So no, the rewinding of published for-pull git trees are not fine in
Linux. It's OK to post a declaredly RFC git tree with some
to-be-reworked patches in it - but it's not OK to publish a git tree for
pull and rebase it after that. You'll certainly see me complain if you
do that (and Linus will complain to me if i do that).
Now with tip:tracing/core we sometimes ran into a situaton where we did
rebases to make the tree fast-forward all the time - but that's not
necessary as Linus has made it clear that the occasional merge commit is
not a problem. (if the mechanism to remove them is to rebase) We can
certainly do that to make the Git flow even more append-only.
Ingo
* David Miller <[email protected]> wrote:
> From: Steven Rostedt <[email protected]>
> Date: Mon, 26 Oct 2009 23:02:38 -0400
>
> > Whether I'm sending a patch to Ingo, Linus, Andrew, or even you, I
> > would do it publicly and have a git repo to pull from for
> > simplicity.
>
> Sure, but just don't do it with a GIT repo that's going to make it's
> way to Linus.
>
> When you commit to any such GIT repo, you're making a permanent
> irrevocable change whose history cannot be changed.
>
> And given what you get in return for that, it's a pretty reasonable
> constraint.
Agreed - and the people asking us to rebase trees miss this point,
completely.
The thing is, i've been on both sides of the equation - three years ago
i ran a patch-queue with 1500 patches in it, two years ago i ran a daily
rebasing/rewinding Git tree and later i ran an occasionally-rebasing
flow as well.
By far the best method is to not rebase/rewind maintainer trees.
If Linus can manage without rebasing _ever_, with _his_ rate of 10,000
commits every 3 months, 300+ merges, with up to 50 direct commits
authored by him per cycle then i'm quite sure other maintainers should
be able to do that too. (at minimum the larger ones)
The networking tree has been doing this for a long amount of time - and
all the trees i co-maintain are very close to doing this as well. So if
there's any trend for the trees i'm involved with it's in the direction
of _less_ rebasing/rewinding, not more of it.
Ingo
David Miller <[email protected]> wrote:
> From: Steven Rostedt <[email protected]>
> Date: Mon, 26 Oct 2009 19:51:01 -0400
>> I get something working and then commit it. Then I do more changes
>> and commit that. I don't use quilt anymore for this.
>
> And you can do this all day long if you like.
>
> What you can't do is _PUBLISH_ this anywhere to a tree that people
> also do development against _UNTIL_ you get those acks and tested-by
> tags back from people.
>
> Once your acks etc. come in, you can pop all of those pending patches
> out of your tree, add the ack tags to the commit messages, then
> reapply them.
>
> Then you can push to your public tree, but no sooner.
I use stacked git for my patches and I may rewind the series (fixes
following reviews/testing, acked-by lines etc.) before patches are
pushed into mainline (nothing new here). But to make it easier for
others to test or develop on top of such branch, I added a "stg
publish" command which creates a separate merge-friendly branch that
is never rebased nor rewound (for some implementation details, see
http://procode.org/stgit/doc/stg-publish.html). The top commit sha1
(and history) of the published and series branches differ but they
always have the same tree so that people using the published branch
always get the same source. You even get to add a comment about what
was modified in the series when publishing the change.
There is currently no way to publish commit message changes like
adding Acked-by (unless the "publish" command would do a revert of the
original commit, though not sure it's worth as it makes the history
more unreadable).
For mainline merging you can send a pull request on the series
(rewound) branch once you are happy with it and don't foresee further
changes (when rebasing a series on the latest mainline, the "publish"
command generates a "merge" with mainline so that people pulling both
the published branch and mainline don't need to resolve the possible
conflicts with your series).
A similar script could be easily done for plain git (Stefan Richter
mentioned something like this as well).
--
Catalin
On Tue, 2009-10-27 at 08:59 +0100, Ingo Molnar wrote:
> For any reasonably complex change you simply need to wait a bit anyway
> to gather feedback. And for trivial/obvious/small patches it makes
> little sense to upset the Git history just to add the tags.
>
> (And if there's frequent problems with small changes that were supposed
> to be easy you need to revisit the quality process.)
Sure, but I'm not talking about changing the patch, I'm talking about a
late "reviewed by" or "tested-by". These usually do come after a patch
set has been moved into the final git push. It would be nice to flag a
commit that it was tested by someone.
If we have to change the commit to add tested-by, then it changes the
SHA1, and if you're going to change the commit, you may be tempted to
make a small fix somewhere too. And that small fix might have an
unexpected result and nullifies the "tested-by".
Sure you can say, "don't do that" but its human nature. Someone will,
and when this patch is proven to break something, it will be
embarrassing to the tester who put their name on it.
>
> So no, the rewinding of published for-pull git trees are not fine in
> Linux. It's OK to post a declaredly RFC git tree with some
> to-be-reworked patches in it - but it's not OK to publish a git tree for
> pull and rebase it after that. You'll certainly see me complain if you
> do that (and Linus will complain to me if i do that).
I agree with this to. And for complex changes, I usually do a RFC post
first. But after comments and we get a good commit going, then I
sometimes get "tested-by" after the final version has been posted.
Because they tested that version.
My suggestion is to add an "annotation" commit that can tag another
commit with something like a "tested-by". This way a tester can say,
they tested this particular commit, and if someone changes it, the SHA1
will no longer match. And then we can also have a record of what commit
was tested by who, after the fact that it went into git.
A tester could have a git tree that you could pull from that only adds
annotations of what they tested. This would add to git a history of what
changes were tested by who.
Just my $0.02.
-- Steve
On Tue, 27 Oct 2009, Steven Rostedt wrote:
> Sure, but I'm not talking about changing the patch, I'm talking about a
> late "reviewed by" or "tested-by". These usually do come after a patch
> set has been moved into the final git push. It would be nice to flag a
> commit that it was tested by someone.
>
[...]
>
> My suggestion is to add an "annotation" commit that can tag another
> commit with something like a "tested-by". This way a tester can say,
> they tested this particular commit, and if someone changes it, the SHA1
> will no longer match. And then we can also have a record of what commit
> was tested by who, after the fact that it went into git.
There is a "git notes" feature from Johan Herland being carried in thepu
branch of the git repository. I strongly suggest to those who might
think this could be useful in this context to express their interest by
posting to [email protected]. With a concrete need and usage scenario
(and offer to help at least with testing) this feature might become
usable sooner.
Nicolas
On Mon, Oct 26, 2009 at 07:01:57PM +0100, Stefan Richter wrote:
> > Suppose I update the 40th patch of a 50th patch series to add check
> > for kmalloc() returning NULL that had been inadvertently left out, or
> > some other error checking is added. Or suppose I add a new tracepoint
> > definition to a 50 patch series.
>
> ...are bad examples in the context of linux-next, IMO. A missing
> allocation failure check or a missing tracepoint don't break
> bisectability. So why discard this history? (It was already published
> in a release preview.)
There are multiple issues for rewinding patches. One is to avoid
breaking bisectability. Other is to keep related changes in
functionality in a single place. 2-3 years for now, does anyone
really care about retaining development history? In the human memory,
one of the most important parts of long-term memory formation is
*forgetting*; that is, editing down everything that happened down to
the most cogent and importants bits of history. This is what is
disrupted when people don't get enough sleep.
Similarly, there is absolutely no point in preserving the v1, v2, v3,
v4... versions of patches that appeared in LKML in Linus's git tree
--- surely people agree that's true? And if something is being
maintained in quilt, and there are v1, v2, v3, v4 versions of a patch,
there's no reason why we should put it in git, right? So if it's in a
rewinding git branch, such as what happens in the pu branch in git
development, the history isn't preserved either ---- and that's O.K.
- Ted
On 10/27/2009 7:07 PM, Theodore Tso wrote:
> On Mon, Oct 26, 2009 at 07:01:57PM +0100, Stefan Richter wrote:
>> A missing
>> allocation failure check or a missing tracepoint don't break
>> bisectability. So why discard this history? (It was already published
>> in a release preview.)
>
> There are multiple issues for rewinding patches. One is to avoid
> breaking bisectability. Other is to keep related changes in
> functionality in a single place. 2-3 years for now, does anyone
> really care about retaining development history? In the human memory,
> one of the most important parts of long-term memory formation is
> *forgetting*; that is, editing down everything that happened down to
> the most cogent and importants bits of history.
[...]
Sure. But when is the deadline for doing this? The evening before you
send a pull request to Linus? Or already when you commit to the branch
from which Stephen pulls into linux-next, which means: This is code and
history which I would ask Linus to pull right now if he was in merge
mode today.
[I for one would indeed add non-essential credits or otherwise touch up
the history even if it already was in linux-next but Linus' next merge
window isn't there yet. But I would batch such rewinds up for a single
occasion between merge windows, and only do them at all if there are
several such changes to make it worthwhile. And I only do that because
I work at driver code with limited interaction with other subsystems,
and virtually no co-developers in the project.]
--
Stefan Richter
-=====-==--= =-=- ==-==
http://arcgraph.de/sr/
On Sat, 24 Oct 2009, Stefan Richter wrote:
>
> Per linux-next submission rules, all /essential/ credits are already
> present. But I agree that it is worth rewinding a for-next branch in
> order to add (non-essential) credits later.
I'd actually personally prefer that people do _not_ generally add
"credits" later.
Quite frankly, if something was committed without having been ack'ed by
some person, then later - when that person sees it on a commit list, for
example - it's worthless adding somebodys late ack.
Same largely goes for 'tested-by' lines.
And 'signed-off-by' are actively _wrong_ to add later. If it didn't come
with a sign-off in the first place, it shouldn't have been committed.
One reason I say this is that I really think it's wrong to even give
credit to some late-comer that pipes in after the patch has already made
it into somebody's tree. If they didn't comment on it while it was passed
around as a patch on mailing lists, what's the point? By the time it's in
somebody elses published tree, any "ack" is worthless, and that person
should simply _not_ get credit for being late to the party.
So I think that there are many good reasons to rebase patches in your own
tree, but I do _not_ think that "adding late acks" is one such reason. If
you've published your tree to others, then it's done. Don't lie about
getting an ack that you didn't get before you made that patch public.
Linus
On Tue, 2009-10-27 at 12:06 -0700, Linus Torvalds wrote:
> Don't lie about
> getting an ack that you didn't get before you made that patch public.
But how do you get your ack without making it public?
But I do agree Signed-off-by is a must for every commit (if one is
missing, something is horribly broken). When I need an Acked-by
(touching a maintainer's code) I post an RFC branch (not to be pulled).
But for Tested-by, it is much easier for a tester to pull and test some
git branch than it is to pull patches from LKML.
And as we know, the easier it is to test, the more testers we have.
-- Steve
On Tue, 27 Oct 2009 15:27:04 EDT, Steven Rostedt said:
> On Tue, 2009-10-27 at 12:06 -0700, Linus Torvalds wrote:
> > Don't lie about
> > getting an ack that you didn't get before you made that patch public.
>
> But how do you get your ack without making it public?
I'm going to guess that Linus meant "causing the patch to show up in a
public git tree" - i.e. shop the patch around on lkml and similar lists a
bit and get some acked-by's to attach to it *before* it goes out in a tree
that everybody might pull from.
On Tue, 27 Oct 2009, Steven Rostedt wrote:
>
> On Tue, 2009-10-27 at 12:06 -0700, Linus Torvalds wrote:
> > Don't lie about
> > getting an ack that you didn't get before you made that patch public.
>
> But how do you get your ack without making it public?
There's a difference between exposing your git tree to the public, and
showing your patches to others.
A public git tree is _not_ the place to ask for comments for patches. If
you haven't gotten Ack's, you have two choices:
- that commit should not show up in a tree that is marked for -next,
because it's still waiting for feedback and people to test it. It may
be in a _private_ git tree of course, but it's not "ready" yet.
- you are going to commit it regardless of acks or not, and you don't
care, and you shouldn't lie about it later.
Those are the two obvious choices. Adding ack's later, after it has
already been exported and tested in -next is kind of pointless, isn't it?
It's basically lying about how the patch came to be.
If you want Ack's from people, then send that patch around by email.
NOTE! If you know the people you want acks from are git users, then the
email can certainly be something like "look at so-and-so branch of my git
tree <here>". You can certainly use private git branches as a way to talk
to other developers about code you're not fully happy with yet, with the
clear understanding that you want acks and comments on it.
Put another way: there's a big conceptual difference between "public git
tree" and "private git branch that may well be available to others and
is meant for development". In git itself, Junio has the 'pu' branch that
is clearly marked as being rebased etc.
Linus
On Tue, 2009-10-27 at 15:39 -0400, [email protected] wrote:
> On Tue, 27 Oct 2009 15:27:04 EDT, Steven Rostedt said:
> > On Tue, 2009-10-27 at 12:06 -0700, Linus Torvalds wrote:
> > > Don't lie about
> > > getting an ack that you didn't get before you made that patch public.
> >
> > But how do you get your ack without making it public?
>
> I'm going to guess that Linus meant "causing the patch to show up in a
> public git tree" - i.e. shop the patch around on lkml and similar lists a
> bit and get some acked-by's to attach to it *before* it goes out in a tree
> that everybody might pull from.
Yeah, I was being a little facetious there. The rest of my email I was
being serious.
-- Steve
* Steven Rostedt <[email protected]> wrote:
> On Tue, 2009-10-27 at 08:59 +0100, Ingo Molnar wrote:
>
> > For any reasonably complex change you simply need to wait a bit
> > anyway to gather feedback. And for trivial/obvious/small patches it
> > makes little sense to upset the Git history just to add the tags.
> >
> > (And if there's frequent problems with small changes that were
> > supposed to be easy you need to revisit the quality process.)
>
> Sure, but I'm not talking about changing the patch, I'm talking about
> a late "reviewed by" or "tested-by". These usually do come after a
> patch set has been moved into the final git push. It would be nice to
> flag a commit that it was tested by someone.
If you want to make your patches dependent on acks then you first need
to send out those patches with an '[RFC]' added, and put into a branch
hierarchy (rfc/* would be fine) that i sure wont pull from you.
You can also send obvious/small/own patches out for immediate pull. In
that case extra review can still be done after you sent patches out.
Mistakes noticed during review will be credited in terms of future
Reported-by tags.
People do this frequently.
Ingo
* Theodore Tso <[email protected]> wrote:
> On Mon, Oct 26, 2009 at 07:01:57PM +0100, Stefan Richter wrote:
> > > Suppose I update the 40th patch of a 50th patch series to add check
> > > for kmalloc() returning NULL that had been inadvertently left out, or
> > > some other error checking is added. Or suppose I add a new tracepoint
> > > definition to a 50 patch series.
> >
> > ...are bad examples in the context of linux-next, IMO. A missing
> > allocation failure check or a missing tracepoint don't break
> > bisectability. So why discard this history? (It was already published
> > in a release preview.)
>
> There are multiple issues for rewinding patches. One is to avoid
> breaking bisectability. Other is to keep related changes in
> functionality in a single place. 2-3 years for now, does anyone
> really care about retaining development history? In the human memory,
> one of the most important parts of long-term memory formation is
> *forgetting*; that is, editing down everything that happened down to
> the most cogent and importants bits of history. This is what is
> disrupted when people don't get enough sleep.
Fact is that the overwhelming majority of git-rebase uses i've seen were
not done for ack-adding purposes.
They were used to prettify up a tree shortly before it's pushed
upstream, and as a happy side-effect to whitewash out the more
embarrassing bits of the history via backmerges. It presents a rosy
picture about problem-free development and a perfectly going, fluid
workflow with perfect foresight, good testing and few mistakes.
A real git tree will contain fixes for brown paperbag bugs, it will
contain reverts, it will contain the occasional messy changelog. It is
also, because it's more real life, far more trustable to pull from. The
thing is, nothing improves a workflow more than public embarrassment -
but rebasing takes away much of that public embarrassment factor.
Also, i dont buy the bisectability argument either. I bisect all the
time. The number of bisections i've done on the kernel in the past two
years are in the hundreds. Still i dont see any actual, frequent
bisectability problems. (and if i see them i do raise it on lkml)
How come i dont see frequent bisection problems, despite the majority of
trees and commits (well in excess of 50%) being maintained in
append-only mode? By your argument i should be seeing an increasing
amount of bisection problems. In fact i see the opposite: if i see some
bad bisection bug it is often _due_ to a rebase - a patch was reordered
without real testing being injected into the new tree, breaking hundreds
(sometimes thousands) of commits.
So to sum it up - i am sure that there are people who can run nice trees
responsibly, using just about _any_ source code management tool, be that
Quilt or stgit. Heck i think Andrew could maintain -mm by scribbling
patches on tree bark and sending them to Linus via swarms of pigeons.
But the fundamental fact remains, from all the workflows i tried (and i
ran 1000+ patches quilt trees just a few years ago), Git append-only
workflows are the most gradual, most resilient, most scalable, most
symmetric and hence most trustable source of changes to me.
So it's no wonder Linus mandates all big Git trees to use an append-only
workflow.
Ingo
On Tue, 2009-10-27 at 13:18 -0400, Nicolas Pitre wrote:
> On Tue, 27 Oct 2009, Steven Rostedt wrote:
>
> > Sure, but I'm not talking about changing the patch, I'm talking about a
> > late "reviewed by" or "tested-by". These usually do come after a patch
> > set has been moved into the final git push. It would be nice to flag a
> > commit that it was tested by someone.
> >
> [...]
> >
> > My suggestion is to add an "annotation" commit that can tag another
> > commit with something like a "tested-by". This way a tester can say,
> > they tested this particular commit, and if someone changes it, the SHA1
> > will no longer match. And then we can also have a record of what commit
> > was tested by who, after the fact that it went into git.
>
> There is a "git notes" feature from Johan Herland being carried in thepu
> branch of the git repository. I strongly suggest to those who might
> think this could be useful in this context to express their interest by
> posting to [email protected]. With a concrete need and usage scenario
> (and offer to help at least with testing) this feature might become
> usable sooner.
>
Thanks for the pointer Nicolas, but it looks like my ideas have already
been NAK'd by Linus.
Oh well,
-- Steve