Feature #8860

Tails Installer should autodetect if Tails is installed on the destination device

Added by sajolida 2015-02-04 22:16:00 . Updated 2017-09-28 18:48:47 .

Status:
Resolved
Priority:
Elevated
Assignee:
Category:
Installation
Target version:
Tails_3.2
Start date:
2015-02-04
Due date:
% Done:

100%

Feature Branch:
feature/8860-tails-installer-improvements installer:master,tails/master
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Installer
Deliverable for:

Description

To simplify the workflow regarding install or upgrade (and as long as Tails Installer does the upgrades, see Feature #7499) we propose the following:

  • Tails Installer start from its main interface straight away (see Feature #8859).
  • When the destination device has Tails installed already, the “Install” button is rename “Reinstall” (or something similar) and an “Upgrade” button appears below it.

Files

attachment-0003.png (29428 B) sajolida, 2016-12-28 21:14:28
install-upgrade-switching.webm (640647 B) sajolida, 2017-05-23 17:37:50

Subtasks

Related issues

Related to Tails - Bug #8862: Display version number of destination device in Tails Installer Confirmed 2015-02-04
Blocks Tails - Feature #13424: Core work 2017Q3: User experience Resolved 2017-07-05
Blocked by Tails - Bug #14695: /tmp/TailsToaster is too small for current Tails Installer Resolved 2017-09-21

History

#1 Updated by sajolida 2015-02-04 22:25:34

  • related to Bug #8862: Display version number of destination device in Tails Installer added

#2 Updated by sajolida 2015-02-04 22:25:55

  • Category set to Installation
  • Affected tool set to Installer

#3 Updated by sajolida 2015-02-04 22:29:50

  • Subject changed from Tails Installer should autodetec is Tails is installed on the destination device to Tails Installer should autodetect if Tails is installed on the destination device

#4 Updated by intrigeri 2015-03-01 21:52:05

#5 Updated by intrigeri 2015-03-01 23:04:07

  • blocks Feature #8859: Get rid of the splash screen of Tails Installer added

#6 Updated by intrigeri 2015-03-03 18:15:08

#7 Updated by intrigeri 2015-03-03 18:15:50

  • blocked by deleted (Feature #8859: Get rid of the splash screen of Tails Installer)

#8 Updated by intrigeri 2015-03-03 18:17:33

  • blocked by Feature #8859: Get rid of the splash screen of Tails Installer added

#9 Updated by intrigeri 2015-03-03 18:17:49

  • blocks deleted (Feature #8859: Get rid of the splash screen of Tails Installer)

#10 Updated by intrigeri 2015-03-03 18:18:01

  • blocks Feature #8859: Get rid of the splash screen of Tails Installer added

#11 Updated by intrigeri 2015-03-03 18:25:00

  • blocks Feature #9006: Simplify the Tails Installer splash screen added

#12 Updated by intrigeri 2015-03-03 18:25:25

  • blocked by deleted (Feature #8859: Get rid of the splash screen of Tails Installer)

#13 Updated by kurono 2015-08-20 09:15:52

  • Assignee set to kurono
  • Target version set to Tails_1.7

#14 Updated by kurono 2015-09-28 07:22:08

  • Target version changed from Tails_1.7 to 2016

#15 Updated by kurono 2016-05-18 15:40:42

  • Target version changed from 2016 to Tails_2.5
  • QA Check set to Ready for QA
  • Feature Branch set to kurono/feature/9005-Improve-tails-installer

#16 Updated by intrigeri 2016-05-18 16:17:02

Why 2.5 (point-release) and not 2.4 (major release)?
Also, if that’s ready for QA, you’ll want to reassign to the release manager (anonym for 2.4, myself for 2.5).

#17 Updated by sajolida 2016-05-18 17:36:40

kurono: it’s super cool that you’re working on this! Just the other day I was updating the blueprint as part of Bug #10640. I don’t think I changed much regarding this section but the updated version is here, still under the #simplify anchor, but not merged yet:

https://git-tails.immerda.ch/tails/tree/wiki/src/blueprint/bootstrapping/installer.mdwn?h=web/10640-assistant-design-doc

#18 Updated by intrigeri 2016-05-19 09:56:01

> kurono: it’s super cool that you’re working on this!

Yeah! I’m excited about it :)

#19 Updated by intrigeri 2016-08-02 09:31:54

  • Target version changed from Tails_2.5 to Tails_2.6

#20 Updated by anonym 2016-09-20 16:53:47

  • Target version changed from Tails_2.6 to Tails_2.7

#21 Updated by BitingBird 2016-10-16 04:47:46

  • Status changed from Confirmed to In Progress
  • Target version changed from Tails_2.7 to Tails_2.9.1
  • % Done changed from 0 to 50

Assigning to next major release (but the assignee should probably be changed)

#22 Updated by kurono 2016-11-28 11:32:11

  • Assignee changed from kurono to sajolida

#23 Updated by sajolida 2016-12-12 18:35:24

  • Target version deleted (Tails_2.9.1)

#24 Updated by sajolida 2016-12-28 21:14:45

Adding screenshot of what this would look like.

#25 Updated by sajolida 2016-12-28 21:23:05

  • Assignee changed from sajolida to kurono
  • QA Check changed from Ready for QA to Dev Needed

I didn’t see this in action during my tests but I might have missed something…

#26 Updated by kurono 2017-01-31 21:03:26

  • Assignee changed from kurono to sajolida
  • QA Check changed from Dev Needed to Ready for QA

It works for me, could you send me a screenshot when the opposite happens to you?

#27 Updated by sajolida 2017-03-18 10:53:35

  • Assignee changed from sajolida to kurono
  • % Done changed from 50 to 70
  • QA Check changed from Ready for QA to Dev Needed

I got that now! Cool!

Still, as noted by mail:

> 6. If I plug two USB sticks, one with Tails and one without, and switch
> from Upgrade to Install and then back to Upgrade, then the Reinstall
> link disappears.

And fixing this should be part of this ticket I think.

#28 Updated by sajolida 2017-03-18 11:04:19

  • blocked by deleted (Feature #9006: Simplify the Tails Installer splash screen)

#29 Updated by kurono 2017-04-24 16:12:53

  • Assignee changed from kurono to sajolida
  • QA Check changed from Dev Needed to Ready for QA

sajolida wrote:
> I got that now! Cool!
>
> Still, as noted by mail:
>
> > 6. If I plug two USB sticks, one with Tails and one without, and switch
> > from Upgrade to Install and then back to Upgrade, then the Reinstall
> > link disappears.
>
> And fixing this should be part of this ticket I think.

Ok, this has been fixed.

#30 Updated by sajolida 2017-05-23 17:37:54

Unfortunately, not for me :(

Attaching a screencast to clarify the issue.

#31 Updated by kurono 2017-06-08 16:57:04

  • Assignee changed from kurono to sajolida
  • QA Check changed from Dev Needed to Ready for QA

#32 Updated by sajolida 2017-06-30 15:55:51

  • Assignee changed from sajolida to intrigeri
  • Feature Branch changed from kurono/feature/9005-Improve-tails-installer to feature/9005-Improve-tails-installer

It’s fixed for me now!

Reassigning to intrigeri for the final code review.

#33 Updated by intrigeri 2017-06-30 15:57:35

  • Assignee changed from intrigeri to anonym
  • Target version set to Tails_3.2

Reassigning to the RM for the next major release. If I am needed for something specific, just say the word (and tell me what that thing is).

#34 Updated by intrigeri 2017-06-30 16:00:43

  • Feature Branch changed from feature/9005-Improve-tails-installer to installer:feature/9005-Improve-tails-installer

#35 Updated by sajolida 2017-06-30 16:01:30

#36 Updated by sajolida 2017-07-05 19:08:52

#37 Updated by sajolida 2017-07-05 19:08:57

#38 Updated by anonym 2017-09-15 00:40:14

  • Assignee changed from anonym to kurono
  • QA Check changed from Ready for QA to Dev Needed
  • Feature Branch changed from installer:feature/9005-Improve-tails-installer to tails:feature/12705-bump-system-partition-size installer:feature/9005-Improve-tails-installer

First of all, great work, kurono!

I want to ship this in Tails 3.2~rc1 despite there being some problems (I’ll use this ticket for comments about Feature #8859 too). It seems to me like they should be fairly easy to fix, and so possible to get done post-freeze, so I went ahead and merged.

kurono, are you available to work on this in the next week? The hard deadline for Tails 3.2 is 2017-09-25, noon (CEST). (If do, please base your fixes on the installer’s master branch.)

Note: below I describe some of the problems through which automated test scenario that failed for me to discover them; look in features/usb_install.feature for the steps to reproduce.

Scenario: Writing a Tails isohybrid to a USB drive and booting it, then trying to upgrading it but ending up having to do a fresh installation, which boots

Tails Installer doesn’t detect that the isohybrid is problematic to upgrade (previously it wrote “It is impossible to upgrade the device” and a suggestion to try a “Clone and Install” instead). Nothing is shown in the status, and the “Install” button is active, but trying it results in:

Partitioning device /dev/sda1
GDBus.Error:org.freedesktop.UDisks2.Error.NotSupported: This partition cannot be modified because it contains a partition table; please reinitialize layout of the whole device.
Tails installation failed!
udisks-error-quark: GDBus.Error:org.freedesktop.UDisks2.Error.NotSupported: This partition cannot be modified because it contains a partition table; please reinitialize layout of the whole device. (11)

Note that it tries to install it to /dev/sda1 and not /dev/sda.

Scenario: Try installing Tails to a too small USB drive with GPT and a FAT partition

Here there’s no status telling us that the drive is “too small”. The Install button is there and active, and pressing it leads to a failure (didn’t catch the message or target device this time, but it felt similar to the above failure).

Inconsistent button label

I’ve seen the button say “Install Tails”, but it probably should only be “Install” in that situation too. While not important in itself, it feels like it might point to some sort of inconsistency.

#39 Updated by anonym 2017-09-15 01:14:31

Not translating strings part of dynamic GUI

I see a lot of instance like this:

self.__button_start.set_label('Upgrade')

which means that the we’ll set the button’s label to ‘Upgrade’ not matter locale. Please make the GUI fully translatable by doing this instead:

self.__button_start.set_label(_('Upgrade'))

#40 Updated by intrigeri 2017-09-15 06:30:53

Meta @kurono: I’m aware that your work has been Ready for QA for 3 month, and that the previous review roundtrips took between 1 and 1.5 months already. So I think we can’t fairly expect you to fix stuff on short notice now. But for the better or the worst, this has been merged already, so someone is gonna have to fix it. I’d rather see anonym do it himself as he took responsibility for merging this branch, but if needed and if asked explicitly in advance I can help a bit.

Here’s a code review.

<property name="uri">https://tails.boum.org/install/os/index.en.html</property> feels wrong: it’s not translatable and will forcefully send everyone to the English page, even though their browser is configured to prefer another language in which our website is translated. Just write https://tails.boum.org/install/os/ and the language negotiation magic will pick the best language automatically :)

I’d like to make sure that this change:

-            if self.opts.partition and not parent_block \
-                  and not self.is_device_big_enough_for_installation(data['size']):
+            if parent_block is None \
+               and not self.is_device_big_enough_for_installation(data['size']):

… doesn’t prevent upgrading a 4GB USB drive with a 2.5GB system partition. I’ll check that our test suite would catch that and will test manually if unsure.

This feels wrong:

+        if self.opts.unprivileged == None:
+            self.opts.unprivileged = True
+        if self.opts.noverify == None:
+            self.opts.noverify = True

given we have:

parser.add_option('-n', '--noverify', dest='noverify', action='store_true',
help='Skip checksum verification')
[…]
parser.add_option('-u', '--unprivileged', dest='unprivileged',
action='store_true', help='Allow not being root')

With this change we’re now setting different defaults for the GUI than for the CLI, with no (obvious) way to override them when using the GUI at runtime. I bet that our code hasn’t supported not passing -n -u for a while anyway, so I think we should entirely drop these options and default to unprivileged + noverify in all cases. While we’re at it, instead of fixing the previously buggy --no-xo option, we could drop it.

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 on_radio_changed method is much less generic than its name suggests. Let’s give it a name that matches better what it does.

The is_ISO_selected name suggests it’s a simple predicate but this method 1. checks if we need an ISO in the first place (which the name doesn’t convey); 2. has side effects (while a predicate must not). I think some refactoring is in order. From a caller’s PoV I would do:

if self.need_ISO() and not self.is_ISO_selected():
    self.warn_ISO_not_selected()

… i.e. two predicates and a third method that does the side-effect.

device_with_persistence is a list that can apparently contain multiple devices, so s/device/devices/ perhaps?

#41 Updated by intrigeri 2017-09-15 07:19:23

Also, after looking at anonym’s corresponding changes that I see on the devel branch:

  • tails-installer-launcher.desktop should be renamed to tails-installer.desktop and adjusted
  • the launcher code can entirely go away, right? I assume it’s not needed on Debian/Ubuntu either
  • anonym, please push your packaging tags & branches
  • the Debian packaging needs to be updated: tails-installer shouldn’t be moved out of the $PATH anymore, and anything related to the launcher should be dropped; I don’t know what subset has been done already as the updated packaging was not pushed yet
  • once this is done config/chroot_local-hooks/50-tails-installer-launcher shall disappear
  • /^I fail to (.*)$/ is a bit too generic a name for a step specific to Tails Installer; I see it’s used in one single place so it should be easy to improve :)

#42 Updated by intrigeri 2017-09-15 07:32:20

The confirmation dialog displayed when clicking “Upgrade” shows the “pretty name” of the device (great!) but not its size. This is inconsistent with what we display in the device selection widget, and IMO the size is a very useful distinguisher (some users will know the size of their USB stick better than its brand, especially for devices with no visible brand/model on the physical model).

This being said, I love the new splash-screen-less interface :)

#43 Updated by anonym 2017-09-15 11:15:25

intrigeri wrote:
> Meta @kurono: I’m aware that your work has been Ready for QA for 3 month, and that the previous review roundtrips took between 1 and 1.5 months already. So I think we can’t fairly expect you to fix stuff on short notice now. But for the better or the worst, this has been merged already, so someone is gonna have to fix it. I’d rather see anonym do it himself as he took responsibility for merging this branch

+1. If I do it …

> Here’s a code review.

… I hope I’ll be able to do better code reviews from having more direct experience with the code base, which would be nice (in the end I did not feel very confident reviewing these very big changes).

> * tails-installer-launcher.desktop should be renamed to tails-installer.desktop and adjusted
> * the launcher code can entirely go away, right? I assume it’s not needed on Debian/Ubuntu either
> * the Debian packaging needs to be updated: tails-installer shouldn’t be moved out of the $PATH anymore, and anything related to the launcher should be dropped; I don’t know what subset has been done already as the updated packaging was not pushed yet

[Eh, so I see I didn’t copy paste what I wrote about this while reviewing into my comment. Sorry that you had to reproduce my work…]

My understanding is that nothing at all has been done towards actually removing the launcher — the installer has just been made completely independent of it, which is the big/important/hard work. :)

> * once this is done config/chroot_local-hooks/50-tails-installer-launcher shall disappear

Exactly.

> * anonym, please push your packaging tags & branches

Done! :S

> * /^I fail to (.*)$/ is a bit too generic a name for a step specific to Tails Installer; I see it’s used in one single place so it should be easy to improve :)

Right. I’m thinking /^Tails installer fails to (.*)$/.

#44 Updated by anonym 2017-09-15 17:47:38

  • Feature Branch deleted (tails:feature/12705-bump-system-partition-size installer:feature/9005-Improve-tails-installer)

#45 Updated by kurono 2017-09-18 17:01:03

intrigeri wrote:
> Meta @kurono: I’m aware that your work has been Ready for QA for 3 month, and that the previous review roundtrips took between 1 and 1.5 months already. So I think we can’t fairly expect you to fix stuff on short notice now. But for the better or the worst, this has been merged already, so someone is gonna have to fix it. I’d rather see anonym do it himself as he took responsibility for merging this branch, but if needed and if asked explicitly in advance I can help a bit.

thanks intrigeri and anonym for the deep review :)
I indeed was expecting another iteration on the changes made to the installer code, in order to actually delete the launcher related stuff. I do have some time this week, but unfortunately I can’t commit on a deadline :/

#46 Updated by intrigeri 2017-09-18 17:31:31

  • Assignee changed from kurono to anonym
  • Priority changed from Normal to Elevated

> I indeed was expecting another iteration on the changes made to the installer code, in order to actually delete the launcher related stuff. I do have some time this week, but unfortunately I can’t commit on a deadline :/

OK, fair enough. Reassigning to anonym as planned then. Up to him to decide if he wants to fix the issues raised during the review, or rollback and give you more time.

#47 Updated by anonym 2017-09-20 15:10:58

So IMHO the blockers that has to be solved to keep this in 3.2 are:

  • Make automated test pass: Scenario: Writing a Tails isohybrid to a USB drive and booting it, then trying to upgrading it but ending up having to do a fresh installation, which boots (this test will also need to be updated)
  • Make automated test pass: Scenario: Try installing Tails to a too small USB drive with GPT and a FAT partition. I can add that this also affects too small isohybrids. In fact, it probably affects all too small drives that already has a partition. That also has to be fixed.
  • Make sure we can upgrade 4GB USB drive with a 2.5GB system partition that Tails already was installed too.
  • Make sure that the “self.opts.partition vs. self.force_reinstall semantics” issue at least doesn’t break anything. I really tried to wrap my head around this, but failed. 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 this from TailsInstallerWindow and LinuxTailsInstallerCreator, and make these properties of each drive, probably set when they are scanned in detect_supported_drives. I will not attempt this, I’ll just make sure that everything we want to support works as expected.
  • Drop everything related to tails-installer-launcher.
  • Update the Debian packaging needs to be updated vs the above point.
  • Drop config/chroot_local-hooks/50-tails-installer-launcher in Tails
  • Find a less generic name for the /^I fail to (.*)$/ automated test step.

I think we could drop some of these, but I believe I already have fixes for all of them (except Debian packaging, but that is one of those true blockers :)) so let’s not bother with trimming the list unless we notice my fix for something unimportant doesn’t work.

#48 Updated by anonym 2017-09-20 15:13:38

  • Assignee changed from anonym to intrigeri
  • QA Check changed from Dev Needed to Info Needed

#49 Updated by anonym 2017-09-20 18:31:30

intrigeri, if you agree with my blockers, then please review’n’merge this ticket and Feature #8859. If you don’t, let me know! :)

#50 Updated by anonym 2017-09-20 18:36:43

  • Feature Branch set to feature/8860-tails-installer-improvements installer:master,tails/stretch

For the review’n’merge: in an attempt to save some time, I prepared tails-installer 4.20 which I hope is good enough to ship in Tails 3.2 (unless we receive enough translations in the next couple of days, which would warrant another release).

#51 Updated by anonym 2017-09-20 19:08:20

On my system, features/usb_install.feature, features/usb_upgrade.feature and features/persistence.feature are all green now!

Let’s see what Jenkins thinks: https://jenkins.tails.boum.org/job/test_Tails_ISO_feature-8860-tails-installer-improvements/1/

#52 Updated by anonym 2017-09-20 19:26:12

Here are some clarifications for why some of the above blockers are solved:

anonym wrote:
> * Make sure we can upgrade 4GB USB drive with a 2.5GB system partition that Tails already was installed too.

I have tested this, and it works.

> * Make sure that the “self.opts.partition vs. self.force_reinstall semantics” issue at least doesn’t break anything. I really tried to wrap my head around this, but failed. 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 this from TailsInstallerWindow and LinuxTailsInstallerCreator, and make these properties of each drive, probably set when they are scanned in detect_supported_drives. I will not attempt this, I’ll just make sure that everything we want to support works as expected.

The automated test suite passes for me locally, and in addition I’ve tested the only other relevant case (upgrade an old 4GB Tails installation) so IMHO that is enough confirmation that the code works well enough to ship in Tails 3.2.

> * Update the Debian packaging needs to be updated vs the above point.

Now done!

#53 Updated by anonym 2017-09-21 04:28:26

anonym wrote:
> Let’s see what Jenkins thinks: https://jenkins.tails.boum.org/job/test_Tails_ISO_feature-8860-tails-installer-improvements/1/

This and job 2 failed, but for unrelated reasons (disk space issues and network issue => failed to clone Git submodule, respectively). I hope job 3 will have better success: https://jenkins.tails.boum.org/job/test_Tails_ISO_feature-8860-tails-installer-improvements/3/

#54 Updated by intrigeri 2017-09-21 09:03:06

  • blocked by Bug #14695: /tmp/TailsToaster is too small for current Tails Installer added

#55 Updated by intrigeri 2017-09-21 09:07:04

> This and job 2 failed, but for unrelated reasons (disk space issues and network issue => failed to clone Git submodule, respectively). I hope job 3 will have better success: https://jenkins.tails.boum.org/job/test_Tails_ISO_feature-8860-tails-installer-improvements/3/

It failed for disk space reasons too (I’m not surprised as such problems rarely go away magically just by re-running the tests) ⇒ Bug #14695. I’ll fix that and while I’ll wait for new, hopefully useful test results, I’ll do the code review :)

#56 Updated by intrigeri 2017-09-21 09:54:37

>> * Make sure that the “self.opts.partition vs. self.force_reinstall semantics” issue at least doesn’t break anything. I really tried to wrap my head around this, but failed. 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 this from TailsInstallerWindow and LinuxTailsInstallerCreator, and make these properties of each drive, probably set when they are scanned in detect_supported_drives. I will not attempt this, I’ll just make sure that everything we want to support works as expected.

> The automated test suite passes for me locally, and in addition I’ve tested the only other relevant case (upgrade an old 4GB Tails installation) so IMHO that is enough confirmation that the code works well enough to ship in Tails 3.2.

Agreed, but please file a ticket so that this problem is tracked: the fact that neither you nor I understands very well how this whole thing works is a pretty strong indicator that it’s not good enough on the long-term.

>> * Update the Debian packaging needs to be updated vs the above point.

> Now done!

Cool! I’ll post my review of the packaging update here:

  • debian/changelog seems to have been updated in a fully automated way, without any kind of manual editing. As a result it contains a bunch of entries that have a really bad signal/noise ratio such as “Simplify”. I probably would not care much if this package targeted Tails only, but that’s not the case, and e.g. Debian users who have apt-listchanges installed will see that, which doesn’t make us look like good package maintainers. FTR the 4.4.19+dfsg-0tails1 entry was even worse; back then I did shut up because we were in a hurry, under a lot of stress, and there were a bunch of more critical issues to report about. I won’t block on this problem this time, but let’s please improve this in the future, OK? :)
  • I’m unconvinced by the move of tails-installer to $PATH: the major reason why it was installed outside of $PATH is that we don’t support running it on the command line. I think this still applies. Users who run it via their desktop environment app launcher don’t care where the executable lives, and those who insist on running it by hand will find it even in /usr/lib. I’ve installed 4.20+dfsg-0tails1 on a Debian sid system and running tails-installer errors out and fails to start the GUI, which seems to be a good indicator that a tails-installer.desktop file that merely runs this command is not good enough in the current state of the code. I’ve not investigated why the same thing works in Tails — perhaps the updated code does the right thing only when running from Tails, in which case this would not be purely a packaging problem?

