Fix python flavors and default links

[dbn]

Summary

USES=python doesn't work correctly under the following situation:

1. PY_FLAVOR is incorrectly set to FLAVOR when:
   1. the port manually specifies FLAVORS

NOTE: the check here is redundant as _PYTHON_VERSION will contain the correct version based on FLAVOR

Test Plan

See:

1. https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=216479
2. https://reviews.freebsd.org/D13448

[mat]

A really, really, really simpler version was committed today.

[dbn]

I assume you are only referring to point 3, as the other two points were not addressed in commit r456026. I think we also disagree on the definition of "really, really, really simpler":

Your commit:

@@ -567,7 +567,9 @@ RUN_DEPENDS+=       cython-${PYTHON_VER}:lang/cython@${PY_FLAVOR}
 
 .if defined(_PYTHON_FEATURE_CONCURRENT)
 _USES_POST+=           uniquefiles:dirs
-.if ${PYTHON_VERSION} == ${PYTHON_DEFAULT_VERSION}
+.if defined(_PYTHON_FEATURE_FLAVORS) && ${FLAVOR} == ${FLAVORS:[1]}
+UNIQUE_DEFAULT_LINKS=  yes
+.elif !defined(_PYTHON_FEATURE_FLAVORS) && ${PYTHON_VERSION} == ${PYTHON_DEFAULT_VERSION}
 UNIQUE_DEFAULT_LINKS=  yes
 .else
 UNIQUE_DEFAULT_LINKS=  no

Note that the above change only fixes UNIQUE_DEFAULT_LINKS if python flavors is used. In the following example it does not work:

# cd /usr/ports/Mk
# make -f bsd.port.mk -V UNIQUE_DEFAULT_LINKS USES=python:3.3+ USE_PYTHON=concurrent
no
# make -f bsd.port.mk -V UNIQUE_DEFAULT_LINKS USES=python:3.3+ USE_PYTHON="concurrent flavors"
yes

Whereas my commit (ignoring some code blocks that were moved around):

@@ -567,7 +559,7 @@ RUN_DEPENDS+=       cython-${PYTHON_VER}:lang/cython@${PY_FLAVOR}
 
 .if defined(_PYTHON_FEATURE_CONCURRENT)
 _USES_POST+=           uniquefiles:dirs
-.if ${PYTHON_VERSION} == ${PYTHON_DEFAULT_VERSION}
+.if ${PYTHON_VERSION:S/python//} == ${_VALID_PYTHON_VERSIONS:[1]}
 UNIQUE_DEFAULT_LINKS=  yes
 .else
 UNIQUE_DEFAULT_LINKS=  no

How shall we proceed regarding:

• the points (1) and (2) that I assume are uncontentious; and
• point (3) where alternative approaches have been suggested?

[mat]

Ok, I see what you trying to do.

It is by design that the selection of the Python version through flavors is not limited to when python.mk sets FLAVORS, so this is wrong. I will try to think of something for people who use conflicting flavor names.

[mat]

In D13449#281137, @dbn wrote:

Note that the above change only fixes UNIQUE_DEFAULT_LINKS if python flavors is used. In the following example it does not work:

# cd /usr/ports/Mk
# make -f bsd.port.mk -V UNIQUE_DEFAULT_LINKS USES=python:3.3+ USE_PYTHON=concurrent
no
# make -f bsd.port.mk -V UNIQUE_DEFAULT_LINKS USES=python:3.3+ USE_PYTHON="concurrent flavors"
yes

Well, USE_PYTHON=concurrent when you only support one of Python 2 or 3, or without flavors makes absolutely no sense, so the first line is really not a bug in the framework but in the port doing something stupid. So, this part of your patch is also rejected, the framework is not here to make make sure every stupid thing a port can do is handled correctly. I will try to add a DEV_WARNING about it.

[mat]

In D13449#281137, @dbn wrote:

Note that the above change only fixes UNIQUE_DEFAULT_LINKS if python flavors is used. In the following example it does not work:

# cd /usr/ports/Mk
# make -f bsd.port.mk -V UNIQUE_DEFAULT_LINKS USES=python:3.3+ USE_PYTHON=concurrent
no
# make -f bsd.port.mk -V UNIQUE_DEFAULT_LINKS USES=python:3.3+ USE_PYTHON="concurrent flavors"
yes

Is there actually one port that does this ? I was trying to test the DEV_WARNING I was adding, and I could not find any.

[dbn]

In D13449#281338, @mat wrote:

In D13449#281137, @dbn wrote:

Note that the above change only fixes UNIQUE_DEFAULT_LINKS if python flavors is used. In the following example it does not work:

# cd /usr/ports/Mk
# make -f bsd.port.mk -V UNIQUE_DEFAULT_LINKS USES=python:3.3+ USE_PYTHON=concurrent
no
# make -f bsd.port.mk -V UNIQUE_DEFAULT_LINKS USES=python:3.3+ USE_PYTHON="concurrent flavors"
yes

Is there actually one port that does this ? I was trying to test the DEV_WARNING I was adding, and I could not find any.

No, I don't know any port specifically.

[dbn]

In D13449#281299, @mat wrote:

Ok, I see what you trying to do.

It is by design that the selection of the Python version through flavors is not limited to when python.mk sets FLAVORS, so this is wrong. I will try to think of something for people who use conflicting flavor names.

Interesting, I'm curious as to the design intent. Perhaps allow a port to manuallt override the FLAVORS?

[mat]

I prefer it the way it currently is, it shows in a very simple way where PY_FLAVOR come from.

[dbn]

In D13449#281454, @mat wrote:

I prefer it the way it currently is, it shows in a very simple way where PY_FLAVOR come from.

The problem is the way it currently is does not work under some conditions:

# cd /usr/ports/Mk
# make -f bsd.port.mk -V PY_FLAVOR USES=python FLAVOR=foobar
foobar

I think there is a trade-off between transparency (clearly showing that PY_FLAVOR comes from FLAVOR) and potential inconsistency (if _PYTHON_VERSION is no longer directly defined by FLAVOR). I naturally prefer preventing the latter, but I do see the value in the former.

I have updated the patch to maintain transparency but to fix the issue.

[dbn]

Fixed in r456293