Skip to content

Commit 82842df

Browse files
FloLeyinmantaci
authored andcommitted
Fix errors due to venv modifications (PR #7298)
# Description Fix issues introduced when modifying the PythonEnvironment class during #7107 # Self Check: Strike through any lines that are not applicable (`~~line~~`) then check the box - [ ] Attached issue to pull request - [x] Changelog entry - [ ] Type annotations are present - [ ] Code is clear and sufficiently documented - [ ] No (preventable) type errors (check using make mypy or make mypy-diff) - [x] Sufficient test cases (reproduces the bug/tests the requested feature) - [ ] Correct, in line with design - [ ] End user documentation is included or an issue is created for end-user documentation (add ref to issue here: ) - [ ] If this PR fixes a race condition in the test suite, also push the fix to the relevant stable branche(s) (see [test-fixes](https://internal.inmanta.com/development/core/tasks/build-master.html#test-fixes) for more info)
1 parent 0009175 commit 82842df

File tree

3 files changed

+30
-26
lines changed

3 files changed

+30
-26
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
description: Fix errors due to venv modifications
2+
change-type: patch
3+
destination-branches: [master]

src/inmanta/env.py

+26-25
Original file line numberDiff line numberDiff line change
@@ -548,11 +548,14 @@ class PythonEnvironment:
548548
Inmanta product packages don't change.
549549
"""
550550

551+
_invalid_chars_in_path_re = re.compile(r'["$`]')
552+
551553
def __init__(self, *, env_path: Optional[str] = None, python_path: Optional[str] = None) -> None:
552554
if (env_path is None) == (python_path is None):
553555
raise ValueError("Exactly one of `env_path` and `python_path` needs to be specified")
554556
self.env_path: str
555557
self.python_path: str
558+
self._parent_python: Optional[str] = None
556559
if env_path is not None:
557560
self.env_path = env_path
558561
self.python_path = self.get_python_path_for_env_path(self.env_path)
@@ -564,9 +567,31 @@ def __init__(self, *, env_path: Optional[str] = None, python_path: Optional[str]
564567
self.env_path = self.get_env_path_for_python_path(self.python_path)
565568
if not self.python_path:
566569
raise ValueError("The python_path cannot be an empty string.")
570+
self.validate_path(self.env_path)
567571
self.site_packages_dir: str = self.get_site_dir_for_env_path(self.env_path)
568572
self._path_pth_file = os.path.join(self.site_packages_dir, "inmanta-inherit-from-parent-venv.pth")
569573

574+
def validate_path(self, path: str) -> None:
575+
"""
576+
The given path is used in the `./bin/activate` file of the created venv without escaping any special characters.
577+
As such, we refuse all special characters here that might cause the given path to be interpreted incorrectly:
578+
579+
* $: Character used for variable expansion in bash strings.
580+
* `: Character used to perform command substitution in bash strings.
581+
* ": Character that will be interpreted incorrectly as the end of the string.
582+
583+
:param path: Path to validate.
584+
"""
585+
if not path:
586+
raise ValueError("Cannot create virtual environment because the provided path is an empty string.")
587+
588+
match = PythonEnvironment._invalid_chars_in_path_re.search(path)
589+
if match:
590+
raise ValueError(
591+
f"Cannot create virtual environment because the provided path `{path}` contains an"
592+
f" invalid character (`{match.group()}`)."
593+
)
594+
570595
@classmethod
571596
def get_python_path_for_env_path(cls, env_path: str) -> str:
572597
"""
@@ -603,6 +628,7 @@ def init_env(self) -> None:
603628
"""
604629
Initialize the virtual environment.
605630
"""
631+
self._parent_python = sys.executable
606632
LOGGER.info("Initializing virtual environment at %s", self.env_path)
607633

608634
# check if the virtual env exists
@@ -1171,36 +1197,11 @@ class VirtualEnv(ActiveEnv):
11711197
Creates and uses a virtual environment for this process. This virtualenv inherits from the previously active one.
11721198
"""
11731199

1174-
_invalid_chars_in_path_re = re.compile(r'["$`]')
1175-
11761200
def __init__(self, env_path: str) -> None:
11771201
super().__init__(env_path=env_path)
1178-
self.validate_path(env_path)
11791202
self.env_path: str = env_path
11801203
self.virtual_python: Optional[str] = None
11811204
self._using_venv: bool = False
1182-
self._parent_python: Optional[str] = None
1183-
1184-
def validate_path(self, path: str) -> None:
1185-
"""
1186-
The given path is used in the `./bin/activate` file of the created venv without escaping any special characters.
1187-
As such, we refuse all special characters here that might cause the given path to be interpreted incorrectly:
1188-
1189-
* $: Character used for variable expansion in bash strings.
1190-
* `: Character used to perform command substitution in bash strings.
1191-
* ": Character that will be interpreted incorrectly as the end of the string.
1192-
1193-
:param path: Path to validate.
1194-
"""
1195-
if not path:
1196-
raise ValueError("Cannot create virtual environment because the provided path is an empty string.")
1197-
1198-
match = VirtualEnv._invalid_chars_in_path_re.search(path)
1199-
if match:
1200-
raise ValueError(
1201-
f"Cannot create virtual environment because the provided path `{path}` contains an"
1202-
f" invalid character (`{match.group()}`)."
1203-
)
12041205

12051206
def exists(self) -> bool:
12061207
"""

tests/test_env.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -680,7 +680,7 @@ def test_basic_logging(tmpdir, caplog):
680680

681681
log_sequence = LogSequence(caplog)
682682
log_sequence.assert_not("inmanta.env", logging.INFO, f"Creating new virtual environment in {env_dir1}")
683-
log_sequence.contains("inmanta.env", logging.INFO, f"Using virtual environment at {env_dir1}")
683+
log_sequence.contains("inmanta.env", logging.INFO, f"Initializing virtual environment at {env_dir1}")
684684

685685

686686
@pytest.mark.slowtest

0 commit comments

Comments
 (0)