#57 Updated by intrigeri 2017-09-21 10:20:38

  • Assignee changed from intrigeri to anonym
  • QA Check changed from Info Needed to Dev Needed

> intrigeri, if you agree with my blockers,

I’ll look into this after I’m done with the code review.

> then please review’n’merge this ticket and Feature #8859.

Let’s go! :)

Wrt. changes in tails.git:

  • Why commit:c2c3d8ecf0f5fcce7a188a723afed814abd339ae? It feels:
    • unneeded: IIRC tails/jessie is meant to host the packaging of upstream code that works on Jessie and newer. I doubt we’re breaking compatibility with Jessie here , are we?
    • incomplete: some other parts of this doc refer to tails/jessie
  • “Scenario: Writing a Tails isohybrid to a USB drive and booting it, then trying to upgrading it but ending up having to do a fresh installation, which boots” does not match the actual tests we’re doing there anymore.

Wrt. changes in Tails Installer itself:

  • tails-installer-launcher.1.markdown had useful content that got lost in favour of the not-very-descriptive tails-installer.1.markdown; also, I suspect that tails-installer.1.markdown is lagging behind the current behavior of the program; and it lists options we can’t pretend we support nowadays. This did not matter much back when tails-installer was outside of $PATH, but if we change that (not sure it’s a good idea, see my review of the packaging) then we need a better manpage IMO.
  • Thanks a lot for commit c3fe6892b530623a7c01fe37f48e645840c11451 and other code cleanups! :)
  • I’m a tiny bit worried about the new isohybrid detection code (commit b7dcb936a58bb0016981645c9224caee4493112a): I’ve seen very weird stuff happening when the kernel detects these devices, kinda lacking determinism (sometimes it would be recognized in a way, sometimes in the other) which is not surprising given such devices are really two different things at the same time. Let’s not bother for now but keep it in mind if we get related bug reports.

