[pkg-cryptsetup-devel] Bug#585496: Bug#585496: Bug#585496: Bug#585496: cryptsetup: replace check scripts by new versions

[Jonas Meurer]

hey christoph,

On 16/06/2010 Christoph Anton Mitterer wrote:
> On Wed, 2010-06-16 at 15:50 +0200, Jonas Meurer wrote:
> > thanks, i've applied some changes to the check scripts in svn trunk,
> > please take a look. unfortunately (as written in the previous mail),
> > your scripts are cluttered. these scripts really should be as simple as
> > possible, and the code in most cases is obvious enough that
> > documentation isn't required.
>
> Wow it's really strange how you refuse to simply use
> well-documented/tested code as is... especially for such a security
> sensitive package where strict documentation should be considered to be
> a must, and where uncertainties and doubts are killers.

sorry, i really don't want to offend you. i see the point that the
constant rejections of your patches seems unpolite to you, but i try to
make my decisions transparent ...

even more, i agree with you that documentation is good to have, and in
fact i do agree with a lot of your suggestions as well, but that's not
the point. in fact my objections are mostly not about content but about
form and style.
this might sound ridiculous to you, but it isn't.

> Especially as really no one is harmed by such documentation,...

i disagree here. good documentation at the right place is a key feature,
especially for security software. i agree with you on that. but simple
scripts, cluttered with tons of comment lines, which expatiate obvious
things, repeat information which have been documented at other places,
and last but not least make it hard to survey the script, don't help
anybody.

> > all of these are applied, take a look at the svn trunk.
>
> well...
> - You still use sed for no reason I can see

fair point, fixed that one.

> - Not sure whether you check e.g. for the availability of blkid which is
> nearly guaranteed to be there, but not for whether the device exists is
> readable and a block device.

you don't need to check for the device, as this is already done in
cryptdisks.

> > > Other changes I've made:
> > > - No longer check for the existence of blkid (this should be always there as util-linux
> > >   is essential/required).
> > kept this one.
> Is there a special reason? If not I don't see how this fits your usual
> arguments when rejecting my code, about dropping unnecessary
> documentation and/or safety-checks.

yes, this is a check not done in cryptdisks, so you cannot rely on blkid
being available. one could argue that this check is useless as
util-linux is essential, but checking for the binaries is  a good thing
to do in scripts.

> > checks aren't supported in cryptroot at all so far. feel free to provide
> > a patch,
> Well.... I guess it's not to difficult to do this for you :),... any I
> guess my patches would be rejected anyway (and I do currently not
> consider to change my coding style ;) )

you're right, it's not that hard. but nobody asked for it yet, and i
didn't get the impression that you intend to use that feature. you just
want it there for the sake of "completeness". and i'm already rather
busy at the moment. in fact maintaining cryptsetup is already very
time-consuming for me.

... and to be honest, the mail conversations with you keep me away from
fixing other bugs, which are much more important in my eyes. but we all
know this phenomenon called "procrastination" :-)

> With respect to my replacements for the check scripts:
> > given that it is kept simple, small
> I guess they were even simpler as the previous versions, not only
> regarding the stuff I've dropped (sed/dd/etc), but also regarding how to
> read the code e.g. splitted up the "compelex" boolen expressions in the
> if-statements to simpler expressions with a nested if-statement.
> Apart from that, the code was basically identical.
> 
> 
> > and uses common coding
> > style.
> Uhm,.. that's the coding and documentation style used throughout many
> manpages, all POSIX,... so it's not that uncommon, is it?

see above.

> > vol_id check scripts don't need to be
> > depreciated. either one still has vol_id from udev available, or one
> > doesn't.
> Don't understand this... I thought vol_id was completely dropped from
> udev, thus it should be generally not available starting with everything
> post lenny?!

yes, and as you might have seen in svn, i completely removed the *vol_id
check scripts from the package.

after all, i really appreciate your dedication and the intension to help
me with the cryptsetup packages. but if you really want the packages to
become better, you should pick some of the longstanding open bugs and
try to reproduce and debug them, rather than starting long discussions
about the style of keyscripts.

i have to keep track of the whole package, and that's far more than
keyscripts and checkscripts. in fact, bugs like #514367, #521761,
#546610, #554506 and #555676 are far more important to be fixed.

greetings,
 jonas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-cryptsetup-devel/attachments/20100616/1568ed84/attachment.pgp>