python portgroup overwrites PYTHONPATH in test.env

[dgilman (David Gilman)]

I believe this is a defect in the Python portgroup.

The PYTHONPATH variable is not getting propagated from test.env down to the actual running of the tests. This is breaking the case where you are trying to test against a package that has not yet been installed so there's no package in the PYTHONPATH to import.

Attached is a simple portfile to reproduce the issue. Here is the guts of the portfile:

    python.test_framework \
                    pytest
    test.run        yes
    test.target
    test.env        FOO=bar PYTHONPATH=${worksrcpath}/src

Truncated test output. Note FOO is set, but PYTHONPATH is empty:

DEBUG: Executing org.macports.test (py311-pyproject_hooks)
DEBUG: Environment:
CC_PRINT_OPTIONS='YES'
CC_PRINT_OPTIONS_FILE='/opt/local/var/macports/build/_Users_david_macports-ports-local_python_py-pyproject_hooks/py311-pyproject_hooks/work/.CC_PRINT_OPTIONS'
CPATH='/opt/local/include'
DEVELOPER_DIR='/Library/Developer/CommandLineTools'
FOO='bar'
LIBRARY_PATH='/opt/local/lib'
MACOSX_DEPLOYMENT_TARGET='10.15'
PYTHONPATH=''
SDKROOT='/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk'
Executing:  cd "/opt/local/var/macports/build/_Users_david_macports-ports-local_python_py-pyproject_hooks/py311-pyproject_hooks/work/pyproject_hooks-1.0.0" && py.test-3.11 -o addopts=''
DEBUG: system:  cd "/opt/local/var/macports/build/_Users_david_macports-ports-local_python_py-pyproject_hooks/py311-pyproject_hooks/work/pyproject_hooks-1.0.0" && py.test-3.11 -o addopts=''
======================================================================== test session starts =========================================================================
platform darwin -- Python 3.11.2, pytest-7.2.1, pluggy-1.0.0
rootdir: /opt/local/var/macports/build/_Users_david_macports-ports-local_python_py-pyproject_hooks/py311-pyproject_hooks/work/pyproject_hooks-1.0.0, configfile: pytest.ini
collected 0 items / 3 errors

=============================================================================== ERRORS ===============================================================================
_____________________________________________________________ ERROR collecting tests/test_call_hooks.py ______________________________________________________________
ImportError while importing test module '/opt/local/var/macports/build/_Users_david_macports-ports-local_python_py-pyproject_hooks/py311-pyproject_hooks/work/pyproject_hooks-1.0.0/tests/test_call_hooks.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/opt/local/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/importlib/__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/test_call_hooks.py:13: in <module>
    from pyproject_hooks import (
E   ModuleNotFoundError: No module named 'pyproject_hooks'

[dgilman (David Gilman)]

example portfile

[reneeotten (Renee Otten)]

Thanks for the report, I’ll take a look! The value you’re setting PYTHONPATH to does not look correct; the python PG sets it correctly for you, but it’s possible that I didn’t append things and assumed that people wouldn’t set anything in the Portfile for test.env. So far that has been a reasonable assumption….

[jmroot (Joshua Root)]

If a *.env option is set to something like FOO=a FOO=b, the actual FOO environment variable will be set to each value in succession, so effectively only the last occurrence of each variable counts. Since the portgroup is adding its value in pre-test, it will usually be the one that counts, and so it must be setting an empty value in this particular case.

[dgilman (David Gilman)]

A portfile with only python.test_framework pytest and test.run yes also emits an empty PYTHONPATH, and so do a handful of other permutations that I tried while trying to get this thing to work.

[reneeotten (Renee Otten)]

Replying to dgilman:

A portfile with only python.test_framework pytest and test.run yes also emits an empty PYTHONPATH, and so do a handful of other permutations that I tried while trying to get this thing to work.

then that is probably a non-issue as this works just fine for dozens of ports. It’s set in the pre-test phase so that it will work also correctly for ports that compile stuff. As I said I will take a look but perhaps there is just something specific what you’re trying to do.

[jmroot (Joshua Root)]

Replying to dgilman:

A portfile with only python.test_framework pytest and test.run yes also emits an empty PYTHONPATH

That at least is expected, since it gets the value by globbing in $worksrcpath, so if that's empty, so will be the glob result. At minimum it probably makes sense to not set the environment variable if the glob doesn't match anything.

[reneeotten (Renee Otten)]

Replying to jmroot:

Replying to dgilman:

A portfile with only python.test_framework pytest and test.run yes also emits an empty PYTHONPATH

That at least is expected, since it gets the value by globbing in $worksrcpath, so if that's empty, so will be the glob result. At minimum it probably makes sense to not set the environment variable if the glob doesn't match anything.

It appears that when using setuptools as the backend for python.pep517 you always get something in ${worksrcpath}/build/lib*, whereas at least in this case with flit as backend you don't (not sure if that's always true). However, if the globbing is empty and thus as a result PYTHONPATH is so as well; this is set as test.env-append in the pre-test phase, which then overriden in the Portfile with

