On 04/30/20 03:20, Roland Illig wrote: > On 29.04.2020 17:22, tmp%bh0.amt.ru@localhost wrote: >> Hello. >> >> I ported akpop3d from OpenBSD ports collection >> (https://cvsweb.openbsd.org/ports/mail/akpop3d/). >> >> The package builds and runs fine on my NetBSD 8.2 ((GENERIC) #0: Tue Mar >> 31 05:08:40 UTC 2020 >> mkrepro%mkrepro.NetBSD.org@localhost:/usr/src/sys/arch/i386/compile/GENERIC i386) >> with up to date packages. >> >> Could you please review and commit it to pkgsrc if found good? > Here are some comments that pkglint won't give you right now: > > files/akpop3d.sh looks fine. OK > patches/patch-akpop3d_8 has a hard-coded /etc. Pkglint already throws an > error at you if you hard-code /usr/pkg, but it doesn't yet detect /etc > and /var. To fix this, add a SUBST block to the package Makefile. See > chat/kgb-bot/Makefile for an example, or mk/subst.mk for the > documentation. To see pkglint's explanation, you can temporarily add > the string /usr/pkg to the patch and run pkglint on it. fixed > patches/patch-akpop3d_8: Since the manual page has been modified, the > date in line 3 should be updated. fixed > patches/patch-authenticate.c contains "MAX_TEMPSTR 100 /* obvious */". > That's not correct. It is far from obvious why the perfect maximum size > of the temporary string is exactly 100. Why not 128, or 97? fixed > patches/patch-authenticate.c contains lots of newly written code. In > that code, there should be no parentheses around the "return" value. fixed > That patch should also document whether it is intended to be submitted > upstream, or if not, why not. And if it is intended for upstream, it > should follow the indentation style of the remaining files from that > project. The author does not maintain this software. > patches/patch-authenticate.c contains %sysconfdir% at the bottom. Where > is that placeholder actually replaced? The package Makefile doesn't lose > a word about this topic, so I guess it's in the package's upstream > Makefile. Why are there two placeholders, SYSCONFDIR and %sysconfdir%? fixed > patches/patch-lock_maildrop_c: why the umask change? This must be > documented. I do not know (maybe for SGID akpop3d and group writable users email indoxes directory). The umask has been changed with the comment "create/use _akpop3d group rather than the non-existent 'mail' one" during the patch committing to the OpenBSD port collection on 2005-12-14. > patches/patch-main_c: the #ifndef at the top is not needed since this > macro will always be defined by the pkgsrc package Makefile. fixed > patches/patch-main_c: why did the timeout change? Who is going to wait > 10 minutes for a mail client? I am not that patient. I do not know (maybe for email inboxes on NFS). The timeout has been changed without comments during the initial port committing to the OpenBSD port collection on 2004-11-08. > patches/patch-main_c: in "int c,i", there is a space missing after the > comma. fixed > patches/patch-main_c: in "maxfd = -1, i=0", spacing must be made > consistent. Either both assignments must get a space around the "=" or > none of them. fixed > patches/patch-pop3_session_c: it's strange that the argument to > write_line must include the trailing "\r\n". That's already encoded in > the function name. But that's upstream's business, not yours. OK > patches/patch-tcp_listen_c: inconsistent spacing after comma: LISTENQ, > deffds. fixed > Makefile: When a package gets initially imported, PKGREVISION must not > be set. It does not matter what the PKGREVISION was in the OpenBSD ports > collection, counting starts anew in pkgsrc. fixed > Makefile: The HOMEPAGE should use https instead of http since we're not > in the 1990s anymore. fixed > Makefile: PKGSRC_COMPILER must not be set by a package since it is a > user-settable variable. See mk/compiler.mk. Just remove that line. fixed > Makefile: Only one CONFIGURE_ARGS per line, please. (This is something > that pkglint _could_ also remark, but it's not implemented yet.) This is > to prevent overlooking some of the configure options. fixed > Makefile: LIBS=-lc is rarely necessary. Especially not _before_ the > other libraries. fixed > Makefile: Before any ?= assignments of user-settable variables, .include > "../../mk/bsd.prefs.mk". fixed > Makefile: In do-install, it's inconsistent that there is a trailing > slash after man8/, but not after sbin. fixed > Other than all these remarks, the package looks fine. > > Since the program is so old, be sure to compile it with "gcc -Wall > -Wextra -O2" once, to see whether it has obvious bugs. OK Please see the attachment. -- Alexei
Attachment:
akpop3d.tgz
Description: Binary data