From f60c2d0def62eb689aecd6f6174dc8dd7130b42f Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 2 Aug 2022 11:54:00 +0530 Subject: [PATCH 1/6] fix: Use venv module instead of virtualenv Only migrate-env requires virtualenv wrapper. However, it can be installed and run manually too. Virtualenv wrapper is patched in debian to change the path of bins - which venv is free from. --- bench/bench.py | 11 +++-------- bench/commands/setup.py | 2 +- bench/utils/__init__.py | 8 ++------ bench/utils/bench.py | 3 +++ docs/bench_usage.md | 2 +- 5 files changed, 10 insertions(+), 16 deletions(-) diff --git a/bench/bench.py b/bench/bench.py index 3d286bf1..a331e270 100644 --- a/bench/bench.py +++ b/bench/bench.py @@ -30,7 +30,6 @@ from bench.utils.bench import ( restart_process_manager, remove_backups_crontab, get_venv_path, - get_virtualenv_path, get_env_cmd, ) from bench.utils.render import job, step @@ -71,7 +70,7 @@ class Bench(Base, Validator): @property def python(self) -> str: - return get_env_cmd("python", bench_path=self.name) + return get_env_cmd("python*", bench_path=self.name) @property def shallow_clone(self) -> bool: @@ -347,15 +346,11 @@ class BenchSetup(Base): click.secho("Setting Up Environment", fg="yellow") frappe = os.path.join(self.bench.name, "apps", "frappe") - virtualenv = get_virtualenv_path(verbose=verbose) quiet_flag = "" if verbose else "--quiet" if not os.path.exists(self.bench.python): - if virtualenv: - self.run(f"{virtualenv} {quiet_flag} env -p {python}", cwd=self.bench.name) - else: - venv = get_venv_path(verbose=verbose, python=python) - self.run(f"{venv} env", cwd=self.bench.name) + venv = get_venv_path(verbose=verbose, python=python) + self.run(f"{venv} env", cwd=self.bench.name) self.pip() diff --git a/bench/commands/setup.py b/bench/commands/setup.py index 4139e8fb..4d1fa88a 100755 --- a/bench/commands/setup.py +++ b/bench/commands/setup.py @@ -96,7 +96,7 @@ def setup_backups(): Bench(".").setup.backups() -@click.command("env", help="Setup virtualenv for bench") +@click.command("env", help="Setup Python environment for bench") @click.option( "--python", type=str, default="python3", help="Path to Python Executable." ) diff --git a/bench/utils/__init__.py b/bench/utils/__init__.py index 952ca2a4..778e5319 100644 --- a/bench/utils/__init__.py +++ b/bench/utils/__init__.py @@ -234,7 +234,7 @@ def run_frappe_cmd(*args, **kwargs): f = get_env_cmd("python", bench_path=bench_path) sites_dir = os.path.join(bench_path, "sites") - is_async = False if from_command_line else True + is_async = not from_command_line if is_async: stderr = stdout = subprocess.PIPE else: @@ -247,11 +247,7 @@ def run_frappe_cmd(*args, **kwargs): stderr=stderr, ) - if is_async: - return_code = print_output(p) - else: - return_code = p.wait() - + return_code = print_output(p) if is_async else p.wait() if return_code > 0: sys.exit(return_code) diff --git a/bench/utils/bench.py b/bench/utils/bench.py index 999915ad..f2644db7 100644 --- a/bench/utils/bench.py +++ b/bench/utils/bench.py @@ -172,6 +172,9 @@ def migrate_env(python, backup=False): path = os.getcwd() python = which(python) virtualenv = which("virtualenv") + if not virtualenv: + raise FileNotFoundError("`virtualenv` not found. Install it and try again.") + pvenv = os.path.join(path, nvenv) # Clear Cache before Bench Dies. diff --git a/docs/bench_usage.md b/docs/bench_usage.md index 7bf52a1a..07b7e080 100644 --- a/docs/bench_usage.md +++ b/docs/bench_usage.md @@ -130,7 +130,7 @@ The setup commands used for setting up the Frappe environment in context of the - **sudoers**: Add commands to sudoers list for allowing bench commands execution without root password - - **env**: Setup virtualenv for bench. This sets up a `env` folder under the root of the bench directory. + - **env**: Setup Python virtual environment for bench. This sets up a `env` folder under the root of the bench directory. - **redis**: Generates configuration for Redis - **fonts**: Add Frappe fonts to system - **config**: Generate or over-write sites/common_site_config.json From b37135e5b19888ef6bae5a79cb6d7bc0545e3097 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 2 Aug 2022 11:57:38 +0530 Subject: [PATCH 2/6] refactor: get_env_cmd * Use globbing first to identify cmd in env paths * Unbound cache which clears on chdir * Allow passing patterns like 'python*' as cmd to match --- bench/cli.py | 1 + bench/utils/bench.py | 21 ++++++++++++--------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/bench/cli.py b/bench/cli.py index b38aea30..d01b6eb2 100755 --- a/bench/cli.py +++ b/bench/cli.py @@ -245,6 +245,7 @@ def setup_clear_cache(): def _chdir(*args, **kwargs): Bench.cache_clear() + get_env_cmd.cache_clear() return f(*args, **kwargs) os.chdir = _chdir diff --git a/bench/utils/bench.py b/bench/utils/bench.py index f2644db7..a3a0d140 100644 --- a/bench/utils/bench.py +++ b/bench/utils/bench.py @@ -6,6 +6,8 @@ import os import re import subprocess import sys +from functools import lru_cache +from glob import glob from json.decoder import JSONDecodeError # imports - third party imports @@ -19,19 +21,20 @@ from bench.utils import exec_cmd, get_bench_name, get_cmd_output, log, which logger = logging.getLogger(bench.PROJECT_NAME) +@lru_cache(maxsize=None) def get_env_cmd(cmd, bench_path="."): + # this supports envs' generated by patched virtualenv or venv (which may cause an extra 'local' folder to be created) + + existing_python_bins = glob( + os.path.abspath(os.path.join(bench_path, "env", "**", "bin", cmd)), recursive=True + ) + + if existing_python_bins: + return existing_python_bins[0] + return os.path.abspath(os.path.join(bench_path, "env", "bin", cmd)) -def get_virtualenv_path(verbose=False): - virtualenv_path = which("virtualenv") - - if not virtualenv_path and verbose: - log("virtualenv cannot be found", level=2) - - return virtualenv_path - - def get_venv_path(verbose=False, python="python3"): with open(os.devnull, "wb") as devnull: is_venv_installed = not subprocess.call( From 24b9af605b08849ee506677a6c812c1e87ea3326 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 2 Aug 2022 11:59:34 +0530 Subject: [PATCH 3/6] fix: Use python* to match any pattern in env --- bench/cli.py | 6 +++--- bench/commands/make.py | 2 +- bench/utils/__init__.py | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bench/cli.py b/bench/cli.py index d01b6eb2..e2e462fa 100755 --- a/bench/cli.py +++ b/bench/cli.py @@ -190,13 +190,13 @@ def change_uid(): def app_cmd(bench_path="."): - f = get_env_cmd("python", bench_path=bench_path) + f = get_env_cmd("python*", bench_path=bench_path) os.chdir(os.path.join(bench_path, "sites")) os.execv(f, [f] + ["-m", "frappe.utils.bench_helper"] + sys.argv[1:]) def frappe_cmd(bench_path="."): - f = get_env_cmd("python", bench_path=bench_path) + f = get_env_cmd("python*", bench_path=bench_path) os.chdir(os.path.join(bench_path, "sites")) os.execv(f, [f] + ["-m", "frappe.utils.bench_helper", "frappe"] + sys.argv[1:]) @@ -216,7 +216,7 @@ def get_frappe_commands(): def get_frappe_help(bench_path="."): - python = get_env_cmd("python", bench_path=bench_path) + python = get_env_cmd("python*", bench_path=bench_path) sites_path = os.path.join(bench_path, "sites") try: out = get_cmd_output( diff --git a/bench/commands/make.py b/bench/commands/make.py index 42ce08cd..2895f79d 100755 --- a/bench/commands/make.py +++ b/bench/commands/make.py @@ -227,5 +227,5 @@ def pip(ctx, args): from bench.utils.bench import get_env_cmd - env_py = get_env_cmd("python") + env_py = get_env_cmd("python*") os.execv(env_py, (env_py, "-m", "pip") + args) diff --git a/bench/utils/__init__.py b/bench/utils/__init__.py index 778e5319..9a14920f 100644 --- a/bench/utils/__init__.py +++ b/bench/utils/__init__.py @@ -231,7 +231,7 @@ def run_frappe_cmd(*args, **kwargs): from bench.utils.bench import get_env_cmd bench_path = kwargs.get("bench_path", ".") - f = get_env_cmd("python", bench_path=bench_path) + f = get_env_cmd("python*", bench_path=bench_path) sites_dir = os.path.join(bench_path, "sites") is_async = not from_command_line @@ -386,7 +386,7 @@ def generate_command_cache(bench_path=".") -> List: """ from bench.utils.bench import get_env_cmd - python = get_env_cmd("python", bench_path=bench_path) + python = get_env_cmd("python*", bench_path=bench_path) sites_path = os.path.join(bench_path, "sites") if os.path.exists(bench_cache_file): From 78742b9546dde7f3910ce25cc03f0f9288c0ed3e Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 2 Aug 2022 12:00:08 +0530 Subject: [PATCH 4/6] refactor: Use specific lru_cache imports over entire module's --- bench/app.py | 4 ++-- bench/bench.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bench/app.py b/bench/app.py index 6406260e..aae317a3 100755 --- a/bench/app.py +++ b/bench/app.py @@ -1,5 +1,5 @@ # imports - standard imports -import functools +from functools import lru_cache import json import logging import os @@ -149,7 +149,7 @@ class AppMeta: return f"git@{self.remote_server}:{self.org}/{self.repo}.git" -@functools.lru_cache(maxsize=None) +@lru_cache(maxsize=None) class App(AppMeta): def __init__( self, diff --git a/bench/bench.py b/bench/bench.py index a331e270..8015d0a3 100644 --- a/bench/bench.py +++ b/bench/bench.py @@ -1,6 +1,6 @@ # imports - standard imports import subprocess -import functools +from functools import lru_cache import os import shutil import json @@ -54,7 +54,7 @@ class Validator: validate_app_installed_on_sites(app, bench_path=self.name) -@functools.lru_cache(maxsize=None) +@lru_cache(maxsize=None) class Bench(Base, Validator): def __init__(self, path): self.name = path From 6ae1997bffb3d75083fb5c720336d036b18b22ce Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 2 Aug 2022 12:18:40 +0530 Subject: [PATCH 5/6] fix(utils): Strip * from cmd via get_env_cmd --- bench/utils/bench.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bench/utils/bench.py b/bench/utils/bench.py index a3a0d140..98cfec2e 100644 --- a/bench/utils/bench.py +++ b/bench/utils/bench.py @@ -22,17 +22,18 @@ logger = logging.getLogger(bench.PROJECT_NAME) @lru_cache(maxsize=None) -def get_env_cmd(cmd, bench_path="."): +def get_env_cmd(cmd: str, bench_path: str = ".") -> str: # this supports envs' generated by patched virtualenv or venv (which may cause an extra 'local' folder to be created) existing_python_bins = glob( - os.path.abspath(os.path.join(bench_path, "env", "**", "bin", cmd)), recursive=True + os.path.join(bench_path, "env", "**", "bin", cmd), recursive=True ) if existing_python_bins: return existing_python_bins[0] - return os.path.abspath(os.path.join(bench_path, "env", "bin", cmd)) + cmd = cmd.strip("*") + return os.path.join(bench_path, "env", "bin", cmd) def get_venv_path(verbose=False, python="python3"): From 10473b60076dde4080ce28222031a332ee7d313a Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 2 Aug 2022 12:46:03 +0530 Subject: [PATCH 6/6] fix: Pass abs path from get_env_cmd --- bench/utils/bench.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bench/utils/bench.py b/bench/utils/bench.py index 98cfec2e..cf590e6f 100644 --- a/bench/utils/bench.py +++ b/bench/utils/bench.py @@ -30,10 +30,10 @@ def get_env_cmd(cmd: str, bench_path: str = ".") -> str: ) if existing_python_bins: - return existing_python_bins[0] + return os.path.abspath(existing_python_bins[0]) cmd = cmd.strip("*") - return os.path.join(bench_path, "env", "bin", cmd) + return os.path.abspath(os.path.join(bench_path, "env", "bin", cmd)) def get_venv_path(verbose=False, python="python3"):