#58 Updated by intrigeri 2017-09-21 10:23:27

I’m sorry I wrote “tails-installer shouldn’t be moved out of the $PATH anymore” a week ago and now am questioning this. So to clarify: it should be installed in $PATH if, and only if, it’s ready to be used this way on all platforms we support.

#59 Updated by intrigeri 2017-09-21 10:37:10

Hi again! :)

> So IMHO the blockers that has to be solved to keep this in 3.2 are:

> * Make automated test pass: […]
> * Make automated test pass: […]

I’ll assume it follows that “make the entire automated test suite pass” is a blocker. Correct?

I agree with this list of blockers for 3.2, but really I don’t feel like I have much choice: the code has been merged already, and anonym has already invested quite some time into making it good enough for 3.2 (thanks!) so I would feel really bad adding more pressure here than there has been already… even though I think that would be entirely legitimate given how this code landed in the master branch in the first place. Now, I really don’t want this situation to justify introducing the other issues I’ve raised in Feature #8860#note-40 for more than one release cycle ⇒ they need a ticket for 3.3, with clearly defined responsibility (IMO that would be kurono if he can do it with these time constraints imposed by the hasty merge + my reluctance to introducing such regressions, and anonym otherwise). In other words, let’s not allow a self-imposed emergency mode to lower our code quality expectations.

