Fix default python flavor selection wrt DEFAULT_VERSIONS
ClosedPublic

Authored by AMDmi3 on Dec 1 2017, 8:41 PM.

Details

Summary

After FLAVORS introduction, I've discovered that even though I have

DEFAULT_VERSIONS+=      python=3.6 python3=3.6

in my /etc/make.conf, py27 ports are still built by default.

% make -V DEFAULT_VERSIONS
python=3.6 python3=3.6 pgsql=10 ssl=libressl
% make -C /usr/ports/www/py-flask -V FLAVORS -V FLAVOR
py27 py36
py27

Seems that default flavor is chosen incorrectly in python.mk. While
_VALID_PYTHON_VERSIONS is filled correctly, taking PYTHON_DEFAULT,
PYTHON2_DEFAULT, PYTHON3_DEFAULT in that order and avoiding duplicates
in the resulting set, FLAVORS filled the similar way is filled
incorrectly:

# Decide how many flavors we want.  By default, only generate the default
# versions.
.  if defined(BUILD_ALL_PYTHON_FLAVORS) || defined(_PYTHON_FEATURE_ALLFLAVORS)
FLAVORS=        ${_ALL_PYTHON_FLAVORS}
.  else
.    for _v in ${PYTHON3_DEFAULT} ${PYTHON2_DEFAULT} ${PYTHON_DEFAULT}
_f=     py${_v:S/.//}
.      if ${_ALL_PYTHON_FLAVORS:M${_f}} && !${FLAVORS:M${_f}}
FLAVORS:=       ${_f} ${FLAVORS}
.      endif
.    endfor
.  endif
.  if !empty(FLAVORS) && empty(FLAVOR)
FLAVOR= ${FLAVORS:[1]}
.  endif
.endif

That code will ignore PYTHON_DEFAULT if the value it's set was
already met in PYTHON3_DEFAULT or PYTHON2_DEFAULT, and that's practically
always the case. IMO it should use the same logic as _VALID_PYTHON_VERSIONS
does, however to keep the change minimal, I'm just fixing the logic here:
since defaults are traversed in reverse order, always move the next
flavor to the beginning of the list.

Test Plan

This fixes tha case of python3 by default and doesn't affect the default configuration, as expected:

% DEFAULT_VERSIONS= make -V FLAVORS -V FLAVOR
py27 py36
py27
% DEFAULT_VERSIONS=python=3.6 make -V FLAVORS -V FLAVOR
py36 py27
py36

Also works well with other configurations:

% DEFAULT_VERSIONS=python3=3.5 make -V FLAVORS -V FLAVOR
py27 py35
py27
% DEFAULT_VERSIONS=python3=3.5\ python=3.6 make -V FLAVORS -V FLAVOR
py36 py27 py35
py36

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
AMDmi3 created this revision.Dec 1 2017, 8:41 PM
AMDmi3 edited the summary of this revision. (Show Details)Dec 1 2017, 8:44 PM
antoine accepted this revision.Dec 2 2017, 10:27 PM
antoine added a subscriber: antoine.

This looks good to me.

This revision is now accepted and ready to land.Dec 2 2017, 10:27 PM
mat added inline comments.Dec 3 2017, 6:50 PM
Mk/Uses/python.mk
427–430 ↗(On Diff #36067)

Would probably be more readable to write the whole thing as . for python python2 python3 instead. I'll have a look at it tomorrow.

antoine added inline comments.Dec 3 2017, 7:28 PM
Mk/Uses/python.mk
427–430 ↗(On Diff #36067)

There is a version that does the loop in the other order in https://bz-attachments.freebsd.org/attachment.cgi?id=188490

This revision was automatically updated to reflect the committed changes.
mat added a comment.Dec 4 2017, 9:16 AM

Thank you for committing this while we were still discussing the issue.

In D13326#278836, @mat wrote:

Thank you for committing this while we were still discussing the issue.

I went to commit this before your comment arrived. Also I don't see how the commit hinders your discussion.