[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Tim Gesthuizen]

[-- Attachment #1.1.1: Type: text/plain, Size: 466 bytes --]

Hi,
the attached patches add optimizations to emacs-clang-format and
emacs-clang-rename:

- Only package the required elisp file in the source
- Add a function for generating package definitions of packages
  containing a single elisp file from the clang source.

The patches make clear that the elisp file is the only thing required
for building the package and should save some disk space as the packages
do not need the full clang source tree.

Tim.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: 0001-gnu-Shrink-source-for-emacs-clang-format.patch --]
[-- Type: text/x-patch; name="0001-gnu-Shrink-source-for-emacs-clang-format.patch", Size: 3771 bytes --]

From 5d85956a30ff1bed8f7c529d4b760bb609ac4ab7 Mon Sep 17 00:00:00 2001
From: Tim Gesthuizen <tim.gesthuizen@yahoo.de>
Date: Fri, 30 Nov 2018 14:49:51 +0100
Subject: [PATCH 1/3] gnu: Shrink source for emacs-clang-format.

To build emacs-clang-format only the `clang-format.el` file is needed. In
order to save disk space and make clear that this is the only file needed
modify the downloaded source to only contain `clang-format.el`.

* gnu/packages/emacs.scm (emacs-clang-format): Add source snippet.
* gnu/packages/emacs.scm (emacs-clang-format): Modify phases.
---
 gnu/packages/llvm.scm | 58 ++++++++++++++++++++++++++++---------------
 1 file changed, 38 insertions(+), 20 deletions(-)

