From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Kost Subject: Re: [PATCH 1/4] gnu: node: Add http-parser. Date: Sat, 27 Aug 2016 23:33:57 +0300 Message-ID: <87eg5adk96.fsf@gmail.com> References: <20160827112333.1759-1-jlicht@fsfe.org> <20160827112333.1759-2-jlicht@fsfe.org> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:59551) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bdkIk-0002i6-SE for guix-devel@gnu.org; Sat, 27 Aug 2016 16:34:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bdkIh-0007lw-Ko for guix-devel@gnu.org; Sat, 27 Aug 2016 16:34:02 -0400 Received: from mail-lf0-x241.google.com ([2a00:1450:4010:c07::241]:35159) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bdkIh-0007lq-7P for guix-devel@gnu.org; Sat, 27 Aug 2016 16:33:59 -0400 Received: by mail-lf0-x241.google.com with SMTP id l89so5318441lfi.2 for ; Sat, 27 Aug 2016 13:33:59 -0700 (PDT) In-Reply-To: <20160827112333.1759-2-jlicht@fsfe.org> (Jelle Licht's message of "Sat, 27 Aug 2016 13:23:30 +0200") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Jelle Licht Cc: guix-devel@gnu.org Jelle Licht (2016-08-27 14:23 +0300) wrote: > * gnu/packages/node.scm (http-parser): New variable. > * gnu/packages/node.scm (define-module): Import gnu packages tls with > tls: prefix These are 2 independent things, so I think it should be 2 separate patches, but actually why is it needed to prefix tls? > --- > gnu/packages/node.scm | 39 +++++++++++++++++++++++++++++++++++++-- > 1 file changed, 37 insertions(+), 2 deletions(-) > > > diff --git a/gnu/packages/node.scm b/gnu/packages/node.scm > index 2b27774..545aaaa 100644 > --- a/gnu/packages/node.scm > +++ b/gnu/packages/node.scm > @@ -32,7 +32,42 @@ > #:use-module (gnu packages linux) > #:use-module (gnu packages perl) > #:use-module (gnu packages python) > - #:use-module (gnu packages tls)) > + #:use-module ((gnu packages tls) #:prefix tls:) > + #:use-module (gnu packages valgrind)) > + > +(define-public http-parser > + (package > + (name "http-parser") > + (version "2.7.1") > + (source (origin > + (method url-fetch) > + (uri (string-append "https://github.com/nodejs/http-parser/archive/v" > + version ".tar.gz")) Here the following line is needed: (file-name (string-append name "-" version ".tar.gz")) Without it the source tarball will have "...-v2.7.1.tar.gz" name, which is not very understandable. > + (sha256 (base32 "1cw6nf8xy4jhib1w0jd2y0gpqjbdasg8b7pkl2k2vpp54k9rlh3h")))) > + (build-system gnu-build-system) > + (arguments > + '(#:make-flags (list "CC=gcc" (string-append "DESTDIR=" (assoc-ref %outputs "out"))) Does it work if it will be PREFIX instead of DESTDIR? I forgot the exact reason, but we try to avoid using DESTDIR directly, and prefer to set PREFIX instead. Also please don't make such long lines :-) > + #:test-target "test-valgrind" > + #:phases > + (modify-phases %standard-phases > + (delete 'configure) > + (add-before 'build 'patch-makefile > + (lambda* (#:key inputs outputs #:allow-other-keys) Just (lambda _ ...) Keywords are not needed here > + (substitute* '("Makefile") > + (("/usr/local") "")))) Please add #t in the end of this phase: succeeded phases should return non-false value, and the return value of 'substitute*' is not specified. > + (replace 'build > + (lambda* (#:key make-flags #:allow-other-keys) > + (zero? (apply system* "make" "library" make-flags)))) > + ))) Please move these lonely parentheses to the previous line. > + (inputs '()) This line is not needed as the empty list is the default value. > + (native-inputs `(("valgrind" ,valgrind))) > + (home-page "https://github.com/nodejs/http-parser") > + (synopsis "HTTP request/response parser for C") > + (description "HTTP parser is a parser for HTTP messages written in C. It > +parses both requests and responses. The parser is designed to be used in > +performance HTTP applications. It does not make any syscalls nor allocations, > +it does not buffer data, it can be interrupted at anytime.") > + (license expat))) > > (define-public node > (package > @@ -118,7 +153,7 @@ > ("which" ,which))) > (inputs > `(("libuv" ,libuv) > - ("openssl" ,openssl) > + ("openssl" ,tls:openssl) > ("zlib" ,zlib))) > (synopsis "Evented I/O for V8 JavaScript") > (description "Node.js is a platform built on Chrome's JavaScript runtime -- Alex