Update the subvolume menu - fix for #1278 (#1297)

* Update subvolume

* Add mypy compliance

Co-authored-by: Daniel Girtler <girtler.daniel@gmail.com>
Co-authored-by: Anton Hvornum <anton@hvornum.se>

1 files changed, 32 insertions, 62 deletions

diff --git a/archinstall/lib/disk/btrfs/btrfs_helpers.py b/archinstall/lib/disk/btrfs/btrfs_helpers.py
index 5fa94314..ab528388 100644
--- a/archinstall/lib/disk/btrfs/btrfs_helpers.py
+++ b/archinstall/lib/disk/btrfs/btrfs_helpers.py
@@ -1,72 +1,42 @@
-import pathlib
 import logging
-from typing import Optional
+from pathlib import Path
+from typing import Optional, Dict, Any, TYPE_CHECKING
 
+from ...models.subvolume import Subvolume
 from ...exceptions import SysCallError, DiskError
 from ...general import SysCommand
 from ...output import log
 from ..helpers import get_mount_info
-from .btrfssubvolume import BtrfsSubvolume
+from .btrfssubvolumeinfo import BtrfsSubvolumeInfo
 
+if TYPE_CHECKING:
+	from .btrfspartition import BTRFSPartition
+	from ...installer import Installer
 
-def mount_subvolume(installation, device, name, subvolume_information):
-	# we normalize the subvolume name (getting rid of slash at the start if exists. In our implementation has no semantic load.
-	# Every subvolume is created from the top of the hierarchy- and simplifies its further use
-	name = name.lstrip('/')
-
-	# renormalize the right hand.
-	mountpoint = subvolume_information.get('mountpoint', None)
-	if not mountpoint:
-		return None
-
-	if type(mountpoint) == str:
-		mountpoint = pathlib.Path(mountpoint)
 
-	installation_target = installation.target
-	if type(installation_target) == str:
-		installation_target = pathlib.Path(installation_target)
+def mount_subvolume(installation: 'Installer', device: 'BTRFSPartition', subvolume: Subvolume):
+	# we normalize the subvolume name (getting rid of slash at the start if exists.
+	# In our implementation has no semantic load.
+	# Every subvolume is created from the top of the hierarchy- and simplifies its further use
+	name = subvolume.name.lstrip('/')
+	mountpoint = Path(subvolume.mountpoint)
+	installation_target = Path(installation.target)
 
 	mountpoint = installation_target / mountpoint.relative_to(mountpoint.anchor)
 	mountpoint.mkdir(parents=True, exist_ok=True)
-
-	mount_options = subvolume_information.get('options', [])
-	if not any('subvol=' in x for x in mount_options):
-		mount_options += [f'subvol={name}']
+	mount_options = subvolume.options + [f'subvol={name}']
 
 	log(f"Mounting subvolume {name} on {device} to {mountpoint}", level=logging.INFO, fg="gray")
 	SysCommand(f"mount {device.path} {mountpoint} -o {','.join(mount_options)}")
 
 
-def setup_subvolumes(installation, partition_dict):
-	"""
-	Taken from: ..user_guides.py
-
-	partition['btrfs'] = {
-		"subvolumes" : {
-			"@":           "/",
-			"@home":       "/home",
-			"@log":        "/var/log",
-			"@pkg":        "/var/cache/pacman/pkg",
-			"@.snapshots": "/.snapshots"
-		}
-	}
-	"""
+def setup_subvolumes(installation: 'Installer', partition_dict: Dict[str, Any]):
 	log(f"Setting up subvolumes: {partition_dict['btrfs']['subvolumes']}", level=logging.INFO, fg="gray")
-	for name, right_hand in partition_dict['btrfs']['subvolumes'].items():
+
+	for subvolume in partition_dict['btrfs']['subvolumes']:
 		# we normalize the subvolume name (getting rid of slash at the start if exists. In our implementation has no semantic load.
 		# Every subvolume is created from the top of the hierarchy- and simplifies its further use
