Bug 199081 - [revive port] mail/smfsav: Add staging support
Summary: [revive port] mail/smfsav: Add staging support
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Dmitry Marakasov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-01 08:26 UTC by Kan Sasaki
Modified: 2015-06-02 13:48 UTC (History)
2 users (show)

See Also:


Attachments
smfsav.shar (8.07 KB, text/plain)
2015-04-01 08:26 UTC, Kan Sasaki
no flags Details
patch for UIDs (634 bytes, patch)
2015-04-01 08:26 UTC, Kan Sasaki
no flags Details | Diff
patch for GIDs (262 bytes, patch)
2015-04-01 08:27 UTC, Kan Sasaki
no flags Details | Diff
patch for two files (1.82 KB, patch)
2015-06-01 02:20 UTC, Kan Sasaki
no flags Details | Diff
patch.txt (6.46 KB, patch)
2015-06-02 06:28 UTC, Kan Sasaki
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kan Sasaki 2015-04-01 08:26:12 UTC
Created attachment 155077 [details]
smfsav.shar

Adds staging support to mail/smfsav.
Comment 1 Kan Sasaki 2015-04-01 08:26:47 UTC
Created attachment 155078 [details]
patch for UIDs
Comment 2 Kan Sasaki 2015-04-01 08:27:09 UTC
Created attachment 155079 [details]
patch for GIDs
Comment 3 John Marino freebsd_committer freebsd_triage 2015-04-01 21:09:27 UTC
whoever picks this up, please follow https://www.freebsd.org/doc/en/articles/committers-guide/ports.html#ports-qa-re-adding 