diff --git a/gnu/packages/llvm.scm b/gnu/packages/llvm.scm
index dace546a4..e7dd6477a 100644
--- a/gnu/packages/llvm.scm
+++ b/gnu/packages/llvm.scm
@@ -461,28 +461,46 @@ code analysis tools.")
     (license license:bsd-3)))
 
 (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-format.el")
-               (emacs-substitute-variables "clang-format.el"
-                 ("clang-format-executable"
-                  (string-append clang "/bin/clang-format"))))
-             #t)))))
-    (synopsis "Format code using clang-format")
-    (description "This package allows to filter code through @code{clang-format}
+  (let ((target-file "clang-format.el"))
+    (package
+      (inherit clang)
+      (name "emacs-clang-format")
+      (source (let ((orig (package-source clang)))
+                (origin
+                  (method (origin-method orig))
+                  (uri (origin-uri orig))
+                  (sha256 (origin-sha256 orig))
+                  (modules '((guix build utils)
+                             (srfi srfi-1)
+                             (ice-9 ftw)))
+                  (snippet
+                   `(begin
+                      ;; Copy target file to source root and delete all other files
+                      (copy-file (string-append "tools/clang-format/" ,target-file)
+                                 ,target-file)
+                      (map delete-file-recursively
+                           (fold delete
+                                 (scandir ".")
+                                 '("." ".." ,target-file)))
+                      #t)))))
+      (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")))
+                 (emacs-substitute-variables ,target-file
+                   ("clang-format-executable"
+                    (string-append 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
 C/C++/Obj-C code according to a set of style options, see
-@url{http://clang.llvm.org/docs/ClangFormatStyleOptions.html}.")))
+@url{http://clang.llvm.org/docs/ClangFormatStyleOptions.html}."))))
 
 (define-public emacs-clang-rename
   (package
-- 
2.19.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.3: 0002-gnu-Shrink-source-for-emacs-clang-rename.patch --]
[-- Type: text/x-patch; name="0002-gnu-Shrink-source-for-emacs-clang-rename.patch", Size: 3623 bytes --]

From 29a1992253908ea191ef0227462972f78ce20da3 Mon Sep 17 00:00:00 2001
From: Tim Gesthuizen <tim.gesthuizen@yahoo.de>
Date: Fri, 30 Nov 2018 15:01:10 +0100
Subject: [PATCH 2/3] gnu: Shrink source for emacs-clang-rename.

To build emacs-clang-rename only the `clang-rename.el` file is needed. In
order to save disk space and make clear that this is the only file needed
modify the downloaded source to only contain `clang-rename.el`.

* gnu/packages/emacs.scm (emacs-clang-rename): Add source snippet.
* gnu/packages/emacs.scm (emacs-clang-rename): Modify phases.
---
 gnu/packages/llvm.scm | 59 ++++++++++++++++++++++++++++---------------
 1 file changed, 39 insertions(+), 20 deletions(-)

diff --git a/gnu/packages/llvm.scm b/gnu/packages/llvm.scm
index e7dd6477a..3eb0fb29f 100644
--- a/gnu/packages/llvm.scm
+++ b/gnu/packages/llvm.scm
@@ -503,23 +503,42 @@ C/C++/Obj-C code according to a set of style options, see
 @url{http://clang.llvm.org/docs/ClangFormatStyleOptions.html}."))))
 
 (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-rename.el")
-               (emacs-substitute-variables "clang-rename.el"
-                 ("clang-rename-binary"
-                  (string-append 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 point
-using @code{clang-rename}.")))
+  (let ((target-file "clang-rename.el"))
+   (package
+     (inherit clang)
+     (name "emacs-clang-rename")
+     (source (let ((orig (package-source clang)))
+               (origin
+                 (method (origin-method orig))
+                 (uri (origin-uri orig))
+                 (sha256 (origin-sha256 orig))
+                 (file-name (string-append name
+                                           (package-version clang)))
+                 (modules '((guix build utils)
+                            (srfi srfi-1)
+                            (ice-9 ftw)))
+                 (snippet
+                  `(begin
+                     (copy-file (string-append "tools/clang-rename/" ,target-file)
+                                ,target-file)
+                     (map delete-file-recursively
+                          (fold delete
+                                (scandir ".")
+                                '("." ".." ,target-file)))
+                     #t)))))
+     (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")))
+                (emacs-substitute-variables ,target-file
+                  ("clang-rename-binary"
+                   (string-append 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 point
+using @code{clang-rename}."))))
-- 
2.19.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.4: 0003-gnu-Add-package-from-clang-elisp-file.patch --]
[-- Type: text/x-patch; name="0003-gnu-Add-package-from-clang-elisp-file.patch", Size: 7486 bytes --]

From e4fdd028c0e71118073aee34c5976a4948b46511 Mon Sep 17 00:00:00 2001
From: Tim Gesthuizen <tim.gesthuizen@yahoo.de>
Date: Fri, 30 Nov 2018 15:13:51 +0100
Subject: [PATCH 3/3] gnu: Add package-from-clang-elisp-file

emacs-clang-format and emacs-clang-rename both are packages that are build by
extracting a single elisp file and building it as an emacs package.
This concept is encapsulated in the package-from-clang-elisp-file
function. This reduces repeating of concepts in the two package definitions.

* gnu/packages/llvm.scm (package-from-clang-elisp-file): New function
* gnu/packages/llvm.scm (emacs-clang-format): Use new function
* gnu/packages/llvm.scm (emacs-clang-rename): Use new function
---
 gnu/packages/llvm.scm | 138 +++++++++++++++++++-----------------------
 1 file changed, 61 insertions(+), 77 deletions(-)

diff --git a/gnu/packages/llvm.scm b/gnu/packages/llvm.scm
index 3eb0fb29f..ad71c967a 100644
--- a/gnu/packages/llvm.scm
+++ b/gnu/packages/llvm.scm
@@ -460,85 +460,69 @@ code analysis tools.")
      "This package provides a Python binding to LLVM for use in Numba.")
     (license license:bsd-3)))
 
+;;; Returns a package definition that packages an emacs-lisp file from the
+;;; clang source. The package has the name PKGNAME and packages the file
+;;; SRC-FILE from the clang source in its root directory with the name
+;;; TARGET-FILE where SUBST substitutions will be performed on the elisp file
+;;; and SYN and DESC as the package synopsis an description.
+(define (package-from-clang-elisp-file pkgname src-file target-file subst syn desc)
+  (package
+    (inherit clang)
+    (name pkgname)
+    (source (let ((orig (package-source clang)))
+              (origin
+                (method (origin-method orig))
+                (uri (origin-uri orig))
+                (sha256 (origin-sha256 orig))
+                (file-name (string-append pkgname "-" (package-version clang)))
+                (modules '((guix build utils)
+                           (srfi srfi-1)
+                           (ice-9 ftw)))
+                (snippet
+                 `(begin
+                    ;; Copy target file to source root and delete all other files
+                    (copy-file (string-append ,src-file)
+                               ,target-file)
+                    (map delete-file-recursively
+                         (fold delete
+                               (scandir ".")
+                               '("." ".." ,target-file)))
+                    #t)))))
+    (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")))
+               (emacs-substitute-variables ,target-file
+                 ,subst))
+             #t)))))
+    (synopsis syn)
+    (description desc)))
+
 (define-public emacs-clang-format
-  (let ((target-file "clang-format.el"))
-    (package
-      (inherit clang)
-      (name "emacs-clang-format")
-      (source (let ((orig (package-source clang)))
-                (origin
-                  (method (origin-method orig))
-                  (uri (origin-uri orig))
-                  (sha256 (origin-sha256 orig))
-                  (modules '((guix build utils)
-                             (srfi srfi-1)
-                             (ice-9 ftw)))
-                  (snippet
-                   `(begin
-                      ;; Copy target file to source root and delete all other files
-                      (copy-file (string-append "tools/clang-format/" ,target-file)
-                                 ,target-file)
-                      (map delete-file-recursively
-                           (fold delete
-                                 (scandir ".")
-                                 '("." ".." ,target-file)))
-                      #t)))))
-      (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")))
-                 (emacs-substitute-variables ,target-file
-                   ("clang-format-executable"
-                    (string-append clang "/bin/clang-format"))))
-               #t)))))
-      (synopsis "Format code using clang-format")
-      (description "This package allows to filter code through @code{clang-format}
+  (package-from-clang-elisp-file
+   "emacs-clang-format"
+   "tools/clang-format/clang-format.el"
+   "clang-format.el"
+   '("clang-format-executable"
+                    (string-append clang "/bin/clang-format"))
+   "Format code using clang-format"
+   "This package allows to filter code through @code{clang-format}
 to fix its formatting.  @code{clang-format} is a tool that formats
 C/C++/Obj-C code according to a set of style options, see
-@url{http://clang.llvm.org/docs/ClangFormatStyleOptions.html}."))))
+@url{http://clang.llvm.org/docs/ClangFormatStyleOptions.html}."))
 
 (define-public emacs-clang-rename
-  (let ((target-file "clang-rename.el"))
-   (package
-     (inherit clang)
-     (name "emacs-clang-rename")
-     (source (let ((orig (package-source clang)))
-               (origin
-                 (method (origin-method orig))
-                 (uri (origin-uri orig))
-                 (sha256 (origin-sha256 orig))
-                 (file-name (string-append name
-                                           (package-version clang)))
-                 (modules '((guix build utils)
-                            (srfi srfi-1)
-                            (ice-9 ftw)))
-                 (snippet
-                  `(begin
-                     (copy-file (string-append "tools/clang-rename/" ,target-file)
-                                ,target-file)
-                     (map delete-file-recursively
-                          (fold delete
-                                (scandir ".")
-                                '("." ".." ,target-file)))
-                     #t)))))
-     (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")))
-                (emacs-substitute-variables ,target-file
-                  ("clang-rename-binary"
-                   (string-append 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 point
-using @code{clang-rename}."))))
+  (package-from-clang-elisp-file
+   "emacs-clang-rename"
+   "tools/clang-rename/clang-rename.el"
+   "clang-rename.el"
+   '("clang-rename-binary"
+                   (string-append clang "/bin/clang-rename"))
+   "Rename every occurrence of a symbol using clang-rename"
+   "This package renames every occurrence of a symbol at point
+using @code{clang-rename}."))
-- 
2.19.2


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Ludovic Courtès]

Hello Tim,

Sorry for the delay, it looks like these patches fell through the
cracks!

Tim Gesthuizen <tim.gesthuizen@yahoo.de> skribis:

> the attached patches add optimizations to emacs-clang-format and
> emacs-clang-rename:
>
> - Only package the required elisp file in the source
> - Add a function for generating package definitions of packages
>   containing a single elisp file from the clang source.
>
> The patches make clear that the elisp file is the only thing required
> for building the package and should save some disk space as the packages
> do not need the full clang source tree.

Nice!  Pierre, could you take a look and apply them if they look good to you?

Thanks!

Ludo’.

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 1065 bytes --]

Hi Tim!

Thanks for working on this.

I'm just a little confused about the intention, so I have a few questions.
The current package re-uses Clang source and packages clang-format.el /
clang-rename.el only.

In your implementation, you've essentially moved the 'configure phase to a
snippet.  As I understand it, this does not change anything.  What is your
reason for doing this?

You are also deleting all non-required files.  Which saves some space in /tmp
indeed, but this won't change anything to the store or the resulting
substitutes, right?  I don't mind this change, I just want to be clear about
what we are trying to achieve here :)

Last, the factorization into package-from-clang-elisp-file: this is a good idea,
and I would make it even more generic.  Simply rename your function

	package-elisp-from-package

(or something like that) and make "clang" a parameter.  Then we can use this
function in other packages like emacs-agda2-mode.

Am I making sense?  What do you think?

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Tim Gesthuizen]

[-- Attachment #1.1: Type: text/plain, Size: 2598 bytes --]

Hi,

On 14.12.2018 10:23, Pierre Neidhardt wrote:
> In your implementation, you've essentially moved the 'configure phase to a
> snippet.  As I understand it, this does not change anything.  What is your
> reason for doing this?

1. Intention: Stripping the source with the snippet models more clearly
   that the building part is a function that maps the single elisp file
   to the final package.

> You are also deleting all non-required files.  Which saves some space in /tmp
> indeed, but this won't change anything to the store or the resulting
> substitutes, right?  I don't mind this change, I just want to be clear about
> what we are trying to achieve here :)

2. Efficiency: I am not quite sure about this - maybe I am just wrong:
   As far as I understand it at first guix builds a source derivation
   and then the actual package derivation. Even when the source
   derivation is not stored in the store it can be substituted. If this
   is the case we can save some bandwith / disk space / build time by
   moving the file stripping to the source snippet. On my machine the
   most time during the build process is spend unpacking the clang
   source.

> Last, the factorization into package-from-clang-elisp-file: this is a good idea,
> and I would make it even more generic.  Simply rename your function
> 
> 	package-elisp-from-package
> 
> (or something like that) and make "clang" a parameter.  Then we can use this
> function in other packages like emacs-agda2-mode.

I also thought about this but could not find another situation where
this was applicable.
In which module should such a function go?
What would definitely be nice is that such a function can encapsulate
the emacs stuff so that we do not need any other emacs related modules
imported in (gnu packages llvm) for example.

> Am I making sense?  What do you think?

Yes. Maybe we should add some reasoning to the commit message then?
Depends on whether we just want a description of the changes in a commit
message or also some reasoning if things might be unclear.
I think that package-elisp-from-package is a really good idea.
Without seeing the function definition it is really hard to figure out
why it needs the arguments it needs and what their purposes are.
Maybe we want to make the arguments keyword arguments if it becomes more
generic so its usage is more intuitive.
The first two patches are debatable though. If there is a particular
reason against them (unnecessary changing without benefits included) we
might want to not merge them.

Tim.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 2326 bytes --]


> 2. Efficiency: I am not quite sure about this - maybe I am just wrong:
>    As far as I understand it at first guix builds a source derivation
>    and then the actual package derivation. Even when the source
>    derivation is not stored in the store it can be substituted. If this
>    is the case we can save some bandwith / disk space / build time by
>    moving the file stripping to the source snippet. On my machine the
>    most time during the build process is spend unpacking the clang
>    source.

I am not sure sure about this.  Ludovic, do we have such a thing as "source
substitutes"?

> I also thought about this but could not find another situation where
> this was applicable.

Look for "emacs-build-system" in files other than emacs.scm.  It's used in quite
a few places.

> In which module should such a function go?

Hmmm, I guess either guix/build/emacs-utils.scm, or gnu/packages/emacs.scm.

> What would definitely be nice is that such a function can encapsulate
> the emacs stuff so that we do not need any other emacs related modules
> imported in (gnu packages llvm) for example.

What emacs stuff?  You mean the build system?

> Yes. Maybe we should add some reasoning to the commit message then?
> Depends on whether we just want a description of the changes in a commit
> message or also some reasoning if things might be unclear.

Well, the reasoning above is mostly a nit.  What matters most is
- Efficiency, if it really works.
- The abstraction function.

> Without seeing the function definition it is really hard to figure out
> why it needs the arguments it needs and what their purposes are.
> Maybe we want to make the arguments keyword arguments if it becomes more
> generic so its usage is more intuitive.

Sure, that's the very purpose of keyword arguments, make code readable.  If it
reads like an English sentence, it's a win!

> The first two patches are debatable though. If there is a particular
> reason against them (unnecessary changing without benefits included) we
> might want to not merge them.

Even if the efficiency point is moot, the abstraction is more than welcome: try
it on at least one more package, then it will save lots of package code.  I'm
all for it! :)

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Tim Gesthuizen]

[-- Attachment #1.1: Type: text/plain, Size: 1468 bytes --]

On 14.12.2018 11:31, Pierre Neidhardt wrote:
> I am not sure sure about this.  Ludovic, do we have such a thing as "source
> substitutes"?

I have searched a little bit in my store... Looks like its not the case.
After all it seems to me that this is the purpose of a source snippet.

>> I also thought about this but could not find another situation where
>> this was applicable.
> 
> Look for "emacs-build-system" in files other than emacs.scm.  It's used in quite
> a few places.

I will have a look at it.

> What emacs stuff?  You mean the build system?

Yes. With the abstraction we could only import the function and do not
need the emacs-build-system imported in modules that have nothing to do
with emacs otherwise.

>> Yes. Maybe we should add some reasoning to the commit message then?
>> Depends on whether we just want a description of the changes in a commit
>> message or also some reasoning if things might be unclear.
> 
> Well, the reasoning above is mostly a nit.  What matters most is
> - Efficiency, if it really works.
> - The abstraction function.

Then I will apply the changes to the function and send new patches when
I am done. Unless Ludo thinks differently we probably shouldn't merge

- The first two patches if they don't work like I expected them to
- The last patch in its current form until the changes are implemented
  and we can start to use the generic function in package definitions

Tim.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 662 bytes --]


> I have searched a little bit in my store... Looks like its not the case.
> After all it seems to me that this is the purpose of a source snippet.

My understanding is that snippets are mostly meant as a Lispy way to replace
patches.

But hey, it's all Scheme code in the end, should it be executed before or after
does not make much of a difference.

> Yes. With the abstraction we could only import the function and do not
> need the emacs-build-system imported in modules that have nothing to do
> with emacs otherwise.

But you'll have to import the module where the abstraction is defined ;)

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Tim Gesthuizen]

[-- Attachment #1.1: Type: text/plain, Size: 210 bytes --]

On 14.12.2018 13:09, Pierre Neidhardt wrote:
> But you'll have to import the module where the abstraction is defined ;)

But you could only #:select the abstraction and don't import the rest ;)

Tim.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Tim Gesthuizen]

[-- Attachment #1.1: Type: text/plain, Size: 2124 bytes --]

Hi,
I was wondering why I sended patches that do not improve
anything. After some more investigation I found out again what the
patches improve:

The source is repacked by guix and the patches applied before the source
is stored in the store.  With the current guix:

┌────
│ ~/src/guix/build$ tar -tf `guix build -S emacs-clang-format`
│ cfe-6.0.1.src/
│ cfe-6.0.1.src/.arcconfig
│ cfe-6.0.1.src/.clang-format
│ cfe-6.0.1.src/.clang-tidy
│ cfe-6.0.1.src/.gitignore
│ cfe-6.0.1.src/CMakeLists.txt
│ cfe-6.0.1.src/CODE_OWNERS.TXT
│ cfe-6.0.1.src/INPUTS/
│ cfe-6.0.1.src/INPUTS/Cocoa_h.m
│ cfe-6.0.1.src/INPUTS/all-std-headers.cpp
│ cfe-6.0.1.src/INPUTS/c99-intconst-1.c
│ cfe-6.0.1.src/INPUTS/carbon_h.c
│ cfe-6.0.1.src/INPUTS/cfg-big-switch.c
│ cfe-6.0.1.src/INPUTS/cfg-long-chain1.c
│ cfe-6.0.1.src/INPUTS/cfg-long-chain2.c
│ cfe-6.0.1.src/INPUTS/cfg-long-chain3.c
│ cfe-6.0.1.src/INPUTS/cfg-nested-switches.c
│ cfe-6.0.1.src/INPUTS/cfg-nested-var-scopes.cpp
│ cfe-6.0.1.src/INPUTS/iostream.cc
│ cfe-6.0.1.src/INPUTS/macro_pounder_fn.c
│ cfe-6.0.1.src/INPUTS/macro_pounder_obj.c
│ cfe-6.0.1.src/INPUTS/stpcpy-test.c
│ cfe-6.0.1.src/INSTALL.txt
│ cfe-6.0.1.src/LICENSE.TXT
│ cfe-6.0.1.src/ModuleInfo.txt
│ cfe-6.0.1.src/NOTES.txt
│ ... < the rest of clangs sources >
└────

With the patches applied:

┌────
│ ~/src/guix/build$ tar -tf `guix build -S emacs-clang-format`
│ ... < repacking and building the source>
│ successfully built
/gnu/store/yds0n90idgaa65ladnpbcdjkqgs8l61d-emacs-clang-format-6.0.1.tar.xz.drv
│ cfe-6.0.1.src/
│ cfe-6.0.1.src/clang-format.el
└────

So it does save disk space. I also think that these sources can be
substituted (This may still not be the case though).  The term source
derivation is mentioned in the description of guix builds `-S` option.

I would argue that I implement the file deletion in the source snippet
in package-elisp-from-package. I hope this makes clear what the purpose
of the first two patches is.

Tim.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 201 bytes --]

Hi Tim,

Yeah, that makes sense.  Please send a new patch updated with the aforementioned
recommendations and I'll merge.
Thanks for the good work!

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Tim Gesthuizen]

[-- Attachment #1.1.1: Type: text/plain, Size: 730 bytes --]

On 19.12.2018 18:50, Pierre Neidhardt wrote:
> Hi Tim,
> 
> Yeah, that makes sense.  Please send a new patch updated with the aforementioned
> recommendations and I'll merge.
> Thanks for the good work!
> 

Hi,
I had some problems packaging the changes because I had some really
weird issues with the guile modules (please validate that the patches
compile as expected).
They implement the discussed function for generating package definitions
for packages that contain only a single elisp file from another packages
source.
If the attached patches compile, they can be merged and work as
expected. If they cause weird errors to happen Pierre tries to find some
time to investigate into this.

Tim Gesthuizen.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: 0001-gnu-Shrink-source-for-emacs-clang-format.patch --]
[-- Type: text/x-patch; name="0001-gnu-Shrink-source-for-emacs-clang-format.patch", Size: 3771 bytes --]

From beec284f12caec18995be40c7a1cb9e254f83ec7 Mon Sep 17 00:00:00 2001
From: Tim Gesthuizen <tim.gesthuizen@yahoo.de>
Date: Fri, 30 Nov 2018 14:49:51 +0100
Subject: [PATCH 1/5] gnu: Shrink source for emacs-clang-format.

To build emacs-clang-format only the `clang-format.el` file is needed. In
order to save disk space and make clear that this is the only file needed
modify the downloaded source to only contain `clang-format.el`.

* gnu/packages/emacs.scm (emacs-clang-format): Add source snippet.
* gnu/packages/emacs.scm (emacs-clang-format): Modify phases.
---
 gnu/packages/llvm.scm | 58 ++++++++++++++++++++++++++++---------------
 1 file changed, 38 insertions(+), 20 deletions(-)

diff --git a/gnu/packages/llvm.scm b/gnu/packages/llvm.scm
index 7eb785c36..cf7c44600 100644
--- a/gnu/packages/llvm.scm
+++ b/gnu/packages/llvm.scm
@@ -480,28 +480,46 @@ code analysis tools.")
     (license license:bsd-3)))
 
 (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-format.el")
-               (emacs-substitute-variables "clang-format.el"
-                 ("clang-format-executable"
-                  (string-append clang "/bin/clang-format"))))
-             #t)))))
-    (synopsis "Format code using clang-format")
-    (description "This package allows to filter code through @code{clang-format}
+  (let ((target-file "clang-format.el"))
+    (package
+      (inherit clang)
+      (name "emacs-clang-format")
+      (source (let ((orig (package-source clang)))
+                (origin
+                  (method (origin-method orig))
+                  (uri (origin-uri orig))
+                  (sha256 (origin-sha256 orig))
+                  (modules '((guix build utils)
+                             (srfi srfi-1)
+                             (ice-9 ftw)))
+                  (snippet
+                   `(begin
+                      ;; Copy target file to source root and delete all other files
+                      (copy-file (string-append "tools/clang-format/" ,target-file)
+                                 ,target-file)
+                      (map delete-file-recursively
+                           (fold delete
+                                 (scandir ".")
+                                 '("." ".." ,target-file)))
+                      #t)))))
+      (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")))
+                 (emacs-substitute-variables ,target-file
+                   ("clang-format-executable"
+                    (string-append 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
 C/C++/Obj-C code according to a set of style options, see
-@url{http://clang.llvm.org/docs/ClangFormatStyleOptions.html}.")))
+@url{http://clang.llvm.org/docs/ClangFormatStyleOptions.html}."))))
 
 (define-public emacs-clang-rename
   (package
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.3: 0002-gnu-Shrink-source-for-emacs-clang-rename.patch --]
[-- Type: text/x-patch; name="0002-gnu-Shrink-source-for-emacs-clang-rename.patch", Size: 3623 bytes --]

From 93a8266610510e67103d06a852b97feba3d3c4e9 Mon Sep 17 00:00:00 2001
From: Tim Gesthuizen <tim.gesthuizen@yahoo.de>
Date: Fri, 30 Nov 2018 15:01:10 +0100
Subject: [PATCH 2/5] gnu: Shrink source for emacs-clang-rename.

To build emacs-clang-rename only the `clang-rename.el` file is needed. In
order to save disk space and make clear that this is the only file needed
modify the downloaded source to only contain `clang-rename.el`.

* gnu/packages/emacs.scm (emacs-clang-rename): Add source snippet.
* gnu/packages/emacs.scm (emacs-clang-rename): Modify phases.
---
 gnu/packages/llvm.scm | 59 ++++++++++++++++++++++++++++---------------
 1 file changed, 39 insertions(+), 20 deletions(-)

diff --git a/gnu/packages/llvm.scm b/gnu/packages/llvm.scm
index cf7c44600..530d6599b 100644
--- a/gnu/packages/llvm.scm
+++ b/gnu/packages/llvm.scm
@@ -522,23 +522,42 @@ C/C++/Obj-C code according to a set of style options, see
 @url{http://clang.llvm.org/docs/ClangFormatStyleOptions.html}."))))
 
 (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-rename.el")
-               (emacs-substitute-variables "clang-rename.el"
-                 ("clang-rename-binary"
-                  (string-append 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 point
-using @code{clang-rename}.")))
+  (let ((target-file "clang-rename.el"))
+   (package
+     (inherit clang)
+     (name "emacs-clang-rename")
+     (source (let ((orig (package-source clang)))
+               (origin
+                 (method (origin-method orig))
+                 (uri (origin-uri orig))
+                 (sha256 (origin-sha256 orig))
+                 (file-name (string-append name
+                                           (package-version clang)))
+                 (modules '((guix build utils)
+                            (srfi srfi-1)
+                            (ice-9 ftw)))
+                 (snippet
+                  `(begin
+                     (copy-file (string-append "tools/clang-rename/" ,target-file)
+                                ,target-file)
+                     (map delete-file-recursively
+                          (fold delete
+                                (scandir ".")
+                                '("." ".." ,target-file)))
+                     #t)))))
+     (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")))
+                (emacs-substitute-variables ,target-file
+                  ("clang-rename-binary"
+                   (string-append 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 point
+using @code{clang-rename}."))))
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.4: 0003-gnu-Add-package-from-clang-elisp-file.patch --]
[-- Type: text/x-patch; name="0003-gnu-Add-package-from-clang-elisp-file.patch", Size: 7486 bytes --]

From c253da5cbb01eb0218f559559ecf16754bb824da Mon Sep 17 00:00:00 2001
From: Tim Gesthuizen <tim.gesthuizen@yahoo.de>
Date: Fri, 30 Nov 2018 15:13:51 +0100
Subject: [PATCH 3/5] gnu: Add package-from-clang-elisp-file

emacs-clang-format and emacs-clang-rename both are packages that are build by
extracting a single elisp file and building it as an emacs package.
This concept is encapsulated in the package-from-clang-elisp-file
function. This reduces repeating of concepts in the two package definitions.

* gnu/packages/llvm.scm (package-from-clang-elisp-file): New function
* gnu/packages/llvm.scm (emacs-clang-format): Use new function
* gnu/packages/llvm.scm (emacs-clang-rename): Use new function
---
 gnu/packages/llvm.scm | 138 +++++++++++++++++++-----------------------
 1 file changed, 61 insertions(+), 77 deletions(-)

diff --git a/gnu/packages/llvm.scm b/gnu/packages/llvm.scm
index 530d6599b..05147b665 100644
--- a/gnu/packages/llvm.scm
+++ b/gnu/packages/llvm.scm
@@ -479,85 +479,69 @@ code analysis tools.")
      "This package provides a Python binding to LLVM for use in Numba.")
     (license license:bsd-3)))
 
+;;; Returns a package definition that packages an emacs-lisp file from the
+;;; clang source. The package has the name PKGNAME and packages the file
+;;; SRC-FILE from the clang source in its root directory with the name
+;;; TARGET-FILE where SUBST substitutions will be performed on the elisp file
+;;; and SYN and DESC as the package synopsis an description.
+(define (package-from-clang-elisp-file pkgname src-file target-file subst syn desc)
+  (package
+    (inherit clang)
+    (name pkgname)
+    (source (let ((orig (package-source clang)))
+              (origin
+                (method (origin-method orig))
+                (uri (origin-uri orig))
+                (sha256 (origin-sha256 orig))
+                (file-name (string-append pkgname "-" (package-version clang)))
+                (modules '((guix build utils)
+                           (srfi srfi-1)
+                           (ice-9 ftw)))
+                (snippet
+                 `(begin
+                    ;; Copy target file to source root and delete all other files
+                    (copy-file (string-append ,src-file)
+                               ,target-file)
+                    (map delete-file-recursively
+                         (fold delete
+                               (scandir ".")
+                               '("." ".." ,target-file)))
+                    #t)))))
+    (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")))
+               (emacs-substitute-variables ,target-file
+                 ,subst))
+             #t)))))
+    (synopsis syn)
+    (description desc)))
+
 (define-public emacs-clang-format
-  (let ((target-file "clang-format.el"))
-    (package
-      (inherit clang)
-      (name "emacs-clang-format")
-      (source (let ((orig (package-source clang)))
-                (origin
-                  (method (origin-method orig))
-                  (uri (origin-uri orig))
-                  (sha256 (origin-sha256 orig))
-                  (modules '((guix build utils)
-                             (srfi srfi-1)
-                             (ice-9 ftw)))
-                  (snippet
-                   `(begin
-                      ;; Copy target file to source root and delete all other files
-                      (copy-file (string-append "tools/clang-format/" ,target-file)
-                                 ,target-file)
-                      (map delete-file-recursively
-                           (fold delete
-                                 (scandir ".")
-                                 '("." ".." ,target-file)))
-                      #t)))))
-      (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")))
-                 (emacs-substitute-variables ,target-file
-                   ("clang-format-executable"
-                    (string-append clang "/bin/clang-format"))))
-               #t)))))
-      (synopsis "Format code using clang-format")
-      (description "This package allows to filter code through @code{clang-format}
+  (package-from-clang-elisp-file
+   "emacs-clang-format"
+   "tools/clang-format/clang-format.el"
+   "clang-format.el"
+   '("clang-format-executable"
+                    (string-append clang "/bin/clang-format"))
+   "Format code using clang-format"
+   "This package allows to filter code through @code{clang-format}
 to fix its formatting.  @code{clang-format} is a tool that formats
 C/C++/Obj-C code according to a set of style options, see
-@url{http://clang.llvm.org/docs/ClangFormatStyleOptions.html}."))))
+@url{http://clang.llvm.org/docs/ClangFormatStyleOptions.html}."))
 
 (define-public emacs-clang-rename
-  (let ((target-file "clang-rename.el"))
-   (package
-     (inherit clang)
-     (name "emacs-clang-rename")
-     (source (let ((orig (package-source clang)))
-               (origin
-                 (method (origin-method orig))
-                 (uri (origin-uri orig))
-                 (sha256 (origin-sha256 orig))
-                 (file-name (string-append name
-                                           (package-version clang)))
-                 (modules '((guix build utils)
-                            (srfi srfi-1)
-                            (ice-9 ftw)))
-                 (snippet
-                  `(begin
-                     (copy-file (string-append "tools/clang-rename/" ,target-file)
-                                ,target-file)
-                     (map delete-file-recursively
-                          (fold delete
-                                (scandir ".")
-                                '("." ".." ,target-file)))
-                     #t)))))
-     (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")))
-                (emacs-substitute-variables ,target-file
-                  ("clang-rename-binary"
-                   (string-append 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 point
-using @code{clang-rename}."))))
+  (package-from-clang-elisp-file
+   "emacs-clang-rename"
+   "tools/clang-rename/clang-rename.el"
+   "clang-rename.el"
+   '("clang-rename-binary"
+                   (string-append clang "/bin/clang-rename"))
+   "Rename every occurrence of a symbol using clang-rename"
+   "This package renames every occurrence of a symbol at point
+using @code{clang-rename}."))
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.5: 0004-gnu-Add-package-elisp-from-package.patch --]
[-- Type: text/x-patch; name="0004-gnu-Add-package-elisp-from-package.patch", Size: 4025 bytes --]

From 51a9a6ea6615eef9ce66985bb1ad0c3ddec5c8d8 Mon Sep 17 00:00:00 2001
From: Tim Gesthuizen <tim.gesthuizen@yahoo.de>
Date: Fri, 4 Jan 2019 22:34:36 +0100
Subject: [PATCH 4/5] 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 | 64 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
index 52f4018c2..cdee59ce4 100644
--- a/gnu/packages/emacs.scm
+++ b/gnu/packages/emacs.scm
@@ -35,7 +35,7 @@
 ;;; Copyright © 2018 Sohom Bhattacharjee <soham.bhattacharjee15@gmail.com>
 ;;; Copyright © 2018 Mathieu Lirzin <mthl@gnu.org>
 ;;; Copyright © 2018 Pierre Neidhardt <mail@ambrevar.xyz>
-;;; Copyright © 2018 Tim Gesthuizen <tim.gesthuizen@yahoo.de>
+;;; Copyright © 2018, 2019 Tim Gesthuizen <tim.gesthuizen@yahoo.de>
 ;;; Copyright © 2018 Jack Hill <jackhill@jackhill.us>
 ;;; Copyright © 2018 Pierre-Antoine Rouby <pierre-antoine.rouby@inria.fr>
 ;;; Copyright © 2018 Alex Branham <alex.branham@gmail.com>
@@ -333,6 +333,68 @@ editor (without an X toolkit)" )
              (lambda _
                (invoke "mkdir" "-p" "src/deps")))))))))
 
+;;; Returns a package definition that packages an emacs-lisp file from 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-INPUTS are
+;;; added as package inputs and SUBSTITUTIONS substitutions will be performed
+;;; on the elisp file and SYNOPSIS and DESCRIPTION as the package synopsis and
+;;; description.
+(define* (package-elisp-from-package
+          srcpkg pkgname src-file
+          #:key (target-file #f)
+          (inputs '())
+          (native-inputs '())
+          (propagated-inputs '())
+          (substitutions '())
+          (synopsis #f)
+          (description #f))
+  (let ((real-target-file (if target-file
+                              target-file
+                              (basename src-file)))
+        (orig (package-source srcpkg)))
+    (package
+      (inherit srcpkg)
+      (name pkgname)
+      (source (origin
+                (method (origin-method orig))
+                (uri (origin-uri orig))
+                (sha256 (origin-sha256 orig))
+                (file-name (string-append pkgname "-" (package-version srcpkg)))
+                (modules '((guix build utils)
+                           (srfi srfi-1)
+                           (ice-9 ftw)))
+                (snippet
+                 `(begin
+                    ;; Copy target file to source root and delete all other files
+                    (copy-file ,src-file
+                               ,real-target-file)
+                    (map delete-file-recursively
+                         (fold delete
+                               (scandir ".")
+                               '("." ".." ,real-target-file)))
+                    #t))))
+      (build-system emacs-build-system)
+      (inputs inputs)
+      (native-inputs native-inputs)
+      (propagated-inputs propagated-inputs)
+      (arguments
+       `(#:phases
+         (modify-phases %standard-phases
+           (add-after 'unpack 'configure
+             (lambda* (#:key inputs #:allow-other-keys)
+               (emacs-substitute-variables ,real-target-file
+                 ,substitutions)
+               #t)))))
+      (synopsis (if synopsis
+                    synopsis
+                    (package-synopsis srcpkg)))
+      (description (if description
+                       description
+                       (package-description srcpkg))))))
+
+(export package-elisp-from-package)
+
 \f
 ;;;
 ;;; Emacs hacking.
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.6: 0005-gnu-Use-package-elisp-from-package-for-clangs-emacs-.patch --]
[-- Type: text/x-patch; name="0005-gnu-Use-package-elisp-from-package-for-clangs-emacs-.patch", Size: 4983 bytes --]

From fda77fbd33933065a3d18f4db60fbf53f5908970 Mon Sep 17 00:00:00 2001
From: Tim Gesthuizen <tim.gesthuizen@yahoo.de>
Date: Fri, 4 Jan 2019 22:34:56 +0100
Subject: [PATCH 5/5] gnu: Use package-elisp-from-package for clangs emacs lisp
 files

Use package-elisp-from-package for emacs-clang-format and emacs-clang-rename.
Also remove package-from-clang-elisp-file as it is not needed anymore.

* gnu/packages/llvm.scm (emacs-clang-format): Use package-elisp-from-package
* gnu/packages/llvm.scm (emacs-clang-rename): Use package-elisp-from-package
* gnu/packages/llvm.scm (package-from-clang-elisp-file): Remove function
---
 gnu/packages/llvm.scm | 70 ++++++++++---------------------------------
 1 file changed, 15 insertions(+), 55 deletions(-)

diff --git a/gnu/packages/llvm.scm b/gnu/packages/llvm.scm
index 05147b665..c846e2758 100644
--- a/gnu/packages/llvm.scm
+++ b/gnu/packages/llvm.scm
@@ -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)
@@ -479,69 +480,28 @@ code analysis tools.")
      "This package provides a Python binding to LLVM for use in Numba.")
     (license license:bsd-3)))
 
-;;; Returns a package definition that packages an emacs-lisp file from the
-;;; clang source. The package has the name PKGNAME and packages the file
-;;; SRC-FILE from the clang source in its root directory with the name
-;;; TARGET-FILE where SUBST substitutions will be performed on the elisp file
-;;; and SYN and DESC as the package synopsis an description.
-(define (package-from-clang-elisp-file pkgname src-file target-file subst syn desc)
-  (package
-    (inherit clang)
-    (name pkgname)
-    (source (let ((orig (package-source clang)))
-              (origin
-                (method (origin-method orig))
-                (uri (origin-uri orig))
-                (sha256 (origin-sha256 orig))
-                (file-name (string-append pkgname "-" (package-version clang)))
-                (modules '((guix build utils)
-                           (srfi srfi-1)
-                           (ice-9 ftw)))
-                (snippet
-                 `(begin
-                    ;; Copy target file to source root and delete all other files
-                    (copy-file (string-append ,src-file)
-                               ,target-file)
-                    (map delete-file-recursively
-                         (fold delete
-                               (scandir ".")
-                               '("." ".." ,target-file)))
-                    #t)))))
-    (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")))
-               (emacs-substitute-variables ,target-file
-                 ,subst))
-             #t)))))
-    (synopsis syn)
-    (description desc)))
-
 (define-public emacs-clang-format
-  (package-from-clang-elisp-file
+  (package-elisp-from-package
+   clang
    "emacs-clang-format"
    "tools/clang-format/clang-format.el"
-   "clang-format.el"
-   '("clang-format-executable"
-                    (string-append clang "/bin/clang-format"))
-   "Format code using clang-format"
-   "This package allows to filter code through @code{clang-format}
+   #:inputs `(("clang" ,clang))
+   #:substitutions '("clang-format-executable"
+                     (string-append (assoc-ref inputs "clang") "/bin/clang-format"))
+   #: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
 C/C++/Obj-C code according to a set of style options, see
 @url{http://clang.llvm.org/docs/ClangFormatStyleOptions.html}."))
 
 (define-public emacs-clang-rename
-  (package-from-clang-elisp-file
+  (package-elisp-from-package
+   clang
    "emacs-clang-rename"
    "tools/clang-rename/clang-rename.el"
-   "clang-rename.el"
-   '("clang-rename-binary"
-                   (string-append clang "/bin/clang-rename"))
-   "Rename every occurrence of a symbol using clang-rename"
-   "This package renames every occurrence of a symbol at point
+   #:inputs `(("clang" ,clang))
+   #:substitutions '("clang-rename-binary"
+                     (string-append (assoc-ref inputs "clang") "/bin/clang-rename"))
+   #:synopsis "Rename every occurrence of a symbol using clang-rename"
+   #:description "This package renames every occurrence of a symbol at point
 using @code{clang-rename}."))
-- 
2.20.1


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 3042 bytes --]

Hi Tim,

I just reviewed your patch. Looks pretty good overall, thanks!

A few things:

> +(export package-elisp-from-package)

This should be placed at the beginning of the file in the (define-module...
See bootstrap.scm.

> +;;; Returns a package definition that packages an emacs-lisp file from 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-INPUTS are
> +;;; added as package inputs and SUBSTITUTIONS substitutions will be performed
> +;;; on the elisp file and SYNOPSIS and DESCRIPTION as the package synopsis and
> +;;; description.
> +(define* (package-elisp-from-package

Move the ";;;" comment to a docstring, e.g.

--8<---------------cut here---------------start------------->8---
(define* (package-elisp-from-package
          ...)
  "Return ..."
--8<---------------cut here---------------end--------------->8---

> +;;; Returns a package definition that packages an emacs-lisp file from the

"Return", not "Returns".

> +;;; SRCPKG source. The package has the name PKGNAME and packages the file

Separate sentences with two spaces.

> +          srcpkg pkgname src-file

Prefer complete words over abbreviations.  Here I'd suggest

  source-package
  name
  source-file

> +      (synopsis (if synopsis
> +                    synopsis
> +                    (package-synopsis srcpkg)))
> +      (description (if description
> +                       description
> +                       (package-description srcpkg))))))

A more Lispy way:

--8<---------------cut here---------------start------------->8---
 +      (synopsis (or synopsis
 +                    (package-synopsis srcpkg)))
 +      (description (or description
 +                       (package-description srcpkg))))))
--8<---------------cut here---------------end--------------->8---

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 split over
multiple files.

One thing I'm not too sure about is the replication of the structure fields as
keys.
I think it's easier to ignore those and let the user define them as follows:

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

Makes sense?  This would also be more robust in case the package structure
changes someday.

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

Thank you again for the good work!

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Tim Gesthuizen]

[-- Attachment #1.1.1: Type: text/plain, Size: 3888 bytes --]

Hi Pierre,
thank you for reviewing!
On 06.01.19 20:00, Pierre Neidhardt wrote:
> Hi Tim,
> 
> I just reviewed your patch. Looks pretty good overall, thanks!
> 
> A few things:
> 
>> +(export package-elisp-from-package)
> 
> This should be placed at the beginning of the file in the (define-module...
> 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 from 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-INPUTS are
>> +;;; added as package inputs and SUBSTITUTIONS substitutions will be performed
>> +;;; on the elisp file and SYNOPSIS and DESCRIPTION as the package synopsis and
>> +;;; description.
>> +(define* (package-elisp-from-package
> 
> Move the ";;;" comment to a docstring, e.g.
> 
> --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 from the
> 
> "Return", not "Returns".
> 
>> +;;; SRCPKG source. The package has the name PKGNAME and packages the file
> 
> Separate sentences with two spaces.

Done.

>> +          srcpkg pkgname src-file
> 
> Prefer complete words over abbreviations.  Here I'd suggest
> 
>   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))))))
> 
> A more Lispy way:
> 
> --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 split over
> multiple files.
> 
> One thing I'm not too sure about is the replication of the structure fields as
> keys.
> I think it's easier to ignore those and let the user define them as follows:
> 
> --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 structure
> changes someday.
> 
> 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 commits.

Done. Patches are attached.

Tim.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: 0001-gnu-Add-package-elisp-from-package.patch --]
[-- Type: text/x-patch; name="0001-gnu-Add-package-elisp-from-package.patch", Size: 2812 bytes --]

From 738d70333c5f09d4f5914e9e524999e800f9f37a Mon Sep 17 00:00:00 2001
From: Tim Gesthuizen <tim.gesthuizen@yahoo.de>
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 © 2018 Sohom Bhattacharjee <soham.bhattacharjee15@gmail.com>
 ;;; Copyright © 2018 Mathieu Lirzin <mthl@gnu.org>
 ;;; Copyright © 2018 Pierre Neidhardt <mail@ambrevar.xyz>
-;;; Copyright © 2018 Tim Gesthuizen <tim.gesthuizen@yahoo.de>
+;;; Copyright © 2018, 2019 Tim Gesthuizen <tim.gesthuizen@yahoo.de>
 ;;; Copyright © 2018 Jack Hill <jackhill@jackhill.us>
 ;;; Copyright © 2018 Pierre-Antoine Rouby <pierre-antoine.rouby@inria.fr>
 ;;; Copyright © 2018 Alex Branham <alex.branham@gmail.com>
@@ -336,6 +336,36 @@ editor (without an X toolkit)" )
              (lambda _
                (invoke "mkdir" "-p" "src/deps")))))))))
 
+(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 packages
+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-package)))
+                (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)))))))
+
 \f
 ;;;
 ;;; Emacs hacking.
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.3: 0002-gnu-Use-package-elisp-from-package-for-clangs-emacs-.patch --]
[-- Type: text/x-patch; name="0002-gnu-Use-package-elisp-from-package-for-clangs-emacs-.patch", Size: 5195 bytes --]

From 012ba2f96b16d54e0e1c23ee912c8a219355216c Mon Sep 17 00:00:00 2001
From: Tim Gesthuizen <tim.gesthuizen@yahoo.de>
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-rename.
Also remove package-from-clang-elisp-file as it is not needed anymore.

* gnu/packages/llvm.scm (emacs-clang-format): Use package-elisp-from-package
* gnu/packages/llvm.scm (emacs-clang-rename): Use package-elisp-from-package
* 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 © 2018 Marius Bakke <mbakke@fastmail.com>
 ;;; Copyright © 2018 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2018 Efraim Flashner <efraim@flashner.co.il>
-;;; Copyright © 2018 Tim Gesthuizen <tim.gesthuizen@yahoo.de>
+;;; Copyright © 2018, 2019 Tim Gesthuizen <tim.gesthuizen@yahoo.de>
 ;;; Copyright © 2018 Pierre Neidhardt <mail@ambrevar.xyz>
 ;;;
 ;;; 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.")
 
 (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-format.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-keys)
+                                    (chmod "clang-format.el" #o644)
+                                    (emacs-substitute-variables "clang-format.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 options, see
 
 (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-rename.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 point
 using @code{clang-rename}.")))
-- 
2.20.1


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 543 bytes --]

Merged!

> : > +;;; Returns a package definition that packages an emacs-lisp file from the
> : 
> : "Return", not "Returns".

You forgot this!  Anyways, I was confused by the docstring, so I took the
liberty to simplify it a little bit to the following:

  "Return a package definition named PACKAGE-NAME that packages the Emacs Lisp
SOURCE-FILES found in SOURCE-PACKAGE."

If you think this is missing something, let me know and I'll fix it.

Thanks for your great contribution!

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]

Oops, got too fast: there is a circular dependency problem because emacs.scm
depends on llvm.scm.
The function must be moved to some other place.  I'll place it in emacs utils or
something.

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]

Hmmm... Not too sure where to put package-elisp-from-package.
I see the following options:

- build-system/emacs.scm, but is it our policy?
- Or a separate file, but which one?
  
@Ludo?

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Ludovic Courtès]

Hello!

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

> Hmmm... Not too sure where to put package-elisp-from-package.
> I see the following options:
>
> - build-system/emacs.scm, but is it our policy?
> - Or a separate file, but which one?
>   
> @Ludo?

I actually fixed it today right after you reverted the commit (I rebased
and didn’t notice it had been reverted in the meantime):

  https://git.savannah.gnu.org/cgit/guix.git/commit/?id=67eebb19b70acfe997b40d8c7978f9dc0673a4af

With this you should be able to reinstate the rest of the commit.

I tried to explain the reason for the issue in the commit log, let me
know if anything’s unclear.

Thanks,
Ludo’.

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Pierre Neidhardt]

It works, but it's semantically dubious.  For packages like emacs-cmake-mode now
have to use the llvm module to use package-elisp-from-package.
Thoughts?

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Ludovic Courtès]

Hi,

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

> It works, but it's semantically dubious.  For packages like emacs-cmake-mode now
> have to use the llvm module to use package-elisp-from-package.
> Thoughts?

Sure, it probably belongs elsewhere.  I moved it there to quickly fix
the problem, and I purposefully made it private, but I agree, it could
go to some other places if there’s a need for it outside of llvm.scm.
I’m not sure exactly where.

I wonder how often the approach of ‘package-elisp-from-package’ is
applicable or desirable.  There are packages (e.g., recutils, GLOBAL)
that come with elisp files, which automatically get installed upon “make
install.”  I’m not sure we’d want to make them separate.

Thoughts?

Ludo’.

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Pierre Neidhardt]

The benefit of separate Emacs packages is if the Emacs library can be used
without installing the parent package to the user profile.

For instance, emacs-clang-rename can be installed and it will work while the
user does not have to install clang.  (Clang remains an input, obviously.)

For this reason, "package-elisp-from-package" gives maximal flexibility in my
opinion.

Currently, there are a few more packages.  We can look up "emacs-build-system"
outside emacs.scm to find them (e.g. agda2).

Off the top of my head, there is also asymptote.

Now to the ideal place for package-elisp-from-package: it seems that no existing
file would be a fit.  So what about guix/utils/emacs.scm?  Having a separate
file would make sure we run into other meta-circular dependency issues.

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Ludovic Courtès]

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

> The benefit of separate Emacs packages is if the Emacs library can be used
> without installing the parent package to the user profile.
>
> For instance, emacs-clang-rename can be installed and it will work while the
> user does not have to install clang.  (Clang remains an input, obviously.)
>
> For this reason, "package-elisp-from-package" gives maximal flexibility in my
> opinion.

Yes, I agree that it makes a lot of sense for ‘emacs-clang-rename’ for
instance.  I’m just unsure whether the approach generalize to other
packages.

> Currently, there are a few more packages.  We can look up "emacs-build-system"
> outside emacs.scm to find them (e.g. agda2).
>
> Off the top of my head, there is also asymptote.

I’m not convinced sure ‘emacs-agda2-mode’ and ‘asymptote’ need to be
changed; it doesn’t look like a clear win, dunno.

For example, ‘package-elisp-from-package’ preserves the name, synopsis,
and description, so you end up having to do:

  (define foo
    (package
      (inherit (package-elisp-from-package x))
      (name "emacs-foo")
      (license …)
      (synopsis …)
      (description …)))

… which I think it a marginal improvement compared to
‘emacs-agda2-mode’.  Also, the “find *.el” approach may not work out of
the box for all cases, so the procedure may need to be tweaked further,
etc.

> Now to the ideal place for package-elisp-from-package: it seems that no existing
> file would be a fit.  So what about guix/utils/emacs.scm?  Having a separate
> file would make sure we run into other meta-circular dependency issues.

Circular, not meta-circular.  ;-)

So yeah, (guix utils emacs) is one option; another one would be to trim
the list of modules emacs.scm depends on, such that we don’t have this
issue in the first place (that requires care, though.)

However, my suggestion would be to use ‘package-elisp-from-package’ as
Tim intended in the original patch, keeping the procedure private to
llvm.scm, and generalize if and when we see other use cases.

How does that sound?

Ludo’.

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Pierre Neidhardt]

Agreed, the win is not always obvious.
If we are all OK with this, let's close the issue.

Tim, what do you think?

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Tim Gesthuizen]

[-- Attachment #1.1: Type: text/plain, Size: 398 bytes --]

On 08.01.19 11:05, Pierre Neidhardt wrote:
> Agreed, the win is not always obvious.
> If we are all OK with this, let's close the issue.
> 
> Tim, what do you think?
> 

That's no problem for me.
After all my intent was to make sure I don't have clangs source tree 3
times in the store :)
I think we should still put the generic function into llvm.scm and not
the clang specific one.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Tim Gesthuizen]

[-- Attachment #1.1.1: Type: text/plain, Size: 115 bytes --]

Hi,

I implemented the changes we discussed so we can finally close this ticket.
Patches are attached.

Tim.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: 0001-gnu-Add-package-elisp-from-package.patch --]
[-- Type: text/x-patch; name="0001-gnu-Add-package-elisp-from-package.patch", Size: 2749 bytes --]

From 515a9392b8912fbfb067728acb25f84de83d8378 Mon Sep 17 00:00:00 2001
From: Tim Gesthuizen <tim.gesthuizen@yahoo.de>
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/llvm.scm (package-elisp-from-package): New function
---
 gnu/packages/llvm.scm | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/llvm.scm b/gnu/packages/llvm.scm
index b1e41d72f..5518d4d5c 100644
--- a/gnu/packages/llvm.scm
+++ b/gnu/packages/llvm.scm
@@ -8,7 +8,7 @@
 ;;; Copyright © 2018 Marius Bakke <mbakke@fastmail.com>
 ;;; Copyright © 2018 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2018 Efraim Flashner <efraim@flashner.co.il>
-;;; Copyright © 2018 Tim Gesthuizen <tim.gesthuizen@yahoo.de>
+;;; Copyright © 2018, 2019 Tim Gesthuizen <tim.gesthuizen@yahoo.de>
 ;;; Copyright © 2018 Pierre Neidhardt <mail@ambrevar.xyz>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -508,6 +508,36 @@ SOURCE-FILES found in SOURCE-PACKAGE."
                                (append '("." "..") basenames)))
                     #t)))))))
 
+(define (package-elisp-from-package
+         source-package package-name source-files)
+  "Return a package definition that packages emacs-lisp files from the
+SOURCE-PACKAGEs source.  The package has the name PACKAGE-NAME and packages
+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-package)))
+                (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)))))))
+
 (define-public emacs-clang-format
   (package
     (inherit clang)
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.3: 0002-gnu-Use-package-elisp-from-package-for-clangs-emacs-.patch --]
[-- Type: text/x-patch; name="0002-gnu-Use-package-elisp-from-package-for-clangs-emacs-.patch", Size: 4794 bytes --]

From 939547e6e2c0f5a83eb1290ff1c9ee39415ce3d5 Mon Sep 17 00:00:00 2001
From: Tim Gesthuizen <tim.gesthuizen@yahoo.de>
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-rename.
Also remove package-from-clang-elisp-file as it is not needed anymore.

* gnu/packages/llvm.scm (emacs-clang-format): Use package-elisp-from-package
* gnu/packages/llvm.scm (emacs-clang-rename): Use package-elisp-from-package
* gnu/packages/llvm.scm (package-from-clang-elisp-file): Remove function
---
 gnu/packages/llvm.scm | 63 +++++++++++++++++++++----------------------
 1 file changed, 31 insertions(+), 32 deletions(-)

diff --git a/gnu/packages/llvm.scm b/gnu/packages/llvm.scm
index 5518d4d5c..dedad7dc7 100644
--- a/gnu/packages/llvm.scm
+++ b/gnu/packages/llvm.scm
@@ -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)
@@ -540,22 +541,21 @@ the files SOURCE-FILES from the source in its root directory."
 
 (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-format.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-keys)
+                                    (chmod "clang-format.el" #o644)
+                                    (emacs-substitute-variables "clang-format.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
@@ -564,22 +564,21 @@ C/C++/Obj-C code according to a set of style options, see
 
 (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-rename.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 point
 using @code{clang-rename}.")))
-- 
2.20.1


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Pierre Neidhardt]

Err... The patch is already merged, what did you change?

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Tim Gesthuizen]

On 10.01.19 19:40, Pierre Neidhardt wrote:
> Err... The patch is already merged, what did you change?
> 

Sorry... Forgot to pull.
Is there anything left keeping us from closing the ticket?

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Pierre Neidhardt]

No, I think you can close it.  Thank you again for your contribution!

[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename

[Tim Gesthuizen]

[-- Attachment #1.1: Type: text/plain, Size: 1401 bytes --]

On 07.01.19 14:47, Pierre Neidhardt wrote:
> Merged!
> 
>> : > +;;; Returns a package definition that packages an emacs-lisp file from the
>> : 
>> : "Return", not "Returns".
> 
> You forgot this!  Anyways, I was confused by the docstring, so I took the
> liberty to simplify it a little bit to the following:

Whoops... Maybe I did not reformat the patches.

>   "Return a package definition named PACKAGE-NAME that packages the Emacs Lisp
> SOURCE-FILES found in SOURCE-PACKAGE."
> 
> If you think this is missing something, let me know and I'll fix it.

Much better. That was probably just to straight forward for me.

> Oops, got too fast: there is a circular dependency problem because emacs.scm
> depends on llvm.scm.
> The function must be moved to some other place.  I'll place it in emacs utils or
> something.

So that is my "weird issue"!
I first packaged the function in emacs-utils.scm and experienced much
worse things. I suspected it to be a circular dependency as any package
using emacs-build-system includes emacs-utils under the hood and moved
it to packages/emacs.scm.
I thought it was something different because the same error appeared
there too.

Maybe we should have a file with "shortcuts" for package definitions of
special kind and place the function there?
I would claim that we would have the same problems in emacs-utils.scm.

Tim.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]