From e318cb0a70d9598e306187c475388d9823ea20a3 Mon Sep 17 00:00:00 2001 From: 18alantom <2.alan.tom@gmail.com> Date: Wed, 31 Jan 2024 17:30:03 +0530 Subject: [PATCH] fix: del node_modules if frappe supports app_cache --- bench/app.py | 123 ++++++++++++++++++++++++++++++++++----------- bench/utils/app.py | 30 +++++++---- 2 files changed, 112 insertions(+), 41 deletions(-) diff --git a/bench/app.py b/bench/app.py index cf4378985..72cdd3a2c 100755 --- a/bench/app.py +++ b/bench/app.py @@ -12,6 +12,7 @@ from datetime import date from functools import lru_cache from pathlib import Path +from typing import Optional from urllib.parse import urlparse # imports - third party imports @@ -170,7 +171,7 @@ def __init__( branch: str = None, bench: "Bench" = None, soft_link: bool = False, - cache_key = None, + cache_key=None, *args, **kwargs, ): @@ -178,7 +179,7 @@ def __init__( self.soft_link = soft_link self.required_by = None self.local_resolution = [] - self.cache_key = cache_key + self.cache_key = cache_key super().__init__(name, branch, *args, **kwargs) @step(title="Fetching App {repo}", success="App {repo} Fetched") @@ -233,7 +234,7 @@ def install( resolved=False, restart_bench=True, ignore_resolution=False, - using_cached=False + using_cached=False, ): import bench.cli from bench.utils.app import get_app_name @@ -291,8 +292,7 @@ def update_app_state(self): branch=self.tag, required=self.local_resolution, ) - - + """ Get App Cache @@ -310,7 +310,7 @@ def update_app_state(self): Code that updates the `env` and `sites` subdirs still need to be run. """ - + def get_app_path(self) -> Path: return Path(self.bench.name) / "apps" / self.app_name @@ -321,14 +321,14 @@ def get_app_cache_path(self, is_compressed=False) -> Path: ext = "tgz" if is_compressed else "tar" tarfile_name = f"{self.app_name}-{self.cache_key[:10]}.{ext}" return cache_path / tarfile_name - + def get_cached(self) -> bool: if not self.cache_key: return False - + cache_path = self.get_app_cache_path() mode = "r" - + # Check if cache exists without gzip if not cache_path.is_file(): cache_path = self.get_app_cache_path(True) @@ -341,7 +341,7 @@ def get_cached(self) -> bool: app_path = self.get_app_path() if app_path.is_dir(): shutil.rmtree(app_path) - + click.secho(f"Getting {self.app_name} from cache", fg="yellow") with tarfile.open(cache_path, mode) as tar: try: @@ -352,7 +352,7 @@ def get_cached(self) -> bool: return False return True - + def set_cache(self, compress_artifacts=False) -> bool: if not self.cache_key: return False @@ -363,15 +363,15 @@ def set_cache(self, compress_artifacts=False) -> bool: cwd = os.getcwd() cache_path = self.get_app_cache_path(compress_artifacts) - mode = "w:gz" if compress_artifacts else "w" - + mode = "w:gz" if compress_artifacts else "w" + message = f"Caching {self.app_name} app directory" if compress_artifacts: message += " (compressed)" click.secho(message) self.prune_app_directory() - + success = False os.chdir(app_path.parent) try: @@ -384,44 +384,100 @@ def set_cache(self, compress_artifacts=False) -> bool: finally: os.chdir(cwd) return success - + def prune_app_directory(self): app_path = self.get_app_path() - remove_unused_node_modules(app_path) - + if can_frappe_use_cached(self): + remove_unused_node_modules(app_path) + + +def can_frappe_use_cached(app: App) -> bool: + min_frappe = get_required_frappe_version(app) + if not min_frappe: + return False + + import semantic_version as sv + + try: + return sv.Version(min_frappe) in sv.SimpleSpec(">=15.12.0") + except ValueError: + # Passed value is not a version string, it's an expression + pass + + try: + """ + 15.12.0 is the first version to support USING_CACHED, + but there is no way to check the last version without + support. So it's not possible to have a ">" filter. + + Hence this excludes the first supported version. + """ + return sv.Version("15.12.0") not in sv.SimpleSpec(min_frappe) + except ValueError: + click.secho( + f"Invalid value found for frappe version '{min_frappe}'", + fg="yellow" + ) + # Invalid expression + return False + + +def get_required_frappe_version(app: App) -> Optional[str]: + from bench.utils.app import get_pyproject + + apps_path = os.path.join(os.path.abspath(app.bench.name), "apps") + pyproject = get_pyproject(apps_path, app.app_name) + + # Reference: https://github.com/frappe/bench/issues/1524 + req_frappe = ( + pyproject.get("tool", {}) + .get("bench", {}) + .get("frappe-dependencies", {}) + .get("frappe") + ) + + if not req_frappe: + click.secho( + "Required frappe version not set in pyproject.toml, " + "please refer: https://github.com/frappe/bench/issues/1524", + fg="yellow", + ) + + return req_frappe + def remove_unused_node_modules(app_path: Path) -> None: """ Erring a bit the side of caution; since there is no explicit way to check if node_modules are utilized, this function checks if Vite is being used to build the frontend code. - + Since most popular Frappe apps use Vite to build their frontends, this method should suffice. - + Note: root package.json is ignored cause those usually belong to apps that do not have a build step and so their node_modules are utilized during runtime. """ - + for p in app_path.iterdir(): if not p.is_dir(): continue - + package_json = p / "package.json" if not package_json.is_file(): continue - + node_modules = p / "node_modules" if not node_modules.is_dir(): continue - + can_delete = False with package_json.open("r", encoding="utf-8") as f: package_json = json.loads(f.read()) build_script = package_json.get("scripts", {}).get("build", "") can_delete = "vite build" in build_script - + if can_delete: shutil.rmtree(node_modules) @@ -503,14 +559,16 @@ def get_app( from bench.utils.app import check_existing_dir bench = Bench(bench_path) - app = App(git_url, branch=branch, bench=bench, soft_link=soft_link, cache_key=cache_key) + app = App( + git_url, branch=branch, bench=bench, soft_link=soft_link, cache_key=cache_key + ) git_url = app.url repo_name = app.repo branch = app.tag bench_setup = False restart_bench = not init_bench frappe_path, frappe_branch = None, None - + if resolve_deps: resolution = make_resolution_plan(app, bench) click.secho("Following apps will be installed", fg="bright_blue") @@ -560,9 +618,14 @@ def get_app( verbose=verbose, ) return - + if app.get_cached(): - app.install(verbose=verbose, skip_assets=skip_assets, restart_bench=restart_bench, using_cached=True) + app.install( + verbose=verbose, + skip_assets=skip_assets, + restart_bench=restart_bench, + using_cached=True, + ) return dir_already_exists, cloned_path = check_existing_dir(bench_path, repo_name) @@ -589,9 +652,8 @@ def get_app( or click.confirm("Do you want to reinstall the existing application?") ): app.install(verbose=verbose, skip_assets=skip_assets, restart_bench=restart_bench) - - app.set_cache(compress_artifacts) + app.set_cache(compress_artifacts) def install_resolved_deps( @@ -602,6 +664,7 @@ def install_resolved_deps( verbose=False, ): from bench.utils.app import check_existing_dir + if "frappe" in resolution: # Terminal dependency del resolution["frappe"] diff --git a/bench/utils/app.py b/bench/utils/app.py index 75891d5bf..ee2546f57 100644 --- a/bench/utils/app.py +++ b/bench/utils/app.py @@ -4,7 +4,7 @@ import re import sys import subprocess -from typing import List +from typing import List, Any from functools import lru_cache # imports - module imports @@ -230,18 +230,12 @@ def get_app_name(bench_path: str, folder_name: str) -> str: app_name = None apps_path = os.path.join(os.path.abspath(bench_path), "apps") - pyproject_path = os.path.join(apps_path, folder_name, "pyproject.toml") config_py_path = os.path.join(apps_path, folder_name, "setup.cfg") setup_py_path = os.path.join(apps_path, folder_name, "setup.py") - - if os.path.exists(pyproject_path): - try: - from tomli import load - except ImportError: - from tomllib import load - - with open(pyproject_path, "rb") as f: - app_name = load(f).get("project", {}).get("name") + + pyproject = get_pyproject(apps_path, folder_name) + if pyproject: + app_name = pyproject.get("project", {}).get("name") if not app_name and os.path.exists(config_py_path): from setuptools.config import read_configuration @@ -261,6 +255,20 @@ def get_app_name(bench_path: str, folder_name: str) -> str: return folder_name +def get_pyproject(apps_path:str, folder_name:str) -> dict[str, Any] | None: + pyproject_path = os.path.join(apps_path, folder_name, "pyproject.toml") + if not os.path.exists(pyproject_path): + return None + + try: + from tomli import load + except ImportError: + from tomllib import load + + with open(pyproject_path, "rb") as f: + return load(f) + + def check_existing_dir(bench_path, repo_name): cloned_path = os.path.join(bench_path, "apps", repo_name) dir_already_exists = os.path.isdir(cloned_path)