Project

General

Profile

Feature #14720

Feature #9005: Improve Tails Installer UX: phase 1

Fix self.opts.partition vs. self.force_reinstall semantics

Added by anonym 23 days ago. Updated 5 days ago.

Status:
Confirmed
Priority:
Normal
Assignee:
Category:
Installation
Target version:
Start date:
09/25/2017
Due date:
% Done:

0%

QA Check:
Ready for QA
Feature Branch:
feature/9005-Improve-tails-installer
Type of work:
Code
Blueprint:
Easy:
Affected tool:
Installer

Description

This is a follow-up on #8859 and #8860. Details will follow in the comments.

History

#1 Updated by anonym 23 days ago

From intrigeri's code review (#8860#note-40):

I'm a bit confused by the self.opts.partition vs. self.force_reinstall semantics. I understand we need to treat them differently in gui.py, but I wonder why it has to leak into creator.py. But perhaps I'm just confused because some other areas where force_reinstall is used are unclear to me:

  • I've not tested yet but the code suggests that "If there is valid Tails partition", we'll display an "Upgrade" button while we're going to partition the device and thus delete any existing persistent volume. That feels weird to me, but I don't have the big picture in mind. If that's what sajolida wants, fine. If that was unspecified, then I don't know :)
  • We now seem to do "Remove parent drives if a valid partition exists" even for initial installation, while we previously did that only for upgrades. That feels weird at first glance. If this is correct, a comment explaining why would be welcome.

#2 Updated by anonym 23 days ago

From #8860#note-47:

To me it seems like all this is baggage from when Tails Installer only had one mode, and didn't switch between them depending on the target device, like now. In fact, I feel the proper fix is to drop self.opts.partition, self.force_reinstall etc. from TailsInstallerWindow and LinuxTailsInstallerCreator, and make these properties of each drive, probably set when they are scanned in detect_supported_drives().

#3 Updated by kurono 7 days ago

anonym wrote:

From intrigeri's code review (#8860#note-40):

I'm a bit confused by the self.opts.partition vs. self.force_reinstall semantics. I understand we need to treat them differently in gui.py, but I wonder why it has to leak into creator.py. But perhaps I'm just confused because some other areas where force_reinstall is used are unclear to me:

  • I've not tested yet but the code suggests that "If there is valid Tails partition", we'll display an "Upgrade" button while we're going to partition the device and thus delete any existing persistent volume. That feels weird to me, but I don't have the big picture in mind. If that's what sajolida wants, fine. If that was unspecified, then I don't know :)
  • We now seem to do "Remove parent drives if a valid partition exists" even for initial installation, while we previously did that only for upgrades. That feels weird at first glance. If this is correct, a comment explaining why would be welcome.
The general idea is:
  • We have a drop down list with all the available USB sticks, we can use for installing Tails.
  • The selected device comes with all the relevant data for installing or upgrading Tails on it, like size, mount path, etc.
  • If the device has tails, we show in the list the relevant partition, for example /dev/sdc1
  • If not, we show the full device, for example /dev/sdc
  • if the device has tails, we delete the parent, otherwise we would have both /dev/sdc1 and /dev/sdc in the list, which is confusing and prone to errors
  • if the selected device has Tails, lets say /dev/sdc1, we show the upgrade button. If it is pressed, it keeps the partitions unchanged and including the persistent. It just deletes the files and copy the new ones. But in addition to the upgrade button, there is a link called reinstall, which forces to delete the partitions and create new ones (exactly like installing from scratch).
  • So the tricky part, is when the user selects reinstall, because we need to come back to the parent device /dev/sdc, that we have deleted. That is way we rescan the devices and leak force_reinstall to creator.py,
    so we can get the info from the parent.
  • On the other hand elf.opts.partition is an option that comes from the command line and self.force_reinstall is something I have add to seal to this special situation, that we did not have before, because we knew before hand exactly what operation we were going to apply. Now it becomes dinamic, we can switch from install to upgrade in the same device
  • Maybe there is a more elegant solution, to get the parent's info when a partition "/dev/sdc1" is going to be reinstalled.

#4 Updated by kurono 5 days ago

  • QA Check set to Ready for QA
  • Feature Branch set to feature/9005-Improve-tails-installer

I have found a better solution for the "leaking force_reinstall" problem.
Now the installer stores the parent information of valid tails partitions,
so if the user selects the reinstall link, we can get that info without having to make more extra weird tricks at creator.py

For the second point, as @anonym says we need to remove the parents for all the devices now, since we are relaying in the previous splash options. So, if we leave that as it was before, it will list valid partitions and their parents in the dropdown list, which is confusing.

Also available in: Atom PDF