From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:43821) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h7Rv5-0005j8-V0 for guix-patches@gnu.org; Fri, 22 Mar 2019 17:41:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1h7Rpl-0008Pt-K1 for guix-patches@gnu.org; Fri, 22 Mar 2019 17:36:15 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:41234) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1h7Rpa-0008KR-Jy for guix-patches@gnu.org; Fri, 22 Mar 2019 17:36:03 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1h7Rpa-000176-D6 for guix-patches@gnu.org; Fri, 22 Mar 2019 17:36:02 -0400 Subject: [bug#34807] [PATCH 1/2] Add (guix lzlib). Resent-Message-ID: From: Ludovic =?UTF-8?Q?Court=C3=A8s?= References: <20190310180209.11578-1-mail@ambrevar.xyz> Date: Fri, 22 Mar 2019 22:35:02 +0100 In-Reply-To: <20190310180209.11578-1-mail@ambrevar.xyz> (Pierre Neidhardt's message of "Sun, 10 Mar 2019 19:02:09 +0100") Message-ID: <8736ne3855.fsf@gnu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Pierre Neidhardt Cc: 34807@debbugs.gnu.org Hello, Pierre Neidhardt skribis: > * guix/lzlib.scm, tests/lzlib.scm: New files. > * Makefile.am (MODULES): Add guix/lzlib.scm. > (SCM_TESTS): Add tests/lzlib.scm. > * m4/guix.m4 (GUIX_LIBLZ_LIBDIR): New macro. > * configure.ac (LIBLZ_LIBDIR): Use it. Define and substitute > 'LIBLZ'. > * guix/config.scm.in (%liblz): New variable. This looks really nice! Please update =E2=80=98make-config.scm=E2=80=99 in (guix self) so that it d= efines =E2=80=98%liblz=E2=80=99 as well (setting it to #f for now). > +(define %liblz > + ;; TODO: Set this dynamically. > + "/gnu/store/8db7vivi8p9mpkbphb8xy8gh2bkwc4iz-lzlib-1.11/lib/liblz") You can already put "@LIBLZ@" here. > +(define %lzlib > + ;; File name of lzlib's shared library. When updating via 'guix pull', > + ;; '%liblz' might be undefined so protect against it. Updating =E2=80=98make-config.scm=E2=80=99 will fix it. > +(define %error-number-ok > + ;; TODO: How do we get the values of a C enum? See the thread on guix-devel. > +(define lz-compress-open > + (let ((proc (lzlib-procedure '* "LZ_compress_open" (list int int uint6= 4)))) > + ;; TODO: member-size default is INT64_MAX. Is there a better way to= do this with Guile? > + (lambda* (dictionary-size match-length-limit #:optional (member-size= #x7FFFFFFFFFFFFFFF)) You could write (- (expt 2 63) 1) I guess for clarity, but what you wrote i= s OK. Is it also the case on 32-bit platforms? > +(define lz-compress-finish > + (let ((proc (lzlib-procedure int "LZ_compress_finish" '(*)))) > + (lambda (encoder) > + "Use this function to tell that all the data for this member have > +already been written (with the `lz-compress-write' function). It is saf= e to > +call `lz-compress-finish' as many times as needed. After all the produc= ed > +compressed data have been read with `lz-compress-read' and > +`lz-compress-member-finished?' returns #t, a new member can be started w= ith > +'lz-compress-restart-member'." For docstrings, the convention in GNU and Guix is to use the imperative tense and to explicitly refer to the arguments there, like: "Tell ENCODER that all the data for this member have alrady been written. =E2=80=A6" (Same for the other docstrings that start with =E2=80=9CUse this function.= =E2=80=9D) > +(define* (lzread! decoder file-port bv > + #:optional (start 0) (count (bytevector-length bv))) > + "Read up to COUNT bytes from FILE-PORT into BV at offset START. Retur= n the > +number of uncompressed bytes actually read; it is zero if COUNT is zero = or if > +the end-of-stream has been reached." > + (let* ((written 0) > + (read 0) > + (chunk (* 64 1024)) > + (file-bv (get-bytevector-n file-port count))) > + (if (eof-object? file-bv) > + 0 > + (begin > + (while (and (< 0 (lz-decompress-write-size decoder)) > + (< written (bytevector-length file-bv))) > + (set! written (lz-decompress-write decoder file-bv written (= - (bytevector-length file-bv) written)))) > + ;; TODO: When should we call `lz-decompress-finish'? > + ;; (lz-decompress-finish decoder) > + ;; TODO: Loop? > + (set! read (lz-decompress-read decoder bv start > + (- (bytevector-length bv) start= ))) It=E2=80=99s worth figuring out. :-) Are there examples or the code of the actual =E2=80=98lzip=E2=80=99 command= that could help? > +dnl GUIX_LIBLZ_LIBDIR VAR > +dnl > +dnl Attempt to determine liblz's LIBDIR; store the result in VAR. > +AC_DEFUN([GUIX_LIBLZ_LIBDIR], [ > + AC_REQUIRE([PKG_PROG_PKG_CONFIG]) > + AC_CACHE_CHECK([lzlib's library directory], > + [guix_cv_liblz_libdir], > + dnl TODO: This fails because lzlib has no pkg-config. > + [guix_cv_liblz_libdir=3D"`$PKG_CONFIG lzlib --variable=3Dlibdir 2> /= dev/null`"]) > + $1=3D"$guix_cv_liblz_libdir" > +]) I think you could do something like this in the body of =E2=80=98AC_CACHE_C= HECK=E2=80=99 (untested): old_LIBS=3D"$LIBS" LIBS=3D"-llz" AC_LINK_IFELSE([LZ_decompress_open();], [guix_cv_libz_libdir=3D"`ldd conftest$EXEEXT | grep liblz | sed '-es/.*= =3D> \(.*\) .*$/\1/g'`"]) LIBS=3D"$old_LIBS" Regarding testing, it=E2=80=99s easy to get this sort of binding subtly wro= ng IME. :-) I=E2=80=99d suggest looking at things like: 1. Passing short input bytevectors, large input bytevectors, and input that=E2=80=99s equal to liblz=E2=80=99s internal buffer size or off by= one. 2. File descriptors: strace guile while doing =E2=80=98call-with-lzip-input-port=E2=80=99 and =E2=80=98call-with-lzi= p-output-port=E2=80=99 and make sure that file descriptors are not left open and are not closed prematurely either. (This is particularly important for long-running processes like =E2=80=98guix publish=E2=80=99.) But overall, modulo the small issues above, it looks pretty much ready to me. Thank you! Ludo=E2=80=99.