From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:36443) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ggFzg-0003mm-FX for guix-patches@gnu.org; Sun, 06 Jan 2019 16:30:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ggFzf-0006l3-11 for guix-patches@gnu.org; Sun, 06 Jan 2019 16:30:04 -0500 Received: from debbugs.gnu.org ([209.51.188.43]:48102) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1ggFze-0006ky-SD for guix-patches@gnu.org; Sun, 06 Jan 2019 16:30:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1ggFze-0000Kv-OM for guix-patches@gnu.org; Sun, 06 Jan 2019 16:30:02 -0500 Subject: [bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename Resent-Message-ID: References: <871s6l9h54.fsf@gnu.org> <87r2ekea1u.fsf@ambrevar.xyz> <04daca2a-705d-6255-85bd-48132879f5a8@yahoo.de> <87lg4se6wf.fsf@ambrevar.xyz> <8d15b1a4-5f0b-28aa-a3d9-78520431b8ef@yahoo.de> <87imzwe2d3.fsf@ambrevar.xyz> <2df7cbf3-81d5-7d50-35d7-f3b369d1ac0e@yahoo.de> <87ftuth0d7.fsf@ambrevar.xyz> <14de0933-fddc-94d0-d75a-cdf4b49fce1a@yahoo.de> <87wonhipbw.fsf@ambrevar.xyz> From: Tim Gesthuizen Message-ID: <792d34a0-b048-b84f-b7b8-d9d996da2a28@yahoo.de> Date: Sun, 6 Jan 2019 22:29:26 +0100 MIME-Version: 1.0 In-Reply-To: <87wonhipbw.fsf@ambrevar.xyz> Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="nkg7ECbHBShueInR8UYTzCE2v3cpQaMIH" 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: 33598@debbugs.gnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --nkg7ECbHBShueInR8UYTzCE2v3cpQaMIH Content-Type: multipart/mixed; boundary="WJRYWDuVSr7TFOG0SoDMzgjwsaVmFMHh6"; protected-headers="v1" From: Tim Gesthuizen To: Pierre Neidhardt Cc: =?UTF-8?Q?Ludovic_Court=c3=a8s?= , 33598@debbugs.gnu.org Message-ID: <792d34a0-b048-b84f-b7b8-d9d996da2a28@yahoo.de> Subject: Re: [bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename References: <871s6l9h54.fsf@gnu.org> <87r2ekea1u.fsf@ambrevar.xyz> <04daca2a-705d-6255-85bd-48132879f5a8@yahoo.de> <87lg4se6wf.fsf@ambrevar.xyz> <8d15b1a4-5f0b-28aa-a3d9-78520431b8ef@yahoo.de> <87imzwe2d3.fsf@ambrevar.xyz> <2df7cbf3-81d5-7d50-35d7-f3b369d1ac0e@yahoo.de> <87ftuth0d7.fsf@ambrevar.xyz> <14de0933-fddc-94d0-d75a-cdf4b49fce1a@yahoo.de> <87wonhipbw.fsf@ambrevar.xyz> In-Reply-To: <87wonhipbw.fsf@ambrevar.xyz> --WJRYWDuVSr7TFOG0SoDMzgjwsaVmFMHh6 Content-Type: multipart/mixed; boundary="------------5582292179FECF82B6000E45" Content-Language: en-US This is a multi-part message in MIME format. --------------5582292179FECF82B6000E45 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi Pierre, thank you for reviewing! On 06.01.19 20:00, Pierre Neidhardt wrote: > Hi Tim, >=20 > I just reviewed your patch. Looks pretty good overall, thanks! >=20 > A few things: >=20 >> +(export package-elisp-from-package) >=20 > This should be placed at the beginning of the file in the (define-modul= e... > See bootstrap.scm. As the new function can be defined with a normal lambda and not a lambda* I just used define-public. >> +;;; Returns a package definition that packages an emacs-lisp file fro= m the >> +;;; SRCPKG source. The package has the name PKGNAME and packages the = file >> +;;; SRC-FILE from the source in its root directory as TARGET-FILE or = the >> +;;; basename of SRC-FILE where INPUTS NATIVE-INPUTS and PROPAGATED-IN= PUTS are >> +;;; added as package inputs and SUBSTITUTIONS substitutions will be p= erformed >> +;;; on the elisp file and SYNOPSIS and DESCRIPTION as the package syn= opsis and >> +;;; description. >> +(define* (package-elisp-from-package >=20 > Move the ";;;" comment to a docstring, e.g. >=20 > --8<---------------cut here---------------start------------->8--- > (define* (package-elisp-from-package > ...) > "Return ..." > --8<---------------cut here---------------end--------------->8--- Done. >> +;;; Returns a package definition that packages an emacs-lisp file fro= m the >=20 > "Return", not "Returns". >=20 >> +;;; SRCPKG source. The package has the name PKGNAME and packages the = file >=20 > Separate sentences with two spaces. Done. >> + srcpkg pkgname src-file >=20 > Prefer complete words over abbreviations. Here I'd suggest >=20 > source-package > name > source-file Done. name is called package-name. >> + (synopsis (if synopsis >> + synopsis >> + (package-synopsis srcpkg))) >> + (description (if description >> + description >> + (package-description srcpkg)))))) >=20 > A more Lispy way: >=20 > --8<---------------cut here---------------start------------->8--- > + (synopsis (or synopsis > + (package-synopsis srcpkg))) > + (description (or description > + (package-description srcpkg)))))) > --8<---------------cut here---------------end--------------->8--- Obsolete as this is now moved again to the final package definition. Thanks for the tip :) I'm still quite new to scheme. > Regarding the function parameters, I would turn SOURCE-FILE into SOURCE= -FILES to > make it more generic. Indeed, the Emacs library could very well be spl= it over > multiple files. >=20 > One thing I'm not too sure about is the replication of the structure fi= elds as > keys. > I think it's easier to ignore those and let the user define them as fol= lows: >=20 > --8<---------------cut here---------------start------------->8--- > (define-public emacs-clang-rename > (package > (inherit (package-elisp-from-package > clang > "emacs-clang-rename" > "tools/clang-rename/clang-rename.el")) > (arguments ...))) > --8<---------------cut here---------------end--------------->8--- I was also thinking about this. But with stuffing everything into the function to evaluate to the final definition made multiple files difficult as it would complicate the data structure for substitutions. As this is not part of the function this is not a problem anymore. Maybe we could make the function even more generic if we would just let it modify the origin object. > Makes sense? This would also be more robust in case the package struct= ure > changes someday. >=20 > Finally, rebase your changes so that you directly use the last function= , no > need for the clang-specific function to appear in the history of commit= s. Done. Patches are attached. Tim. --------------5582292179FECF82B6000E45 Content-Type: text/x-patch; name="0001-gnu-Add-package-elisp-from-package.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="0001-gnu-Add-package-elisp-from-package.patch" =46rom 738d70333c5f09d4f5914e9e524999e800f9f37a Mon Sep 17 00:00:00 2001 From: Tim Gesthuizen Date: Fri, 4 Jan 2019 22:34:36 +0100 Subject: [PATCH 1/2] gnu: Add package-elisp-from-package Add a function to generate package definitions that packages single elisp= files from other packages. * gnu/packages/emacs.scm (package-elisp-from-package): New function --- gnu/packages/emacs.scm | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm index df0d6144b..a70c9dd9c 100644 --- a/gnu/packages/emacs.scm +++ b/gnu/packages/emacs.scm @@ -35,7 +35,7 @@ ;;; Copyright =C2=A9 2018 Sohom Bhattacharjee ;;; Copyright =C2=A9 2018 Mathieu Lirzin ;;; Copyright =C2=A9 2018 Pierre Neidhardt -;;; Copyright =C2=A9 2018 Tim Gesthuizen +;;; Copyright =C2=A9 2018, 2019 Tim Gesthuizen = ;;; Copyright =C2=A9 2018 Jack Hill ;;; Copyright =C2=A9 2018 Pierre-Antoine Rouby ;;; Copyright =C2=A9 2018 Alex Branham @@ -336,6 +336,36 @@ editor (without an X toolkit)" ) (lambda _ (invoke "mkdir" "-p" "src/deps"))))))))) =20 +(define-public (package-elisp-from-package + source-package package-name source-files) + "Returns a package definition that packages emacs-lisp files from the +SOURCE-PACKAGEs source. The package has the name PACKAGE-NAME and packa= ges +the files SOURCE-FILES from the source in its root directory." + (let ((orig (package-source source-package))) + (package + (inherit source-package) + (name package-name) + (build-system emacs-build-system) + (source (origin + (method (origin-method orig)) + (uri (origin-uri orig)) + (sha256 (origin-sha256 orig)) + (file-name (string-append package-name "-" + (package-version source-packag= e))) + (modules '((guix build utils) + (srfi srfi-1) + (ice-9 ftw))) + (snippet + `(let* ((source-files (quote ,source-files)) + (basenames (map basename source-files))) + (map copy-file + source-files basenames) + (map delete-file-recursively + (fold delete + (scandir ".") + (append '("." "..") basenames))) + #t))))))) + =0C ;;; ;;; Emacs hacking. --=20 2.20.1 --------------5582292179FECF82B6000E45 Content-Type: text/x-patch; name="0002-gnu-Use-package-elisp-from-package-for-clangs-emacs-.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename*0="0002-gnu-Use-package-elisp-from-package-for-clangs-emacs-.pa"; filename*1="tch" =46rom 012ba2f96b16d54e0e1c23ee912c8a219355216c Mon Sep 17 00:00:00 2001 From: Tim Gesthuizen Date: Fri, 4 Jan 2019 22:34:56 +0100 Subject: [PATCH 2/2] gnu: Use package-elisp-from-package for clangs emacs= lisp files Use package-elisp-from-package for emacs-clang-format and emacs-clang-ren= ame. Also remove package-from-clang-elisp-file as it is not needed anymore. * gnu/packages/llvm.scm (emacs-clang-format): Use package-elisp-from-pack= age * gnu/packages/llvm.scm (emacs-clang-rename): Use package-elisp-from-pack= age * gnu/packages/llvm.scm (package-from-clang-elisp-file): Remove function --- gnu/packages/llvm.scm | 65 +++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 33 deletions(-) diff --git a/gnu/packages/llvm.scm b/gnu/packages/llvm.scm index 6dab9c519..32d7ef81b 100644 --- a/gnu/packages/llvm.scm +++ b/gnu/packages/llvm.scm @@ -8,7 +8,7 @@ ;;; Copyright =C2=A9 2018 Marius Bakke ;;; Copyright =C2=A9 2018 Tobias Geerinckx-Rice ;;; Copyright =C2=A9 2018 Efraim Flashner -;;; Copyright =C2=A9 2018 Tim Gesthuizen +;;; Copyright =C2=A9 2018, 2019 Tim Gesthuizen = ;;; Copyright =C2=A9 2018 Pierre Neidhardt ;;; ;;; This file is part of GNU Guix. @@ -39,6 +39,7 @@ #:use-module (gnu packages gcc) #:use-module (gnu packages bootstrap) ;glibc-dynamic-linker #:use-module (gnu packages compression) + #:use-module (gnu packages emacs) #:use-module (gnu packages libffi) #:use-module (gnu packages perl) #:use-module (gnu packages python) @@ -481,22 +482,21 @@ code analysis tools.") =20 (define-public emacs-clang-format (package - (inherit clang) - (name "emacs-clang-format") - (build-system emacs-build-system) - (inputs - `(("clang" ,clang))) - (arguments - `(#:phases - (modify-phases %standard-phases - (add-after 'unpack 'configure - (lambda* (#:key inputs #:allow-other-keys) - (let ((clang (assoc-ref inputs "clang"))) - (copy-file "tools/clang-format/clang-format.el" "clang-fo= rmat.el") - (emacs-substitute-variables "clang-format.el" - ("clang-format-executable" - (string-append clang "/bin/clang-format")))) - #t))))) + (inherit (package-elisp-from-package + clang + "emacs-clang-format" + '("tools/clang-format/clang-format.el"))) + (inputs `(("clang" ,clang))) + (arguments `(#:phases + (modify-phases %standard-phases + (add-after 'unpack 'configure + (lambda* (#:key inputs #:allow-other-k= eys) + (chmod "clang-format.el" #o644) + (emacs-substitute-variables "clang-f= ormat.el" + ("clang-format-executable" + (string-append (assoc-ref inputs = "clang") + "/bin/clang-format= "))) + #t))))) (synopsis "Format code using clang-format") (description "This package allows to filter code through @code{clang= -format} to fix its formatting. @code{clang-format} is a tool that formats @@ -505,22 +505,21 @@ C/C++/Obj-C code according to a set of style option= s, see =20 (define-public emacs-clang-rename (package - (inherit clang) - (name "emacs-clang-rename") - (build-system emacs-build-system) - (inputs - `(("clang" ,clang))) - (arguments - `(#:phases - (modify-phases %standard-phases - (add-after 'unpack 'configure - (lambda* (#:key inputs #:allow-other-keys) - (let ((clang (assoc-ref inputs "clang"))) - (copy-file "tools/clang-rename/clang-rename.el" "clang-re= name.el") - (emacs-substitute-variables "clang-rename.el" - ("clang-rename-binary" - (string-append clang "/bin/clang-rename")))) - #t))))) + (inherit (package-elisp-from-package + clang + "emacs-clang-rename" + '("tools/clang-rename/clang-rename.el"))) + (inputs `(("clang" ,clang))) + (arguments `(#:phases + (modify-phases %standard-phases + (add-after 'unpack 'configure + (lambda* (#:key inputs #:allow-other-keys) + (chmod "clang-rename.el" #o644) + (emacs-substitute-variables "clang-rename.el" + ("clang-rename-binary" + (string-append (assoc-ref inputs "clang") + "/bin/clang-rename"))) + #t))))) (synopsis "Rename every occurrence of a symbol using clang-rename") (description "This package renames every occurrence of a symbol at p= oint using @code{clang-rename}."))) --=20 2.20.1 --------------5582292179FECF82B6000E45-- --WJRYWDuVSr7TFOG0SoDMzgjwsaVmFMHh6-- --nkg7ECbHBShueInR8UYTzCE2v3cpQaMIH Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAEBCgAdFiEEKUiC5+8BRKEri5fa0uWPaa77GdUFAlwyczcACgkQ0uWPaa77 GdVrGQf/YcdROgHmaXWesMbXnXj8AQfnXK1F6M43P092xtD2n9cAFkM294t/yLS4 abNoOODlbigsJ7cUxtYInEg8+vRD3HoFMAebCSAgE1uFM3am5UC53QeEy0o199fh 6zvq4MxZNAWAmAn12NVHpEOp6Yq0haoChkt4kW+bAfhhvir58Qg57gELjyaaZGZK Ev4EFJ0qtRHJ71QAPxxFqnVZol+8eLKp3fPaNIMiYcbVN2O+0zJ8YZnFkm69gZKx Dsa0HIiVkUxesPzF6wvBhnZpOw8WR4+2f+wQ6oV6ptrElGighGsk7ff2WBCSKNrv wWfV7t1S382ZmHpbfZiiS22FlNq1wA== =JByk -----END PGP SIGNATURE----- --nkg7ECbHBShueInR8UYTzCE2v3cpQaMIH--