Page MenuHomeFreeBSD

mail/opensmtpd: Update to 5.9.2
ClosedPublic

Authored by brnrd on May 17 2016, 1:42 PM.
Tags
None
Referenced Files
F128757789: D6421.id16633.diff
Sun, Sep 14, 3:24 AM
F128757274: D6421.id16874.diff
Sun, Sep 14, 3:16 AM
Unknown Object (File)
Thu, Sep 11, 9:07 AM
Unknown Object (File)
Thu, Sep 11, 8:15 AM
Unknown Object (File)
Wed, Sep 10, 10:04 PM
Unknown Object (File)
Mon, Sep 8, 6:13 AM
Unknown Object (File)
Wed, Sep 3, 10:36 PM
Unknown Object (File)
Wed, Sep 3, 6:59 AM
Subscribers

Details

Summary

Proposed commit log:

mail/opensmtpd: Update to 5.9.2

  - Update to 5.9.2p1
  - Remove usernamelen patch (default 255+1)
  - Fix renamed configure args
  - Add symlink for makemap (now included in smtpctl)
  - Align versioning with other OpenBSD projects
  - Add _smtpq group
  - Add UPDATING entry for existing installs
  - Update pkg-message for existing installs

Reviewed_by:	ashish (maintainer)
Approved by:	(maintainer)
Differential_Revision: D6421
Test Plan
  • poudriere testport

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 3785
Build 3828: arc lint + arc unit

Event Timeline

brnrd retitled this revision from to mail/opensmtpd: Update to 5.9.2.
brnrd updated this object.
brnrd edited the test plan for this revision. (Show Details)
brnrd added a reviewer: ashish.
mail/opensmtpd/Makefile
5–6

If p1 means pre-release, and 5.9.2 will follow, you need to set DISTVERSION=5.9.2p1, it will make PORTVERSION=5.9.2.p1

$ pkg version -t 5.9.2.p1 5.9.2
<

If p1 means "patch 1" meaning it is after 5.9.2, then you should set PORTVERSION=5.9.2p1:

$ pkg version -t 5.9.2 5.9.2p1
<
mail/opensmtpd/Makefile
5–6

It's the second one, Bernard.

62

Shouldn't that be

... && empty(PORT:OPTIONS:MSSL_PORT)

Align versioning with standard

  • As per security/openssh
mail/opensmtpd/Makefile
62

If it works with make on 9 & 10, it's ok :-) (I never remember which form works)

brnrd marked 3 inline comments as done.

Fix OpenSSL option

  • No need to check SSL_PORT option when < 10
  • Import of 1.0.1 is version 1000015
  • Move SSL_PORT check to SSL_PORT_VARS
brnrd added inline comments.
mail/opensmtpd/Makefile
5–6

Meaning in OpenBSD derivative projects is "portable" not "patch" (e.g. security/openssh). Aligned PORVERSION to naming in security/openssh.

62

Correct. Yet the options check is unnecessary anyway, OK to clobber WITH_OPENSSL_PORT even if SSL_PORT is set.
Check should be against OpenSSL >= 1.0.1 which is in OSVERSION 1000015
SSL_PORT option moved to SSL_PORT_VARS=

brnrd marked an inline comment as done.

Add UPDATING entry for start failures on existing installs

One change at a time... Remove spurious GIDs addition