-		name = name.lstrip('/')
-
-		# renormalize the right hand.
-		# mountpoint = None
-		subvol_options = []
-
-		match right_hand:
-			# case str(): # backwards-compatability
-			# 	mountpoint = right_hand
-			case dict():
-				# mountpoint = right_hand.get('mountpoint', None)
-				subvol_options = right_hand.get('options', [])
+		name = subvolume.name.lstrip('/')
 
 		# We create the subvolume using the BTRFSPartition instance.
 		# That way we ensure not only easy access, but also accurate mount locations etc.
@@ -76,27 +46,25 @@ def setup_subvolumes(installation, partition_dict):
 		# It will be the main cause of creation of subvolumes which are not to be mounted
 		# it is not an options which can be established by subvolume (but for whole file systems), and can be
 		# set up via a simple attribute change in a directory (if empty). And here the directories are brand new
-		if 'nodatacow' in subvol_options:
+		if subvolume.nodatacow:
 			if (cmd := SysCommand(f"chattr +C {installation.target}/{name}")).exit_code != 0:
 				raise DiskError(f"Could not set  nodatacow attribute at {installation.target}/{name}: {cmd}")
-			# entry is deleted so nodatacow doesn't propagate to the mount options
-			del subvol_options[subvol_options.index('nodatacow')]
+
 		# Make the compress processing now
 		# it is not an options which can be established by subvolume (but for whole file systems), and can be
 		# set up via a simple attribute change in a directory (if empty). And here the directories are brand new
 		# in this way only zstd compression is activaded
 		# TODO WARNING it is not clear if it should be a standard feature, so it might need to be deactivated
 
-		if 'compress' in subvol_options:
+		if subvolume.compress:
 			if not any(['compress' in filesystem_option for filesystem_option in partition_dict.get('filesystem', {}).get('mount_options', [])]):
 				if (cmd := SysCommand(f"chattr +c {installation.target}/{name}")).exit_code != 0:
 					raise DiskError(f"Could not set compress attribute at {installation.target}/{name}: {cmd}")
-			# entry is deleted so compress doesn't propagate to the mount options
-			del subvol_options[subvol_options.index('compress')]
 
-def subvolume_info_from_path(path :pathlib.Path) -> Optional[BtrfsSubvolume]:
+
+def subvolume_info_from_path(path: Path) -> Optional[BtrfsSubvolumeInfo]:
 	try:
-		subvolume_name = None
+		subvolume_name = ''
 		result = {}
 		for index, line in enumerate(SysCommand(f"btrfs subvolume show {path}")):
 			if index == 0:
@@ -110,14 +78,14 @@ def subvolume_info_from_path(path :pathlib.Path) -> Optional[BtrfsSubvolume]:
 				# allows for hooking in a pre-processor to do this we have to do it here:
 				result[key.lower().replace(' ', '_').replace('(s)', 's')] = value.strip()
 
-		return BtrfsSubvolume(**{'full_path' : path, 'name' : subvolume_name, **result})
-
+		return BtrfsSubvolumeInfo(**{'full_path' : path, 'name' : subvolume_name, **result})  # type: ignore
 	except SysCallError as error:
 		log(f"Could not retrieve subvolume information from {path}: {error}", level=logging.WARNING, fg="orange")
 
 	return None
 
-def find_parent_subvolume(path :pathlib.Path, filters=[]):
+
+def find_parent_subvolume(path: Path, filters=[]) -> Optional[BtrfsSubvolumeInfo]:
 	# A root path cannot have a parent
 	if str(path) == '/':
 		return None
@@ -127,6 +95,8 @@ def find_parent_subvolume(path :pathlib.Path, filters=[]):
 			if found_mount['target'] == '/':
 				return None
 
-			return find_parent_subvolume(path.parent, traverse=True, filters=[*filters, found_mount['target']])
+			return find_parent_subvolume(path.parent, filters=[*filters, found_mount['target']])
+
+		return subvolume
 
-		return subvolume
\ No newline at end of file
+	return None