Feature #11420

Please detect libdvd-css failures

Added by CyrilBrulebois 2016-05-15 06:25:26 . Updated 2016-06-08 01:27:32 .

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Build system
Target version:
Tails_2.4
Start date:
2016-05-15
Due date:
% Done:

100%

Feature Branch:
hybridwipe/feature/11420-verify-libdvdcss
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Deliverable for:

Description

While implementing support for snapshots, it was discovered that the initial patch series provided by intrigeri and me was missing support for a few cases in the apt-get wrapper (which makes it possible to keep track of all installed packages, so that they can be referenced in a manifest for snapshotting purposes).

Even though that was already fixed by intrigeri, it was considered bad taste to have a build go through while failures were supposed to be detected as early as possible.

I filed https://bugs.debian.org/824078 in the Debian BTS, but it seems the maintainer doesn’t quite get why the current behaviour is buggy and worth fixing.

Therefore, I suggest implementing checks in config/chroot_local-hooks/59-libdvd-pkg after those lines:

apt-get --yes install libdvd-pkg
dpkg-reconfigure libdvd-pkg

to make sure that the libdvdcss2 package is indeed installed at this stage.

Assigning to intrigeri as we discovered this together, so that he can comment on the issue.

Subtasks

History

#1 Updated by CyrilBrulebois 2016-05-15 06:31:14

This crossed my mind right after filing the report.

In case the check is implemented with dpkg -s, this one might need an arch-qualified name. A quick git grep across tails.git suggests that dpkg-query -W -f='${Version}\n' foo is used a few times already (e.g. in auto/build, auto/config), and that one doesn’t come with this limitation. ;)

#2 Updated by intrigeri 2016-05-18 13:50:41

  • Assignee changed from intrigeri to hybridwipe

> I filed https://bugs.debian.org/824078 in the Debian BTS, but it seems the maintainer doesn’t quite get why the current behaviour is buggy and worth fixing.

After some back’n’forth, the maintainer did change the package somewhat. I’ve not checked the new version.

I agree a check on our side would be great to have.

hybridwipe, what do you think?

#3 Updated by hybridwipe 2016-05-19 06:51:06

intrigeri wrote:
> > I filed https://bugs.debian.org/824078 in the Debian BTS, but it seems the maintainer doesn’t quite get why the current behaviour is buggy and worth fixing.
>
> After some back’n’forth, the maintainer did change the package somewhat. I’ve not checked the new version.
>
> I agree a check on our side would be great to have.
>
> hybridwipe, what do you think?

Sure, I’m in favor of anything that increases build reliance / reproducibility.

I’m not clear on what should be added though. dpkg -s, dpkg-query -W -f=‘${Version}\n’ libdvd-css, apt-get check, something else?

#4 Updated by intrigeri 2016-05-19 09:55:32

  • Status changed from New to Confirmed
  • QA Check set to Dev Needed

> Sure, I’m in favor of anything that increases build reliance / reproducibility.

:)

> I’m not clear on what should be added though. dpkg -s, dpkg-query -W -f=‘${Version}\n’ libdvd-css, apt-get check, something else?

Whatever checks, simply and reliably, that the package we want in the end is indeed installed :)

#5 Updated by hybridwipe 2016-05-20 05:32:25

intrigeri wrote:
> > Sure, I’m in favor of anything that increases build reliance / reproducibility.
>
> :)
>
> > I’m not clear on what should be added though. dpkg -s, dpkg-query -W -f=‘${Version}\n’ libdvd-css, apt-get check, something else?
>
> Whatever checks, simply and reliably, that the package we want in the end is indeed installed :)

Well, testing on my personal machine, I’d say not apt-get check, as it failed for (valid) reasons unrelated to packages I queried (iceweaesel and asdf). dpkg -s / dpkg-query -W both seemed to return valid info. I think we could check both, as I’m not sure which is better, and they’re both very quick, so why not double down :D.