And taking a step back, let’s please not put ourselves into such a situation again. I want to do a blame-free post-mortem with anonym once 3.2 is released about some of the root causes and what we can do about them.

#60 Updated by anonym 2017-09-21 23:52:12

  • Assignee changed from anonym to intrigeri
  • QA Check changed from Dev Needed to Info Needed

intrigeri wrote:
> >> * Make sure that the “self.opts.partition vs. self.force_reinstall semantics” issue at least doesn’t break anything. I really tried to wrap my head around this, but failed. 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 this from TailsInstallerWindow and LinuxTailsInstallerCreator, and make these properties of each drive, probably set when they are scanned in detect_supported_drives. I will not attempt this, I’ll just make sure that everything we want to support works as expected.
>
> > The automated test suite passes for me locally, and in addition I’ve tested the only other relevant case (upgrade an old 4GB Tails installation) so IMHO that is enough confirmation that the code works well enough to ship in Tails 3.2.
>
> Agreed, but please file a ticket so that this problem is tracked: the fact that neither you nor I understands very well how this whole thing works is a pretty strong indicator that it’s not good enough on the long-term.

The plan is to file tickets for every remaining issue, if we close this ticket for Tails 3.2. (If so, that should the issues you reported in comment 40.)

> >> * Update the Debian packaging needs to be updated vs the above point.
>
> > Now done!
>
> Cool! I’ll post my review of the packaging update here:
>
> * debian/changelog seems to have been updated in a fully automated way, without any kind of manual editing. As a result it contains a bunch of entries that have a really bad signal/noise ratio such as “Simplify”. I probably would not care much if this package targeted Tails only, but that’s not the case, and e.g. Debian users who have apt-listchanges installed will see that, which doesn’t make us look like good package maintainers. FTR the 4.4.19+dfsg-0tails1 entry was even worse; back then I did shut up because we were in a hurry, under a lot of stress, and there were a bunch of more critical issues to report about. I won’t block on this problem this time, but let’s please improve this in the future, OK? :)

