rewrite guided disk selection to be more general

Drive various decisions that used to be keyed off core boot capabilities
and special purpose bits of the API by the new guided capability stuff.
I tried to think of a way to do this incrementally and failed. It might
be easier to review the new code rather than the diff in places.

subiquity/client/controllers/filesystem.py

@@ -30,7 +30,6 @@ from subiquity.common.types import (
     GuidedChoiceV2,
     GuidedStorageResponseV2,
     GuidedStorageTargetManual,
-    GuidedStorageTargetReformat,
     StorageResponseV2,
     )
 from subiquity.models.filesystem import (
@@ -43,7 +42,6 @@ from subiquity.ui.views import (
     GuidedDiskSelectionView,
     )
 from subiquity.ui.views.filesystem.probing import (
-    CoreBootClassicError,
     SlowProbing,
     ProbingFailed,
     )
@@ -64,7 +62,6 @@ class FilesystemController(SubiquityTuiController, FilesystemManipulator):
         self.answers.setdefault('guided-index', 0)
         self.answers.setdefault('manual', [])
         self.current_view: Optional[BaseView] = None
-        self.core_boot_capability: Optional[GuidedCapability] = None
 
     async def make_ui(self) -> Callable[[], BaseView]:
         def get_current_view() -> BaseView:
@@ -104,15 +101,6 @@ class FilesystemController(SubiquityTuiController, FilesystemManipulator):
             self.app.show_error_report(status.error_report)
             return ProbingFailed(self, status.error_report)
 
-        reformat_targets = [
-            target
-            for target in status.targets
-            if isinstance(target, GuidedStorageTargetReformat)
-            ]
-
-        self.core_boot_capability = None
-        self.encryption_unavailable_reason = ''
-
         response: StorageResponseV2 = await self.endpoint.v2.GET(
             include_raid=True)
 
@@ -120,27 +108,10 @@ class FilesystemController(SubiquityTuiController, FilesystemManipulator):
             disk.id: disk for disk in response.disks
             }
 
-        disks = []
-
-        for target in reformat_targets:
-            if target.allowed:
-                disks.append(disk_by_id[target.disk_id])
-                for capability in target.allowed:
-                    if capability.is_core_boot():
-                        assert len(target.allowed) == 1
-                        self.core_boot_capability = capability
-            for disallowed in target.disallowed:
-                if disallowed.capability.is_core_boot():
-                    self.encryption_unavailable_reason = disallowed.message
-
-        if not disks and self.encryption_unavailable_reason:
-            return CoreBootClassicError(
-                self, self.encryption_unavailable_reason)
-
         if status.error_report:
             self.app.show_error_report(status.error_report)
 
-        return GuidedDiskSelectionView(self, disks)
+        return GuidedDiskSelectionView(self, status.targets, disk_by_id)
 
     async def run_answers(self):
         # Wait for probing to finish.
@@ -151,15 +122,20 @@ class FilesystemController(SubiquityTuiController, FilesystemManipulator):
             await self.app.confirm_install()
             self.ui.body.done(self.ui.body.form)
         if self.answers['guided']:
+            targets = self.ui.body.form.targets
             if 'guided-index' in self.answers:
-                disk = self.ui.body.form.disks[self.answers['guided-index']]
+                target = targets[self.answers['guided-index']]
             elif 'guided-label' in self.answers:
                 label = self.answers['guided-label']
-                [disk] = [d for d in self.ui.body.form.disks
+                disk_by_id = self.ui.body.form.disk_by_id
-                          if d.label == label]
+                [target] = [
+                    t
+                    for t in targets
+                    if disk_by_id[t.disk_id].label == label
+                    ]
             method = self.answers.get('guided-method')
             value = {
-                'disk': disk,
+                'disk': target,
                 'use_lvm': method == "lvm",
                 }
             passphrase = self.answers.get('guided-passphrase')
@@ -172,7 +148,7 @@ class FilesystemController(SubiquityTuiController, FilesystemManipulator):
                         }
                     }
             self.ui.body.form.guided_choice.value = value
-            self.ui.body.done(self.ui.body.form)
+            self.ui.body.done(None)
             await self.app.confirm_install()
             while not isinstance(self.ui.body, FilesystemView):
                 await asyncio.sleep(0.1)

subiquity/ui/views/filesystem/guided.py

