Feature #8551

Clean up Tails Installer's packaging

Added by intrigeri 2015-01-06 13:53:28 . Updated 2015-10-02 17:24:13 .

Status:
Resolved
Priority:
Elevated
Assignee:
Category:
Installation
Target version:
Start date:
2015-01-06
Due date:
% Done:

100%

Feature Branch:
451f:tails/jessie
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Installer
Deliverable for:

Description


Subtasks


History

#1 Updated by intrigeri 2015-01-06 14:48:08

  • blocks #8538 added

#2 Updated by Anonymous 2015-02-26 15:07:19

  • Target version changed from Hardening_M1 to Tails_1.4

#5 Updated by Anonymous 2015-03-19 09:26:51

Hi,

could you eventually elaborate a little bit on what I should look for?
Would simply running lintian —pedantic be sufficient to find the packaging flaws?

Thanks & Cheers!

#6 Updated by intrigeri 2015-03-19 11:18:26

> could you eventually elaborate a little bit on what I should look for?
> Would simply running lintian —pedantic be sufficient to find the packaging flaws?

Yes, this would be a pretty good start. Once this is sorted out I’ll look deeper for other problems that need fixing.

By the way, please base your work on the debian_feature-jessie branch.

#7 Updated by Anonymous 2015-03-19 16:27:53

  • Status changed from Confirmed to In Progress

Started to look at lintian errors for now.

#8 Updated by Anonymous 2015-05-05 12:28:57

  • % Done changed from 0 to 40

I’ve fixed all the Lintian errors.

Will verify if there is still something else to be done on that one.

#9 Updated by Anonymous 2015-05-05 12:30:16

  • Feature Branch set to installer/debian_feature-jessie_u

#10 Updated by intrigeri 2015-05-06 09:14:00

Woohoo, so many improvements and cleanups. Congrats!

I had a look and I have a few comments:

  • syslinux dependency: do we really need it, now that we’re using the syslinux binary and MBR shipped in the ISO? (and by the way, in the current state of things I bet Tails Installer only works on i386 systems, I’ll add a note about it on Feature #8556)
  • to do anything useful, Tails Installer needs the syslinux command that we ship in the ISO. In Debian, syslinux is only available for i386 and amd64 => likely we’ll ship binaries only for these two architectures in the ISO (Feature #8556) => we need to set the Architecture field of our binary package to amd64 i386, I think. Right?
  • debian/changelog says “Mark old bug numbers as Tails bug numbers” but I don’t see where this change has actually been made.
  • commit 7999758 changes a lot of lines it shouldn’t touch in the upstream ChangeLog file. Please use git add -p, to avoid introducing such errors, if you’re not reviewing your commits after-the-fact. Besides, ChangeLog is part of the upstream source, so should not be modified on the packaging branch this way. Instead: if the change were Debian-specific, then we would need a quilt patch; otherwise, the change needs to be done on the upstream (feature/jessie in the case at hand) branch.
  • commit 53e091b changes one line in the upstream AUTHORS file that it shouldn’t touch. Same comments as above regarding where the correct change this commit introduces should live.
  • The Files: * section in debian/copyright is meant to document whatever upstream claims, so the part of commit 1deb673 that bumps copyright years for Files: * is incorrect. It would be correct if you first bumped copyright years claimed by upstream, on the upstream (feature/jessie) branch, too. Sadly that’s not an atomic commit that could simply be reverted, but you get the idea.
  • The package’s description shouldn’t restrict supported use cases to USB sticks. Also, it should use consistent terminology (it currently says “USB stick” in one place, and “USB flash drive” in another. See the Tails documentation for the hardware support we’re advertising to users, and please have sajolida review the string once you’re happy with your updated description :)
  • Should debian/compat be bumped to 9? Now that we’re depending on debhelper 9, what prevents us from modernizing the packaging this way?
  • I see you’ve declared a switch to source format 3.0 (quilt), which is good. However, this change is only theoretical as long as changes to the upstream source are not managed with quilt, no? I’ll build a package and see what happens, but the current state of things seems quite wrong to me. I’ve elaborated a little bit on this topic above, and in an email you should now have received :)
  • That’s definitely only a matter of taste, but I prefer updating debian/changelog in dedicated commits (e.g. with gbp dch) instead of doing it in the commit that introduces the change it documents: IMO the strategy I prefer makes it easier to cherry-pick/merge stuff between packaging branches, without too many conflicts. Do as you wish, but please pick one strategy and stick to it :)
  • I think the “dfsg” component of the version number should be part of the upstream version number, e.g. 3.11.6+tails1+dfsg-16+jessie4 instead of 3.11.6+tails1-16+jessie4+dfsg. It’s a change applied to the upstream tarball, not to the packaging bits.