Got it! I used to pay attention to this, but got the impression others didn’t and thought I was doing something wrong when hand-editing these machine generated changelogs. I’ll happily comply!

Now, I just noticed that I completely screwed up the version — we just jumped from 4.4.19 to 4.20. Urgh. I no think the best would be that I release a 5.0 (after all, these very user-visible changes could warrant a bump), that drops the entries for 4.4.19 and 4.20, combining them into a nice changelog for 5.0. That wouldn’t be unacceptable in this situation, right?

> * I’m unconvinced by the move of tails-installer to $PATH: the major reason why it was installed outside of $PATH is that we don’t support running it on the command line.

Ah, I was not aware of this (and missed any comment/docs stating this, if any). Any way, it makes sense, because…

> I think this still applies.

… I strongly believe this is the case.

> Users who run it via their desktop environment app launcher don’t care where the executable lives, and those who insist on running it by hand will find it even in /usr/lib. I’ve installed 4.20+dfsg-0tails1 on a Debian sid system and running tails-installer errors out and fails to start the GUI, which seems to be a good indicator that a tails-installer.desktop file that merely runs this command is not good enough in the current state of the code. I’ve not investigated why the same thing works in Tails — perhaps the updated code does the right thing only when running from Tails, in which case this would not be purely a packaging problem?

I’ll have a look!

> > then please review’n’merge this ticket and Feature #8859.
>
> Let’s go! :)
>
> Wrt. changes in tails.git:
>
> * Why commit:c2c3d8ecf0f5fcce7a188a723afed814abd339ae? It feels:
> unneeded: IIRC tails/jessie is meant to host the packaging of upstream code that works on Jessie and newer. I doubt we’re breaking compatibility with Jessie here , are we?

It felt unintuitive for me to continue the work in a branch referring to tails/jessie, it’s just that. How about tails/stable?

> incomplete: some other parts of this doc refer to tails/jessie

True — I just updated the parts of the release process that I actually followed. I’ll update these unless you think think we should revert back to tails/jessie.

> * “Scenario: Writing a Tails isohybrid to a USB drive and booting it, then trying to upgrading it but ending up having to do a fresh installation, which boots” does not match the actual tests we’re doing there anymore.

True! Now fixed!

> Wrt. changes in Tails Installer itself:
>
> * tails-installer-launcher.1.markdown had useful content that got lost in favour of the not-very-descriptive tails-installer.1.markdown; also, I suspect that tails-installer.1.markdown is lagging behind the current behavior of the program; and it lists options we can’t pretend we support nowadays. This did not matter much back when tails-installer was outside of $PATH, but if we change that (not sure it’s a good idea, see my review of the packaging) then we need a better manpage IMO.

I’m convinced of the need to move tails-installer was outside of $PATH, so let’s forget about this.

> * Thanks a lot for commit c3fe6892b530623a7c01fe37f48e645840c11451 and other code cleanups! :)

:)

> * I’m a tiny bit worried about the new isohybrid detection code (commit b7dcb936a58bb0016981645c9224caee4493112a): I’ve seen very weird stuff happening when the kernel detects these devices, kinda lacking determinism (sometimes it would be recognized in a way, sometimes in the other) which is not surprising given such devices are really two different things at the same time. Let’s not bother for now but keep it in mind if we get related bug reports.

Good to know! At least it consistently worked on my system while testing this.

> > So IMHO the blockers that has to be solved to keep this in 3.2 are:
>
> > * Make automated test pass: […]
> > * Make automated test pass: […]
>
> I’ll assume it follows that “make the entire automated test suite pass” is a blocker. Correct?

Yes.

> I agree with this list of blockers for 3.2, but really I don’t feel like I have much choice: the code has been merged already, and anonym has already invested quite some time into making it good enough for 3.2 (thanks!) so I would feel really bad adding more pressure here than there has been already… even though I think that would be entirely legitimate given how this code landed in the master branch in the first place.

Really, I won’t be too sad if you feel we would revert it for 3.2. :)

> Now, I really don’t want this situation to justify introducing the other issues I’ve raised in Feature #8860#note-40 for more than one release cycle ⇒ they need a ticket for 3.3, with clearly defined responsibility (IMO that would be kurono if he can do it with these time constraints imposed by the hasty merge + my reluctance to introducing such regressions, and anonym otherwise). In other words, let’s not allow a self-imposed emergency mode to lower our code quality expectations.

I can afford spending a day on this next cycle, if needed, so this should work out fine. kurono (if you are reading this massive comment…), do you think you’ll have some time to work on this before, say, mid-October?