@@ -50,6 +50,7 @@ from subiquity.common.types import (
     Gap,
     GuidedCapability,
     GuidedChoiceV2,
+    GuidedDisallowedCapabilityReason,
     GuidedStorageTargetManual,
     GuidedStorageTargetReformat,
     Partition,
@@ -166,39 +167,68 @@ class GuidedChoiceForm(SubForm):
 
     def __init__(self, parent):
         super().__init__(parent, initial={'use_lvm': True})
+        self.tpm_choice = None
+
         options = []
         tables = []
         initial = -1
-        for disk in parent.disks:
+
+        all_caps = set()
+
+        for target in parent.targets:
+            all_caps.update(target.allowed)
+            all_caps.update(d.capability for d in target.disallowed)
+            disk = parent.disk_by_id[target.disk_id]
             for obj, cells in summarize_device(disk):
                 table = TablePile([TableRow(cells)])
                 tables.append(table)
-                enabled = False
+                if obj is disk and target.allowed:
-                if obj is disk and disk.ok_for_guided:
-                    enabled = True
                     if initial < 0:
                         initial = len(options)
-                options.append(Option((table, enabled, obj)))
+                    val = target
+                else:
+                    val = None
+                options.append(Option((table, val is not None, val)))
+
         t0 = tables[0]
         for t in tables[1:]:
             t0.bind(t)
+
         self.disk.widget.options = options
         self.disk.widget.index = initial
-        connect_signal(self.use_lvm.widget, 'change', self._toggle)
+        connect_signal(self.disk.widget, 'select', self._select_disk)
-        self.lvm_options.enabled = self.use_lvm.value
+        self._select_disk(None, self.disk.value)
-        if parent.core_boot_capability is not None:
+        connect_signal(self.use_lvm.widget, 'change', self._toggle_lvm)
-            self.remove_field('use_lvm')
+        self._toggle_lvm(None, self.use_lvm.value)
+
+        if GuidedCapability.LVM_LUKS not in all_caps:
             self.remove_field('lvm_options')
-            self.tpm_choice = choices[parent.core_boot_capability]
+        if GuidedCapability.LVM not in all_caps:
+            self.remove_field('use_lvm')
+        core_boot_caps = [c for c in all_caps if c.is_core_boot()]
+        if not core_boot_caps:
+            self.remove_field('use_tpm')
+
+    def _select_disk(self, sender, val):
+        self.use_lvm.enabled = GuidedCapability.LVM in val.allowed
+        core_boot_caps = [c for c in val.allowed if c.is_core_boot()]
+        if core_boot_caps:
+            assert len(val.allowed) == 1
+            cap = core_boot_caps[0]
+            reason = ''
+            for disallowed in val.disallowed:
+                if disallowed.capability == \
+                   GuidedCapability.CORE_BOOT_ENCRYPTED:
+                    reason = disallowed.message
+            self.tpm_choice = choices[cap]
             self.use_tpm.enabled = self.tpm_choice.enabled
             self.use_tpm.value = self.tpm_choice.default
             self.use_tpm.help = self.tpm_choice.help
-            self.use_tpm.help = self.tpm_choice.help.format(
+            self.use_tpm.help = self.tpm_choice.help.format(reason=reason)
-                reason=parent.encryption_unavailable_reason)
         else:
-            self.remove_field('use_tpm')
+            self.tpm_choice = None
 
-    def _toggle(self, sender, val):
+    def _toggle_lvm(self, sender, val):
         self.lvm_options.enabled = val
         self.validated()
 
@@ -213,11 +243,9 @@ class GuidedForm(Form):
 
     cancel_label = _("Back")
 
-    def __init__(self, disks, core_boot_capability,
+    def __init__(self, targets, disk_by_id):
-                 encryption_unavailable_reason):
+        self.targets = targets
-        self.disks = disks
+        self.disk_by_id = disk_by_id
-        self.core_boot_capability = core_boot_capability
-        self.encryption_unavailable_reason = encryption_unavailable_reason
         super().__init__()
         connect_signal(self.guided.widget, 'change', self._toggle_guided)
 
@@ -272,36 +300,58 @@ class GuidedDiskSelectionView(BaseView):
 
     title = _("Guided storage configuration")
 
-    def __init__(self, controller, disks):
+    def __init__(self, controller, targets, disk_by_id):
         self.controller = controller
 
-        if disks:
+        reformats = []
-            if any(disk.ok_for_guided for disk in disks):
+        any_ok = False
-                reason = controller.encryption_unavailable_reason
+        offer_manual = False
-                self.form = GuidedForm(
+        encryption_unavail_reason = ''
-                    disks=disks,
+        GCDR = GuidedDisallowedCapabilityReason
-                    core_boot_capability=self.controller.core_boot_capability,
-                    encryption_unavailable_reason=reason)
 
-                if self.controller.core_boot_capability is not None:
+        for target in targets:
-                    self.form = self.form.guided_choice.widget.form
+            if isinstance(target, GuidedStorageTargetManual):
-                    excerpt = _(
+                offer_manual = True
-                        "Choose a disk to install this core boot classic "
+            if not isinstance(target, GuidedStorageTargetReformat):
-                        "system to:")
+                continue
+            reformats.append(target)
+            if target.allowed:
+                any_ok = True
+            for disallowed in target.disallowed:
+                if disallowed.reason == GCDR.CORE_BOOT_ENCRYPTION_UNAVAILABLE:
+                    encryption_unavail_reason = disallowed.message
+
+        if any_ok:
+            show_form = self.form = GuidedForm(
+                targets=reformats,
+                disk_by_id=disk_by_id)
+
+            if not offer_manual:
+                show_form = self.form.guided_choice.widget.form
+                excerpt = _("Choose a disk to install to:")
             else:
                 excerpt = _(subtitle)
 
-                connect_signal(self.form, 'submit', self.done)
+            connect_signal(show_form, 'submit', self.done)
-                connect_signal(self.form, 'cancel', self.cancel)
+            connect_signal(show_form, 'cancel', self.cancel)
 
             super().__init__(
-                    self.form.as_screen(
+                show_form.as_screen(
                     focus_buttons=False, excerpt=_(excerpt)))
-            else:
+        elif encryption_unavail_reason:
+            super().__init__(
+                screen(
+                    [Text(rewrap(_(encryption_unavail_reason)))],
+                    [other_btn(_("Back"), on_press=self.cancel)],
+                    excerpt=_("Cannot install core boot classic system")))
+        elif disk_by_id and offer_manual:
             super().__init__(
                 screen(
                     [Text(rewrap(_(no_big_disks)))],
-                        [other_btn(_("OK"), on_press=self.manual)]))
+                    [
+                        other_btn(_("OK"), on_press=self.manual),
+                        other_btn(_("Back"), on_press=self.cancel),
+                    ]))
         else:
             super().__init__(
                 screen(
@@ -312,17 +362,19 @@ class GuidedDiskSelectionView(BaseView):
         return (_("Help on guided storage configuration"), rewrap(_(HELP)))
 
     def done(self, sender):
-        results = sender.as_data()
+        results = self.form.as_data()
+        if results['guided']:
+            guided_choice = results['guided_choice']
+            target = guided_choice['disk']
+            tpm_choice = self.form.guided_choice.widget.form.tpm_choice
             password = None
-        capability = None
+            if tpm_choice is not None:
-        disk_id = None
+                if guided_choice.get('use_tpm', tpm_choice.default):
-        if self.controller.core_boot_capability is not None:
-            if results.get('use_tpm', sender.tpm_choice.default):
                     capability = GuidedCapability.CORE_BOOT_ENCRYPTED
-            disk_id = results['disk'].id
+                else:
-        elif results['guided']:
+                    capability = GuidedCapability.CORE_BOOT_UNENCRYPTED
-            if results['guided_choice']['use_lvm']:
+            elif guided_choice.get('use_lvm', False):
-                opts = results['guided_choice'].get('lvm_options', {})
+                opts = guided_choice.get('lvm_options', {})
                 if opts.get('encrypt', False):
                     capability = GuidedCapability.LVM_LUKS
                     password = opts['luks_options']['passphrase']
@@ -330,13 +382,8 @@ class GuidedDiskSelectionView(BaseView):
                     capability = GuidedCapability.LVM
             else:
                 capability = GuidedCapability.DIRECT
-            disk_id = results['guided_choice']['disk'].id
-        else:
-            disk_id = None
-        if disk_id is not None:
             choice = GuidedChoiceV2(
-                target=GuidedStorageTargetReformat(
+                target=target,
-                    disk_id=disk_id, allowed=[capability]),
                 capability=capability,
                 password=password,
                 )

subiquity/ui/views/filesystem/probing.py

@@ -27,7 +27,6 @@ from subiquitycore.ui.spinner import (
     )
 from subiquitycore.ui.utils import (
     button_pile,
-    rewrap,
     screen,
     )
 from subiquitycore.view import BaseView
@@ -84,19 +83,3 @@ class ProbingFailed(BaseView):
 
     def show_error(self, sender=None):
         self.controller.app.show_error_report(self.error_ref)
-
-
-class CoreBootClassicError(BaseView):
-
-    title = _("Cannot install core boot classic system")
-
-    def __init__(self, controller, msg):
-        self.controller = controller
-        super().__init__(screen([
-            Text(rewrap(_(msg))),
-            Text(""),
-            ],
-            [other_btn(_("Back"), on_press=self.cancel)]))
-
-    def cancel(self, result=None):
-        self.controller.cancel()