diff mbox series

[bug#34807,1/2] Add (guix lzlib).

Message ID 878svm5xic.fsf@ambrevar.xyz
State Accepted
Headers show
Series [bug#34807,1/2] Add (guix lzlib). | expand

Checks

Context Check Description
cbaines/applying patch fail Apply failed

Commit Message

Pierre Neidhardt May 4, 2019, 10:23 a.m. UTC
Right on time, I just finished it!

- I've been in touch with Antonio, Lzip's maintainer, for more than a
  week and now I'm confident that I have a decent understanding of the
  library.

- Your m4 suggestion didn't work.  I've included a comment.  We need to
  fix it before merging.  I'm not the right person for this job I'm
  afraid :p  Ludo?

- The convenience functions do not support multi-member archives.
  Multi-member archives are mostly useful for parallelization, but we
  don't use that in Guix, so it's OK.  Should it be required some day,
  we would need to implement it, which requires a little bit more work.
  I've documented all that.

- The implementation of lzread! is subpar because I understood a
  subtlety a bit too late.  But that's alright, it does not affect
  performance nor reliability.

- I've included 11 tests covering all your suggestions.

- I haven't strace'd the Guile process.  The code regarding ports is
  identical to zlib.scm, so it's unlikely there would be an issue in
  this area.  I have never done this before, so out of curiosity, how do
  you run a specific Guix tests without going through `make'?

Next steps? :D

Comments

Ludovic Courtès May 4, 2019, 9:09 p.m. UTC | #1
Hello!

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

> Right on time, I just finished it!
>
> - I've been in touch with Antonio, Lzip's maintainer, for more than a
>   week and now I'm confident that I have a decent understanding of the
>   library.
>
> - Your m4 suggestion didn't work.  I've included a comment.  We need to
>   fix it before merging.  I'm not the right person for this job I'm
>   afraid :p  Ludo?

Sure, I can do it.

> - The convenience functions do not support multi-member archives.
>   Multi-member archives are mostly useful for parallelization, but we
>   don't use that in Guix, so it's OK.  Should it be required some day,
>   we would need to implement it, which requires a little bit more work.
>   I've documented all that.
>
> - The implementation of lzread! is subpar because I understood a
>   subtlety a bit too late.  But that's alright, it does not affect
>   performance nor reliability.
>
> - I've included 11 tests covering all your suggestions.
>
> - I haven't strace'd the Guile process.  The code regarding ports is
>   identical to zlib.scm, so it's unlikely there would be an issue in
>   this area.  I have never done this before, so out of curiosity, how do
>   you run a specific Guix tests without going through `make'?
>
> Next steps? :D

This looks all good to me!

I was about to apply it and add the Autoconf machinery, but I thought we
could also make it a separate project that could be beneficial to other
Guilers out there (like we did with Guile-Gcrypt and Guile-Git).
Incidentally that would also avoid the need for adding the ‘%liblz’
variable in (guix config), which simplifies things a bit.

WDYT?

If you want to take that route, I’m happy to help with the Autotools
machinery (or you could use ‘hall’ from the ‘guile-hall’ package to do
that for you.)

If you don’t feel like taking that route (or at least not yet ;-)),
that’s OK for me too, I don’t feel strongly either way.

Thoughts?

Thank you!

Ludo’.
Pierre Neidhardt May 4, 2019, 9:39 p.m. UTC | #2
Hi!

It's definitely the ideal route.  Something like guile-compress or
guile-archive, with a high-level abstraction for a collection of
bindings including zlib and lzlib for now.

Sadly I don't have the time for it at the moment.  Unless you do (:p) I
suggest we add a TODO item and keep it for later.

Regarding guix publish and the farms, what shall we do?
diff mbox series

Patch

From 7dd8f4207657ae7ad178c21a45f74bef6cc0a314 Mon Sep 17 00:00:00 2001
From: Pierre Neidhardt <mail@ambrevar.xyz>
Date: Sun, 10 Mar 2019 16:40:41 +0100
Subject: [PATCH 2/2] dir-locals.el: Add 'call-with-lzip-input-port' and
 'call-with-lzip-output-port' keywords.

* .dir-locals.el: Add indentation rules for 'call-with-lzip-input-port' and
'call-with-lzip-output-port'.
---
 .dir-locals.el | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.dir-locals.el b/.dir-locals.el
index 550e06ef09..f1196fd781 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
@@ -53,6 +53,8 @@ 
    (eval . (put 'call-with-decompressed-port 'scheme-indent-function 2))
    (eval . (put 'call-with-gzip-input-port 'scheme-indent-function 1))
    (eval . (put 'call-with-gzip-output-port 'scheme-indent-function 1))
+   (eval . (put 'call-with-lzip-input-port 'scheme-indent-function 1))
+   (eval . (put 'call-with-lzip-output-port 'scheme-indent-function 1))
    (eval . (put 'signature-case 'scheme-indent-function 1))
    (eval . (put 'emacs-batch-eval 'scheme-indent-function 0))
    (eval . (put 'emacs-batch-edit-file 'scheme-indent-function 1))
-- 
2.21.0