#11 Updated by sajolida 2015-05-07 14:54:19

Here is what I propose:


Description: Installer for the Tails live operating system
 Tails Installer is a graphical tool to install or upgrade Tails on a
 USB stick or SD card from an ISO image.

 Tails Installer is the only installation technique that allows you to:

   - Benefit from automatic upgrades.
   - Create a persistent encrypted storage for your personal
     files in the rest of the space available.

#12 Updated by Anonymous 2015-05-07 20:18:55

sajolida wrote:
> Here is what I propose:
>
> […]

thanks a lot.

I’ve changed the last sentence though from “space available.” to “available space.”.

#13 Updated by Anonymous 2015-05-07 20:19:52

  • Feature Branch changed from installer/debian_feature-jessie_u to installer/debian_feature-jessie_clean

intrigeri wrote:
> Woohoo, so many improvements and cleanups. Congrats!
>
> I had a look and I have a few comments:
>
> * syslinux dependency: do we really need it, now that we’re using the syslinux binary and MBR shipped in the ISO? (and by the way, in the current state of things I bet Tails Installer only works on i386 systems, I’ll add a note about it on Feature #8556)

that’s still keft to be tested.

Thanks for all the other useful comments.

#14 Updated by Anonymous 2015-05-07 21:27:34

  • Assignee set to intrigeri
  • QA Check set to Ready for QA

I’ve committed the upstream changes to the feature/jessie branch on my repository fyi.

Could you please check that everything is there and in the above mentioned feature branch.

When building, I do not get any lintian errors anymore.

I’ll document how I created the package on Feature #8554

#15 Updated by Anonymous 2015-05-07 21:37:03

Actually the syslinux part is not “Ready for QA”.

And I’ll create a sub ticket for checking the copyright of the upstream icons.

#16 Updated by intrigeri 2015-05-07 23:09:17

> Could you please check that everything is there and in the above mentioned feature branch.
> […]
> I’ll document how I created the package on Feature #8554

Great! OK, I’ll try to check what I can without the documentation, but there are things I won’t be able to guess and I may not have time to bruteforce them, so I can only complete the review once there’s some progress on Feature #8555.

#17 Updated by intrigeri 2015-05-07 23:09:36

> Actually the syslinux part is not “Ready for QA”.

Please create a sub-ticket for it so that it doesn’t slip through the cracks.

> And I’ll create a sub ticket for checking the copyright of the upstream icons.

Excellent!

#18 Updated by intrigeri 2015-05-08 12:32:54

  • Feature Branch changed from installer/debian_feature-jessie_clean to 451f:installer/debian_feature-jessie_clean

(Setting non-ambiguous feature branch.)

#19 Updated by intrigeri 2015-05-08 13:14:31

  • Assignee deleted (intrigeri)
  • QA Check changed from Ready for QA to Dev Needed

OK, so I had another look, and all important things (modulo the syslinux dependency) look good! Some nitpicking:

  • Please keep local gbp configuration changes in the config file that’s meant for those, that is .git/gbp.conf (see gbp.conf(5), instead of setting debian-branch in debian/gbp.conf to something that works only for you.
  • The short description of the binary package has a trailing whitespace => please fix it. I suggest configuring your Git to highlight such mistakes in a super-visible way, e.g. I’m using git config --global color.diff.whitespace "red reverse".
  • I don’t think the long description should end with an empty line (.).

That’s all, we’re almost there! Congrats :)

#20 Updated by Anonymous 2015-05-08 14:24:36

I’ve built a test package without the syslinux dependency and tested it on a Debian Jessie without the syslinux command. I could run the installer. More on Feature #8556

#21 Updated by Anonymous 2015-05-08 14:34:35

intrigeri wrote:
> OK, so I had another look, and all important things (modulo the syslinux dependency) look good! Some nitpicking:

Thanks!

> * Please keep local gbp configuration changes in the config file that’s meant for those, that is .git/gbp.conf (see gbp.conf(5), instead of setting debian-branch in debian/gbp.conf to something that works only for you.

Ok, I had never used this before, but I’m glad to learn that there is a local conf :)

> * The short description of the binary package has a trailing whitespace => please fix it. I suggest configuring your Git to highlight such mistakes in a super-visible way, e.g. I’m using git config --global color.diff.whitespace "red reverse".

Thanks for the config option, but i can’t seem to confirm this trailing whitespace?

> * I don’t think the long description should end with an empty line (.).

Fixed.

> That’s all, we’re almost there! Congrats :)

