Bug#955259: libjcat: Please add consider adding autopkgtest coverage

[Simon McVittie]

On Mon, 30 Mar 2020 at 22:35:11 +, mario.limoncie...@dell.com wrote:
> > From: Simon McVittie 
> > That's a lot like what I did. I'll recheck soon whether my branch would
> > work with their patch instead of mine.

Yes, their patch is fine. I slightly preferred my version,
but not enough for it to be worth taking upstream. I've opened
https://salsa.debian.org/efi-team/libjcat/-/merge_requests/3 which
applies their patch instead of the one attached to this bug, and adds
the same autopkgtests I used before (they still pass).

> As a general statement since we have a super responsive upstream I would 
> rather
> flesh out non debian/* patches upstream immediately and then pull them back as
> cherry-picks and add packaging to match them at the same time.

Yes to a point, but since 0.1.0 didn't compile on the buildds *at all*, I
wanted to get through at least #955234 with downstream patches first, on
the basis that a package is better than no package.

I've simplified the patch that was part of the solution to #955234 further,
and sent it to .

smcv

Bug#955259: libjcat: Please add consider adding autopkgtest coverage

[Mario.Limonciello]

> -Original Message-
> From: Simon McVittie 
> Sent: Monday, March 30, 2020 5:23 PM
> To: Limonciello, Mario
> Cc: 955...@bugs.debian.org; sub...@bugs.debian.org
> Subject: Re: Bug#955259: libjcat: Please add consider adding autopkgtest
> coverage
> 
> 
> [EXTERNAL EMAIL]
> 
> On Mon, 30 Mar 2020 at 19:09:23 +, mario.limoncie...@dell.com wrote:
> > Upstream the following was recently accepted to allow installed-tests to
> work
> > Without the source tree nearby.
> >
> https://github.com/hughsie/libjcat/commit/d6dc3e90b0c805cc50bff9d1d1b87ff57
> 5e53769
> 
> That's a lot like what I did. I'll recheck soon whether my branch would
> work with their patch instead of mine.

Thanks!

> 
> Salsa was mid-upgrade and not working very well when I sent the initial
> patches, but I'll send a MR for the new version.
> 
> smcv

As a general statement since we have a super responsive upstream I would rather
flesh out non debian/* patches upstream immediately and then pull them back as
cherry-picks and add packaging to match them at the same time.

Bug#955259: libjcat: Please add consider adding autopkgtest coverage

[Simon McVittie]

On Mon, 30 Mar 2020 at 19:09:23 +, mario.limoncie...@dell.com wrote:
> Upstream the following was recently accepted to allow installed-tests to work
> Without the source tree nearby.
> https://github.com/hughsie/libjcat/commit/d6dc3e90b0c805cc50bff9d1d1b87ff575e53769

That's a lot like what I did. I'll recheck soon whether my branch would
work with their patch instead of mine.

Salsa was mid-upgrade and not working very well when I sent the initial
patches, but I'll send a MR for the new version.

smcv

Bug#955259: libjcat: Please add consider adding autopkgtest coverage

[Mario.Limonciello]

Upstream the following was recently accepted to allow installed-tests to work
Without the source tree nearby.
https://github.com/hughsie/libjcat/commit/d6dc3e90b0c805cc50bff9d1d1b87ff575e53769


> -Original Message-
> From: Simon McVittie 
> Sent: Saturday, March 28, 2020 3:29 PM
> To: Debian Bug Tracking System
> Subject: Bug#955259: libjcat: Please add consider adding autopkgtest
> coverage
> 
> 
> [EXTERNAL EMAIL]
> 
> Source: libjcat
> Version: 0.1.0-1
> Severity: wishlist
> Tags: patch
> Control: block -1 by 955258
> 
> I noticed that libjcat has GNOME-style installed-tests, which in principle
> make it easy to add autopkgtest coverage, so I tried to contribute a
> trivial patch to do so. Unfortunately, the installed-tests don't actually
> work when run installed (they rely on the original source tree), so it
> turned out to be less trivial than I'd hoped.
> 
> I've attached patches, but the patch added in debian/patches should
> probably be discussed with upstream. As well as making the test pass, it
> also fixes a source of non-reproducibility.
> 
> For it to pass, the test for libjcat-dev requires the patches I attached
> to #955258.
> 
> smcv

Bug#955259: libjcat: Please add consider adding autopkgtest coverage

[Simon McVittie]

Source: libjcat
Version: 0.1.0-1
Severity: wishlist
Tags: patch
Control: block -1 by 955258

I noticed that libjcat has GNOME-style installed-tests, which in principle
make it easy to add autopkgtest coverage, so I tried to contribute a
trivial patch to do so. Unfortunately, the installed-tests don't actually
work when run installed (they rely on the original source tree), so it
turned out to be less trivial than I'd hoped.

I've attached patches, but the patch added in debian/patches should probably
be discussed with upstream. As well as making the test pass, it also fixes a
source of non-reproducibility.

For it to pass, the test for libjcat-dev requires the patches I attached
to #955258.

smcv
>From c33068507c34a1416ee023f4064f19015f9b9c75 Mon Sep 17 00:00:00 2001
From: Simon McVittie 
Date: Sat, 28 Mar 2020 17:37:36 +
Subject: [PATCH 7/9] Install test data so that the installed-tests can pass

---
 ...est-data-next-to-the-test-executable.patch | 273 ++
 debian/patches/series |   1 +
 2 files changed, 274 insertions(+)
 create mode 100644 debian/patches/Install-test-data-next-to-the-test-executable.patch

diff --git a/debian/patches/Install-test-data-next-to-the-test-executable.patch b/debian/patches/Install-test-data-next-to-the-test-executable.patch
new file mode 100644
index 000..6a8d473
--- /dev/null
+++ b/debian/patches/Install-test-data-next-to-the-test-executable.patch
@@ -0,0 +1,273 @@
+From: Simon McVittie 
+Date: Sat, 28 Mar 2020 17:35:46 +
+Subject: Install test data next to the test executable
+
+This is conventional for GNOME-style installed-tests, and lets us find
+it with g_test_build_filename() (which uses the G_TEST_SRCDIR and
+G_TEST_BUILDDIR environment variables, or looks near the executable).
+
+The environment variables avoiding the need to hard-code the
+build directory into the test executable, which isn't suitable for
+reproducible builds.
+
+Signed-off-by: Simon McVittie 
+---
+ data/tests/colorhug/meson.build |  5 +++--
+ data/tests/meson.build  |  5 +
+ data/tests/pki/meson.build  |  6 ++
+ libjcat/jcat-self-test.c| 40 
+ libjcat/meson.build | 13 -
+ 5 files changed, 42 insertions(+), 27 deletions(-)
+
+diff --git a/data/tests/colorhug/meson.build b/data/tests/colorhug/meson.build
+index 1851fbf..475bf46 100644
+--- a/data/tests/colorhug/meson.build
 b/data/tests/colorhug/meson.build
+@@ -1,8 +1,9 @@
+ install_data([
+ 'firmware.bin',
+ 'firmware.bin.asc',
++'firmware.bin.p7b',
+   ],
+-  install_dir: installed_test_datadir,
++  install_dir: installed_test_bindir / 'colorhug',
+ )
+ 
+ if get_option('pkcs7')
+@@ -17,6 +18,6 @@ if get_option('pkcs7')
+ '--infile', '@INPUT@',
+ '--outfile', '@OUTPUT@'],
+ install: true,
+-install_dir: installed_test_datadir,
++install_dir: installed_test_bindir / 'colorhug',
+   )
+ endif
+diff --git a/data/tests/meson.build b/data/tests/meson.build
+index 5bfca03..617edf2 100644
+--- a/data/tests/meson.build
 b/data/tests/meson.build
+@@ -15,5 +15,10 @@ pkcs7_privkey = custom_target('test-privkey.pem',
+ subdir('pki')
+ subdir('colorhug')
+ 
++install_data(
++  'meson.build',
++  install_dir: installed_test_bindir,
++)
++
+ testdatadir_src = meson.current_source_dir()
+ testdatadir_dst = meson.current_build_dir()
+diff --git a/data/tests/pki/meson.build b/data/tests/pki/meson.build
+index f35f2a9..f5bf31d 100644
+--- a/data/tests/pki/meson.build
 b/data/tests/pki/meson.build
+@@ -7,4 +7,10 @@ pkcs7_certificate = custom_target('test.pem',
+   '--template', pkcs7_config,
+   '--load-privkey', '@INPUT@',
+   '--outfile', '@OUTPUT@'],
++  install_dir: installed_test_bindir / 'pki',
++)
++install_data(
++  'GPG-KEY-Linux-Vendor-Firmware-Service',
++  'LVFS-CA.pem',
++  install_dir: installed_test_bindir / 'pki',
+ )
+diff --git a/libjcat/jcat-self-test.c b/libjcat/jcat-self-test.c
+index 7c3558b..f7a8375 100644
+--- a/libjcat/jcat-self-test.c
 b/libjcat/jcat-self-test.c
+@@ -216,7 +216,7 @@ jcat_sha1_engine_func (void)
+ 	g_assert_cmpint (jcat_engine_get_verify_kind (engine), ==, JCAT_ENGINE_VERIFY_KIND_CHECKSUM);
+ 
+ 	/* verify checksum */
+-	fn_pass = g_build_filename (TESTDATADIR_SRC, "colorhug", "firmware.bin", NULL);
++	fn_pass = g_test_build_filename (G_TEST_DIST, "colorhug", "firmware.bin", NULL);
+ 	data_fwbin = jcat_get_contents_bytes (fn_pass, );
+ 	g_assert_no_error (error);
+ 	g_assert_nonnull (data_fwbin);
+@@ -230,7 +230,7 @@ jcat_sha1_engine_func (void)
+ 	g_assert_cmpstr (jcat_result_get_authority (result_pass), ==, NULL);
+ 
+ 	/* verify will fail */
+-	fn_fail = g_build_filename (TESTDATADIR_SRC, "meson.build",NULL);
++	fn_fail = g_test_build_filename (G_TEST_DIST, "meson.build",NULL);
+ 	data_fail = jcat_get_contents_bytes (fn_fail, );
+