Skip to content

Commit

Permalink
fix: del node_modules if frappe supports app_cache
Browse files Browse the repository at this point in the history
  • Loading branch information
18alantom committed Jan 31, 2024
1 parent d84a05e commit 9988b83
Show file tree
Hide file tree
Showing 2 changed files with 112 additions and 41 deletions.
123 changes: 93 additions & 30 deletions bench/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -170,15 +171,15 @@ def __init__(
branch: str = None,
bench: "Bench" = None,
soft_link: bool = False,
cache_key = None,
cache_key=None,
*args,
**kwargs,
):
self.bench = bench
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")
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -291,8 +292,7 @@ def update_app_state(self):
branch=self.tag,
required=self.local_resolution,
)



"""
Get App Cache
Expand All @@ -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

Expand All @@ -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)
Expand All @@ -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:
Expand All @@ -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
Expand All @@ -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:
Expand All @@ -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)

Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand All @@ -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(
Expand All @@ -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"]
Expand Down
30 changes: 19 additions & 11 deletions bench/utils/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import re
import sys
import subprocess
from typing import List
from typing import List, Optional
from functools import lru_cache

# imports - module imports
Expand Down Expand Up @@ -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
Expand All @@ -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) -> Optional[dict]:
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)
Expand Down

0 comments on commit 9988b83

Please sign in to comment.