From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: [PATCH 09/10] gnu: Add ocaml-js-build-tools. Date: Mon, 30 Jan 2017 10:19:21 +0100 Message-ID: <87poj4gb1i.fsf@gnu.org> References: <20170127221228.4370-1-julien@lepiller.eu> <20170127221228.4370-10-julien@lepiller.eu> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:34596) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cY87U-0000rh-6i for guix-devel@gnu.org; Mon, 30 Jan 2017 04:19:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cY87P-00031P-WB for guix-devel@gnu.org; Mon, 30 Jan 2017 04:19:28 -0500 In-Reply-To: <20170127221228.4370-10-julien@lepiller.eu> (Julien Lepiller's message of "Fri, 27 Jan 2017 23:12:27 +0100") 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: Julien Lepiller Cc: guix-devel@gnu.org Hey! Julien Lepiller skribis: > * gnu/packages/ocaml.scm (ocaml-js-build-tools): New variable. > * gnu/packages/patches/ocaml-janestreet-fix-libdir.patch: New file. > * gnu/local.mk (dist_patch_DATA): Add it. [...] > +;; Janestreet packages are found in a similar way and all need the same = patch Period at the end of a sentence please. > +(define (janestreet-origin name version hash) > + (origin (method url-fetch) > + (uri (string-append "https://ocaml.janestreet.com/ocaml-core/" > + (version-major+minor version) "/files/" > + name "-" version ".tar.gz")) > + (sha256 (base32 hash)) > + (patches (search-patches "ocaml-janestreet-fix-libdir.patch")) > + (modules '((guix build utils))) > + (snippet `(substitute* "install.ml" > + (((string-append "lib/" ,name)) > + (string-append "lib/ocaml/site-lib/" ,= name)))))) I prefer not to rely on the ability to use non-literal patterns in =E2=80=98substitute*=E2=80=99, so I would write it this way: (let ((pattern (string-append "lib/" name))) `(substitute* "install.ml" ((,pattern) =E2=80=A6))) Also, could you add a comment about what the snippet does (I suppose Jane Street=E2=80=99s default install.ml files specify a wrong location or something?). Lastly, please pass this through indent-code.el. > +(define-public ocaml-js-build-tools > + (package > + (name "ocaml-js-build-tools") > + (version "113.33.06") > + (source (janestreet-origin "js-build-tools" version > + "0r8z4fz8iy5y6hkdlkpwf6rk4qigcr3dzyv35585xgg2ahf12zy6")) > + (native-inputs > + `(("oasis" ,ocaml-oasis) > + ("opam" ,opam))) > + (build-system ocaml-build-system) > + (arguments janestreet-arguments) > + (home-page "https://github.com/janestreet/js-build-tools") > + (synopsis "Collection of tools to help building Jane Street Packages= ") > + (description "This packages contains tools to help building Jane Str= eet > +Packages, but can be used for other purposes. It contains: s/Packages/packages/ > +@enumerate > +@item an oasis2opam-install tool to produce a .install file from the oas= is build log Nitpick: @command{oasis2opam-install}, @file{.install}, OASIS. > +@item an js_build_tools ocamlbuild plugin with various goodies s/an js_build_tools/a @code{js_build_tools}/ Period at the end. > --- /dev/null > +++ b/gnu/packages/patches/ocaml-janestreet-fix-libdir.patch > @@ -0,0 +1,39 @@ > +This patch adds a --libdir option to opam-installer so it installs the p= lugin > +in the specified directory rather than in the default one (ocaml's direc= tory in > +the store, which is forbidden). s/forbidden/read-only/ :-) Also please mention that this patch is meant to apply to all the packages published by Jane Street (IIUC). Thanks, Ludo=E2=80=99.