Merge pull request #1304 from Aradhya-Tripathi/fixes

fix: removed unnecessary requests while removing apps

bench/app.py

@@ -11,6 +11,7 @@ import typing
 from collections import OrderedDict
 from datetime import date
 from urllib.parse import urlparse
+import os
 
 # imports - third party imports
 import click
@@ -66,6 +67,8 @@ class AppMeta:
 		self.from_apps = False
 		self.is_url = False
 		self.branch = branch
+		self.app_name = None
+		self.git_repo = None
 		self.mount_path = os.path.abspath(
 			os.path.join(urlparse(self.name).netloc, urlparse(self.name).path)
 		)
@@ -73,11 +76,10 @@ class AppMeta:
 
 	def setup_details(self):
 		# fetch meta from installed apps
-		if (
+		if self.bench and os.path.exists(
-			not self.to_clone
+			os.path.join(self.bench.name, "apps", self.name)
-			and hasattr(self, "bench")
-			and os.path.exists(self.mount_path)
 		):
+			self.mount_path = os.path.join(self.bench.name, "apps", self.name)
 			self.from_apps = True
 			self._setup_details_from_mounted_disk()
 
@@ -95,6 +97,13 @@ class AppMeta:
 		else:
 			self._setup_details_from_name_tag()
 
+		if self.git_repo:
+			self.app_name = os.path.basename(
+				os.path.normpath(self.git_repo.working_tree_dir)
+			)
+		else:
+			self.app_name = self.repo
+
 	def _setup_details_from_mounted_disk(self):
 		# If app is a git repo
 		self.git_repo = Repo(self.mount_path)
@@ -116,7 +125,7 @@ class AppMeta:
 		name = url if url else self.name
 		if name.startswith("git@") or name.startswith("ssh://"):
 			self.use_ssh = True
-			_first_part, _second_part = self.name.rsplit(":", 1)
+			_first_part, _second_part = name.rsplit(":", 1)
 			self.remote_server = _first_part.split("@")[-1]
 			self.org, _repo = _second_part.rsplit("/", 1)
 		else:
@@ -187,7 +196,7 @@ class App(AppMeta):
 		from bench.utils.app import get_app_name
 
 		verbose = bench.cli.verbose or verbose
-		app_name = get_app_name(self.bench.name, self.repo)
+		app_name = get_app_name(self.bench.name, self.app_name)
 		if not resolved and self.repo != "frappe" and not ignore_resolution:
 			click.secho(
 				f"Ignoring dependencies of {self.name}. To install dependencies use --resolve-deps",
@@ -231,7 +240,9 @@ class App(AppMeta):
 	def update_app_state(self):
 		from bench.bench import Bench
 		bench = Bench(self.bench.name)
-		bench.apps.sync(self.name, self.tag, self.local_resolution)
+		bench.apps.sync(app_dir=self.app_name, app_name=self.name,
+						branch=self.tag, required_list=self.local_resolution)
+
 
 
 def make_resolution_plan(app: App, bench: "Bench"):
@@ -540,7 +551,7 @@ def install_app(
 	if os.path.exists(os.path.join(app_path, "package.json")):
 		bench.run("yarn install", cwd=app_path)
 
-	bench.apps.sync(app, required=resolution, branch=tag)
+	bench.apps.sync(app_name=app, required=resolution, branch=tag, app_dir=app_path)
 
 	if not skip_assets:
 		build_assets(bench_path=bench_path, app=app)

bench/bench.py

@@ -170,7 +170,13 @@ class BenchApps(MutableSequence):
 		except FileNotFoundError:
 			self.states = {}
 
-	def update_apps_states(self, app_name: Union[str, None] = None, branch: Union[str, None] = None, required:List = []):
+	def update_apps_states(
+			self,
+			app_dir: str = None,
+			app_name: Union[str, None] = None,
+			branch: Union[str, None] = None,
+			required: List = [],
+	):
 		if self.apps and not os.path.exists(self.states_path):
 			# idx according to apps listed in apps.txt (backwards compatibility)
 			# Keeping frappe as the first app.
@@ -200,10 +206,13 @@ class BenchApps(MutableSequence):
 		for app in apps_to_remove:
 			del self.states[app]
 
+		if app_name and not app_dir:
+			app_dir = app_name
+
 		if app_name and app_name not in self.states:
 			version = get_current_version(app_name, self.bench.name)
 
-			app_dir = os.path.join(self.apps_path, app_name)
+			app_dir = os.path.join(self.apps_path, app_dir)
 			if not branch:
 				branch = (
 						subprocess
@@ -227,11 +236,24 @@ class BenchApps(MutableSequence):
 		with open(self.states_path, "w") as f:
 			f.write(json.dumps(self.states, indent=4))
 
-	def sync(self,app_name: Union[str, None] = None, branch: Union[str, None] = None, required:List = []):
+	def sync(
+		self,
+		app_name: Union[str, None] = None,
+		app_dir: Union[str, None] = None,
+		branch: Union[str, None] = None,
+		required: List = []
+	):
 		self.initialize_apps()
+
 		with open(self.bench.apps_txt, "w") as f:
 			f.write("\n".join(self.apps))
-		self.update_apps_states(app_name, branch, required)
+
+		self.update_apps_states(
+			app_name=app_name,
+			app_dir=app_dir,
+			branch=branch,
+			required=required
+		)
 
 	def initialize_apps(self):
 		is_installed = lambda app: app in installed_packages
@@ -413,7 +435,7 @@ class BenchSetup(Base):
 
 		for app in apps:
 			path_to_app = os.path.join(self.bench.name, "apps", app)
-			App(path_to_app, bench=self.bench, to_clone=False).install(
+			app = App(path_to_app, bench=self.bench, to_clone=False).install(
 				skip_assets=True, restart_bench=False, ignore_resolution=True
 			)
 

bench/tests/test_init.py

@@ -12,6 +12,7 @@ from bench.utils import exec_cmd
 from bench.release import get_bumped_version
 from bench.app import App
 from bench.tests.test_base import FRAPPE_BRANCH, TestBenchBase
+from bench.bench import Bench
 
 
 # changed from frappe_theme because it wasn't maintained and incompatible,
@@ -43,7 +44,9 @@ class TestBenchInit(TestBenchBase):
 		self.assert_folders(bench_name)
 		self.assert_virtual_env(bench_name)
 		self.assert_config(bench_name)
-
+		test_bench = Bench(bench_name)
+		app = App("frappe", bench=test_bench)
+		self.assertEqual(app.from_apps, True)
 
 	def basic(self):
 		try:

bench/tests/test_utils.py

@@ -66,7 +66,7 @@ class TestUtils(unittest.TestCase):
 		subprocess.run(["git", "add", "."], cwd=frappe_path, capture_output=True, check=True)
 		subprocess.run(["git", "commit", "-m", "temp"], cwd=frappe_path, capture_output=True, check=True)
 
-		fake_bench.apps.update_apps_states("frappe")
+		fake_bench.apps.update_apps_states(app_name="frappe")
 
 		self.assertIn("frappe", fake_bench.apps.states)
 		self.assertIn("version", fake_bench.apps.states["frappe"])

bench/utils/app.py

@@ -210,25 +210,25 @@ def get_remote(app, bench_path="."):
 		return contents.splitlines()[0].split()[0]
 
 
-def get_app_name(bench_path, repo_name):
+def get_app_name(bench_path, folder_name):
 	app_name = None
 	apps_path = os.path.join(os.path.abspath(bench_path), "apps")
-	config_path = os.path.join(apps_path, repo_name, "setup.cfg")
+	config_path = os.path.join(apps_path, folder_name, "setup.cfg")
 	if os.path.exists(config_path):
 		config = read_configuration(config_path)
 		app_name = config.get("metadata", {}).get("name")
 
 	if not app_name:
 		# retrieve app name from setup.py as fallback
-		app_path = os.path.join(apps_path, repo_name, "setup.py")
+		app_path = os.path.join(apps_path, folder_name, "setup.py")
 		with open(app_path, "rb") as f:
 			app_name = re.search(r'name\s*=\s*[\'"](.*)[\'"]', f.read().decode("utf-8")).group(1)
 
-	if app_name and repo_name != app_name:
+	if app_name and folder_name != app_name:
-		os.rename(os.path.join(apps_path, repo_name), os.path.join(apps_path, app_name))
+		os.rename(os.path.join(apps_path, folder_name), os.path.join(apps_path, app_name))
 		return app_name
 
-	return repo_name
+	return folder_name
 
 def check_existing_dir(bench_path, repo_name):
 	cloned_path = os.path.join(bench_path, "apps", repo_name)