Fix 1875 (#1880)

Co-authored-by: Daniel Girtler <girtler.daniel@gmail.com>

2 files changed, 37 insertions, 23 deletions

diff --git a/archinstall/lib/disk/device_handler.py b/archinstall/lib/disk/device_handler.py
index 4341c53c..2c88e382 100644
--- a/archinstall/lib/disk/device_handler.py
+++ b/archinstall/lib/disk/device_handler.py
@@ -238,41 +238,46 @@ class DeviceHandler(object):
 		info(f'luks2 locking device: {dev_path}')
 		luks_handler.lock()
 
+	def _validate(self, device_mod: DeviceModification):
+		checks = {
+			# verify that all partitions have a path set (which implies that they have been created)
+			lambda x: x.dev_path is None: ValueError('When formatting, all partitions must have a path set'),
+			# crypto luks is not a valid file system type
+			lambda x: x.fs_type is FilesystemType.Crypto_luks: ValueError('Crypto luks cannot be set as a filesystem type'),
+			# file system type must be set
+			lambda x: x.fs_type is None: ValueError('File system type must be set for modification')
+		}
+
+		for check, exc in checks.items():
+			found = next(filter(check, device_mod.partitions), None)
+			if found is not None:
+				raise exc
+
 	def format(
 		self,
-		modification: DeviceModification,
+		device_mod: DeviceModification,
 		enc_conf: Optional['DiskEncryption'] = None
 	):
 		"""
 		Format can be given an overriding path, for instance /dev/null to test
 		the formatting functionality and in essence the support for the given filesystem.
 		"""
-
-		# verify that all partitions have a path set (which implies that they have been created)
-		missing_path = next(filter(lambda x: x.dev_path is None, modification.partitions), None)
-		if missing_path is not None:
-			raise ValueError('When formatting, all partitions must have a path set')
-
-		# crypto luks is not known to parted and can therefore not
-		# be used as a filesystem type in that sense;
-		invalid_fs_type = next(filter(lambda x: x.fs_type is FilesystemType.Crypto_luks, modification.partitions), None)
-		if invalid_fs_type is not None:
-			raise ValueError('Crypto luks cannot be set as a filesystem type')
+		self._validate(device_mod)
 
 		# make sure all devices are unmounted
-		self._umount_all_existing(modification)
+		self._umount_all_existing(device_mod)
 
-		for part_mod in modification.partitions:
+		for part_mod in device_mod.partitions:
 			# partition will be encrypted
 			if enc_conf is not None and part_mod in enc_conf.partitions:
 				self._perform_enc_formatting(
 					part_mod.safe_dev_path,
 					part_mod.mapper_name,
-					part_mod.fs_type,
+					part_mod.safe_fs_type,
 					enc_conf
 				)
 			else:
-				self._perform_formatting(part_mod.fs_type, part_mod.safe_dev_path)
+				self._perform_formatting(part_mod.safe_fs_type, part_mod.safe_dev_path)
 
 	def _perform_partitioning(
 		self,
@@ -312,7 +317,7 @@ class DeviceHandler(object):
 			length=length_sector.value
 		)
 
-		filesystem = FileSystem(type=part_mod.fs_type.value, geometry=geometry)
+		filesystem = FileSystem(type=part_mod.safe_fs_type.value, geometry=geometry)
 
 		partition = Partition(
 			disk=disk,
@@ -325,7 +330,7 @@ class DeviceHandler(object):
 			partition.setFlag(flag.value)
 
 		debug(f'\tType: {part_mod.type.value}')
-		debug(f'\tFilesystem: {part_mod.fs_type.value}')
+		debug(f'\tFilesystem: {part_mod.safe_fs_type.value}')
 		debug(f'\tGeometry: {start_sector.value} start sector, {length_sector.value} length')
 
 		try:
diff --git a/archinstall/lib/disk/device_model.py b/archinstall/lib/disk/device_model.py
index 35fbd40c..97623772 100644
--- a/archinstall/lib/disk/device_model.py
+++ b/archinstall/lib/disk/device_model.py
@@ -283,7 +283,7 @@ class _PartitionInfo:
 	partition: Partition
 	name: str
 	type: PartitionType
-	fs_type: FilesystemType
+	fs_type: Optional[FilesystemType]
 	path: Path
 	start: Size
 	length: Size
@@ -313,7 +313,7 @@ class _PartitionInfo:
 	def from_partition(
 		cls,
 		partition: Partition,
-		fs_type: FilesystemType,
+		fs_type: Optional[FilesystemType],
 		partuuid: str,
 		mountpoints: List[Path],
 		btrfs_subvol_infos: List[_BtrfsSubvolumeInfo] = []
@@ -594,7 +594,7 @@ class PartitionModification:
 	type: PartitionType
 	start: Size
 	length: Size
-	fs_type: FilesystemType
+	fs_type: Optional[FilesystemType]
 	mountpoint: Optional[Path] = None
 	mount_options: List[str] = field(default_factory=list)
 	flags: List[PartitionFlag] = field(default_factory=list)
@@ -613,6 +613,9 @@ class PartitionModification:
 		if self.is_exists_or_modify() and not self.dev_path:
 			raise ValueError('If partition marked as existing a path must be set')
 
+		if self.fs_type is None and self.status == ModificationStatus.Modify:
+			raise ValueError('FS type must not be empty on modifications with status type modify')
+
 	def __hash__(self):
 		return hash(self._obj_id)
 
@@ -628,6 +631,12 @@ class PartitionModification:
 			raise ValueError('Device path was not set')
 		return self.dev_path
 
+	@property
+	def safe_fs_type(self) -> FilesystemType:
+		if self.fs_type is None:
+			raise ValueError('File system type is not set')
+		return self.fs_type
+
 	@classmethod
 	def from_existing_partition(cls, partition_info: _PartitionInfo) -> PartitionModification:
 		if partition_info.btrfs_subvol_infos:
@@ -714,7 +723,7 @@ class PartitionModification:
 			'type': self.type.value,
 			'start': self.start.__dump__(),
 			'length': self.length.__dump__(),
-			'fs_type': self.fs_type.value,
+			'fs_type': self.fs_type.value if self.fs_type else '',
 			'mountpoint': str(self.mountpoint) if self.mountpoint else None,
 			'mount_options': self.mount_options,
 			'flags': [f.name for f in self.flags],
@@ -731,7 +740,7 @@ class PartitionModification:
 			'Type': self.type.value,
 			'Start': self.start.format_size(Unit.MiB),
 			'Length': self.length.format_size(Unit.MiB),
-			'FS type': self.fs_type.value,
+			'FS type': self.fs_type.value if self.fs_type else 'Unknown',
 			'Mountpoint': self.mountpoint if self.mountpoint else '',
 			'Mount options': ', '.join(self.mount_options),
 			'Flags': ', '.join([f.name for f in self.flags]),