[bug#31733] [PATCH] gnu: encfs: Update to 1.9.5.

[bug#31733] [PATCH] gnu: encfs: Update to 1.9.5.

[Pierre Neidhardt]

* gnu/packages/crypto.scm (encfs): Update to 1.9.5.
---
 gnu/packages/crypto.scm | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/crypto.scm b/gnu/packages/crypto.scm
index f96fe0490..4707355f3 100644
--- a/gnu/packages/crypto.scm
+++ b/gnu/packages/crypto.scm
@@ -216,7 +216,7 @@ communication.")
 (define-public encfs
   (package
     (name "encfs")
-    (version "1.9.1")
+    (version "1.9.5")
     (source
      (origin
        (method url-fetch)
@@ -225,7 +225,7 @@ communication.")
                        version "/encfs-" version ".tar.gz"))
        (sha256
         (base32
-         "1906254dg5hwljh0h4gyrw09ms3b57dlhjfzhfzffv50yzpkl837"))
+         "0qzxavvv20577bxvly8s7d3y7bqasqclc2mllp0ddfncjm9z02a7"))
        (modules '((guix build utils)))
        ;; Remove bundled dependencies in favour of proper inputs.
        (snippet '(for-each delete-file-recursively
@@ -244,7 +244,12 @@ communication.")
        ("openssl" ,openssl)
        ("tinyxml2" ,tinyxml2)))
     (arguments
-     `(#:configure-flags (list "-DUSE_INTERNAL_TINYXML=OFF")))
+     `(#:configure-flags (list "-DUSE_INTERNAL_TINYXML=OFF")
+       #:phases
+       (modify-phases %standard-phases
+         (add-before 'check 'make-unittests
+           (lambda _
+             (zero? (system* "make" "unittests")))))))
     (home-page "https://vgough.github.io/encfs")
     (synopsis "Encrypted virtual file system")
     (description
-- 
2.17.0

[bug#31733] [PATCH] gnu: encfs: Update to 1.9.5.

[Kei Kebreau]

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

Hi Pierre,

> * gnu/packages/crypto.scm (encfs): Update to 1.9.5.

Don't forget to mention the new 'make-unittests' phase!

> ---
>  gnu/packages/crypto.scm | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/gnu/packages/crypto.scm b/gnu/packages/crypto.scm
> index f96fe0490..4707355f3 100644
> --- a/gnu/packages/crypto.scm
> +++ b/gnu/packages/crypto.scm
> @@ -216,7 +216,7 @@ communication.")
>  (define-public encfs
>    (package
>      (name "encfs")
> -    (version "1.9.1")
> +    (version "1.9.5")
>      (source
>       (origin
>         (method url-fetch)
> @@ -225,7 +225,7 @@ communication.")
>                         version "/encfs-" version ".tar.gz"))
>         (sha256
>          (base32
> -         "1906254dg5hwljh0h4gyrw09ms3b57dlhjfzhfzffv50yzpkl837"))
> +         "0qzxavvv20577bxvly8s7d3y7bqasqclc2mllp0ddfncjm9z02a7"))
>         (modules '((guix build utils)))
>         ;; Remove bundled dependencies in favour of proper inputs.
>         (snippet '(for-each delete-file-recursively
> @@ -244,7 +244,12 @@ communication.")
>         ("openssl" ,openssl)
>         ("tinyxml2" ,tinyxml2)))
>      (arguments
> -     `(#:configure-flags (list "-DUSE_INTERNAL_TINYXML=OFF")))
> +     `(#:configure-flags (list "-DUSE_INTERNAL_TINYXML=OFF")
> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-before 'check 'make-unittests
> +           (lambda _
> +             (zero? (system* "make" "unittests")))))))

Any particular reason you decided to use '(zero? (system* ...))' rather
than '(invoke ...)' for this phase? I tried it with 'invoke' and it
worked fine.

>      (home-page "https://vgough.github.io/encfs")
>      (synopsis "Encrypted virtual file system")
>      (description

Other than the above, this patch looks good! :)

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

[bug#31733] [PATCH] gnu: encfs: Update to 1.9.5.

[Kei Kebreau]

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

Kei Kebreau <kkebreau@posteo.net> writes:

> Hi Pierre,
>
>> * gnu/packages/crypto.scm (encfs): Update to 1.9.5.
>
> Don't forget to mention the new 'make-unittests' phase!
>
>> ---
>>  gnu/packages/crypto.scm | 11 ++++++++---
>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/gnu/packages/crypto.scm b/gnu/packages/crypto.scm
>> index f96fe0490..4707355f3 100644
>> --- a/gnu/packages/crypto.scm
>> +++ b/gnu/packages/crypto.scm
>> @@ -216,7 +216,7 @@ communication.")
>>  (define-public encfs
>>    (package
>>      (name "encfs")
>> -    (version "1.9.1")
>> +    (version "1.9.5")
>>      (source
>>       (origin
>>         (method url-fetch)
>> @@ -225,7 +225,7 @@ communication.")
>>                         version "/encfs-" version ".tar.gz"))
>>         (sha256
>>          (base32
>> -         "1906254dg5hwljh0h4gyrw09ms3b57dlhjfzhfzffv50yzpkl837"))
>> +         "0qzxavvv20577bxvly8s7d3y7bqasqclc2mllp0ddfncjm9z02a7"))
>>         (modules '((guix build utils)))
>>         ;; Remove bundled dependencies in favour of proper inputs.
>>         (snippet '(for-each delete-file-recursively
>> @@ -244,7 +244,12 @@ communication.")
>>         ("openssl" ,openssl)
>>         ("tinyxml2" ,tinyxml2)))
>>      (arguments
>> -     `(#:configure-flags (list "-DUSE_INTERNAL_TINYXML=OFF")))
>> +     `(#:configure-flags (list "-DUSE_INTERNAL_TINYXML=OFF")
>> +       #:phases
>> +       (modify-phases %standard-phases
>> +         (add-before 'check 'make-unittests
>> +           (lambda _
>> +             (zero? (system* "make" "unittests")))))))
>
> Any particular reason you decided to use '(zero? (system* ...))' rather
> than '(invoke ...)' for this phase? I tried it with 'invoke' and it
> worked fine.
>
>>      (home-page "https://vgough.github.io/encfs")
>>      (synopsis "Encrypted virtual file system")
>>      (description
>
> Other than the above, this patch looks good! :)

Also, do note that the snippet needs to be updated, as bundled libraries
(now including googletest) have moved to the "vendor" subdirectory. Once
you remove the "vendor" subdirectory, you may have to add 'googletest'
from '(gnu packages check)' to the native-inputs.

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

[bug#31733] [PATCH] gnu: encfs: Update to 1.9.5.

[Pierre Neidhardt]

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


>> Any particular reason you decided to use '(zero? (system* ...))' rather
>> than '(invoke ...)' for this phase? I tried it with 'invoke' and it
>> worked fine.

I did not know about `invoke' :)
I just found its definition in guix/build/utils.scm.

> Also, do note that the snippet needs to be updated, as bundled libraries
> (now including googletest) have moved to the "vendor" subdirectory. Once
> you remove the "vendor" subdirectory, you may have to add 'googletest'
> from '(gnu packages check)' to the native-inputs.

So the vender directory has 3 dependencies:

- tinyxml which is already used externally, so it's safe to remove the
  embedded version.

- easyloggingpp which is not packaged in Guix I believe.  We need to
  package it first before we can remove it.

- googletest: this one does not have an option in CMakeList.txt to be
  used externally.  So either we report upstream or we patch the
  CMakeList.txt.

All things considered, why not sticking to the vendored libraries?

What do you think?

-- 
Pierre Neidhardt

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

[bug#31733] [PATCH] gnu: encfs: Update to 1.9.5.

[Kei Kebreau]

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

Pierre Neidhardt <ambrevar@gmail.com> writes:

>>> Any particular reason you decided to use '(zero? (system* ...))' rather
>>> than '(invoke ...)' for this phase? I tried it with 'invoke' and it
>>> worked fine.
>
> I did not know about `invoke' :)
> I just found its definition in guix/build/utils.scm.
>

Well now you know! :)

>> Also, do note that the snippet needs to be updated, as bundled libraries
>> (now including googletest) have moved to the "vendor" subdirectory. Once
>> you remove the "vendor" subdirectory, you may have to add 'googletest'
>> from '(gnu packages check)' to the native-inputs.
>
> So the vender directory has 3 dependencies:
>
> - tinyxml which is already used externally, so it's safe to remove the
>   embedded version.
>
> - easyloggingpp which is not packaged in Guix I believe.  We need to
>   package it first before we can remove it.
>

Observation agrees with both of these.

> - googletest: this one does not have an option in CMakeList.txt to be
>   used externally.  So either we report upstream or we patch the
>   CMakeList.txt.
>

I see that while CMakeLists.txt doesn't have an external option to
control this, it *does* look for googletest in the vendor
directory. Perhaps we can add our own googletest as a native input and
remove the bundled googletest and CMakeLists.txt's references to it in a
snippet?

> All things considered, why not sticking to the vendored libraries?
>

We try to unbundle third-party software in our packages in order to ease
the burden of security maintenance and avoid duplication of work in
general. There are probably other reasons that I can't remember right
now, too.

> What do you think?

This sounds actionable to me!

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

[bug#31733] [PATCH] gnu: encfs: Update to 1.9.5.

[Pierre Neidhardt]

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


Kei Kebreau <kkebreau@posteo.net> writes:

>> - googletest: this one does not have an option in CMakeList.txt to be
>>   used externally.  So either we report upstream or we patch the
>>   CMakeList.txt.
>>
>
> I see that while CMakeLists.txt doesn't have an external option to
> control this, it *does* look for googletest in the vendor
> directory. Perhaps we can add our own googletest as a native input and
> remove the bundled googletest and CMakeLists.txt's references to it in a
> snippet?

I have no experience at all with googletest.  Do you know how, when and
why to use it?
If so, could you help me with this process?  Thanks!

-- 
Pierre Neidhardt

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

[bug#31733] [PATCH] gnu: encfs: Update to 1.9.5.

[Kei Kebreau]

[-- Attachment #1.1: Type: text/plain, Size: 2491 bytes --]

Pierre Neidhardt <ambrevar@gmail.com> writes:

> Kei Kebreau <kkebreau@posteo.net> writes:
>
>>> - googletest: this one does not have an option in CMakeList.txt to be
>>>   used externally.  So either we report upstream or we patch the
>>>   CMakeList.txt.
>>>
>>
>> I see that while CMakeLists.txt doesn't have an external option to
>> control this, it *does* look for googletest in the vendor
>> directory. Perhaps we can add our own googletest as a native input and
>> remove the bundled googletest and CMakeLists.txt's references to it in a
>> snippet?
>
> I have no experience at all with googletest.  Do you know how, when and
> why to use it?
> If so, could you help me with this process?  Thanks!

I don't have experience with it either, but I did find an interesting
way around the issue in the Guix code. (Usually if I have an issue
packaging something, I try "grepping" around the existing code to see if
someone has already solved a similar problem.) I've attached a patch
(based on yours) that covers the method:

[...]
        (snippet '(begin
                    (for-each delete-file-recursively
-                             (find-files "internal" "^tinyxml2-[0-9]"
-                                         #:directories? #t))
+                             '("vendor/github.com/leethomason/tinyxml2"
+                               "vendor/github.com/google/googletest"))
                    #t))))

Here I just remove the tinyxml2 and googletest directories as discussed.

[...]
        ;; Test dependencies.
        ("expect" ,expect)
+       ("googletest-source" ,(package-source googletest))
        ("perl" ,perl)))
     (inputs
      `(("attr" ,attr)

As seen in the definition for gnucash, the idea is to extract *our*
googletest source code in place of the bundled version. This seems to be
the easiest way to deal with replacing googletest in this case rather
than completely removing it.

[...]
+       #:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'unpack-googletest
+           (lambda* (#:key inputs #:allow-other-keys)
+             (mkdir-p "vendor/github.com/google/googletest")
+             (invoke "tar" "xvf" (assoc-ref inputs "googletest-source")
+                     "-C" "vendor/github.com/google/googletest"
+                     "--strip-components=1")))

This phase does the extracting bit.

As for the Easylogging++ and google/benchmark replacements, we'd have to
start by packaging them.

Did this help at all?


[-- Attachment #1.2: 0001-gnu-encfs-Update-to-1.9.5.patch --]
[-- Type: text/plain, Size: 3060 bytes --]

From 52ce45f9ee4961207b14e7b3a2835a1db78501fd Mon Sep 17 00:00:00 2001
From: Pierre Neidhardt <ambrevar@gmail.com>
Date: Wed, 6 Jun 2018 11:47:30 +0200
Subject: [PATCH] gnu: encfs: Update to 1.9.5.

* gnu/packages/crypto.scm (encfs): Update to 1.9.5.

Signed-off-by: Kei Kebreau <kkebreau@posteo.net>
---
 gnu/packages/crypto.scm | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/gnu/packages/crypto.scm b/gnu/packages/crypto.scm
index 105760afa..338db04f5 100644
--- a/gnu/packages/crypto.scm
+++ b/gnu/packages/crypto.scm
@@ -32,6 +32,7 @@
   #:use-module (gnu packages attr)
   #:use-module (gnu packages autotools)
   #:use-module (gnu packages boost)
+  #:use-module (gnu packages check)
   #:use-module (gnu packages compression)
   #:use-module (gnu packages cryptsetup)
   #:use-module (gnu packages databases)
@@ -217,7 +218,7 @@ communication.")
 (define-public encfs
   (package
     (name "encfs")
-    (version "1.9.1")
+    (version "1.9.5")
     (source
      (origin
        (method url-fetch)
@@ -226,13 +227,13 @@ communication.")
                        version "/encfs-" version ".tar.gz"))
        (sha256
         (base32
-         "1906254dg5hwljh0h4gyrw09ms3b57dlhjfzhfzffv50yzpkl837"))
+         "0qzxavvv20577bxvly8s7d3y7bqasqclc2mllp0ddfncjm9z02a7"))
        (modules '((guix build utils)))
        ;; Remove bundled dependencies in favour of proper inputs.
        (snippet '(begin
                    (for-each delete-file-recursively
-                             (find-files "internal" "^tinyxml2-[0-9]"
-                                         #:directories? #t))
+                             '("vendor/github.com/leethomason/tinyxml2"
+                               "vendor/github.com/google/googletest"))
                    #t))))
     (build-system cmake-build-system)
     (native-inputs
@@ -240,6 +241,7 @@ communication.")
 
        ;; Test dependencies.
        ("expect" ,expect)
+       ("googletest-source" ,(package-source googletest))
        ("perl" ,perl)))
     (inputs
      `(("attr" ,attr)
@@ -247,7 +249,18 @@ communication.")
        ("openssl" ,openssl)
        ("tinyxml2" ,tinyxml2)))
     (arguments
-     `(#:configure-flags (list "-DUSE_INTERNAL_TINYXML=OFF")))
+     `(#:configure-flags (list "-DUSE_INTERNAL_TINYXML=OFF")
+       #:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'unpack-googletest
+           (lambda* (#:key inputs #:allow-other-keys)
+             (mkdir-p "vendor/github.com/google/googletest")
+             (invoke "tar" "xvf" (assoc-ref inputs "googletest-source")
+                     "-C" "vendor/github.com/google/googletest"
+                     "--strip-components=1")))
+         (add-before 'check 'make-unittests
+           (lambda _
+             (invoke "make" "unittests"))))))
     (home-page "https://vgough.github.io/encfs")
     (synopsis "Encrypted virtual file system")
     (description
-- 
2.17.1


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

[bug#31733] [PATCH] gnu: encfs: Update to 1.9.5.

[Pierre Neidhardt]

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


This last patch looks good to me.
Can we merge it?

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

bug#31733: [PATCH] gnu: encfs: Update to 1.9.5.

[Kei Kebreau]

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

Pierre Neidhardt <ambrevar@gmail.com> writes:

> This last patch looks good to me.
> Can we merge it?

Sorry for the delay! I went ahead and pushed the change. As more
dependencies are unbundled, we may update the package definition.

Thanks,
Kei

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