diff mbox series

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

Message ID 87a7fzffip.fsf@gnu.org
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

Ludovic Courtès May 6, 2019, 9:18 p.m. UTC 
Hi Pierre,

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

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

Sounds good!

Below are the Autoconf-related changes I made.  Committed!

We’ll take care of (guix self) when (guix lzlib) is actually used by
other parts of the code.

> Regarding guix publish and the farms, what shall we do?

I think we should arrange for the client part, ‘guix substitute’, to be
ready to lzip-decode as soon as it talks to an lzip-capable server.

Then we should add support in ‘guix publish’.  At some later point, we’d
deploy it on the build farms.

For this migration to be incremental, we need (1) clients to be able to
transparently switch to lzip when it’s available, and (2) servers to be
able to produce both lzip archives (for new clients) and gzip archives
(for old clients) during the transition period.

That’s a bit of work in ‘guix publish’.  It’ll be extra CPU and storage
usage on the build farm since during the transition period it’d have to
produce and store both gzip and lzip archives for each store item.  I
don’t really see any way around that, though.

A difficulty is that narinfos currently include a fixed compression
scheme:

--8<---------------cut here---------------start------------->8---
$ wget -q -O - https://ci.guix.info/nrkm1683p1cqnkcmhlmhiig9q9qd7xqh.narinfo | head -3
StorePath: /gnu/store/nrkm1683p1cqnkcmhlmhiig9q9qd7xqh-sed-4.5
URL: nar/gzip/nrkm1683p1cqnkcmhlmhiig9q9qd7xqh-sed-4.5
Compression: gzip
--8<---------------cut here---------------end--------------->8---

So, depending on the client, ‘guix publish’ should return either a
narinfo-for-gzip or a narinfo-for-lzip.  To make it possible, new
clients could send an extra HTTP header, say ‘X-Guix-Compression’, that
would specify their preferred compression method(s).  ‘guix publish’
would take that into account when replying.

How does that sound?

Thanks,
Ludo’.

Comments

Tobias Geerinckx-Rice May 6, 2019, 11:28 p.m. UTC | #1 
Ludo',

Ludovic Courtès wrote:
> So, depending on the client, ‘guix publish’ should return either 
> a
> narinfo-for-gzip or a narinfo-for-lzip.  To make it possible, 
> new
> clients could send an extra HTTP header, say 
> ‘X-Guix-Compression’, that
> would specify their preferred compression method(s).  ‘guix 
> publish’
> would take that into account when replying.

There's a standard[0] HTTP header for that: ‘Accept-Encoding’.

Unfortunately (and for reasons that I cannot fathom), it doesn't 
use standard MIME types, but pseudostandard strings like ‘gzip’ 
and ‘br’.  We can boldly add ‘lzip’ to that :-)

Similarly, servers can send ‘Content-Encoding’[1] HTTP headers, 
but I don't see a need for it here.

Kind regards,

T G-R

[0]: 
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept-Encoding
[1]: 
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Encoding
Pierre Neidhardt May 7, 2019, 7:02 a.m. UTC | #2 
All good.

I'm very busy (with Next browser) these days, so I won't have much time.
Maybe I can give (1) a shot (lzip-decoding for clients), don't think
I'll have time for the guix publish part before a while.

Anyone?
Ludovic Courtès May 7, 2019, 8:19 a.m. UTC | #3 
Tobias Geerinckx-Rice <me@tobias.gr> skribis:

> Ludovic Courtès wrote:
>> So, depending on the client, ‘guix publish’ should return either a
>> narinfo-for-gzip or a narinfo-for-lzip.  To make it possible, new
>> clients could send an extra HTTP header, say ‘X-Guix-Compression’,
>> that
>> would specify their preferred compression method(s).  ‘guix publish’
>> would take that into account when replying.
>
> There's a standard[0] HTTP header for that: ‘Accept-Encoding’.
>
> Unfortunately (and for reasons that I cannot fathom), it doesn't use
> standard MIME types, but pseudostandard strings like ‘gzip’ and ‘br’.
> We can boldly add ‘lzip’ to that :-)

Well, that’s why I thought about using a new header.  :-)

Ludo’.
Ludovic Courtès May 7, 2019, 3:44 p.m. UTC | #4 
Pierre Neidhardt <mail@ambrevar.xyz> skribis:

> I'm very busy (with Next browser) these days, so I won't have much time.
> Maybe I can give (1) a shot (lzip-decoding for clients), don't think
> I'll have time for the guix publish part before a while.

I’ll take a look at it, probably after 1.0.1.

Anyway, we can close this issue and open new ones for the remaining
bits.

