patch security/py-kerberos to optionally use Kerberos from base, heimdal, or from MIT

[dvl]

Summary

see also: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=199123

Commit message will be:

• Optionally use Kerberos from base, heimdal, or from MIT
• take maintainership

PR: 199123
Approved by: X (mentor)

Test Plan

Compiled with all three options:

• http://services.unixathome.org/poudriere/data/102amd64-testing/2015-11-09_21h19m23s/logs/py27-kerberos-1.1.1_5.log
• http://services.unixathome.org/poudriere/data/102amd64-testing/2015-11-09_21h21m39s/logs/py27-kerberos-1.1.1_5.log
• http://services.unixathome.org/poudriere/data/102amd64-testing/2015-11-09_21h08m29s/logs/py27-kerberos-1.1.1_5.log

[john_saltant.com]

Nice work!

security/py-kerberos/Makefile

6

New PORTREVISION should be 2.

16–22

While I readily acknowledge that upstream prefers MIT, wouldn't it be a nice benefit of your patches that this port could be installed from stock packages without hauling in security/krb5 if this defaulted to GSSAPI_BASE?

22

USES=gssapi:heimdal already appends this to BUILD_ and RUN_DEPENDS. Is LIB_DEPENDS definitely needed, or is this mainly just to preserve status quo?

From /usr/ports/Mk/Uses/gssapi.mk:

_HEIMDAL_DEPENDS=${GSSAPILIBDIR}/libgssapi.so:${PORTSDIR}/security/heimdal
# [...]
.elif ${_local} == "heimdal"
HEIMDAL_HOME?=  ${LOCALBASE}
GSSAPIBASEDIR=  ${HEIMDAL_HOME}
GSSAPILIBDIR=   ${GSSAPIBASEDIR}/lib/heimdal
GSSAPIINCDIR=   ${GSSAPIBASEDIR}/include/heimdal
_HEADERS+=      gssapi/gssapi.h gssapi/gssapi_krb5.h krb5.h
.if !defined(_KRB_BOOTSTRAP)
BUILD_DEPENDS+= ${_HEIMDAL_DEPENDS}
RUN_DEPENDS+=   ${_HEIMDAL_DEPENDS}
.else

24

Ditto for USES=gssapi:mit.

37–40

In this case, I would be inclined to use the two step method: regular patch to place %%GSSAPIBASEDIR%% exactly where desired, followed by ${REINPLACE_CMD} to replace that (guaranteed-unique) string. "krb5" has kind of an ambiguous feel to it.

[mat]

security/py-kerberos/Makefile

16–22

The default should be _BASE, yes.

20–24

The [[ https://www.freebsd.org/doc/en/books/porters-handbook/uses-gssapi.html | USES=gssapi documentation in the handbook ]] says that it already handles the dependencies on the kerberos ports, so you don't need to add it again here.

37–40

Would be better to have a more specific sed lines, yes, patching and then replacing is overkill.

[dvl]

Implement some of the suggestions; still one to go.

[dvl]

Some suggestion completed; one more to go.

security/py-kerberos/Makefile

16–22

Yes. I agree.

[dvl]

Do a better post-patch by providing a more specific line to sed.

[dvl]

Better line for SED supplied.

[dvl]

security/py-kerberos/Makefile

37

The original lines are:

$ grep krb5 /var/ports/usr/ports/security/py-kerberos/work/kerberos-1.1.1/setup.py

extra_link_args = commands.getoutput("krb5-config --libs gssapi").split(),
extra_compile_args = commands.getoutput("krb5-config --cflags gssapi").split(),

[mat]

security/py-kerberos/Makefile

25

These where better the other way round. autoplist depends on distutils.

29–34

You don't really need the bsd.port.options.mk include, instead you should do:

GSSAPI_BASE_EXTRA_PATCHES=        ${PATCHDIR}/extra-patch-src_kerberosbasic.h \
              ${PATCHDIR}/extra-patch-src_kerberosgss.c \
              ${PATCHDIR}/extra-patch-src_kerberosgss.h \
              ${PATCHDIR}/extra-patch-src_kerberospw.h
GSSAPI_HEIMDAL_EXTRA_PATCHES=${GSSAPI_BASE_EXTRA_PATCHES}

[dvl]

Recent suggestions

[dvl]

Done.

[mat]

Looks good. (when you commit, do note that the maintainer change was approved by the former maintainer.)

[john_saltant.com]

security/py-kerberos/Makefile

25

I don't wish to re-litigate this change but I'm genuinely curious to know whether the order matters. From what I can tell, it doesn't, which is why your comment puzzles me.

As of r401622, the following lines of Mk/Uses/python.mk appear to be the relevant ones.

225 # Make each individual feature available as _PYTHON_FEATURE_<FEATURENAME>
226 .for var in ${USE_PYTHON}
227 _PYTHON_FEATURE_${var:tu}=  yes
228 .endfor

Each feature flag is tested starting at line 440 in a static order, independent of the order in which the features are enabled by the port.

Elsewhere I've seen recommendations to alphabetize USE_PYTHON. As quasi-arbitrary standards go, that one seems better because it depends only on information accessible by direct inspection.

Am I missing something?

[mat]

security/py-kerberos/Makefile

25–34

The order doesn't matter, there are a few people that are kind of a sort nazi police that want to sort everything, it just does not make sense.

In this case, autoplist can't be used without distutils, so it's more logical to have first distutil, then autoplist.

[dvl]

See https://svnweb.freebsd.org/ports?view=revision&revision=401816

[koobs]

For future reference, the following line in the commit log will automatically close a Phabricator review:

Differential Revision: <full url>

[dvl]

In D4115#88133, @koobs wrote:

For future reference, the following line in the commit log will automatically close a Phabricator review:

Differential Revision: <full url>

Thanks.

I keep wanting to add that into my code review, but the system does not allow it. I have to remember at commit time, and by then, I'm copy/pasting what appears here.

[dvl]

In D4115#87974, @mat wrote:

Looks good. (when you commit, do note that the maintainer change was approved by the former maintainer.)

I apologize. I missed this message and did not acknowledge that in my commit.