Tails

#1 Updated by segfault 2018-04-11 12:48:40

• Deliverable for set to 299

#2 Updated by intrigeri 2018-04-15 13:00:38

• Status changed from Confirmed to In Progress

#3 Updated by bertagaz 2018-05-10 11:09:22

• Target version changed from Tails_3.7 to Tails_3.8

#5 Updated by intrigeri 2018-06-03 17:59:21

• QA Check set to Dev Needed

General comments:

• Many of the debdiff’s modify a number of old debian/changelog entries; please fix that. Maybe your editor is too clever? :)
• Some debian/control are modified on top of debian/control.in (and sometimes were not kept in sync); it’ll be confusing next time someone else than you has to update this package, e.g. to cope with a DSA in gjs.
• It would be nice if the added debian/patches/* referenced (with http://dep.debian.net/deps/dep3/) where the patch comes from: in your case, a link to the Git branch + commit ID should be enough; this way, whenever the branch (based on upstream) is updated, we can easily tell whether the packaging has the last version or not.

And about specific packages:

• gjs: I’m worried about the maintenance cost of (and meaning conveyed by) the versioned dependencies you’ve added on exact versions. I understand it’s a good way to enforce rebuilding this package when needed (otherwise it simply cannot be installed) but perhaps there’s a middle ground to be found? I see the official Debian packages has >= dependencies (on rather old versions) which leads me to suspect that we don’t really need to rebuild this package every time we update any of the 3 dependencies we patch. Right? Perhaps it would be enough to have versioned dependencies such as libglib2.0-dev (>= 2.50.3-2.0tails2)? That should be enough to ensure the new pieces of API we need are available.
• gvfs:
  • same as gjs, I suspect the versioned deps are too strict.
  • your diff is modifying unrelated lines in debian/control.in.
• gobject-introspection: same as above, too strict versioned deps
• gnome-shell: same as above, too strict versioned deps

#6 Updated by intrigeri 2018-06-03 18:22:15

• related to Feature #14481: Release Beta for VeraCrypt support in Tails added

#8 Updated by intrigeri 2018-06-04 19:35:06

• Target version changed from Tails_3.8 to Tails_3.9

This can (and should) wait post-3.8.

#11 Updated by segfault 2018-07-04 20:13:34

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

#12 Updated by intrigeri 2018-07-05 07:28:40

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

>> debian/patches/0002-ShellMountPasswordDialog-Improve-how-Disks-is-linked.patch
>> patches some more CSS files than
>> https://gitlab.gnome.org/segfault3/gnome-shell/commit/73b7ab60e251fe1ba996e70ed9dff3fadbf98850;
>> does that mean these CSS files are not generated/updated from source at package
>> build time? If that’s the case, it may be a bug in the Debian packaging, that would
>> be worth reporting.

> No, the additional CSS files are simply not present any more in the upstream master branch, but are in 3.22.3.

To be more precise, the generated CSS not being in upstream Git is not sufficient: what really matters is whether the generated CSS are part of the upstream tarball and if they are, whether they’re *re*generated from source as part of the Debian package build. So I’ve verified that these generated CSS are not on the debian/master branch of the packaging Vcs-Git (good news!). I guess that’s because https://gitlab.gnome.org/GNOME/gnome-shell/commit/c62e7a6a82fd94a3e66c8ebf02e112b45431eb9d says it will “always generate CSS files from Sass sources”. Case closed, then!

>> debian/patches/0001-Support-TCRYPT-in-ShellMountPasswordDialog.patch seems to
>> be slightly outdated: it lacks
>> https://gitlab.gnome.org/segfault3/gnome-shell/commit/b907db9b4052623dfe221c482e65c6064760b073

> That’s also specific to Stretch. The error messages differ because in Stretch it comes from cryptsetup, in more recent versions it comes from libblockdev.

OK. Oh crap, I did not get initially that we were checking hardcoded error messages coming from another piece of software. I know it’s not your fault (https://bugs.freedesktop.org/show_bug.cgi?id=51271). Fine, then.

So we’re good wrt. my Feature #15521#note-9 review.

Next step: go through Feature #15521#note-5 to check if everything in there was fixed already.

#14 Updated by segfault 2018-07-05 12:41:00

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

#16 Updated by intrigeri 2018-07-06 12:23:05

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

Since there’s no source package readily available to debdiff and your Git branch is not based on anything I can compare it to with Git — both for good reasons, I’m not complaining! — the only way for me to review this fully would be to clone 4 Git repos and build the source packages myself. That does not seem to be worth the effort considering what changes we’re talking about. We’ll have to rebuild these 4 packages one last time anyway before we merge the branch, I’ll debdiff vs. the Stretch packages once we’re there. So I took a quick look via the GitLab web UI.

Everything looks good but my previous comment wrt. gjs versioned deps still stands:

• https://gitlab.com/segfault3/tails-packaging-gjs/blob/master/gjs-1.46.0/debian/control.in#L12
• https://gitlab.com/segfault3/tails-packaging-gjs/blob/master/gjs-1.46.0/debian/control.in#L13

Regarding sync’ing control.in to control: I doubt that’s needed, I think control is generated from control.in during the build, so if I’m correct, better keep the debdiff clean and meaningful and not touch control at all. Which will avoid desynchronization problems. Sorry I was unclear last month which might have lead you to understand I wanted you to sync’ these files: my comment about the fact some control files had been modified but not been kept in sync’ was rather meant to suggest that modifying them was not needed.

#17 Updated by segfault 2018-07-06 17:25:00

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

intrigeri wrote:
> Since there’s no source package readily available to debdiff and your Git branch is not based on anything I can compare it to with Git — both for good reasons, I’m not complaining! — the only way for me to review this fully would be to clone 4 Git repos and build the source packages myself. That does not seem to be worth the effort considering what changes we’re talking about. We’ll have to rebuild these 4 packages one last time anyway before we merge the branch, I’ll debdiff vs. the Stretch packages once we’re there.

ack

> Everything looks good but my previous comment wrt. gjs versioned deps still stands:
>
> * https://gitlab.com/segfault3/tails-packaging-gjs/blob/master/gjs-1.46.0/debian/control.in#L12
> * https://gitlab.com/segfault3/tails-packaging-gjs/blob/master/gjs-1.46.0/debian/control.in#L13

I really don’t get how I could miss this twice. I’m sure I checked the control.in files of all packages. Fixed now.

> Regarding sync’ing control.in to control: I doubt that’s needed, I think control is generated from control.in during the build, so if I’m correct, better keep the debdiff clean and meaningful and not touch control at all. Which will avoid desynchronization problems. Sorry I was unclear last month which might have lead you to understand I wanted you to sync’ these files: my comment about the fact some control files had been modified but not been kept in sync’ was rather meant to suggest that modifying them was not needed.

Yes, yesterday I read the “(and sometimes were not kept in sync)” as implying that I should sync them. It was my point before that we don’t have to commit these changes, because the file is generated during build (see Feature #14481#note-29). I removed these commits from my branches now.

#18 Updated by intrigeri 2018-07-07 09:40:24

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

>> Everything looks good but my previous comment wrt. gjs versioned deps still stands: […]

> I really don’t get how I could miss this twice. I’m sure I checked the control.in files of all packages. Fixed now.

Good.

>> Regarding sync’ing control.in to control: […]

> Yes, yesterday I read the “(and sometimes were not kept in sync)” as implying that I should sync them. It was my point before that we don’t have to commit these changes, because the file is generated during build (see Feature #14481#note-29). I removed these commits from my branches now.

Great. So we’re done here for now! Let’s come back to it once we have other changes to apply on the packages or we’re confident the current state is what we want to merge, whichever comes first.

#19 Updated by intrigeri 2018-07-07 09:41:13

• blocked by Feature #14480: Fix bugs and UX issues of VeraCrypt support added

#20 Updated by intrigeri 2018-07-07 09:41:22

• blocked by Feature #15589: Process community feedback added

#21 Updated by segfault 2018-07-09 20:14:24

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

I found and fixed the issue I was talking about in Feature #14481#note-64. It required a small patch to GTK, so I built a new package. Please review and upload.

#22 Updated by intrigeri 2018-07-10 08:23:54

• Assignee changed from intrigeri to segfault
• QA Check changed from Ready for QA to Info Needed

> I found and fixed the issue I was talking about in Feature #14481#note-64. It required a small patch to GTK, so I built a new package. Please review and upload.

The packaging is perfect this time :)

Two questions:

• Is it on purpose that you left the patch that adds debug output enabled? FWIW one option could be to leave it in debian/patches but drop it from debian/patches/series so it’s not applied.
• Regarding 0002-gtkmountoperation-Support-TCRYPT-options.patch: is this something that needs to be upstreamed? If not, maybe add a Forwarded: not-needed DEP-3 header so none of the future reviewers/uploaders of this custom package of ours asks the same question again :)

This being said, I want to optimize in favour of trying to put the beta out by tomorrow afternoon (if possible), so I’ll build and upload regardless.

#23 Updated by intrigeri 2018-07-10 08:38:18

• QA Check changed from Info Needed to Dev Needed

The package FTBFS for me:

make[4]: *** [gtk-query-settings] Error 1
./.libs/libgtk-3.so: undefined reference to `g_mount_operation_set_hidden_volume'
./.libs/libgtk-3.so: undefined reference to `g_mount_operation_set_pim'
./.libs/libgtk-3.so: undefined reference to `g_mount_operation_set_system_volume'
collect2: error: ld returned 1 exit status
Makefile:2981: recipe for target 'gtk-launch' failed
make[4]: *** [gtk-launch] Error 1
./.libs/libgtk-3.so: undefined reference to `g_mount_operation_set_hidden_volume'
./.libs/libgtk-3.so: undefined reference to `g_mount_operation_set_pim'
./.libs/libgtk-3.so: undefined reference to `g_mount_operation_set_system_volume'
collect2: error: ld returned 1 exit status
Makefile:2985: recipe for target 'gtk-query-immodules-3.0' failed
make[4]: *** [gtk-query-immodules-3.0] Error 1
g-ir-scanner: link: /bin/bash ../libtool --mode=link --tag=CC gcc -o /<<PKGBUILDDIR>>/debian/build/shared/gtk/tmp-introspecthygc6yxk/Gtk-3.0 -export-dynamic -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -Wall -Wl,-z,relro -Wl,-O1 tmp-introspecthygc6yxk/<<PKGBUILDDIR>>/debian/build/shared/gtk/tmp-introspecthygc6yxk/Gtk-3.0.o -L. libgtk-3.la ../gdk/libgdk-3.la -lgio-2.0 -lgobject-2.0 -Wl,--export-dynamic -lgmodule-2.0 -pthread -lglib-2.0
libtool: link: gcc -o /<<PKGBUILDDIR>>/debian/build/shared/gtk/tmp-introspecthygc6yxk/.libs/Gtk-3.0 -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -Wall -Wl,-z -Wl,relro -Wl,-O1 tmp-introspecthygc6yxk/<<PKGBUILDDIR>>/debian/build/shared/gtk/tmp-introspecthygc6yxk/Gtk-3.0.o -Wl,--export-dynamic -pthread -Wl,--export-dynamic  -L. ./.libs/libgtk-3.so ../gdk/.libs/libgdk-3.so -lgio-2.0 -lgobject-2.0 -lgmodule-2.0 -lglib-2.0 -pthread
./.libs/libgtk-3.so: undefined reference to `g_mount_operation_set_hidden_volume'
./.libs/libgtk-3.so: undefined reference to `g_mount_operation_set_pim'
./.libs/libgtk-3.so: undefined reference to `g_mount_operation_set_system_volume'
collect2: error: ld returned 1 exit status

Maybe we need a versioned build-dep on some newer library headers?

#28 Updated by segfault 2018-07-10 11:55:14

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

intrigeri wrote:
> > Yes, that needs to be upstreamed. I will create a merge request and a ticket to track the status. I think it would make sense to track this as a subtask of Feature #14467, because it affects Nautilus (beside others).
>
> Feature #14467 is part of our old set of tickets that maps 1-1 to the sponsor deliverables and that we don’t use much for organizing this work. So instead I’ve created a subtask of Feature #15244: Feature #15724 :)

Thanks!

I uploaded GTK 3.22.11-1.0tails4.

#29 Updated by intrigeri 2018-07-10 13:04:50

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

segfault wrote:
> I uploaded GTK 3.22.11-1.0tails4.

Me too! And triggered a Jenkins build. So we can put this ticket on the back burner until it’s time to rebuild + upload with all recent packaging fixes.

#32 Updated by segfault 2018-08-12 11:22:03

• Description updated
• Assignee changed from segfault to intrigeri
• QA Check set to Ready for QA
• Feature Branch set to segfault:tails-tcrypt-packages

I pushed new packages for glib, gtk, gvfs, gobject-introspection, gjs, and gnome-shell to my repository.

The gtk patch to include a spinner in the sidebar causes some tests to fail during packaging. After spending some hours trying to fix this (unsuccessfully), I removed the patch for now. I hope that I can still fix this in time for 3.9, but I’ll only continue working on this after finishing my reviews for 3.9.

#33 Updated by intrigeri 2018-08-13 09:37:22

• Description updated

#34 Updated by intrigeri 2018-08-13 09:57:00

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

segfault wrote:
> I pushed new packages for glib, gtk, gvfs, gobject-introspection, gjs, and gnome-shell to my repository.

Blockers:

• Please sign the .dsc files: I’m not going to blindly trust gitlab.com.

And below, here are a few comments that don’t block the merge but should be useful so you learn how to submit better packaging next time :)

• glib:
  • 0001-Support-TCRYPT-volumes.patch seems to be leftover cruft; not a blocker but still, please fix this in your packaging repo so the next iteration gets this right. I’m surprised because I thought you were using gbp pq, which addresses this problem automatically.
  • Please run Lintian in the future, so you don’t forget the non-maintainer upload line in debian/changelog.
• gtk+3.0
  • The changelog does not document the libglib2.0-dev versioned build-dep bump. FYI gbp dch helps updating debian/changelog automatically, which avoids such problems.

I’ll review the other packages once I can get their source in an authenticated manner.

#35 Updated by segfault 2018-08-13 22:10:15

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

intrigeri wrote:
> segfault wrote:
> > I pushed new packages for glib, gtk, gvfs, gobject-introspection, gjs, and gnome-shell to my repository.
>
> Blockers:
>
> * Please sign the .dsc files: I’m not going to blindly trust gitlab.com.

Sorry! I signed them now, and also created a pre-push hook that checks whether all .dsc, .changes, and .buildinfo files are signed.

> * glib:
> 0001-Support-TCRYPT-volumes.patch seems to be leftover cruft; not a blocker but still, please fix this in your packaging repo so the next iteration gets this right.

Fixed.

> I’m surprised because I thought you were using gbp pq, which addresses this problem automatically.

No, I’m using quilt to import and apply the patches. I will try to use gbp pq next time.

> Please run Lintian in the future, so you don’t forget the non-maintainer upload line in debian/changelog.

I didn’t know about lintian. I just tried it (version 2.5.95), but it doesn’t tell me anything about a missing non-maintainer upload line. Instead, it says that feature-14481-tcrypt-support-beta is a bad distribution.

> * gtk+3.0
> The changelog does not document the libglib2.0-dev versioned build-dep bump.

Should I fix this in the changelog, so that it is included in the next release?

> FYI gbp dch helps updating debian/changelog automatically, which avoids such problems.

That’s good to know! Unfortunately, gbp dch doesn’t find debian/changelog in my repository, because it’s not in the repository root :/

#37 Updated by intrigeri 2018-08-14 07:35:58

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

Blockers:

• glib: the debian/libglib2.0-0.symbols claims that symbols like g_mount_operation_get_is_tcrypt_hidden_volume were introduced in 2.50.3-2.0tails2, while I think they’re introduced in this new version (2.50.3-2.0tails3). This will prevent the packaging toolchain from automatically adding the correct versioned dependencies (${shlibs:Depends}) when building reverse build-deps. This prevents me from building this package and in turn all others. When fixing this:
  • Don’t bother rebuilding binary packages, I’ll have to do it anyway.
  • Exceptionally, don’t bump the version number; instead, re-release 2.50.3-2.0tails3 so we don’t have to update versioned build-deps in all other packages.
• gnome-shell:
  • The debdiff against gnome-shell_3.22.3-3.dsc modifies old debian/changelog entries.
  • Do we really need strict dependencies against specific versions of libgjs-dev and libglib2.0-dev or should we use >= as everywhere else?

Nitpicking aka. skills sharing: in several debian/changelog you wrote things like “Update PACKAGE to VERSION” which is not an accurate description of the corresponding change. Something like “Bump versioned build-dependency on PACKAGE to >=VERSION” would be vastly better Don’t bother changing past entries to fix that but please keep it in mind for the future :)

#38 Updated by segfault 2018-08-14 10:11:05

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

intrigeri wrote:
> >> * glib:
> >> 0001-Support-TCRYPT-volumes.patch seems to be leftover cruft; not a blocker but still, please fix this in your packaging repo so the next iteration gets this right.
>
> > Fixed.
>
> I think you forgot to push: https://gitlab.com/segfault3/tails-packaging-glib/blob/master/glib2.0-2.50.3/debian/patches/0001-Support-TCRYPT-volumes.patch

Right, I thought that would only be needed once I release a new version. But I now included it in the re-release of 2.50.3-2.0tails3.

> >> Please run Lintian in the future, so you don’t forget the non-maintainer upload line in debian/changelog.
>
> > I didn’t know about lintian. I just tried it (version 2.5.95), but it doesn’t tell me anything about a missing non-maintainer upload line. Instead, it says that feature-14481-tcrypt-support-beta is a bad distribution.
>
> Interesting. Perhaps the source packages checks were not run. Which file did you call Lintian on?
> FTR I was expecting https://lintian.debian.org/tags/changelog-should-mention-nmu.html.

I ran it in the packaging repository. When I run it on the resulting *.dsc file, it does indeed tell me about `changelog-should-mention-nmu` and doesn’t mention the bad distribution anymore.

> >> * gtk+3.0
> >> The changelog does not document the libglib2.0-dev versioned build-dep bump.
>
> > Should I fix this in the changelog, so that it is included in the next release?
>
> Yes, please.

Done, and included in the re-release.

> Blockers:
>
> * glib: the debian/libglib2.0-0.symbols claims that symbols like g_mount_operation_get_is_tcrypt_hidden_volume were introduced in 2.50.3-2.0tails2, while I think they’re introduced in this new version (2.50.3-2.0tails3).

Correct.

> This will prevent the packaging toolchain from automatically adding the correct versioned dependencies (${shlibs:Depends}) when building reverse build-deps. This prevents me from building this package and in turn all others. When fixing this:
> Don’t bother rebuilding binary packages, I’ll have to do it anyway.

I did it anyway, because it helps me keep track of which versions we released.

> Exceptionally, don’t bump the version number; instead, re-release 2.50.3-2.0tails3 so we don’t have to update versioned build-deps in all other packages.

Done.

> * gnome-shell:
> The debdiff against gnome-shell_3.22.3-3.dsc modifies old debian/changelog entries.

Fixed.

> Do we really need strict dependencies against specific versions of libgjs-dev and libglib2.0-dev or should we use >= as everywhere else?

No we don’t, I fixed it.

> Nitpicking aka. skills sharing: in several debian/changelog you wrote things like “Update PACKAGE to VERSION” which is not an accurate description of the corresponding change. Something like “Bump versioned build-dependency on PACKAGE to >=VERSION” would be vastly better Don’t bother changing past entries to fix that but please keep it in mind for the future :)

ACK

#41 Updated by intrigeri 2018-08-14 19:03:05

• blocks deleted (Feature #14480: Fix bugs and UX issues of VeraCrypt support)

#42 Updated by intrigeri 2018-08-14 19:03:08

• blocks deleted (Feature #15589: Process community feedback)

#43 Updated by intrigeri 2018-08-14 19:04:00

• Subject changed from Iteration 1: Create Debian packages to ship our VeraCrypt patches in Tails 3.x to Iteration 1: Create Debian packages to ship our VeraCrypt patches in Tails 3.9
• Status changed from In Progress to Resolved
• Assignee deleted (intrigeri)
• QA Check changed from Ready for QA to Pass

Built and uploaded everything.