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 writes: > Hi Justus, > > Justus Winter 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 >> +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 >> +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 >> +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 >> +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