Project

General

Profile

Feature #15521

Feature #14468: Add VeraCrypt support to Tails

Feature #15214: Iteration 1: Support unlocking VeraCrypt partitions in GNOME

Iteration 1: Create Debian packages to ship our VeraCrypt patches in Tails 3.9

Added by segfault over 1 year ago. Updated over 1 year ago.

Status:
Resolved
Priority:
Normal
Assignee:
-
Category:
-
Target version:
Start date:
04/11/2018
Due date:
% Done:

100%

Feature Branch:
segfault:tails-tcrypt-packages
Type of work:
Code
Blueprint:
Starter:
Affected tool:

Description

On #15037 we decided to build custom packages of our changes to have them in Tails 3.x and maintain these custom packages during the Stretch cycle.

Packaging repositories:

The build order is documented on #14481#note-13.


Subtasks

Feature #15522: Iteration 1: Create custom Debian package for udisksResolved

Feature #15523: Iteration 1: Create custom Debian package for DisksResolved


Related issues

Related to Tails - Feature #14481: Release Beta for VeraCrypt support in Tails Resolved 12/10/2017

History

#2 Updated by intrigeri over 1 year ago

  • Status changed from Confirmed to In Progress

#3 Updated by bertagaz over 1 year ago

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

#4 Updated by segfault over 1 year ago

I created all the required Debian packages as part of 14481, see my comment there.

#5 Updated by intrigeri over 1 year ago

  • 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 over 1 year ago

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

#7 Updated by intrigeri over 1 year ago

intrigeri wrote:

  • 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.

libudisks2-dev (= 2.1.8-1tails1) is wrong so I've replaced it with 2.1.8-1.0tails1) and did s/=/>=/ for that specific dependency while I was at it.

#8 Updated by intrigeri over 1 year ago

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

This can (and should) wait post-3.8.

#9 Updated by intrigeri over 1 year ago

Like last time, I'll report here issues that don't block #14481 IMO but need to be solved before we merge all this into devel for 3.9:

#10 Updated by segfault over 1 year ago

intrigeri wrote:

Like last time, I'll report here issues that don't block #14481 IMO but need to be solved before we merge all this into devel for 3.9:

  • gnome-shell 3.22.3-3.0tails3:
    • please set the target dist to feature-14481-tcrypt-support-beta; as explained on #14481#note-32 the one you've set is not correct (my bad initially but I've updated the instructions since then) and makes me do additional work before uploading; I've fixed that in gnome-shell 3.22.3-3.0tails3, please base .0tails4 on the one I've uploaded

Done in 8d5649501a3a3754e475a9302f5462eb5cb2c938.

The reason is that only after I backported this patch to 3.22.3 it occurred to me that it would better fit the current code style to make use of the Params module, but I didn't want to waste time backporting a style fix.

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

  • is debian/patches/0003-Fix-binding-of-PIM-entry-activation.patch Stretch-specific? I can't find the corresponding change in your branches based on upstream

Yes, that is specific to the 3.22.3 version of my patches. The corresponding commit in the branches based on upstream is 35676264aac64dfb43522edff892b6d65aef4ced, in the 3.22.3-support-tcrypt branch, which you might want to use for reviewing these patches, instead of the support-tcrypt branch, which is based on upstream master.

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.

#11 Updated by segfault over 1 year ago

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

#12 Updated by intrigeri over 1 year ago

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

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!

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 #15521#note-9 review.

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

#13 Updated by segfault over 1 year ago

intrigeri wrote:

General comments:

  • Many of the debdiff's modify a number of old debian/changelog entries; please fix that. Maybe your editor is too clever? :)

I was sure I had already fixed that in all packages, but when I just checked, I noticed that both the gnome-shell and gvfs packages still included inadvertent removal of trailing whitespace caused by a vim plugin I since disabled. gnome-shell also still had too strict versioned dependencies. I pushed fixes for these issues now, so they will be fixed in the next versions of those packages.

  • 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.

I now synced debian/control and debian/control.in in all packages.

  • 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.

To do this for all the patches I already created seems like a lot of work to me. Also, the existing patches already include the commit ID, because I create them via git format-patch on the branch based on upstream.

#14 Updated by segfault over 1 year ago

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

#15 Updated by intrigeri over 1 year ago

  • Many of the debdiff's modify a number of old debian/changelog entries; please fix that. Maybe your editor is too clever? :)

I was sure I had already fixed that in all packages, but when I just checked, I noticed that both the gnome-shell and gvfs packages still included inadvertent removal of trailing whitespace caused by a vim plugin I since disabled. gnome-shell also still had too strict versioned dependencies. I pushed fixes for these issues now, so they will be fixed in the next versions of those packages.

  • 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.

I now synced debian/control and debian/control.in in all packages.

Great, I'm going to take a look. Meta: we'll want to keep this ticket open until the fixed packages are uploaded, to ensure we don't forget.

  • 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.

To do this for all the patches I already created seems like a lot of work to me. Also, the existing patches already include the commit ID, because I create them via git format-patch on the branch based on upstream.

Fair enough.

#16 Updated by intrigeri over 1 year ago

  • 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:

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 over 1 year ago

  • 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:

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 #14481#note-29). I removed these commits from my branches now.

#18 Updated by intrigeri over 1 year ago

  • 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 #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 over 1 year ago

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