woohoo!

#22 Updated by Anonymous 2015-05-08 14:43:44

  • % Done changed from 40 to 50

#23 Updated by intrigeri 2015-05-08 15:22:52

> I’ve built a test package without the syslinux dependency and tested it on a Debian Jessie without the syslinux command. I could run the installer.

It would be good, if that’s not what you did, to test on a system that has none of the binary packages (syslinux, syslinux-efi, extlinux, isolinux, pxelinux, syslinux-common, syslinux-utils) installed: Tails Installer might be using other bits they ship than the syslinux command. Sorry I should have provided this information earlier :/

#24 Updated by intrigeri 2015-05-08 15:30:21

> Ok, I had never used this before, but I’m glad to learn that there is a local conf :)

And debian/gbp.conf now says debian-branch = installer/debian_feature-jessie_u… which is maybe better, but still not what we want. Please reset it to its original value, that was debian_feature-jessie. Thanks in advance :)

Once this is done, I think we’re almost good on this front for now. Please reassign to me for QA once:

  • the syslinux dependency topic has been addressed
  • the package is using pristine-tar, as requested elsewhere
  • Feature #8554 and Feature #8555 are mostly completed, and I’ll test the entire packaging workflow to validate it as well as the updated packaging itself.

Yay!

> Thanks for the config option, but i can’t seem to confirm this trailing whitespace?

Indeed, trailing whitespace was added in 3769c3f, and then removed in a1232c2. Sorry for the noise.

#25 Updated by Anonymous 2015-05-11 07:33:24

note from intrigeri: “it’ll soon be time to debdiff your resulting pkgs with the one currently on feature-jessie (both the .dsc and the .deb) so that we can make sure that the set of files we ship is correct, etc.”

#26 Updated by Anonymous 2015-05-11 19:41:58

intrigeri wrote:
> > I’ve built a test package without the syslinux dependency and tested it on a Debian Jessie without the syslinux command. I could run the installer.
>
> It would be good, if that’s not what you did, to test on a system that has none of the binary packages (syslinux, syslinux-efi, extlinux, isolinux, pxelinux, syslinux-common, syslinux-utils) installed: Tails Installer might be using other bits they ship than the syslinux command. Sorry I should have provided this information earlier :/

I’ve finally managed to test the package in a clean Jessie environment without any of those packages installed. I get this warning when running liveusb-creator:

creator.py: 1233 (get_ext_linux_version) WARNING: extlinux not found. Only FAT filesystems will be supported.