Ludo’.
Pierre Neidhardt May 7, 2019, 3:51 p.m. UTC | #5 
OK, feel free to open the corresponding issues and forward me the
messages, I'll see what I can do.
diff mbox series

Patch

diff --git a/configure.ac b/configure.ac
index ee7aa25742..3918550a79 100644
--- a/configure.ac
+++ b/configure.ac
@@ -251,14 +251,13 @@  AC_MSG_RESULT([$LIBZ])
 AC_SUBST([LIBZ])
 
 dnl Library name of lzlib suitable for 'dynamic-link'.
-GUIX_LIBLZ_LIBDIR([liblz_libdir])
-if test "x$liblz_libdir" = "x"; then
+GUIX_LIBLZ_FILE_NAME([LIBLZ])
+if test "x$LIBLZ" = "x"; then
   LIBLZ="liblz"
 else
-  LIBLZ="$liblz_libdir/liblz"
+  # Strip the .so or .so.1 extension since that's what 'dynamic-link' expects.
+  LIBLZ="`echo $LIBLZ | sed -es'/\.so\(\.[[0-9.]]\+\)\?//g'`"
 fi
-AC_MSG_CHECKING([for lzlib's shared library name])
-AC_MSG_RESULT([$LIBLZ])
 AC_SUBST([LIBLZ])
 
 dnl Check for Guile-SSH, for the (guix ssh) module.
diff --git a/guix/config.scm.in b/guix/config.scm.in
index cd7036ca7e..0ada0f3c38 100644
--- a/guix/config.scm.in
+++ b/guix/config.scm.in
@@ -34,10 +34,10 @@ 
 
             %system
             %libz
+            %liblz
             %gzip
             %bzip2
-            %xz
-            %liblz))
+            %xz))
 
 ;;; Commentary:
 ;;;
@@ -91,6 +91,9 @@ 
 (define %libz
   "@LIBZ@")
 
+(define %liblz
+  "@LIBLZ@")
+
 (define %gzip
   "@GZIP@")
 
@@ -100,9 +103,4 @@ 
 (define %xz
   "@XZ@")
 
-(define %liblz
-  ;; TODO: Set this dynamically.
-  ;; "@LIBLZ@"
-  "/gnu/store/8db7vivi8p9mpkbphb8xy8gh2bkwc4iz-lzlib-1.11/lib/liblz")
-
 ;;; config.scm ends here
diff --git a/guix/self.scm b/guix/self.scm
index 983f3514d3..74ea65240c 100644
--- a/guix/self.scm
+++ b/guix/self.scm
@@ -925,6 +925,7 @@  Info manual."
                                %store-database-directory
                                %config-directory
                                %libz
+                               ;; TODO: %liblz
                                %gzip
                                %bzip2
                                %xz))
@@ -971,8 +972,7 @@  Info manual."
 
                    (define %libz
                      #+(and zlib
-                            (file-append zlib "/lib/libz")))
-                   (define %liblz #f))
+                            (file-append zlib "/lib/libz"))))
 
                ;; Guile 2.0 *requires* the 'define-module' to be at the
                ;; top-level or the 'toplevel-ref' in the resulting .go file are
diff --git a/m4/guix.m4 b/m4/guix.m4
index 78cc3777f8..d0c5ec0f08 100644
--- a/m4/guix.m4
+++ b/m4/guix.m4
@@ -1,5 +1,5 @@ 
 dnl GNU Guix --- Functional package management for GNU
-dnl Copyright © 2012, 2013, 2014, 2015, 2016, 2018 Ludovic Courtès <ludo@gnu.org>
+dnl Copyright © 2012, 2013, 2014, 2015, 2016, 2018, 2019 Ludovic Courtès <ludo@gnu.org>
 dnl Copyright © 2014 Mark H Weaver <mhw@netris.org>
 dnl Copyright © 2017 Efraim Flashner <efraim@flashner.co.il>
 dnl
@@ -312,20 +312,18 @@  AC_DEFUN([GUIX_LIBZ_LIBDIR], [
   $1="$guix_cv_libz_libdir"
 ])
 
-dnl GUIX_LIBLZ_LIBDIR VAR
+dnl GUIX_LIBLZ_FILE_NAME VAR
 dnl
