Skip to content

Commit

Permalink
fix: Environment variable substitution in cmd scripts
Browse files Browse the repository at this point in the history
Fixes #2542

Signed-off-by: Frost Ming <[email protected]>
  • Loading branch information
frostming committed Jan 9, 2024
1 parent 6471521 commit 55385e6
Show file tree
Hide file tree
Showing 9 changed files with 49 additions and 23 deletions.
1 change: 0 additions & 1 deletion news/2488.bugfix.md

This file was deleted.

1 change: 0 additions & 1 deletion news/2495.bugfix.md

This file was deleted.

1 change: 0 additions & 1 deletion news/2502.bugfix.md

This file was deleted.

1 change: 0 additions & 1 deletion news/2507.bugfix.md

This file was deleted.

1 change: 0 additions & 1 deletion news/2510.doc.md

This file was deleted.

1 change: 1 addition & 0 deletions news/2542.bugfix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix the environment variable substitution for `cmd` scripts.
23 changes: 11 additions & 12 deletions src/pdm/cli/commands/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from pdm.exceptions import PdmUsageError
from pdm.project import Project
from pdm.signals import pdm_signals
from pdm.utils import is_path_relative_to
from pdm.utils import expand_env_vars, is_path_relative_to

if TYPE_CHECKING:
from typing import Any, Callable, Iterator, TypedDict
Expand All @@ -44,8 +44,8 @@ def exec_opts(*options: TaskOptions | None) -> dict[str, Any]:
)


RE_ARGS_PLACEHOLDER = re.compile(r"{args(?::(?P<default>[^}]*))?}")
RE_PDM_PLACEHOLDER = re.compile(r"{pdm}")
RE_ARGS_PLACEHOLDER = re.compile(r"\{args(?::(?P<default>[^}]*))?\}")
RE_PDM_PLACEHOLDER = re.compile(r"\{pdm\}")


def _interpolate_args(script: str, args: Sequence[str]) -> tuple[str, bool]:
Expand All @@ -63,11 +63,9 @@ def replace(m: re.Match[str]) -> str:
def _interpolate_pdm(script: str) -> str:
"""Interpolate the `{pdm} placeholder in a string"""
executable_path = Path(sys.executable)
pdm_executable = shlex.join([executable_path.as_posix(), "-m", "pdm"])

def replace(m: re.Match[str]) -> str:
return shlex.join([executable_path.as_posix(), "-m", "pdm"])

interpolated = RE_PDM_PLACEHOLDER.sub(replace, script)
interpolated = RE_PDM_PLACEHOLDER.sub(pdm_executable, script)
return interpolated


Expand Down Expand Up @@ -141,7 +139,7 @@ def get_task(self, script_name: str) -> Task | None:
return Task(kind, script_name, value, cast("TaskOptions", options))