#27 Updated by intrigeri 2015-05-11 20:22:23

> I get this warning when running liveusb-creator:

>

> creator.py: 1233 (get_ext_linux_version) WARNING: extlinux not found. Only FAT filesystems will be supported.
> 

No problem, we don’t support non-FAT anyway.

#28 Updated by Anonymous 2015-05-13 09:22:37

  • Target version changed from Tails_1.4 to Tails_1.4.1

#29 Updated by Anonymous 2015-06-28 10:04:12

  • Target version changed from Tails_1.4.1 to Tails_1.5

#30 Updated by Anonymous 2015-07-14 04:26:19

  • Assignee set to intrigeri
  • QA Check changed from Dev Needed to Ready for QA
  • Feature Branch changed from 451f:installer/debian_feature-jessie_clean to 451f:tails/jessie

#31 Updated by Anonymous 2015-07-14 04:26:28

  • % Done changed from 50 to 70

#32 Updated by intrigeri 2015-07-18 04:53:32

  • Assignee deleted (intrigeri)
  • QA Check changed from Ready for QA to Dev Needed

There’s no tails/jessie branch in your repo, please push it :)

#33 Updated by Anonymous 2015-07-18 07:52:28

  • Assignee set to intrigeri
  • QA Check changed from Dev Needed to Ready for QA

incredible. pushed it again.

#34 Updated by intrigeri 2015-07-18 08:24:57

  • Assignee deleted (intrigeri)
  • QA Check changed from Ready for QA to Dev Needed

Excellent! We’re almost there, and I only have a few nitpicking comments left:

  • Please merge current debian_feature-jessie into your branch: some things have changed there two weeks ago (and more recently), and as a result there are a few (trivial) merge conflicts.
  • I’ve pushed 4eb4720 to your branch, that removes trailing whitespaces in the debian/ directory. Sorry if I sound like a broken record, but if not done yet, please do set up your Git so that it prevents you from introducing such mistakes.
  • I don’t understand why liveusb-creator-launcher.1 is introduced as part of the Debian delta, instead of directly upstream. Sorry if we’ve discussed this already and I forgot.

#35 Updated by intrigeri 2015-07-18 08:28:34

Also, please push your pristine-tar branch, otherwise nobody but you can build the package, which prevents reviewing :)

#36 Updated by Anonymous 2015-07-25 01:55:01

intrigeri wrote:
> Excellent! We’re almost there, and I only have a few nitpicking comments left:
>
> * Please merge current debian_feature-jessie into your branch: some things have changed there two weeks ago (and more recently), and as a result there are a few (trivial) merge conflicts.

done.

> * I’ve pushed 4eb4720 to your branch, that removes trailing whitespaces in the debian/ directory. Sorry if I sound like a broken record, but if not done yet, please do set up your Git so that it prevents you from introducing such mistakes.

thanks. I’ve added the corresponding git config line now.

> * I don’t understand why liveusb-creator-launcher.1 is introduced as part of the Debian delta, instead of directly upstream. Sorry if we’ve discussed this already and I forgot.

Added to upstream repo with your modifications.

#37 Updated by Anonymous 2015-07-25 01:58:20

As noted on Feature #8554 by intrigeri:

  • I see a dependency on python-qt4: I think this is the result of a mistake when resolving a merge conflict.
    debian/copyright: it’s a bit unclear if data/usb-creator-gtk.svg is under GPL v3 or v3+.

This has been fixed.

  • I believe Lintian should warn you about too long lines in debian/changelog. But maybe it needs to be run with —pedantic? In any case, those lines should be fixed.

This should be fixed.

  • debian/changelog has a merge conflict marker: >>>>>>> debian_feature-jessie

Can’t see this on this branch, but need to verify this.

#38 Updated by intrigeri 2015-08-03 05:00:12

Should I review the current state of your work, or?

