[bug#42404] [PATCH 3/5] gnu: Add rnp.

[Justus Winter <justus@sequoia-pgp.org>]

[-- Attachment #1: Type: text/plain, Size: 5466 bytes --]

Hi Ludo’ :)

Some context:  RNP is the OpenPGP implementation that Thunderbird 78
will use.  Furthermore, we (Sequoia) created an OpenPGP interoperability
test suite (https://tests.sequoia-pgp.org/) to test various
implementations, and we'd like to use Guix to build the implementations
for consistent, reproducible results.

Ludovic Courtès <ludo@gnu.org> writes:

> Hi Justus,
>
> Justus Winter <justus@sequoia-pgp.org> skribis:
>
>> * gnu/packages/openpgp.scm (rnp): New variable.
>> * gnu/packages/patches/rnp-disable-ruby-rnp-tests.patch: New file.
>> * gnu/packages/patches/rnp-fix-cp.patch: New file.
>> * gnu/packages/patches/rnp-fix-gnupg-list-packets.patch: New file.
>> * gnu/packages/patches/rnp-fix-test-setup.patch: New file.
>> * gnu/packages/patches/rnp-fix-test.patch: New file.
>> * gnu/packages/patches/rnp-fix-true-false.patch: New file.
>> * gnu/packages/patches/rnp-unbundle-googletest.patch: New file.
>> * gnu/packages/patches/rnp-update-expiration-16ecb289.patch: New file.
>
> Could you add the patches to gnu/local.mk?

Sure!

>
>> +    (home-page "https://www.rnpgp.com/")
>> +    (license (list license:bsd-2 license:asl2.0 license:bsd-3))))
>
> Could you add a comment (a couple of lines) stating whether it’s triple
> licensing or rather that several parts come under different licenses?

I will.

> The patch overall LGTM but I’m concerned by the relatively large number
> of patches and the fact that their upstream status is unknown.  Seems to
> me that many of them ought to be upstream no?  If they are upstream, can
> we instead either wait for a release that includes them, or take a
> snapshot that includes them?

I agree.  I upstreamed my changes, and they got merged today:

  https://github.com/rnpgp/rnp/pull/1213

Those not included in the merge were meanwhile fixed upstream.  I don't
know whether they will make a release soon.  I assumed that Guix would
prefer to package the released version, but we could go for a snapshot
as well.  Your call.

>> diff --git a/gnu/packages/patches/rnp-disable-ruby-rnp-tests.patch b/gnu/packages/patches/rnp-disable-ruby-rnp-tests.patch
>> new file mode 100644
>> index 0000000000..5c8c06524d
>> --- /dev/null
>> +++ b/gnu/packages/patches/rnp-disable-ruby-rnp-tests.patch
>> @@ -0,0 +1,25 @@
>> +From 9f3c07601393e219cc5979f93fda57bf2d07dee7 Mon Sep 17 00:00:00 2001
>> +From: Justus Winter <teythoon@avior.uberspace.de>
>> +Date: Tue, 21 Jul 2020 16:10:21 +0200
>> +Subject: [PATCH 6/6] Disable ruby-rnp tests.
>
> What’s the rationale?  Would #:tests? #f have the same effect?

AIUI #:tests? #f would disable all tests.  This patch prevents cmake
from cloning the ruby-rnp repository to run its tests.  There is no
cmake switch to disable it, therefore I patched it out.  I should try to
propose a better solution to the upstream project, but I'm not familiar
with cmake.

>> diff --git a/gnu/packages/patches/rnp-fix-cp.patch b/gnu/packages/patches/rnp-fix-cp.patch
>> new file mode 100644
>> index 0000000000..039912d953
>> --- /dev/null
>> +++ b/gnu/packages/patches/rnp-fix-cp.patch
>> @@ -0,0 +1,27 @@
>> +From c163e1b12511e9e7df752a01767a2a8ba56c4196 Mon Sep 17 00:00:00 2001
>> +From: Justus Winter <teythoon@avior.uberspace.de>
>> +Date: Tue, 21 Jul 2020 15:52:37 +0200
>> +Subject: [PATCH 1/6] Make copying more robust.
>> +
>> +Let the shell locate 'cp'.  This is more robust in environments such
>> +as Guix or Nix that do not provide /bin/cp.
>> +---
>> + src/tests/support.cpp | 2 +-
>> + 1 file changed, 1 insertion(+), 1 deletion(-)
>> +
>> +diff --git a/src/tests/support.cpp b/src/tests/support.cpp
>> +index 3d6a6dc9..d260e166 100644
>> +--- a/src/tests/support.cpp
>> ++++ b/src/tests/support.cpp
>> +@@ -283,7 +283,7 @@ copy_recursively(const char *src, const char *dst)
>> +     // TODO: maybe use fts or something less hacky
>> +     char buf[2048];
>> + #ifndef _WIN32
>> +-    snprintf(buf, sizeof(buf), "/bin/cp -a '%s' '%s'", src, dst);
>> ++    snprintf(buf, sizeof(buf), "cp -a '%s' '%s'", src, dst);
>
> I’d rather add a build phase that replaces /bin/cp with (which "cp") or
> similar.  That way, the package would be self-contained (no need to
> manually add Coreutils on $PATH).

Ok, I'll try :)

>> +++ b/gnu/packages/patches/rnp-fix-true-false.patch
>> @@ -0,0 +1,253 @@
>> +From 028a2f50fbf47d989bbf79be589945bec55b4825 Mon Sep 17 00:00:00 2001
>> +From: Justus Winter <teythoon@avior.uberspace.de>
>> +Date: Tue, 21 Jul 2020 15:57:57 +0200
>> +Subject: [PATCH 3/6] Use 'true' and 'false' instead of 'TRUE' and 'FALSE'.
>> +
>> +The latter are not guaranteed to be defined.
>
> This should probably be upstream, but if we have to have it, how about
> making this change with ‘substitute*’ instead?

Fixed upstream.

>> +++ b/gnu/packages/patches/rnp-update-expiration-16ecb289.patch
>> @@ -0,0 +1,208 @@
>> +commit 16ecb28974b18e51f9060c86c7229f3b7e1cbb88
>> +Author: Nickolay Olshevsky <o.nickolay@gmail.com>
>> +Date:   Fri Apr 10 16:29:52 2020 +0300
>> +
>> +    Update expiration date of test keys.
>
> This is bound to expire again.  :-)
>
> How about using faketime instead, as is done for the ‘nss’ package?

That is much better indeed.  I had hoped that the upstream change would
just make the keys don't expire, but they still expire in 2030...


Thanks for the review :)
Justus

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]