GNU bug report logs - #31733
[PATCH] gnu: encfs: Update to 1.9.5.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 31733 in the body.
You can then email your comments to 31733 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Pierre Neidhardt <ambrevar <at> gmail.com>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: encfs: Update to 1.9.5.
Date: Wed,  6 Jun 2018 11:47:30 +0200

* 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

Message #8 received at 31733 <at> debbugs.gnu.org (full text, mbox):

From: Kei Kebreau <kkebreau <at> posteo.net>
To: Pierre Neidhardt <ambrevar <at> gmail.com>
Cc: 31733 <at> debbugs.gnu.org
Subject: Re: [bug#31733] [PATCH] gnu: encfs: Update to 1.9.5.
Date: Thu, 14 Jun 2018 10:53:49 -0400

[Message part 1 (text/plain, inline)]

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! :)

[signature.asc (application/pgp-signature, inline)]

Message #11 received at 31733 <at> debbugs.gnu.org (full text, mbox):

From: Kei Kebreau <kkebreau <at> posteo.net>
To: Pierre Neidhardt <ambrevar <at> gmail.com>
Cc: 31733 <at> debbugs.gnu.org
Subject: Re: [bug#31733] [PATCH] gnu: encfs: Update to 1.9.5.
Date: Thu, 14 Jun 2018 11:04:30 -0400

[Message part 1 (text/plain, inline)]

Kei Kebreau <kkebreau <at> 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.

[signature.asc (application/pgp-signature, inline)]

Message #14 received at 31733 <at> debbugs.gnu.org (full text, mbox):

From: Pierre Neidhardt <ambrevar <at> gmail.com>
To: Kei Kebreau <kkebreau <at> posteo.net>
Cc: 31733 <at> debbugs.gnu.org
Subject: Re: [bug#31733] [PATCH] gnu: encfs: Update to 1.9.5.
Date: Sat, 16 Jun 2018 16:04:57 +0200

[Message part 1 (text/plain, inline)]

>> 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

[signature.asc (application/pgp-signature, inline)]

Message #17 received at 31733 <at> debbugs.gnu.org (full text, mbox):

From: Kei Kebreau <kkebreau <at> posteo.net>
To: Pierre Neidhardt <ambrevar <at> gmail.com>
Cc: 31733 <at> debbugs.gnu.org
Subject: Re: [bug#31733] [PATCH] gnu: encfs: Update to 1.9.5.
Date: Sat, 16 Jun 2018 11:23:33 -0400

[Message part 1 (text/plain, inline)]

Pierre Neidhardt <ambrevar <at> 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!

[signature.asc (application/pgp-signature, inline)]

Message #20 received at 31733 <at> debbugs.gnu.org (full text, mbox):

From: Pierre Neidhardt <ambrevar <at> gmail.com>
To: Kei Kebreau <kkebreau <at> posteo.net>
Cc: 31733 <at> debbugs.gnu.org
Subject: Re: [bug#31733] [PATCH] gnu: encfs: Update to 1.9.5.
Date: Sat, 16 Jun 2018 17:44:27 +0200

[Message part 1 (text/plain, inline)]

Kei Kebreau <kkebreau <at> 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

[signature.asc (application/pgp-signature, inline)]

Message #23 received at 31733 <at> debbugs.gnu.org (full text, mbox):

From: Kei Kebreau <kkebreau <at> posteo.net>
To: Pierre Neidhardt <ambrevar <at> gmail.com>
Cc: 31733 <at> debbugs.gnu.org
Subject: Re: [bug#31733] [PATCH] gnu: encfs: Update to 1.9.5.
Date: Sat, 16 Jun 2018 16:35:19 -0400

[Message part 1 (text/plain, inline)]

Pierre Neidhardt <ambrevar <at> gmail.com> writes:

> Kei Kebreau <kkebreau <at> 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?

[0001-gnu-encfs-Update-to-1.9.5.patch (text/plain, attachment)]

[signature.asc (application/pgp-signature, inline)]

Message #26 received at 31733 <at> debbugs.gnu.org (full text, mbox):

From: Pierre Neidhardt <ambrevar <at> gmail.com>
To: Kei Kebreau <kkebreau <at> posteo.net>
Cc: 31733 <at> debbugs.gnu.org
Subject: Re: [bug#31733] [PATCH] gnu: encfs: Update to 1.9.5.
Date: Sun, 17 Jun 2018 19:11:19 +0200

[Message part 1 (text/plain, inline)]

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

[signature.asc (application/pgp-signature, inline)]

Message #31 received at 31733-done <at> debbugs.gnu.org (full text, mbox):

From: Kei Kebreau <kkebreau <at> posteo.net>
To: Pierre Neidhardt <ambrevar <at> gmail.com>
Cc: 31733-done <at> debbugs.gnu.org
Subject: Re: [bug#31733] [PATCH] gnu: encfs: Update to 1.9.5.
Date: Sun, 17 Jun 2018 22:01:08 -0400

[Message part 1 (text/plain, inline)]

Pierre Neidhardt <ambrevar <at> 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

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.