-dnl Attempt to determine liblz's LIBDIR; store the result in VAR.
-AC_DEFUN([GUIX_LIBLZ_LIBDIR], [
+dnl Attempt to determine liblz's absolute file name; store the result in VAR.
+AC_DEFUN([GUIX_LIBLZ_FILE_NAME], [
   AC_REQUIRE([PKG_PROG_PKG_CONFIG])
-  AC_CACHE_CHECK([lzlib's library directory],
+  AC_CACHE_CHECK([lzlib's file name],
     [guix_cv_liblz_libdir],
-    [guix_cv_liblz_libdir="`$PKG_CONFIG lzlib --variable=libdir 2> /dev/null`"])
-    dnl TODO: lzlib has no pkg-config so we need the following trick to find its directory.
-    dnl old_LIBS="$LIBS"
-    dnl LIBS="-llz"
-    dnl AC_LINK_IFELSE([LZ_decompress_open();],
-    dnl   [guix_cv_libz_libdir="`ldd conftest$EXEEXT | grep liblz | sed '-es/.*=> \([^ ]*\).*$/\1/g'`"])
-    dnl LIBS="$old_LIBS"
+    [old_LIBS="$LIBS"
+     LIBS="-llz"
+     AC_LINK_IFELSE([AC_LANG_SOURCE([int main () { return LZ_decompress_open(); }])],
+       [guix_cv_liblz_libdir="`ldd conftest$EXEEXT | grep liblz | sed '-es/.*=> \(.*\) .*$/\1/g'`"])
+     LIBS="$old_LIBS"])
   $1="$guix_cv_liblz_libdir"
 ])
 
diff --git a/tests/lzlib.scm b/tests/lzlib.scm
index a6631ce91c..cf53a9417d 100644
--- a/tests/lzlib.scm
+++ b/tests/lzlib.scm
@@ -26,8 +26,11 @@ 
 
 ;; Test the (guix lzlib) module.
 
-(unless (lzlib-available?)
-  (exit 77))
+(define-syntax-rule (test-assert* description exp)
+  (begin
+    (unless (lzlib-available?)
+      (test-skip 1))
+    (test-assert description exp)))
 
 (test-begin "lzlib")
 
@@ -68,41 +71,41 @@ 
                     (port-closed? parent)
                     (bytevector=? received data)))))))))))
 
-(test-assert "null bytevector"
+(test-assert* "null bytevector"
   (compress-and-decompress (make-bytevector (+ (random 100000)
                                                (* 20 1024)))))
 
-(test-assert "random bytevector"
+(test-assert* "random bytevector"
   (compress-and-decompress (random-bytevector (+ (random 100000)
                                                  (* 20 1024)))))
-(test-assert "small bytevector"
+(test-assert* "small bytevector"
   (compress-and-decompress (random-bytevector 127)))
 
-(test-assert "1 bytevector"
+(test-assert* "1 bytevector"
   (compress-and-decompress (random-bytevector 1)))
 
-(test-assert "Bytevector of size relative to Lzip internal buffers (2 * dictionary)"
+(test-assert* "Bytevector of size relative to Lzip internal buffers (2 * dictionary)"
   (compress-and-decompress
    (random-bytevector
     (* 2 (car (car (assoc-ref (@@ (guix lzlib) %compression-levels)
                               (@@ (guix lzlib) %default-compression-level))))))))
 
-(test-assert "Bytevector of size relative to Lzip internal buffers (64KiB)"
+(test-assert* "Bytevector of size relative to Lzip internal buffers (64KiB)"
   (compress-and-decompress (random-bytevector (* 64 1024))))
 
-(test-assert "Bytevector of size relative to Lzip internal buffers (64KiB-1)"
+(test-assert* "Bytevector of size relative to Lzip internal buffers (64KiB-1)"
   (compress-and-decompress (random-bytevector (1- (* 64 1024)))))
 
-(test-assert "Bytevector of size relative to Lzip internal buffers (64KiB+1)"
+(test-assert* "Bytevector of size relative to Lzip internal buffers (64KiB+1)"
   (compress-and-decompress (random-bytevector (1+ (* 64 1024)))))
 
-(test-assert "Bytevector of size relative to Lzip internal buffers (1MiB)"
+(test-assert* "Bytevector of size relative to Lzip internal buffers (1MiB)"
   (compress-and-decompress (random-bytevector (* 1024 1024))))
 
-(test-assert "Bytevector of size relative to Lzip internal buffers (1MiB-1)"
+(test-assert* "Bytevector of size relative to Lzip internal buffers (1MiB-1)"
   (compress-and-decompress (random-bytevector (1- (* 1024 1024)))))
 
-(test-assert "Bytevector of size relative to Lzip internal buffers (1MiB+1)"
+(test-assert* "Bytevector of size relative to Lzip internal buffers (1MiB+1)"
   (compress-and-decompress (random-bytevector (1+ (* 1024 1024)))))
 
 (test-end)