brnrd added a reviewer: adamw.
brnrd removed a subscriber: adamw.
UPDATING
17 ↗(On Diff #16633)

Do those directories need to get manually chgrp'ed as well?

mail/opensmtpd/Makefile
35

Correct me if I'm wrong, but doesn't using ${OPENSSLBASE} have to happen after bsd.port.pre.mk?

If somebody sets OPENSSL_PORT=security/libressl but hasn't built any ports yet, won't it fall back to security/openssl?

brnrd added inline comments.
UPDATING
17 ↗(On Diff #16633)

After executing

Performing sanity check on smtpd configuration:
configuration OK
Starting smtpd.
unknown group _smtpq
/usr/local/etc/rc.d/smtpd: WARNING: failed to start smtpd
/var/spool/smtpd/offline

thsi is solved by adding _smtpq group (pkg does that now).
Then starting smtpd failed to start due to the mode on /var/spool/smtpd/offline and /var/spool/smtpd/purge.
After executing

chmod 770 /var/spool/smtpd/offline
chmod 700 /var/spool/smtpd/purge

It has been running 5.9.2 fine since, but it'd be great if someone could upgrade a 5.7 system to see if this replicates!

Just found another regression, no matches for 'smtpd: \[priv\]' . Not sure if something failed or process names changed... According to Gilles this will remail the case, got to adapt the rc-script. check for /usr/local/sbin/smtpd instead.

The pkg-message also has text on upgrading, got to add this as well!

brnrd marked an inline comment as done.

Fix regressions in rc-script, add info to pkg-message

This port fails to build as non-root.

You'll need to do the following:

Makefile:
post-patch:
    ${REINPLACE_CMD} -e '/chmod 2555/d' ${WRKSRC}/mk/smtpctl/Makefile.in

pkg-plist:
@(,,2555) sbin/smtpctl
UPDATING
17 ↗(On Diff #16659)

On a fresh installation, /var/spool/smtpd/offline is created with user _smtpq. Users should do that too.

# chgrp _smtpq /var/spool/smtpd/offline
20 ↗(On Diff #16659)

This sentence makes no sense. I'm not even sure what you were trying to say here.

Was is "Stop the smtpd service before performing these commands"? If so, before the chmod commands put

# service smtpd stop
# chmod 770 [...]
mail/opensmtpd/Makefile
50

Bernard, this doesn't work at all. None of the OPTIONS stuff can come after bsd.port.pre.mk.

Put it back where it was, and then right change the .include <bsd.port.options.mk> line to this:

.include <bsd.port.pre.mk>

CONFIGURE_ARGS+=    --with-ssl=${OPENSSLBASE}

.if ${OPSYS} == FreeBSD && [etc.]

Of course, delete the --with-ssl line from the original CONFIGURE_ARGS line.

brnrd edited edge metadata.

Fix regressions in rc-script, add info to pkg-message

Update patches

  • Use proper order of bsd.{options,pre,post}.mk
  • Allow building as non-root
  • Add upgrade instructions to pkg-message
  • Improve UPDATING entry

Learned something again... (order of includes, modes, non-root building)
Even though I don't build as root, apparently the non-privileged user I
su into as root is not proper for testing as non-root!

brnrd added inline comments.
UPDATING
20 ↗(On Diff #16659)

Hope this makes more sense.

UPDATING
23 ↗(On Diff #16871)

I still think you should add:

# chgrp _smtpq /var/spool/smtpd/offline
mail/opensmtpd/Makefile
73

Bernard, buddy, this doesn't work either :-) It clobbers the stuff from the OPTIONS helpers.

Either make this +=, or just add the --with-ssl line after .pre.mk.

brnrd marked an inline comment as done.

Don't clobber CONFIGURE_ARGS

Good work on this, Bernard!

The only remaining thing for me is the chgrp _smtpq thing. Please add it, or show me why I'm wrong about it. After that, I'll give this a green-light review!

brnrd added inline comments.
UPDATING
23 ↗(On Diff #16871)

Seems so indeed, but smtpd performs that chgrp itself. No clue why it doesn't do the other modes itself but fails on these :sad:

mail/opensmtpd/Makefile
73

Argghhh... Absolutely...
Got to setup a separate jail for building that does not have LibreSSL in base. Diff is becoming unwieldy to properly port!

brnrd marked 2 inline comments as done.

Just found out chgrp _is_ required and so is -R

UPDATING
17 ↗(On Diff #16633)

Testing helps... Is required! Added chown and -R for people that already have spool files in these dirs.

Use chown not chgrp in UPDATING

adamw edited edge metadata.

Nice work Bernard!

This looks good to me, I'm happy to put a green-light review on this.

Technically you have an implicit approval from ashish, so it's up to you whether you want to get a formal approval from him.

mail/opensmtpd/files/pkg-message.in
7 ↗(On Diff #16875)

If you're using service(8) below, why not use it here?

This revision is now accepted and ready to land.May 25 2016, 8:00 PM
This revision was automatically updated to reflect the committed changes.
In D6421#138701, @adamw wrote:

Technically you have an implicit approval from ashish, so it's up to you whether you want to get a formal approval from him.

Hope that Ashish doesn't mind me committing so shortly before he returns.

It's been running fine here for a week now including some of the filters. Setup decribed on the FreeBSD wiki