and do not simply treat it as a new port.
Comment 4 Dmitry Marakasov freebsd_committer freebsd_triage 2015-05-27 15:45:55 UTC
Are you willing to become maintainer of this port?
Comment 5 Kan Sasaki 2015-05-29 04:20:30 UTC
(In reply to Dmitry Marakasov from comment #4)
Yes. I will.
Comment 6 Dmitry Marakasov freebsd_committer freebsd_triage 2015-05-29 18:25:44 UTC
Why don't we change user name to "smfs" which is upstream default?

# Run as a selected user (smf-sav must be started by root)
#
# Default: smfs

Also I suggest the following set of changes:

- Add LICENSE_FILE
- Use -lpthread as threading ldflags as it has been decided
- Simplify installation - no need to split do-install and post-install, do need to install docs conditionally as this is handled automatically

diff -ruN smfsav.orig/Makefile smfsav/Makefile
--- smfsav.orig/Makefile        2015-05-29 21:20:18.470625000 +0300
+++ smfsav/Makefile     2015-05-29 21:15:04.043203000 +0300
@@ -13,18 +13,17 @@
 COMMENT=       Sendmail Sender Address Validator
 
 LICENSE=       GPLv2
+LICENSE_FILE=  ${WRKSRC}/COPYING
 
 USERS=         smfsav
 GROUPS=                smfsav
 
 OPTIONS_DEFINE=        DOCS
 
-.include <bsd.port.options.mk>
-
 USE_RC_SUBR=   smfsav
 
 CPPFLAGS+=     -D_REENTRANT
-LDFLAGS+=      -lmilter -pthread
+LDFLAGS+=      -lmilter -lpthread
 
 SMFSAV_RUN_DIR?=/var/run/smfsav
 SUB_FILES+=    pkg-message
@@ -38,12 +37,8 @@
 do-install:
        ${INSTALL_PROGRAM} ${WRKSRC}/smf-sav ${STAGEDIR}${PREFIX}/sbin/smfsav
        ${INSTALL_DATA} ${WRKSRC}/smf-sav.conf ${STAGEDIR}${PREFIX}/etc/smfsav.conf.sample
-
-post-install:
        @${MKDIR} ${STAGEDIR}${SMFSAV_RUN_DIR}
-.if ${PORT_OPTIONS:MDOCS}
        @${MKDIR} ${STAGEDIR}${DOCSDIR}
        cd ${WRKSRC} && ${INSTALL_DATA} ChangeLog readme ${STAGEDIR}${DOCSDIR}
-.endif
 
 .include <bsd.port.mk>
diff -ruN smfsav.orig/files/pkg-message.in smfsav/files/pkg-message.in
--- smfsav.orig/files/pkg-message.in    2015-05-29 21:20:18.464386000 +0300
+++ smfsav/files/pkg-message.in 2015-05-29 21:12:35.276515000 +0300
@@ -5,5 +5,5 @@
     define(`confMILTER_MACROS_HELO', confMILTER_MACROS_HELO`, {verify}')dnl
     INPUT_MAIL_FILTER(`smfsav', `S=unix:/var/run/smfsav/smfsav.sock, T=S:30s;R:4m')dnl
 3. Put line smfsav_enable="YES" to /etc/rc.conf file
-4. Run service smfsav start
+4. Run `service smfsav start`
 =====================================================================================
Comment 7 Kan Sasaki 2015-05-30 01:08:52 UTC
(In reply to Dmitry Marakasov from comment #6)
I totally agree with you.
Comment 8 commit-hook freebsd_committer freebsd_triage 2015-05-31 14:16:45 UTC
A commit references this bug:

Author: amdmi3
Date: Sun May 31 14:16:23 UTC 2015
New revision: 388093
URL: https://svnweb.freebsd.org/changeset/ports/388093

Log:
  - Revive mail/smfsav
  - Fix staging
  - Add LICENSE_FILE
  - Modernize pthread flags usage
  - Modernize user/group handling
  - Regenerate patches
  - Pass maintainership to submitter

  PR:		199081
  Submitted by:	sasaki@fcc.ad.jp

Changes:
  head/GIDs
  head/MOVED
  head/UIDs
  head/mail/Makefile
  head/mail/smfsav/
  head/mail/smfsav/Makefile
  head/mail/smfsav/distinfo
  head/mail/smfsav/files/patch-readme
  head/mail/smfsav/files/patch-smf-sav.c
  head/mail/smfsav/files/patch-smf-sav.conf
  head/mail/smfsav/files/pkg-message.in
  head/mail/smfsav/files/smfsav.in
  head/mail/smfsav/pkg-deinstall
  head/mail/smfsav/pkg-install
  head/mail/smfsav/pkg-plist
Comment 9 Kan Sasaki 2015-06-01 02:20:10 UTC
Two patch files have to be changed because of user change.
Comment 10 Kan Sasaki 2015-06-01 02:20:43 UTC
Created attachment 157320 [details]
patch for two files
Comment 11 Dmitry Marakasov freebsd_committer freebsd_triage 2015-06-01 13:50:57 UTC
Sorry for missing that. Shouldn't we remove socket path modification as well? I don't see reason for that either:

-# Default: unix:/var/run/smfs/smf-sav.sock
+# Default: unix:/var/run/smfsav/smfsav.sock
 #
-#Socket                unix:/var/run/smfs/smf-sav.sock
+#Socket                unix:/var/run/smfsav/smfsav.sock
Comment 12 Kan Sasaki 2015-06-02 01:22:43 UTC
(In reply to Dmitry Marakasov from comment #11)
I suppose we should hold the modification at minimum because this is
the revived port. I think user name modification is ok because it was
once removed, but socket path modification is too much.

If I create this port as new one, I want to hold original
program/path/user name. But this is revived port, so I think
aggressive modification is no good. What do you think about this?

I found an error in Makefile. readme has two %%PREFIX%% on the same line.

 post-patch:
-       @${REINPLACE_CMD} -e 's|%%PREFIX%%|${PREFIX}|' ${WRKSRC}/readme
+       @${REINPLACE_CMD} -e 's|%%PREFIX%%|${PREFIX}|g' ${WRKSRC}/readme
Comment 13 Dmitry Marakasov freebsd_committer freebsd_triage 2015-06-02 02:42:05 UTC
> If I create this port as new one, I want to hold original
> program/path/user name. But this is revived port, so I think
> aggressive modification is no good. What do you think about this?

Whether this port is new or revived does not matter - either way it's better to fix things now than when it's used by more people. There's absolutely no reason to hold back changes when port is revived.

> I found an error in Makefile. readme has two %%PREFIX%% on the same line.

Added to the diff.
Comment 14 Kan Sasaki 2015-06-02 06:28:13 UTC
Created attachment 157366 [details]
patch.txt
Comment 15 Kan Sasaki 2015-06-02 06:29:28 UTC
(In reply to Dmitry Marakasov from comment #13)
> Whether this port is new or revived does not matter - either way it's better to
> fix things now than when it's used by more people. There's absolutely no reason
> to hold back changes when port is revived.

I got it.

I've attached a patch which includes the following set of changes:

- Prefer upstream default
- Replace hardcoded /usr/local by %%PREFIX%%
- Add required_files and command_args to rc script
- Fix REINPLACE_CMD args in Makefile
Comment 16 Dmitry Marakasov freebsd_committer freebsd_triage 2015-06-02 13:47:26 UTC
This does not apply. Please send single patch against
Comment 17 Dmitry Marakasov freebsd_committer freebsd_triage 2015-06-02 13:48:10 UTC
(In reply to Dmitry Marakasov from comment #16)
> This does not apply. Please send single patch against

Oops, please ignore this comment. The patch applied cleanly and was committed.
Comment 18 commit-hook freebsd_committer freebsd_triage 2015-06-02 13:48:16 UTC
A commit references this bug:

Author: amdmi3
Date: Tue Jun  2 13:47:23 UTC 2015
New revision: 388358
URL: https://svnweb.freebsd.org/changeset/ports/388358

Log:
  - Don't override upstream paths
  - Replace hardcoded /usr/local by ${PREFIX}
  - Add required_files and command_args to rc script
  - Fix REINPLACE_CMD args in Makefile

  PR:		199081
  Submitted by:	sasaki@fcc.ad.jp (maintainer)

Changes:
  head/mail/smfsav/Makefile
  head/mail/smfsav/files/patch-readme
  head/mail/smfsav/files/patch-smf-sav.c
  head/mail/smfsav/files/patch-smf-sav.conf
  head/mail/smfsav/files/pkg-message.in
  head/mail/smfsav/files/smfsav.in
  head/mail/smfsav/pkg-plist