#20 Updated by intrigeri over 1 year ago

#21 Updated by segfault over 1 year ago

  • 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 #14481#note-64. It required a small patch to GTK, so I built a new package. Please review and upload.

#22 Updated by intrigeri over 1 year ago

  • 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 #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 over 1 year ago

  • 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?

#24 Updated by segfault over 1 year ago

intrigeri wrote:

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.

Yes, I added the "Add debug output" patch to GTK on purpose, because I hope that will make debugging easier. The debug messages will only be printed if G_MESSAGES_DEBUG is set.

  • 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 :)

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 #14467, because it affects Nautilus (beside others).

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.

Good.

The package FTBFS for me:

I will look into that now.

#25 Updated by segfault over 1 year ago

intrigeri wrote:

The package FTBFS for me:

[...]

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

Yes, this requires libglib2.0-dev 2.50.3-2.0tails2. I will add the dependency. Do I have to bump the GTK version again or is it ok if I sneak this into 3.22.11-1.0tails3 and force push?

#26 Updated by intrigeri over 1 year ago

Do I have to bump the GTK version again or is it ok if I sneak this into 3.22.11-1.0tails3 and force push?

Replied on XMPP. In general, never change the content referenced by an already published version.

#27 Updated by intrigeri over 1 year ago

intrigeri wrote:

  • Is it on purpose that you left the patch that adds debug output enabled?

Yes, I added the "Add debug output" patch to GTK on purpose, because I hope that will make debugging easier. The debug messages will only be printed if G_MESSAGES_DEBUG is set.

OK!

  • Regarding 0002-gtkmountoperation-Support-TCRYPT-options.patch: is this something that needs to be upstreamed?

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 #14467, because it affects Nautilus (beside others).

#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 #15244: #15724 :)

#28 Updated by segfault over 1 year ago

  • 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 #14467, because it affects Nautilus (beside others).

#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 #15244: #15724 :)

Thanks!

I uploaded GTK 3.22.11-1.0tails4.

#29 Updated by intrigeri over 1 year ago

  • 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.

#30 Updated by intrigeri over 1 year ago

intrigeri wrote:

So we can put this ticket on the back burner until it's time to rebuild + upload with all recent packaging fixes.

I think it's now time (both for packaging fixes and recent improvements such as #15671, #15667 etc).

#31 Updated by intrigeri over 1 year ago

For some Redmine does not let me bump the priority — with the usual trick of doing so on the subtasks — but this does blocks merging feature/14481-TCRYPT-support-beta into devel.

#32 Updated by segfault over 1 year ago

  • Description updated (diff)
  • 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 over 1 year ago

  • Description updated (diff)

#34 Updated by intrigeri over 1 year ago

  • 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 over 1 year ago

  • 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 :/

#36 Updated by intrigeri over 1 year ago

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

Amazing!

  • 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

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.

Actually that would only work with a more standard packaging repo layout than what I see e.g. on https://gitlab.com/segfault3/tails-packaging-glib.

  • 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.

  • 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.

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 :/

Right, again the "creative" packaging repo layout prevents using standard tools. Anyway, no big deal :)

I'll now review and hopefully build+upload!

#37 Updated by intrigeri over 1 year ago

  • 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 over 1 year ago

  • 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

#39 Updated by intrigeri over 1 year ago

Regression in the re-released glib2.0 (2.50.3-2.0tails3): it modifies old debian/changelog entries. At this point in the release cycle I'm not going to block on it but that's happening too often for my taste and this should be identified and fixed before it's submitted for QA, so please start to run debdiff yourself in the future, just like you would git diff a Git topic branch before submitting it :)

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.

OK, so tl;dr: Lintian has checks for binary packages and for source packages. If you run it on a .dsc it will run the source checks (since the .dsc describes a source package). I generally run it on the .changes so it checks both.

  • 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.

I've cleaned up my local stuff and re-downloaded https://gitlab.com/segfault3/tails-tcrypt-packages/raw/master/gtk+3.0_3.22.11-1.0tails5.dsc and cannot see that fix in the changelog. I'm not going to block on this but it would be nice to ensure this fix lands in some place that will be used next time we build this package.

I'm now done with the review and will build and upload the packages.

#40 Updated by segfault over 1 year ago

intrigeri wrote:

Regression in the re-released glib2.0 (2.50.3-2.0tails3): it modifies old debian/changelog entries. At this point in the release cycle I'm not going to block on it but that's happening too often for my taste and this should be identified and fixed before it's submitted for QA, so please start to run debdiff yourself in the future, just like you would git diff a Git topic branch before submitting it :)

Oh no, you already reviewed this. I noticed soon after that I recommitted the old glib 2.50.3-2.0tails3, which I fixed now.

  • 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.

That was incorrect, I did not re-release gtk, but only glib. The fix will be included in the next release. Sorry to waste your time.

#41 Updated by intrigeri over 1 year ago

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

#42 Updated by intrigeri over 1 year ago

#43 Updated by intrigeri over 1 year ago

  • 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.

#44 Updated by u over 1 year ago

OK, so tl;dr: Lintian has checks for binary packages and for source packages. If you run it on a .dsc it will run the source checks (since the .dsc describes a source package). I generally run it on the .changes so it checks both.

And we generally run it using the "--pedantic" parameter :)

Also available in: Atom PDF