Re: Sendmail update

[Greg Troxel <gdt%lexort.com@localhost>]

Hauke Fath <hauke%Espresso.Rhein-Neckar.DE@localhost> writes:

> I have checked in a wip/sendmail818 package for sendmail 8.18.0.2, 
> which mitigates the recent MTA SMTP smuggling issue.
>
> If you have an interest in sendmail, please have a look at the package 
> - as mentioned, pkglint is borken for me.

It would be great if others tested or reviewed.   I can tell that nobody
has tested because it doesn't build :-(

I had a look, even though I don't use sendmail.

I understand the basic point - update to get a security bug fix, but
especially since we'll be very likely doing this update without hearing
from $MAINTAINER, I'd like to have a little more clarity.  Thus these
comments/requests are likely pickier than usual.

I observe that once this is in wip those worried can run it.  So it is
urgent to fix, but I lean to as fast as is possible to so safely, vs
faster.                                                            

1) The patch files are all renamed.  While I get the motivation, I'm
uncomfortable folding that rototilling into an update, instead of it
being a rototill-only update.  However I am not quite sure it is
reasonable to insist on you unwinding it.  I am guessing it is probably
a fairly quick script to find the name and git mv back but I haven't
tried to write it.

2) I'd like to see COMMIT_MSG in wip with that the message will be.
Specifically including:

  a) What is the status/release type of 18.18.0.2?  Is it a regular
  release, being 8.18 vs 8.17?  Somethign else?  Has upstream said "this
  is a releas and all users should upgrade"?  Something else?

  b) If you don't fix the patch renaming, an explanation of that patches
  were preserved.  Either way, comments explaining which were dropped
  and what changed, other than defuzzing.   With the renaming, it's
  hard to understand what is changing.

  c) An explanation of the testing that has happened.  I would expect
  that you 've built the package and installed it on a machine with real
  use and watched the logs but the commit message doesn't say that.

3) There's a lot of commenting out blocks and then adding new blocks
slightly different.  I find this needlessly hard to read in diffs and
the package shouldn't end up this way Please make the file just change
what has to be changed, tending to minimal diffs when that's reasonable.
E.g.

  -.include "../../mail/sendmail/Makefile.common"
  +#.include "../../mail/sendmail/Makefile.common"
  +.include "../../hauke/sendmail8_18/Makefile.common"

should just be

  -.include "../../mail/sendmail/Makefile.common"
  +.include "../../wip/sendmail8_18/Makefile.common"

and this particular one undone on commit.  (As it is, the package does
not build.)  Another example is

  -DISTINFO_FILE= ${.CURDIR}/../../mail/sendmail/distinfo
  -FILESDIR=      ${.CURDIR}/../../mail/sendmail/files
  -PATCHDIR=      ${.CURDIR}/../../mail/sendmail/patches
  +#DISTINFO_FILE=        ${.CURDIR}/../../mail/sendmail/distinfo
  +#FILESDIR=     ${.CURDIR}/../../mail/sendmail/files
  +#PATCHDIR=     ${.CURDIR}/../../mail/sendmail/patches
  +DISTINFO_FILE= ${.CURDIR}/../../wip/sendmail818/distinfo
  +FILESDIR=      ${.CURDIR}/../../wip/sendmail818/files
  +PATCHDIR=      ${.CURDIR}/../../wip/sendmail818/patches

4) I see there is a new MASTER_SITES (because appparently this isn't a
release and I don't have the answer to 2a), and if so please just add it
and don't turn off the rest.  I am expecting a get-well plan from
upstrem and for us to align for it.

5) I changed OWNER to MAINTAINER and would prefer to have MAINTAINER
resets be done by the named person or pmc, so please resolve the merge
conflict to leave it.



Thanks,
Greg