def expand_command(self, command: str) -> str:
expanded_command = os.path.expanduser(os.path.expandvars(command))
expanded_command = os.path.expanduser(command)
if expanded_command.replace(os.sep, "/").startswith(("./", "../")):
abspath = os.path.abspath(expanded_command)
if not os.path.isfile(abspath):
Expand Down Expand Up @@ -191,16 +189,17 @@ def _run_process(
process_env.update(env)
if shell:
assert isinstance(args, str)
expanded_args: str | Sequence[str] = os.path.expandvars(args)
# environment variables will be expanded by shell
process_cmd: str | Sequence[str] = args
else:
assert isinstance(args, Sequence)
command, *args = args
command, *args = (expand_env_vars(arg, env=process_env) for arg in args)
if command.endswith(".py"):
args = [command, *args]
command = str(project.environment.interpreter.executable)
expanded_command = self.expand_command(command)
real_command = os.path.realpath(expanded_command)
expanded_args = [os.path.expandvars(arg) for arg in [expanded_command, *args]]
process_cmd = [expanded_command, *args]
if (
project_env.is_local
and not site_packages
Expand All @@ -222,7 +221,7 @@ def forward_signal(signum: int, frame: FrameType | None) -> None:

handle_term = signal.signal(signal.SIGTERM, forward_signal)
handle_int = signal.signal(signal.SIGINT, forward_signal)
process = subprocess.Popen(expanded_args, cwd=cwd, env=process_env, shell=shell, bufsize=0)
process = subprocess.Popen(process_cmd, cwd=cwd, env=process_env, shell=shell, bufsize=0)
process.wait()
signal.signal(signal.SIGTERM, handle_term)
signal.signal(signal.SIGINT, handle_int)
Expand Down
10 changes: 6 additions & 4 deletions src/pdm/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import warnings
from os import name as os_name
from pathlib import Path
from typing import TYPE_CHECKING
from typing import TYPE_CHECKING, Mapping

from packaging.version import Version, _cmpkey

Expand Down Expand Up @@ -221,17 +221,19 @@ def path_to_url(path: str) -> str:
return url


def expand_env_vars(credential: str, quote: bool = False) -> str:
def expand_env_vars(credential: str, quote: bool = False, env: Mapping[str, str] | None = None) -> str:
"""A safe implementation of env var substitution.
It only supports the following forms:
${ENV_VAR}
Neither $ENV_VAR and %ENV_VAR is not supported.
Neither $ENV_VAR and %ENV_VAR is supported.
"""
if env is None:
env = os.environ

def replace_func(match: Match) -> str:
rv = os.getenv(match.group(1), match.group(0))
rv = env.get(match.group(1), match.group(0))
return parse.quote(rv) if quote else rv

return re.sub(r"\$\{(.+?)\}", replace_func, credential)
Expand Down
33 changes: 31 additions & 2 deletions tests/cli/test_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,8 @@ def test_run_shell_script_with_pdm_placeholder(project, pdm):
def test_run_expand_env_vars(project, pdm, capfd, monkeypatch):
(project.root / "test_script.py").write_text("import os; print(os.getenv('FOO'))")
project.pyproject.settings["scripts"] = {
"test_cmd": 'python -c "foo, bar = 0, 1;print($FOO)"',
"test_cmd_no_expand": "python -c 'print($FOO)'",
"test_cmd": 'python -c "foo, bar = 0, 1;print(${FOO})"',
"test_cmd_no_expand": "python -c 'print(${FOO})'",
"test_script": "python test_script.py",
"test_cmd_array": ["python", "test_script.py"],
"test_shell": {"shell": "echo $FOO"},
Expand All @@ -351,6 +351,35 @@ def test_run_expand_env_vars(project, pdm, capfd, monkeypatch):
assert capfd.readouterr()[0].strip() == "bar"


def test_run_expand_env_vars_from_config(project, pdm, capfd):
(project.root / "test_script.py").write_text("import os; print(os.getenv('FOO'))")
project.pyproject.settings["scripts"] = {
"test_cmd": 'python -c "foo, bar = 0, 1;print(${FOO})"',
"test_cmd_no_expand": "python -c 'print(${FOO})'",
"test_script": "python test_script.py",
"test_cmd_array": ["python", "test_script.py"],
"test_shell": {"shell": "echo $FOO"},
"_": {"env": {"FOO": "bar"}},
}
project.pyproject.write()
capfd.readouterr()
with cd(project.root):
pdm(["run", "test_cmd"], obj=project)
assert capfd.readouterr()[0].strip() == "1"

result = pdm(["run", "test_cmd_no_expand"], obj=project)
assert result.exit_code == 1

pdm(["run", "test_script"], obj=project)
assert capfd.readouterr()[0].strip() == "bar"

pdm(["run", "test_cmd_array"], obj=project)
assert capfd.readouterr()[0].strip() == "bar"

pdm(["run", "test_shell"], obj=project)
assert capfd.readouterr()[0].strip() == "bar"


def test_run_script_with_env_defined(project, pdm, capfd):
(project.root / "test_script.py").write_text("import os; print(os.getenv('FOO'))")
project.pyproject.settings["scripts"] = {"test_script": {"cmd": "python test_script.py", "env": {"FOO": "bar"}}}
Expand Down

0 comments on commit 55385e6

Please sign in to comment.