> And taking a step back, let’s please not put ourselves into such a situation again. I want to do a blame-free post-mortem with anonym once 3.2 is released about some of the root causes and what we can do about them.

Sure!

#61 Updated by intrigeri 2017-09-22 06:00:18

  • Assignee changed from intrigeri to anonym
  • QA Check changed from Info Needed to Dev Needed

> The plan is to file tickets for every remaining issue, if we close this ticket for Tails 3.2. (If so, that should the issues you reported in comment 40.)

OK, great.

> Now, I just noticed that I completely screwed up the version — we just jumped from 4.4.19 to 4.20. Urgh. I no think the best would be that I release a 5.0 (after all, these very user-visible changes could warrant a bump), that drops the entries for 4.4.19 and 4.20, combining them into a nice changelog for 5.0. That wouldn’t be unacceptable in this situation, right?

Sounds great, assuming we have no already existing versioning policy that disagrees.

>> * I’m unconvinced by the move of tails-installer to $PATH: the major reason why it was installed outside of $PATH is that we don’t support running it on the command line.

> Ah, I was not aware of this (and missed any comment/docs stating this, if any).

Perhaps that was not good enough doc for you, but it was documented in debian/patches/tails-installer-as-helper that you deleted, so it should have popped up in your git diff --cached or similar. I don’t know how to better document such things in the future; any idea?

>> * Why commit:c2c3d8ecf0f5fcce7a188a723afed814abd339ae? It feels:
>> unneeded: IIRC tails/jessie is meant to host the packaging of upstream code that
>> works on Jessie and newer. I doubt we’re breaking compatibility with Jessie here ,
>> are we?

> It felt unintuitive for me to continue the work in a branch referring to tails/jessie, it’s just that.

OK, got it.

> How about tails/stable?

This would be inconsistent with what “stable” means in tails.git. Anyway, re-reading our (outdated) release process doc about branches, I think the best would be to follow DEP-14 conventions. In this case that would be tails/master for “Packages uploaded to the current development release”, and tails/stable when we prepare some kind of hotfix for the stable branch of tails.git while testing/devel have already a newer version.

>> * “Scenario: Writing a Tails isohybrid to a USB drive and booting it, then trying to upgrading it but ending up having to do a fresh installation, which boots” does not match the actual tests we’re doing there anymore.

> True! Now fixed!

LGTM

>> Now, I really don’t want this situation to justify introducing the other issues I’ve raised in Feature #8860#note-40 for more than one release cycle ⇒ they need a ticket for 3.3, with clearly defined responsibility (IMO that would be kurono if he can do it with these time constraints imposed by the hasty merge + my reluctance to introducing such regressions, and anonym otherwise). In other words, let’s not allow a self-imposed emergency mode to lower our code quality expectations.

> I can afford spending a day on this next cycle, if needed, so this should work out fine. kurono (if you are reading this massive comment…), do you think you’ll have some time to work on this before, say, mid-October?

Great.

#62 Updated by anonym 2017-09-23 11:29:55

  • QA Check changed from Dev Needed to Ready for QA
  • Feature Branch changed from feature/8860-tails-installer-improvements installer:master,tails/stretch to feature/8860-tails-installer-improvements installer:master,tails/master

So I’ve built and uploaded tails-installer 5.0, that squashes versions 4.4.19 and 4.20 as described above. I believe it resolves all issues above (and worry not! when closing this ticket I will comb through this ticket and open new tickets for all remaining issues). Notes:

  • As for tails-installer vs PATH, I kept it in PATH but disabled the CLI, and removed the manpage, which IMHO is the best solution.
  • I move Tails’ packaging to tails/master and updated the tails-installer release process (in Tails’ Git).
  • I cannot reproduce your issue with starting tails-installer on Sid; 5.0 works (incl. being able to do an install/upgrade) on Tails, Stretch and Sid for me.

Let’s see what Jenkins thinks:

#63 Updated by intrigeri 2017-09-23 15:48:36

> * As for tails-installer vs PATH, I kept it in PATH but disabled the CLI, and removed the manpage, which IMHO is the best solution.

Debian Policy won’t be happy if a binary in $PATH lacks a manpage, which (I think) is why Ulrike wrote the nice manpage for the launcher in the first place. Would be sad to regress on this front and dump this nice work, no?

#64 Updated by anonym 2017-09-24 21:21:49

intrigeri wrote:
> > * As for tails-installer vs PATH, I kept it in PATH but disabled the CLI, and removed the manpage, which IMHO is the best solution.
>
> Debian Policy won’t be happy if a binary in $PATH lacks a manpage, which (I think) is why Ulrike wrote the nice manpage for the launcher in the first place. Would be sad to regress on this front and dump this nice work, no?

Huh, even for strictly graphical applications that doesn’t look at the command line arguments?