test.env        FOO=bar PYTHONPATH=${worksrcpath}/src

so I fail to see how this empty variable can be originating from the Portfile....

I can try and figure out how to not set this if the glob-result is empty but don't understand how/if this is the actual underlying reason for the reported error.

[edit: @jmroot, I guess I could check for the existence of {{{${worksrcpath}/build}}} since if that's present the glob will not be empty; or is there a better way to check whether the result is empty? My Tcl is a bit rusty...]

[jmroot (Joshua Root)]

Replying to reneeotten:

However, if the globbing is empty and thus as a result PYTHONPATH is so as well; this is set as test.env-append in the pre-test phase, which then overriden in the Portfile

The main body of the Portfile is executed before any phases are run, so it's the other way round: the portgroup overrides the value set in the Portfile.

[jmroot (Joshua Root)]

My suggested fix would be something like this:

_resources/port1.0/group/python-1.0.tcl

@@ -572 +572 @@
 
 pre-test {
     # set PYTHONPATH
-    test.env-append PYTHONPATH=[join [glob -nocomplain ${worksrcpath}/build/lib*] :]
+    set libdirs [glob -nocomplain -directory ${worksrcpath}/build lib*]
+    if {$libdirs ne "" && [lsearch ${test.env} PYTHONPATH=*] == -1} {
+        test.env-append PYTHONPATH=[join $libdirs :]
+    }
 }

[reneeotten (Renee Otten)]

Replying to jmroot:

My suggested fix would be something like this:

_resources/port1.0/group/python-1.0.tcl

@@ -572 +572 @@
 
 pre-test {
     # set PYTHONPATH
-    test.env-append PYTHONPATH=[join [glob -nocomplain ${worksrcpath}/build/lib*] :]
+    set libdirs [glob -nocomplain -directory ${worksrcpath}/build lib*]
+    if {$libdirs ne "" && [lsearch ${test.env} PYTHONPATH=*] == -1} {
+        test.env-append PYTHONPATH=[join $libdirs :]
+    }
 }

thanks Josh for refreshing my memory on the order of which Portfiles are executed - that does ring a bell now, but I clearly didn't get that in the beginning... I appreciated your suggested fix as well, that makes total sense the lsearch is indeed useful as well so that people can set PYTHONPATH different than the "default" set by the python PG.

Since this is your change anyway do you just want to commit this directly?

[jmroot (Joshua Root)]

Sure, I can commit it. Just wanted to verify that it fixes the originally reported problem first.

[dgilman (David Gilman)]

Confirmed it works, I'm seeing a sensible PYTHONPATH:

PYTHONPATH='/opt/local/var/macports/build/_Users_david_macports-ports-local_python_py-pyproject_hooks/py311-pyproject_hooks/work/pyproject_hooks-1.0.0/src'

This is the Portfile with test.env PYTHONPATH=${worksrcpath}/src.

[jmroot (Joshua Root)]

In 84f65e6dea4ac33d992da5ec7ab4376b2dae6514/macports-ports (master):

python pg: test.env improvements

Don't override PYTHONPATH if already set by the Portfile. Don't set it
to an empty value if there are no lib* dirs found.

Closes: #67055