From 5bfea9df6b2926599190ac6a74621cf32ec92c13 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Thu, 26 Oct 2023 15:47:38 +0100 Subject: [PATCH 1/6] add handling for ROSE_PYTHONPATH --- CHANGES.md | 7 ++++++ metomi/rose/rose.py | 32 +++++++++++++++++++++++-- metomi/rose/tests/test_rose.py | 44 ++++++++++++++++++++++++++++++++++ tox.ini | 2 ++ 4 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 metomi/rose/tests/test_rose.py diff --git a/CHANGES.md b/CHANGES.md index cdc413c641..ce48c8120c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -13,6 +13,13 @@ ones in. --> ## 2.1.0 (Released 2023-07-21) +### Breaking Changes + +[2736](https://github.com/metomi/rose/pull/2736) +Rose now ignores `PYTHONPATH` to make it more robust to task environments +which set this value. If you want to add to the Rose environment itself, +e.g. to write a rose-ana test, use `ROSE_PYTHONPATH`. + ### Fixes [#2699](https://github.com/metomi/rose/pull/2699) - diff --git a/metomi/rose/rose.py b/metomi/rose/rose.py index be4606cffb..e9c3473579 100644 --- a/metomi/rose/rose.py +++ b/metomi/rose/rose.py @@ -14,11 +14,39 @@ # # You should have received a copy of the GNU General Public License # along with Rose. If not, see . + +import os +import sys + + +def pythonpath_manip(): + """Stop PYTHONPATH contaminating the Cylc Environment + + * Remove PYTHONPATH items from sys.path to prevent PYTHONPATH + contaminating the Rose Environment. + * Add items from ROSE_PYTHONPATH to sys.path. + + See Also: + https://github.com/cylc/cylc-flow/issues/5124 + """ + if 'ROSE_PYTHONPATH' in os.environ: + for item in os.environ['ROSE_PYTHONPATH'].split(os.pathsep): + print(f'extracted {item} from ROSE_PYTHONPATH') + abspath = os.path.abspath(item) + sys.path.insert(0, abspath) + if 'PYTHONPATH' in os.environ: + 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 inspect import signature -import os from pathlib import Path -import sys from pkg_resources import ( DistributionNotFound, diff --git a/metomi/rose/tests/test_rose.py b/metomi/rose/tests/test_rose.py new file mode 100644 index 0000000000..81745fd568 --- /dev/null +++ b/metomi/rose/tests/test_rose.py @@ -0,0 +1,44 @@ +# Copyright (C) British Crown (Met Office) & Contributors. +# +# This file is part of Rose, a framework for meteorological suites. +# +# Rose is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Rose is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Rose. If not, see . +"""Test metomi/rose/rose.py +""" + +import os +import sys + +from metomi.rose.rose import pythonpath_manip + + +def test_pythonpath_manip(monkeypatch): + """pythonpath_manip removes items in PYTHONPATH from sys.path + and adds items from ROSE_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('ROSE_PYTHONPATH', '/add-to-sys.path') + pythonpath_manip() + assert sys.path == ['/add-to-sys.path', '/leave-alone'] diff --git a/tox.ini b/tox.ini index 8dad1c4452..83a93c182c 100644 --- a/tox.ini +++ b/tox.ini @@ -13,3 +13,5 @@ ignore= E731, # no longer best practice: W503 + ; module level import not at top of file + E402, From ec8d188419384a174d8468c2da338b2cf18338eb Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Fri, 24 Nov 2023 10:08:35 +0000 Subject: [PATCH 2/6] Update metomi/rose/rose.py Co-authored-by: Oliver Sanders --- metomi/rose/rose.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metomi/rose/rose.py b/metomi/rose/rose.py index e9c3473579..5b4e25e463 100644 --- a/metomi/rose/rose.py +++ b/metomi/rose/rose.py @@ -20,7 +20,7 @@ def pythonpath_manip(): - """Stop PYTHONPATH contaminating the Cylc Environment + """Stop PYTHONPATH contaminating the Rose Environment * Remove PYTHONPATH items from sys.path to prevent PYTHONPATH contaminating the Rose Environment. From f39035269e0aaa96b738b8c84476e66d8624bfbc Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Tue, 28 Nov 2023 13:14:46 +0000 Subject: [PATCH 3/6] Apply suggestions from code review Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- metomi/rose/rose.py | 12 ++++++++---- metomi/rose/tests/test_rose.py | 12 ++++++------ 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/metomi/rose/rose.py b/metomi/rose/rose.py index 5b4e25e463..512761544e 100644 --- a/metomi/rose/rose.py +++ b/metomi/rose/rose.py @@ -30,10 +30,14 @@ def pythonpath_manip(): https://github.com/cylc/cylc-flow/issues/5124 """ if 'ROSE_PYTHONPATH' in os.environ: - for item in os.environ['ROSE_PYTHONPATH'].split(os.pathsep): - print(f'extracted {item} from ROSE_PYTHONPATH') - abspath = os.path.abspath(item) - sys.path.insert(0, abspath) + paths = [ + os.path.abspath(item) + for item in os.environ['ROSE_PYTHONPATH'].split(os.pathsep) + ] + print(f"Extracted {paths} from ROSE_PYTHONPATH") + paths.extend(sys.path) + sys.path = paths + if 'PYTHONPATH' in os.environ: for item in os.environ['PYTHONPATH'].split(os.pathsep): abspath = os.path.abspath(item) diff --git a/metomi/rose/tests/test_rose.py b/metomi/rose/tests/test_rose.py index 81745fd568..7f7e953464 100644 --- a/metomi/rose/tests/test_rose.py +++ b/metomi/rose/tests/test_rose.py @@ -28,17 +28,17 @@ def test_pythonpath_manip(monkeypatch): and adds items from ROSE_PYTHONPATH """ # If PYTHONPATH is set... - monkeypatch.setenv('PYTHONPATH', '/remove-from-sys.path') - monkeypatch.setattr('sys.path', ['/leave-alone', '/remove-from-sys.path']) + monkeypatch.setenv('PYTHONPATH', '/remove1:/remove2') + monkeypatch.setattr('sys.path', ['/leave-alone', '/remove1', '/remove2']) pythonpath_manip() # ... we don't change PYTHONPATH - assert os.environ['PYTHONPATH'] == '/remove-from-sys.path' + assert os.environ['PYTHONPATH'] == '/remove1:/remove2' # ... 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 + # If ROSE_PYTHONPATH is set we retrieve its contents and # add them to the sys.path: - monkeypatch.setenv('ROSE_PYTHONPATH', '/add-to-sys.path') + monkeypatch.setenv('ROSE_PYTHONPATH', '/add1:/add2') pythonpath_manip() - assert sys.path == ['/add-to-sys.path', '/leave-alone'] + assert sys.path == ['/add1', '/add2', '/leave-alone'] From a8acbc55da440571fc50f705a3dbe4ec36232ce6 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Tue, 28 Nov 2023 13:16:57 +0000 Subject: [PATCH 4/6] fix changelog --- CHANGES.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index ce48c8120c..fb4ad7f71b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -11,7 +11,7 @@ ones in. --> -------------------------------------------------------------------------------- -## 2.1.0 (Released 2023-07-21) +## 2.2.0 (Upcoming) ### Breaking Changes @@ -20,6 +20,8 @@ Rose now ignores `PYTHONPATH` to make it more robust to task environments which set this value. If you want to add to the Rose environment itself, e.g. to write a rose-ana test, use `ROSE_PYTHONPATH`. +## 2.1.0 (Released 2023-07-21) + ### Fixes [#2699](https://github.com/metomi/rose/pull/2699) - From 24071509412a0710d13b59b4e815b2c26f35f779 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Tue, 28 Nov 2023 14:57:48 +0000 Subject: [PATCH 5/6] Update CHANGES.md Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index fb4ad7f71b..625213baa2 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -15,7 +15,7 @@ ones in. --> ### Breaking Changes -[2736](https://github.com/metomi/rose/pull/2736) +[#2736](https://github.com/metomi/rose/pull/2736) - Rose now ignores `PYTHONPATH` to make it more robust to task environments which set this value. If you want to add to the Rose environment itself, e.g. to write a rose-ana test, use `ROSE_PYTHONPATH`. From 6c7438b93c0ddece425e0ffb8fc8fccfbf7c0a66 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Thu, 30 Nov 2023 10:53:35 +0000 Subject: [PATCH 6/6] Update metomi/rose/rose.py --- metomi/rose/rose.py | 1 - 1 file changed, 1 deletion(-) diff --git a/metomi/rose/rose.py b/metomi/rose/rose.py index 512761544e..9b0f065323 100644 --- a/metomi/rose/rose.py +++ b/metomi/rose/rose.py @@ -34,7 +34,6 @@ def pythonpath_manip(): os.path.abspath(item) for item in os.environ['ROSE_PYTHONPATH'].split(os.pathsep) ] - print(f"Extracted {paths} from ROSE_PYTHONPATH") paths.extend(sys.path) sys.path = paths