Fix 1875 (#1880)

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

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.
 		"""
-
+		self._validate(device_mod)
-		# 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')
 
 		# 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:

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]),

archinstall/lib/installer.py

@@ -667,28 +667,29 @@ class Installer:
 	):
 		for mod in self._disk_config.device_modifications:
 			for part in mod.partitions:
-				if (pkg := part.fs_type.installation_pkg) is not None:
+				if part.fs_type is not None:
-					self.base_packages.append(pkg)
+					if (pkg := part.fs_type.installation_pkg) is not None:
-				if (module := part.fs_type.installation_module) is not None:
+						self.base_packages.append(pkg)
-					self.modules.append(module)
+					if (module := part.fs_type.installation_module) is not None:
-				if (binary := part.fs_type.installation_binary) is not None:
+						self.modules.append(module)
-					self._binaries.append(binary)
+					if (binary := part.fs_type.installation_binary) is not None:
+						self._binaries.append(binary)
 
-				# There is not yet an fsck tool for NTFS. If it's being used for the root filesystem, the hook should be removed.
+					# There is not yet an fsck tool for NTFS. If it's being used for the root filesystem, the hook should be removed.
-				if part.fs_type.fs_type_mount == 'ntfs3' and part.mountpoint == self.target:
+					if part.fs_type.fs_type_mount == 'ntfs3' and part.mountpoint == self.target:
-					if 'fsck' in self._hooks:
+						if 'fsck' in self._hooks:
-						self._hooks.remove('fsck')
+							self._hooks.remove('fsck')
 
-				if part in self._disk_encryption.partitions:
+					if part in self._disk_encryption.partitions:
-					if self._disk_encryption.hsm_device:
+						if self._disk_encryption.hsm_device:
-						# Required bby mkinitcpio to add support for fido2-device options
+							# Required bby mkinitcpio to add support for fido2-device options
-						self._pacstrap('libfido2')
+							self._pacstrap('libfido2')
 
-						if 'sd-encrypt' not in self._hooks:
+							if 'sd-encrypt' not in self._hooks:
-							self._hooks.insert(self._hooks.index('filesystems'), 'sd-encrypt')
+								self._hooks.insert(self._hooks.index('filesystems'), 'sd-encrypt')
-					else:
+						else:
-						if 'encrypt' not in self._hooks:
+							if 'encrypt' not in self._hooks:
-							self._hooks.insert(self._hooks.index('filesystems'), 'encrypt')
+								self._hooks.insert(self._hooks.index('filesystems'), 'encrypt')
 
 		if not SysInfo.has_uefi():
 			self.base_packages.append('grub')
@@ -857,7 +858,7 @@ class Installer:
 					# blkid doesn't trigger on loopback devices really well,
 					# so we'll use the old manual method until we get that sorted out.
 
-					options_entry = f'rw rootfstype={root_partition.fs_type.fs_type_mount} {" ".join(self._kernel_params)}\n'
+					options_entry = f'rw rootfstype={root_partition.safe_fs_type.fs_type_mount} {" ".join(self._kernel_params)}\n'
 
 					for sub_vol in root_partition.btrfs_subvols:
 						if sub_vol.is_root():
@@ -868,7 +869,7 @@ class Installer:
 					if self._zram_enabled:
 						options_entry = "zswap.enabled=0 " + options_entry
 
-					if root_partition.fs_type.is_crypto():
+					if root_partition.safe_fs_type.is_crypto():
 						# TODO: We need to detect if the encrypted device is a whole disk encryption,
 						#       or simply a partition encryption. Right now we assume it's a partition (and we always have)
 						debug('Root partition is an encrypted device, identifying by PARTUUID: {root_partition.partuuid}')
@@ -900,15 +901,15 @@ class Installer:
 
 		_file = "/etc/default/grub"
 
-		if root_partition.fs_type.is_crypto():
+		if root_partition.safe_fs_type.is_crypto():
 			debug(f"Using UUID {root_partition.uuid} as encrypted root identifier")
 
-			cmd_line_linux = f"sed -i 's/GRUB_CMDLINE_LINUX=\"\"/GRUB_CMDLINE_LINUX=\"cryptdevice=UUID={root_partition.uuid}:cryptlvm rootfstype={root_partition.fs_type.value}\"/'"
+			cmd_line_linux = f"sed -i 's/GRUB_CMDLINE_LINUX=\"\"/GRUB_CMDLINE_LINUX=\"cryptdevice=UUID={root_partition.uuid}:cryptlvm rootfstype={root_partition.safe_fs_type.value}\"/'"
 			enable_cryptdisk = "sed -i 's/#GRUB_ENABLE_CRYPTODISK=y/GRUB_ENABLE_CRYPTODISK=y/'"
 
 			SysCommand(f"/usr/bin/arch-chroot {self.target} {enable_cryptdisk} {_file}")
 		else:
-			cmd_line_linux = f"sed -i 's/GRUB_CMDLINE_LINUX=\"\"/GRUB_CMDLINE_LINUX=\"rootfstype={root_partition.fs_type.value}\"/'"
+			cmd_line_linux = f"sed -i 's/GRUB_CMDLINE_LINUX=\"\"/GRUB_CMDLINE_LINUX=\"rootfstype={root_partition.safe_fs_type.value}\"/'"
 
 		SysCommand(f"/usr/bin/arch-chroot {self.target} {cmd_line_linux} {_file}")
 
@@ -984,14 +985,14 @@ class Installer:
 			# blkid doesn't trigger on loopback devices really well,
 			# so we'll use the old manual method until we get that sorted out.
 
-			if root_partition.fs_type.is_crypto():
+			if root_partition.safe_fs_type.is_crypto():
 				# TODO: We need to detect if the encrypted device is a whole disk encryption,
 				#       or simply a partition encryption. Right now we assume it's a partition (and we always have)
 				debug(f'Identifying root partition by PARTUUID: {root_partition.partuuid}')
-				kernel_parameters.append(f'cryptdevice=PARTUUID={root_partition.partuuid}:luksdev root=/dev/mapper/luksdev rw rootfstype={root_partition.fs_type.value} {" ".join(self._kernel_params)}')
+				kernel_parameters.append(f'cryptdevice=PARTUUID={root_partition.partuuid}:luksdev root=/dev/mapper/luksdev rw rootfstype={root_partition.safe_fs_type.value} {" ".join(self._kernel_params)}')
 			else:
 				debug(f'Root partition is an encrypted device identifying by PARTUUID: {root_partition.partuuid}')
-				kernel_parameters.append(f'root=PARTUUID={root_partition.partuuid} rw rootfstype={root_partition.fs_type.value} {" ".join(self._kernel_params)}')
+				kernel_parameters.append(f'root=PARTUUID={root_partition.partuuid} rw rootfstype={root_partition.safe_fs_type.value} {" ".join(self._kernel_params)}')
 
 			device = disk.device_handler.get_device_by_partition_path(boot_partition.safe_dev_path)
 

pyproject.toml

@@ -63,6 +63,7 @@ packages = ["archinstall"]
 python_version = "3.10"
 files = "archinstall/"
 exclude = "tests"
+#check_untyped_defs=true
 
 [tool.bandit]
 targets = ["archinstall"]