Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cylc to ignore PYTHONPATH: Use CYLC_PYTHONPATH for custom Python for Cylc #5727

Merged
merged 4 commits into from
Oct 25, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changes.d/5727.break.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Cylc now ignores `PYTHONPATH` to make it more robust to task environments which set this value.
If you want to add to the Cylc environment itself, e.g. to install a Cylc extension, use `CYLC_PYTHONPATH`.
wxtim marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
[[[environment]]]
# These environment variables ensure that tasks can
# run in the same environment as the workflow:
{% from "sys" import path, executable %}
PYTHONPATH = {{':'.join(path)}}
{% from "sys" import executable %}
PYTHONPATH = {{executable}}/../lib/python:$PYTHONPATH
wxtim marked this conversation as resolved.
Show resolved Hide resolved
PATH = $(dirname {{executable}}):$PATH
29 changes: 27 additions & 2 deletions cylc/flow/scripts/cylc.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,35 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
"""cylc main entry point"""

import argparse
from contextlib import contextmanager
import os
import sys


def pythonpath_manip():
"""Stop PYTHONPATH contaminating the Cylc Environment

* Remove PYTHONPATH items from sys.path to prevent PYTHONPATH
contaminating the Cylc Environment.
* Add items from CYLC_PYTHONPATH to sys.path.

See Also:
https://github.com/cylc/cylc-flow/issues/5124
"""
if 'CYLC_PYTHONPATH' in os.environ:
for item in os.environ['CYLC_PYTHONPATH'].split(os.pathsep):
abspath = os.path.abspath(item)
sys.path.insert(0, abspath)
if 'PYTHONPATH' in os.environ:
wxtim marked this conversation as resolved.
Show resolved Hide resolved
wxtim marked this conversation as resolved.
Show resolved Hide resolved
for item in os.environ['PYTHONPATH'].split(os.pathsep):
abspath = os.path.abspath(item)
if abspath in sys.path:
sys.path.remove(abspath)


pythonpath_manip()

import argparse
from contextlib import contextmanager
from typing import Iterator, NoReturn, Optional, Tuple

from ansimarkup import parse as cparse
Expand Down
26 changes: 25 additions & 1 deletion tests/unit/scripts/test_cylc.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,16 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

import os
import pkg_resources
import sys
from types import SimpleNamespace
from typing import Callable
from unittest.mock import Mock

import pytest

from cylc.flow.scripts.cylc import iter_commands
from cylc.flow.scripts.cylc import iter_commands, pythonpath_manip

from ..conftest import MonkeyMock

Expand Down Expand Up @@ -136,3 +138,25 @@ def test_execute_cmd(
# the "bad" entry point should raise an exception
with pytest.raises(ModuleNotFoundError):
execute_cmd('bad')


def test_pythonpath_manip(monkeypatch):
"""pythonpath_manip removes items in PYTHONPATH from sys.path

and adds items from CYLC_PYTHONPATH
"""
# If PYTHONPATH is set...
monkeypatch.setenv('PYTHONPATH', '/remove-from-sys.path')
monkeypatch.setattr('sys.path', ['/leave-alone', '/remove-from-sys.path'])
pythonpath_manip()
# ... we don't change PYTHONPATH
assert os.environ['PYTHONPATH'] == '/remove-from-sys.path'
# ... but we do remove PYTHONPATH items from sys.path, and don't remove
# items there not in PYTHONPATH
assert sys.path == ['/leave-alone']

# If CYLC_PYTHONPATH is set we retrieve its contents and
# add them to the sys.path:
monkeypatch.setenv('CYLC_PYTHONPATH', '/add-to-sys.path')
pythonpath_manip()
assert sys.path == ['/add-to-sys.path', '/leave-alone']
Loading