#6 Updated by hybridwipe 2016-05-20 06:07:42

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

intrigeri wrote:
> > Sure, I’m in favor of anything that increases build reliance / reproducibility.
>
> :)
>
> > I’m not clear on what should be added though. dpkg -s, dpkg-query -W -f=‘${Version}\n’ libdvd-css, apt-get check, something else?
>
> Whatever checks, simply and reliably, that the package we want in the end is indeed installed :)

https://git-tails.immerda.ch/hybridwipe/tails/?h=feature%2F11420-verify-libdvdcss

I can’t build to verify at the moment, but I think that should satisfy this requirement :)

#7 Updated by intrigeri 2016-05-20 18:32:08

  • Subject changed from please detect libdvd-css failures to Please detect libdvd-css failures
  • Status changed from Confirmed to In Progress
  • Assignee changed from intrigeri to anonym
  • Target version set to Tails_2.4
  • % Done changed from 0 to 50
  • QA Check changed from Info Needed to Ready for QA

(I guess it’s no huge deal if this has to be delayed to 2.5, or to post-2.4 freeze, due to lack of reviewing time.)

#8 Updated by anonym 2016-05-22 17:46:25

  • Assignee changed from anonym to hybridwipe
  • QA Check changed from Ready for QA to Dev Needed
  • Feature Branch set to hybridwipe/feature/11420-verify-libdvdcss

Build fails with:

[...]
dpkg-deb: building package `libdvd-pkg' in `../libdvd-pkg_1.4.0-1-1~bpo8+1+fake1_all.deb'.

The package has been created.
Attention, the package has been created in the current directory,
not in ".." as indicated by the message above!
(Reading database ... 171399 files and directories currently installed.)
Preparing to unpack libdvd-pkg_1.4.0-1-1~bpo8+1+fake1_all.deb ...
Unpacking libdvd-pkg (1.4.0-1-1~bpo8+1+fake1) over (1.4.0-1-1~bpo8+1) ...
Setting up libdvd-pkg (1.4.0-1-1~bpo8+1+fake1) ...
dpkg-query: package 'libdvd-css' is not installed and no information is available
Use dpkg --info (= dpkg-deb --info) to examine archive files,
and dpkg --contents (= dpkg-deb --contents) to list their contents.
E: config/chroot_local-hooks/59-libdvd-pkg failed (exit non-zero). You should check for errors.
[...]


Please also merge or rebase on the devel branch in any new pull request.

#9 Updated by hybridwipe 2016-05-23 01:21:13

Thanks for testing, I couldn’t test at the time since we hadn’t solved the build yet ;).

https://git-tails.immerda.ch/hybridwipe/tails/?h=feature/11420-verify-libdvdcss is updated and builds, based on devel as of 5 minutes ago. Not sure if you wanted the commits squashed or not though, for reviewing.

Note that the set -x change wasn’t meant to be committed permanently. I force pushed an update but gitweb is still showing the old commit for me, at least.

#10 Updated by intrigeri 2016-05-23 08:40:53

  • Assignee changed from hybridwipe to anonym
  • QA Check changed from Dev Needed to Ready for QA

(Hopefully I got it right..)

#11 Updated by anonym 2016-05-24 23:08:39

  • Status changed from In Progress to Fix committed
  • Assignee deleted (anonym)
  • % Done changed from 50 to 100
  • QA Check changed from Ready for QA to Pass

hybridwipe wrote:
> Thanks for testing, I couldn’t test at the time since we hadn’t solved the build yet ;).
>
> https://git-tails.immerda.ch/hybridwipe/tails/?h=feature/11420-verify-libdvdcss is updated and builds, based on devel as of 5 minutes ago.

Yay that you can build again! :)

> Not sure if you wanted the commits squashed or not though, for reviewing.

What you did is correct, but for a tiny change like this history rewriting would be ok too.

#12 Updated by anonym 2016-06-08 01:27:32

  • Status changed from Fix committed to Resolved