Dearest HCH,
Please consider adhering to a quality control process. No patch comes
out of Namesys without a second person testing it (that includes
compiling it). Users should not be burdened with code that has not been
tested by a second person. Everyone makes mistakes of this kind, the
difference is that some persons use a quality control process to avoid
burdening more than one other person with them.
Hans
Jeff Mahoney wrote:
> Hans Reiser wrote:
>
> >http://bugzilla.kernel.org/show_bug.cgi?id=6016
>
> > Summary: reiserfs doesn't build with REISERFS_FS_POSIX_ACL=n
> > Kernel Version: v2.6.16-rc2-g5b7b644
> > Status: NEW
> > Severity: normal
> > Owner: [email protected]
> > Submitter: [email protected]
>
>
> >Most recent kernel where this bug did not occur: 2.6.16-rc1
> >Distribution: Gentoo
> >Hardware Environment: amd64
> >Problem Description:
>
> >reiserfs doesn't build with REISERFS_FS_POSIX_ACL=n
>
> >Steps to reproduce:
> >$ make
> > CHK include/linux/version.h
> > SPLIT include/linux/autoconf.h -> include/config/*
> > CHK include/linux/compile.h
> > CHK usr/initramfs_list
> > GZIP kernel/config_data.gz
> > IKCFG kernel/config_data.h
> > CC kernel/configs.o
> > LD kernel/built-in.o
> > CC fs/reiserfs/xattr.o
> >fs/reiserfs/xattr.c: In function `reiserfs_check_acl':
> >fs/reiserfs/xattr.c:1330: error: called object is not a function
> >make[2]: *** [fs/reiserfs/xattr.o] Error 1
> >make[1]: *** [fs/reiserfs] Error 2
> >make: *** [fs] Error 2
>
> >Reverting ec191574b9c3cb7bfb95e4f803b63f7c8dc52690
> >---
> >[PATCH] reiserfs: use generic_permission
>
> >Use the generic_permission code with a proper wrapper and callback
> instead
> >of having a local copy.
> >---
> >fixes the problem, but causes some warnings about unused symbols.
>
>
> This was a patch from hch, not me. There's already a patch in -mm to
> fix it.
>
> -Jeff
>
> --
> Jeff Mahoney
> SUSE Labs
On Feb 05, 2006, at 22:15, Hans Reiser wrote:
> Jeff Mahoney wrote:
>> Hans Reiser wrote:
>>> http://bugzilla.kernel.org/show_bug.cgi?id=6016
>>> Summary: reiserfs doesn't build with
>>> REISERFS_FS_POSIX_ACL=n
>>> Kernel Version: v2.6.16-rc2-g5b7b644
>>
>> This was a patch from hch, not me. There's already a patch in -mm to
>> fix it.
>
> Please consider adhering to a quality control process.
It's a GIT version of an RC patch for grief's sake! You don't
seriously expect people to quadruple-check every trivial patch that
goes into Linus GIT tree before sending it, do you? The whole point
of the RC is to indicate that only smaller patches should be applied
(and this one was for the most part) so that we can do some kind of
global-kernel QC.
> Everyone makes mistakes of this kind, the difference is that some
> persons use a quality control process to avoid burdening more than
> one other person with them.
Precisely! The guy who reported the bug is the one person who was
burdened with it. It will get fixed in the GIT tree, and only
somebody who happened to test the dev tree between the two patches
with that particular .config will have noticed.
BTW: Nice way to CC a private thread to a public list without the
consent of all parties. You also made the grievous errors of (A) top-
posting, (B) fullquoting without snipping irrelevant material, and
(C) sending flamebait to the list (which I am so stupidly responding
to).
Cheers,
Kyle Moffett
--
Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are, by
definition, not smart enough to debug it.
-- Brian Kernighan
On Sun, Feb 05 2006, Kyle Moffett wrote:
> On Feb 05, 2006, at 22:15, Hans Reiser wrote:
> >Jeff Mahoney wrote:
> >>Hans Reiser wrote:
> >>>http://bugzilla.kernel.org/show_bug.cgi?id=6016
> >>> Summary: reiserfs doesn't build with
> >>>REISERFS_FS_POSIX_ACL=n
> >>> Kernel Version: v2.6.16-rc2-g5b7b644
> >>
> >>This was a patch from hch, not me. There's already a patch in -mm to
> >>fix it.
> >
> >Please consider adhering to a quality control process.
>
> It's a GIT version of an RC patch for grief's sake! You don't
> seriously expect people to quadruple-check every trivial patch that
> goes into Linus GIT tree before sending it, do you? The whole point
> of the RC is to indicate that only smaller patches should be applied
> (and this one was for the most part) so that we can do some kind of
> global-kernel QC.
Eh, but you are expected to do that. If everybody sent in half-assed not
tested patches "just because" it's a pre-rc, things would look bad.
Compile testing is the least time consuming and easiest thing to to
test, so you should at least do that. If nobody did that, no one would
get snapshots tested because they would never compile for anyone.
For developers it's equally annoying. I typically update my tree every
morning and run with that for the various stuff I'm working on, and it
is really annoying to have to spent time on hunting down and fixing
compile errors.
This mail is about the mentality displayed, not the specific change that
spawned it.
--
Jens Axboe
> BTW: Nice way to CC a private thread to a public list without the
> consent of all parties. You also made the grievous errors of (A) top-
> posting, (B) fullquoting without snipping irrelevant material, and
> (C) sending flamebait to the list (which I am so stupidly responding
> to).
... which is a nice co-incidence in a thread on quality control :)))
Martin
Kyle Moffett <[email protected]> writes:
>
> It's a GIT version of an RC patch for grief's sake! You don't
> seriously expect people to quadruple-check every trivial patch that
> goes into Linus GIT tree before sending it, do you?
No quadruple check, but every patch going to Linus should get at least
some basic testing and it's definitely suppose to compile at least
in one .config combination.
> The whole point
> of the RC is to indicate that only smaller patches should be applied
> (and this one was for the most part) so that we can do some kind of
> global-kernel QC.
global kernel QA doesn't replace individual patch QA. Black box
testing is no replacement for targetted tests.
-Andi
On Feb 06, 2006, at 06:09, Andi Kleen wrote:
> Kyle Moffett <[email protected]> writes:
>> It's a GIT version of an RC patch for grief's sake! You don't
>> seriously expect people to quadruple-check every trivial patch
>> that goes into Linus GIT tree before sending it, do you?
>
> No quadruple check, but every patch going to Linus should get at
> least some basic testing and it's definitely suppose to compile at
> least in one .config combination.
Well, yes, and it did. The problem was that if you turned off ACLs,
it didn't work; only one or two variants of about 6 or 8 ways to
configure reiserfs stopped working. Given that, I can't see how Hans
is complaining about lack of QC. Nobody is going to test patches
against every possible kernel configuration; that's why we do an RC,
so that we can get a lot of different configs tested.
Cheers,
Kyle Moffett
--
I didn't say it would work as a defense, just that they can spin that
out for years in court if it came to it.
-- Rob Landley
On Mon, Feb 06 2006, Kyle Moffett wrote:
> On Feb 06, 2006, at 06:09, Andi Kleen wrote:
> >Kyle Moffett <[email protected]> writes:
> >>It's a GIT version of an RC patch for grief's sake! You don't
> >>seriously expect people to quadruple-check every trivial patch
> >>that goes into Linus GIT tree before sending it, do you?
> >
> >No quadruple check, but every patch going to Linus should get at
> >least some basic testing and it's definitely suppose to compile at
> >least in one .config combination.
>
> Well, yes, and it did. The problem was that if you turned off ACLs,
> it didn't work; only one or two variants of about 6 or 8 ways to
> configure reiserfs stopped working. Given that, I can't see how Hans
Look, it's really simple: lets say I make a change that has to do with
PM, you do a quick compile test with and _without_ PM just to check you
didn't screw anything up with that change. You change reiserfs acl
stuff, you do a quick compile test with and without that configured.
It's a pretty standard procedure, and contrary to what you think, it
_is_ required before submitting a patch. No one is asking anyone to
check all possible configure options, but the interesting data set is
typically extremely easy to guess looking at a change.
--
Jens Axboe
On Mon, Feb 06, Andi Kleen wrote:
> Kyle Moffett <[email protected]> writes:
> >
> > It's a GIT version of an RC patch for grief's sake! You don't
> > seriously expect people to quadruple-check every trivial patch that
> > goes into Linus GIT tree before sending it, do you?
>
> No quadruple check, but every patch going to Linus should get at least
> some basic testing and it's definitely suppose to compile at least
> in one .config combination.
Right. We have now git-bisect, and it helped me to nail down a few bugs.
Just now I track down some scsi or whatever breakage in -rc1. And guess
what, not a single compile error so far, with a full featured config!
So you guys better send tested patches, via akpm, to keep Linus tree in
a reasonable shape.
--
short story of a lazy sysadmin:
alias appserv=wotan
Jens Axboe <[email protected]> wrote:
>
> Look, it's really simple: lets say I make a change that has to do with
> PM, you do a quick compile test with and _without_ PM just to check you
> didn't screw anything up with that change. You change reiserfs acl
> stuff, you do a quick compile test with and without that configured.
>
> It's a pretty standard procedure, and contrary to what you think, it
> _is_ required before submitting a patch. No one is asking anyone to
> check all possible configure options, but the interesting data set is
> typically extremely easy to guess looking at a change.
<rofl>
bix:/usr/src/op> find patches -name '*build-fix*' | wc -l
533
bix:/usr/src/op> find patches -name '*fix.patch' | wc -l
5109
A lot of people don't make the slightest effort. But it's not a big
problem, really. Silly build errors are reported early and are almost
always trivial to fix. The major drawback is that they can wreck a -mm
release for many testers.
On Tue, Feb 07 2006, Andrew Morton wrote:
> Jens Axboe <[email protected]> wrote:
> >
> > Look, it's really simple: lets say I make a change that has to do with
> > PM, you do a quick compile test with and _without_ PM just to check you
> > didn't screw anything up with that change. You change reiserfs acl
> > stuff, you do a quick compile test with and without that configured.
> >
> > It's a pretty standard procedure, and contrary to what you think, it
> > _is_ required before submitting a patch. No one is asking anyone to
> > check all possible configure options, but the interesting data set is
> > typically extremely easy to guess looking at a change.
>
> <rofl>
>
> bix:/usr/src/op> find patches -name '*build-fix*' | wc -l
> 533
>
> bix:/usr/src/op> find patches -name '*fix.patch' | wc -l
> 5109
>
> A lot of people don't make the slightest effort. But it's not a big
> problem, really. Silly build errors are reported early and are almost
> always trivial to fix. The major drawback is that they can wreck a -mm
> release for many testers.
That's precisely the problem, it may be really simple to fix but often
will stop people from testing.
Your fix count probably isn't totally accurate either, I bet a lot of
these are fixups due to conflicts with other patches. I'm talking about
the fact that someone sends Linus a patch which doesn't compile for the
case you could (and should) have trivially checked. A little edumacation
never hurt :-)
--
Jens Axboe