On 2020/03/02 16:43, Jeremie Courreges-Anglas wrote:
> On Mon, Mar 02 2020, Frederic Cambus <[email protected]> wrote:
> > Hi ports@,
> >
> > Here is a new port: net/dbip.
> >
> > Following the decision from MaxMind [1] to stop releasing their GeoLite2
> > databases under a Creative Commons license, I have been looking for an
> > alternative we could package.
> >
> > DB-IP has provided free databases in CSV format for a while now, and are
> > now also providing the databases in MMDB format which can be directly
> > consumed with net/libmaxminddb. These databases are licensed under the
> > Creative Commons Attribution 4.0 International License (CC BY 4.0).
> >
> > They are not a full replacement for GeoLite2 yet, as there is no free
> > version of the IP to ASN database at the moment.
> >
> > Comments? OK?
>
> Regarding Makefile.inc:
> --8<--
> ...
> NO_TEST = Yes
>
> do-install:
> ${INSTALL_DATA_DIR} ${PREFIX}/share/examples/dbip
> ${INSTALL_DATA} ${WRKDIST}/*.mmdb ${PREFIX}/share/examples/dbip
> chown -R ${SHAREOWN}:${SHAREGRP} ${PREFIX}/share/examples/dbip
> -->8--
>
> I would expect a Makefile.inc to set default values that can be
> overriden from underlying ports, a la:
>
> --8<--
> NO_TEST ?= Yes
>
> .if !target(do-install)
> do-install:
> ${INSTALL_DATA_DIR} ${PREFIX}/share/examples/dbip
> ${INSTALL_DATA} ${WRKDIST}/*.mmdb ${PREFIX}/share/examples/dbip
> chown -R ${SHAREOWN}:${SHAREGRP} ${PREFIX}/share/examples/dbip
> .endif
> -->8--
It's useful sometimes where there's more divergence between the ports
in subdirectories, but that does seems a bit overengineered for this,
especially the ".if !target".
> Regarding the PLISTs,
> --8<--
> @comment $OpenBSD: PLIST,v$
> share/examples/dbip/
> @sample ${LOCALSTATEDIR}/db/dbip/
> share/examples/dbip/dbip-city-lite.mmdb
> @sample ${LOCALSTATEDIR}/db/dbip/dbip-city-lite.mmdb
> -->8--
>
> Why use @sample here? Is that the way the old geoip database port
> worked? I'm not strongly objecting but I'm not sure there's a point in
> providing a user-editable file when there's no easy way to edit and
> update it.
Agreed, in the case of old geoip1 the filenames were pretty much fixed and
the @sample method made sense (plus you might run an update tool to fetch
new files in place of the old ones). For maxminddb it seems standard to
configure the filename instead and if users are updating separately it
would seem sensible to use different files for it, so I think it would
be better to skip the @sample and just set PREFIX instead so they are
installed directly to ${LOCALSTATEDIR}/db/dbip..
> Except for this, the port looks good, lightly tested with my VPN
> address and mmdblookup. ok jca@ with the points mentioned above
> addressed.
>
> --
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
>