I’ll prepare a 5.0.1, but I hope it’s ok if you review 5.0, and I prepare 5.0.1 afterwards, without a review (i.e. no different than usual, when just importing translations. (?)

#65 Updated by intrigeri 2017-09-25 03:56:18

redmine@labs.riseup.net:
> Huh, even for strictly graphical applications that doesn’t look at the command line arguments?

Yes: https://lintian.debian.org/tags/binary-without-manpage.html (which is actually more specific than Policy). The reasoning is that if you (as the packager) makes a program readily available on the command line by installing it in $PATH, then it should have some doc that at least explains what it does. Anyway, we already have the right content for the missing manpage.

> I’ll prepare a 5.0.1, but I hope it’s ok if you review 5.0, and I prepare 5.0.1 afterwards, without a review (i.e. no different than usual, when just importing translations. (?)

Sounds fine as long as it’s only about translation updates. Now I’m confused: is your work ready to be reviewed? It’s not been reassigned to me yet. I’ll take a look at 5.0 soon anyway, as you’re suggesting here.

#66 Updated by intrigeri 2017-09-25 06:01:53

  • QA Check changed from Ready for QA to Dev Needed

Wrt. release process changes (none of these are blockers, feel free to copy this to Bug #12152 and postpone to 3.3):

  • Debian “Testing” and “Unstable” are creations of your own. “testing/sid” (that we had before) is common as it used to be what one had in /etc/debian_version when running testing or unstable. “testing/unstable” would work too.
  • I should not have let you bump the version to 5.x: our versioning scheme is meant to convey useful info to downstream package maintainers, and in that scheme this bump means “we’re breaking compatibility with Jessie” (which I hope we’re not). Anyway, too late, no big deal :)
  • Your update to versions, branches etc. is much welcome, but the info about Jessie was lost: our current main upstream dev branch is supposed to support it (https://tails.boum.org/install/debian/usb/).
  • Your changes in the “In practice, it’s expected that Tails contributors submit bugfix and” paragraph lose info that’s useful when we’re maintaining two dev branches at the same time (one for current Tails and one for Tails based on next Debian). No huge deal but it feels a bit sad to regress here.
  • Similarly, with your changes in the “Update the Debian package” section, it now assumes that tails/master is the branch that works on current testing/sid, which is not always the case: sometimes we have to start a new (e.g. 6.x) dev branch to keep supporting it, while we’re still shipping something else (e.g. 5.x) in current Tails based on Debian stable. I think you can keep them all as-is (it’s good to have examples that work in the most common case) but please write something like “This assumes that the latest upstream release has been imported into a Tails packaging branch (e.g. `tails/master` or `tails/buster`) already” in the intro, to leave room for “tails/master is not always the right branch”.
  • I understand what follows but I think many other readers will think these two bullet points contradicts each other as it’s not clear that tails/master is an exception to the 2nd rule:
* The `tails/master` branch is used to prepare packages that we upload
to the Tails APT repo for stable releases, but not to Debian.
* The `tails/$codename` branch is used to prepare packages that we upload
to the Tails APT repo but for Tails based on Debian `$codename`. Again,
these packages will not be uploaded to Debian.

Wrt. changes done in the Installer’s master and packaging branches:

  • I don’t know when this was broken, but “Set verbose mode if the DEBUG environment variable is set to 1” does not work anymore. Thankfully -v does the same, so well, fine.
  • tails-installer still documents the --console option, that you’ve removed support for. Once removed, please reference this new commit + 1111fa4cc0e4ec018b8ded72f126404b4777b3d2 on Feature #8861 (whose status / future is unclear but still, let’s be nice to kurono or whoever resumes work on it some day).
  • There are a few too long lines in debian/changelog. IIRC either dch or Lintian complains about it.
  • s/tails installer/Tails Installer/ in debian/changelog.
  • nitpicking: “Detect when […]” has “and” twice.
  • missing manpage but that’s been raised here already.

I’ve installed 5.0+dfsg-0tails1 on sid.

It works fine for me when started from the GNOME Overview, but it fails when started on the command line:

$ tails-installer
Traceback (most recent call last):
File "/bin/tails-installer", line 91, in <module>
main()
File "/bin/tails-installer", line 85, in main
win = TailsInstallerWindow(opts=opts, args=sys.argv)
File "/usr/lib/python2.7/dist-packages/tails_installer/gui.py", line 314, in __init__
self._build_ui()
File "/usr/lib/python2.7/dist-packages/tails_installer/gui.py", line 379, in _build_ui
os.path.join(_get_datadir(), 'tails-installer.ui'))
File "/usr/lib/python2.7/posixpath.py", line 70, in join
elif path == '' or path.endswith('/'):
AttributeError: 'NoneType' object has no attribute 'endswith'

… unless I explicitly run it as /usr/bin/tails-installer. IOW, our code breaks on merged-/usr systems if /bin happens to be before /usr/bin in $PATH. Same with 4.4.18+dfsg-1 so I’ll file a dedicated ticket.

I think I’ve found a bug:

  1. dd a Tails ISO to a USB stick
  2. leave that USB stick plugged
  3. start Tails Installer
  4. /dev/sdc1 is automatically selected
  5. the “Upgrade” button is visible (weird) but disabled (OK)
  6. the “Reinstall (delete all data)” button is enabled, which is inconsistent wrt. the “Upgrade” button (but if I click on it I’m told “No ISO image selected” as expected); that’s another bug, please handle it separately
  7. once I select an ISO image the “Upgrade” button is enabled
  8. clicking “Upgrade” fails with “Unknown GLib exception while trying to mount device: udisks-error-quark: GDBus.Error:org.freedesktop.UDisks2.Error.Failed: Error mounting /dev/sdb1 at /media/intrigeri/Tails2: Wrong fs type, /dev/sdb1 has an invalid superblock or missing helper program. (0)”
  9. unplug / re-plug the USB stick => I see an “Install” button, which works.

That’s a regression because in 4.4.18+dfsg-1 when clicking “Upgrade” I was told:

It is impossible to upgrade the device TOSHIBA TransMemory (7.8 GB) - /dev/sdb1 because it was not created using Tails Installer. You should instead use "Install from ISO" to upgrade Tails on this device.

… and when clicking “Install” the correct device (/dev/sdb) is selected.

Granted, it’s a corner case and thankfully our doc does not instructs anyone to do that. But it makes the developer experience a bit painful, and (worse) confusing when working on the code that handles isohybrid’ed devices. Also, I had doubts wrt. the robustness of some changes made in our isohybrid detection code on this ticket, and this experience decreases a bit my level of confidence.

#67 Updated by anonym 2017-09-25 12:48:10

intrigeri wrote:
> Sounds fine as long as it’s only about translation updates.

And the manpage. However, your remarks (e.g. --console) unfortunately forces me to do a few other changes. Well, actually, since you probably anticipated the two commits related to the manpage, I guess the only “surprise” for you is commit:f071b0f41272196712facbe1a5052ec39d7608ad.

> Now I’m confused: is your work ready to be reviewed? It’s not been reassigned to me yet. I’ll take a look at 5.0 soon anyway, as you’re suggesting here.

I failed to assign it to you, so you did the right thing.

> Wrt. changes done in the Installer’s master and packaging branches:
>
> * I don’t know when this was broken, but “Set verbose mode if the DEBUG environment variable is set to 1” does not work anymore. Thankfully -v does the same, so well, fine.

Apparently I broke this, because it was only part of the launcher’s code. Fixed in commit:f071b0f41272196712facbe1a5052ec39d7608ad!

> * tails-installer still documents the --console option, that you’ve removed support for. Once removed, please reference this new commit + 1111fa4cc0e4ec018b8ded72f126404b4777b3d2 on Feature #8861 (whose status / future is unclear but still, let’s be nice to kurono or whoever resumes work on it some day).

I have now completely disabled all options except --verbose in commit:f071b0f41272196712facbe1a5052ec39d7608ad.

> * There are a few too long lines in debian/changelog. IIRC either dch or Lintian complains about it.

So the process fix is to add runs of lintian at appropriate places (imho, in all Debian packaging we do), and update debian/*.lintian-overrides so it ignores the things we don’t care about.

> * s/tails installer/Tails Installer/ in debian/changelog.
> * nitpicking: “Detect when […]” has “and” twice.

According to u it is verboten to edit past entries, so I’m not sure what to do with these.

> * missing manpage but that’s been raised here already.

FTR, nothing in either of the old manpages was still relevant, so I wrote a new from scratch.

> I think I’ve found a bug:
>
> # dd a Tails ISO to a USB stick
> # leave that USB stick plugged
> # start Tails Installer
> # /dev/sdc1 is automatically selected
> # the “Upgrade” button is visible (weird) but disabled (OK)
> # the “Reinstall (delete all data)” button is enabled, which is inconsistent wrt. the “Upgrade” button (but if I click on it I’m told “No ISO image selected” as expected); that’s another bug, please handle it separately
> # once I select an ISO image the “Upgrade” button is enabled
> # clicking “Upgrade” fails with “Unknown GLib exception while trying to mount device: udisks-error-quark: GDBus.Error:org.freedesktop.UDisks2.Error.Failed: Error mounting /dev/sdb1 at /media/intrigeri/Tails2: Wrong fs type, /dev/sdb1 has an invalid superblock or missing helper program. (0)”
> # unplug / re-plug the USB stick => I see an “Install” button, which works.

So this is exactly what we do in the Writing a Tails isohybrid to a USB drive [...] scenario, except that the dd:ing doesn’t happen during the same session we start tails-installer. I’m hypothesizing that the flakiness you’ve seen re: “isohybrid detection” only occurs during the session the isohybrid is dd:ed to the disk, due to a race between the writing of the partition table to the disk, and the scanning of said partition table, possibly in an intermediate state of the write.

> That’s a regression because in 4.4.18+dfsg-1 when clicking “Upgrade” I was told:
>
> […]
>
> … and when clicking “Install” the correct device (/dev/sdb) is selected.
>
> Granted, it’s a corner case and thankfully our doc does not instructs anyone to do that. But it makes the developer experience a bit painful, and (worse) confusing when working on the code that handles isohybrid’ed devices. Also, I had doubts wrt. the robustness of some changes made in our isohybrid detection code on this ticket, and this experience decreases a bit my level of confidence.

If my hypothesis is correct (which seems supported that this haven’t failed yet in any automated test suite run) it indeed is a pretty peculiar corner case that I doubt will appear outside of testing situations (when would you dd an isohybrid only to overwrite it later during the same session?). A ticket shall be filed about this, however.

#68 Updated by anonym 2017-09-25 13:20:34

anonym wrote:
> commit:f071b0f41272196712facbe1a5052ec39d7608ad.

Not that this commit was force pushed, so Redmin probably told you another commit in your email notification.

> > * There are a few too long lines in debian/changelog. IIRC either dch or Lintian complains about it.
[…]
> > * s/tails installer/Tails Installer/ in debian/changelog.
> > * nitpicking: “Detect when […]” has “and” twice.
>
> According to u it is verboten to edit past entries, so I’m not sure what to do with these.

She now reasoned that it should not be a problem for versions that were never released, which arguably is the case for 5.0, which only appeared in Tails development builds. So I went ahead and fixed up the 5.0 changelog entry.

#69 Updated by anonym 2017-09-25 17:31:24

  • Status changed from In Progress to Fix committed
  • % Done changed from 70 to 100

Applied in changeset commit:d88127540b6a3d9c56d5bf339a71c7fe523405b2.

#70 Updated by anonym 2017-09-25 17:44:41

  • Assignee deleted (anonym)
  • QA Check changed from Dev Needed to Pass

Merged!

These tickets were filed:

And in these instances I just fed the new info into the relevant, existing tickets:

  • Feature #8861 - everything related to running from the command line
  • Bug #12152 - everything related to the Tails Installer release process

#71 Updated by intrigeri 2017-09-26 05:41:28

  • Assignee set to intrigeri
  • % Done changed from 100 to 90
  • QA Check changed from Pass to Ready for QA

I should review the last changes that went into 3.2.

#72 Updated by intrigeri 2017-09-26 06:54:33

> I guess the only “surprise” for you is commit:f071b0f41272196712facbe1a5052ec39d7608ad

Looks good!

>> * I don’t know when this was broken, but “Set verbose mode if the DEBUG environment variable is set to 1” does not work anymore. Thankfully -v does the same, so well, fine.

> Apparently I broke this, because it was only part of the launcher’s code. Fixed in commit:f071b0f41272196712facbe1a5052ec39d7608ad!

ACK and works for me on Debian sid. Thanks :)

> I have now completely disabled all options

Nice hack.

>> * There are a few too long lines in debian/changelog. IIRC either dch or Lintian complains about it.

> So the process fix is to add runs of lintian at appropriate places (imho, in all Debian packaging we do), and update debian/*.lintian-overrides so it ignores the things we don’t care about.

Yes. I’m personally using https://anonscm.debian.org/cgit/pkg-perl/packages/pkg-perl-tools.git/tree/examples/check-build (apt install pkg-perl-tools). Now, frankly I’m not thrilled at the idea of adding more manual steps everywhere; this should be automated and is already covered by Feature #6220#note-11 so I’ll skip this for now.

>> * missing manpage but that’s been raised here already.

> FTR, nothing in either of the old manpages was still relevant, so I wrote a new from scratch.

Ooops, I’m very sorry I have been insisting that we already had the right content in the old manpages :/
Now, I would argue that instead of writing a new description of tails-installer from scratch, we should reuse the one (that’s been improved by our tech writers) from debian/control. Whatever, I can live with the current state of things.

>> I think I’ve found a bug: […]

> If my hypothesis is correct (which seems supported that this haven’t failed yet in any automated test suite run)

I think it is also supported by the fact that after unplugging + re-plugging the USB stick, I can’t reproduce the bug. So I’m very confident that hypothesis is correct.

> it indeed is a pretty peculiar corner case that I doubt will appear outside of testing situations (when would you dd an isohybrid only to overwrite it later during the same session?). A ticket shall be filed about this, however.

I totally agree that this specific bug is unlikely to make anyone experience’s worse, except developers’. To clarify, aside of caring about developers’ experience, my concern is primarily that this bug is a symptom of new fragility introduced in our isohybrid handling code, that could have other, more problematic effects in real-world scenarios we haven’t discovered yet. So I’m glad you’ve created Bug #14724 :)

#73 Updated by intrigeri 2017-09-26 06:56:39

  • QA Check changed from Ready for QA to Pass

:)

> These tickets were filed:

[…]

> And in these instances I just fed the new info into the relevant, existing tickets:

Thanks! I did not double-check that all issues I had raised are correctly tracked, I’ll trust you on that one.

#74 Updated by intrigeri 2017-09-26 06:57:06

  • Assignee deleted (intrigeri)

#75 Updated by intrigeri 2017-09-26 06:57:46

  • % Done changed from 90 to 100

#76 Updated by anonym 2017-09-28 18:48:48

  • Status changed from Fix committed to Resolved