#39 Updated by intrigeri 2015-08-04 08:32:53

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

#40 Updated by Anonymous 2015-09-22 14:43:41

question to elucidate: do we add the requirements that are needed only for the package outside of Tails to the debian/sid pkg branch which would then diverge from the tails/jessie pkg branch?

#41 Updated by Anonymous 2015-09-22 15:25:13

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

#42 Updated by intrigeri 2015-09-22 16:01:44

> question to elucidate: do we add the requirements that are needed only for the package outside of Tails to the debisan/sid pkg branch which would then diverge from the tails/jessie pkg branch?

Please clarify what are these “requirements”, I’m not sure I get it.

#43 Updated by Anonymous 2015-09-23 04:01:06

  • Target version deleted (Tails_1.7)

Sorry for not being clear enough. I mean the Depends: field in debian/control, precisely.

#44 Updated by intrigeri 2015-09-23 04:43:07

> Target version deleted (Tails_1.7)

Sounds like a mistake, no?

> Sorry for not being clear enough. I mean the Depends: field in debian/control, precisely.

OK. So:

> do we add the requirements that are needed only for the package outside of Tails to the debisan/sid pkg branch which would then diverge from the tails/jessie pkg branch?

I can’t find anymore where the branch layout is documented, but IIRC the tails/jessie branch is supposed to be a backport of the Debian stuff, with Tails-specific adjustments as needed, rather than the other way round.

What dependencies are only needed outside of Tails?

#45 Updated by Anonymous 2015-09-23 08:16:40

  • Priority changed from Normal to Elevated
  • Target version set to Tails_1.7

intrigeri wrote:
> > Target version deleted (Tails_1.7)
>
> Sounds like a mistake, no?

yep.

> > Sorry for not being clear enough. I mean the Depends: field in debian/control, precisely.
>
> OK. So:
>
> > do we add the requirements that are needed only for the package outside of Tails to the debisan/sid pkg branch which would then diverge from the tails/jessie pkg branch?
>
> I can’t find anymore where the branch layout is documented, but IIRC the tails/jessie branch is supposed to be a backport of the Debian stuff, with Tails-specific adjustments as needed, rather than the other way round.
>
> What dependencies are only needed outside of Tails?

that is policykit-1 and syslinux imo.

#46 Updated by Anonymous 2015-09-23 08:17:25

u wrote:
> intrigeri wrote:
> > > Target version deleted (Tails_1.7)
> >
> > Sounds like a mistake, no?
>
> yep.
>
> > > Sorry for not being clear enough. I mean the Depends: field in debian/control, precisely.
> >
> > OK. So:
> >
> > > do we add the requirements that are needed only for the package outside of Tails to the debisan/sid pkg branch which would then diverge from the tails/jessie pkg branch?
> >
> > I can’t find anymore where the branch layout is documented, but IIRC the tails/jessie branch is supposed to be a backport of the Debian stuff, with Tails-specific adjustments as needed, rather than the other way round.

that sounds reasonable.

> > What dependencies are only needed outside of Tails?
>
> that is policykit-1 and syslinux imo.

#47 Updated by intrigeri 2015-09-25 03:24:50

>> What dependencies are only needed outside of Tails?

> that is policykit-1 and syslinux imo.

We install them in Tails via other means and for other reasons anyway, and I doubt it’ll change any time soon, so IMO it’s fine to keep these deps in the Tails Installer packaging. Not worth creating/growing a delta at all IMO.

#48 Updated by Anonymous 2015-10-02 15:14:34

  • Assignee set to intrigeri
  • QA Check changed from Dev Needed to Ready for QA

I think everything has been fixed now.

#49 Updated by intrigeri 2015-10-02 17:24:13

  • Status changed from In Progress to Resolved
  • Assignee deleted (intrigeri)
  • % Done changed from 70 to 100
  • QA Check changed from Ready for QA to Pass

Indeed, the packaging looks clean and solid to me now.