bug#26559: [PATCH] build: emacs: Install only a subset of files.

bug#26559: [PATCH] build: emacs: Install only a subset of files.

[Arun Isaac]

* guix/build/emacs-build-system.scm (install): Install files matching
  #:include while excluding files matching #:exclude.
* guix/build-system/emacs.scm (emacs-build): Add keyword arguments #:include
  and #:exclude.
---
 guix/build-system/emacs.scm       |  6 ++++++
 guix/build/emacs-build-system.scm | 24 +++++++++++++++++++++---
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/guix/build-system/emacs.scm b/guix/build-system/emacs.scm
index a7982002b..e6c021c7e 100644
--- a/guix/build-system/emacs.scm
+++ b/guix/build-system/emacs.scm
@@ -83,6 +83,10 @@
                       (phases '(@ (guix build emacs-build-system)
                                   %standard-phases))
                       (outputs '("out"))
+                      (include ''(".*.el$" ".*.el.in$" "^dir$"
+                                 ".*.info$" ".*.texi$" ".*.texinfo$"
+                                 "doc/dir" "doc/*.info$" "doc/*.texi$" "doc/*.texinfo$"))
+                      (exclude ''("^.dir-locals.el$" "^test.el$" "^tests.el$" ".*-test.el$" ".*-tests.el$"))
                       (search-paths '())
                       (system (%current-system))
                       (guile #f)
@@ -108,6 +112,8 @@
                     #:tests? ,tests?
                     #:phases ,phases
                     #:outputs %outputs
+                    #:include ,include
+                    #:exclude ,exclude
                     #:search-paths ',(map search-path-specification->sexp
                                           search-paths)
                     #:inputs %build-inputs)))
diff --git a/guix/build/emacs-build-system.scm b/guix/build/emacs-build-system.scm
index 44e8b0d31..579596d72 100644
--- a/guix/build/emacs-build-system.scm
+++ b/guix/build/emacs-build-system.scm
@@ -28,6 +28,7 @@
   #:use-module (ice-9 rdelim)
   #:use-module (ice-9 regex)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 ftw)
   #:export (%standard-phases
             emacs-build))
 
@@ -93,14 +94,31 @@ store in '.el' files."
           (substitute-cmd))))
     #t))
 
-(define* (install #:key outputs #:allow-other-keys)
+(define* (install #:key outputs
+                  (include '(".*.el$" ".*.el.in$" "^dir$"
+                             ".*.info$" ".*.texi$" ".*.texinfo$"
+                             "^doc/dir" "^doc/*.info$" "^doc/*.texi$" "^doc/*.texinfo$"))
+                  (exclude '("^.dir-locals.el$" "^test.el$" "^tests.el$" ".*-test.el$" ".*-tests.el$"))
+                  #:allow-other-keys)
   "Install the package contents."
+
+  (define (include-file? file)
+    (and (any (cut string-match <> file) include)
+         (not (any (cut string-match <> file) exclude))))
+
   (let* ((out (assoc-ref outputs "out"))
          (elpa-name-ver (store-directory->elpa-name-version out))
          (src-dir (getcwd))
          (tgt-dir (string-append out %install-suffix "/" elpa-name-ver)))
-    (copy-recursively src-dir tgt-dir)
-    #t))
+    (ftw src-dir
+         (lambda (file stat flag)
+           (let ((stripped-file (substring file (string-length src-dir))))
+             (when (and (eq? flag 'regular)
+                        (include-file? (string-trim stripped-file #\/)))
+               (format #t "`~a' -> `~a'~%"
+                       file (string-append tgt-dir stripped-file))
+               (install-file file tgt-dir)))
+           #t))))
 
 (define* (move-doc #:key outputs #:allow-other-keys)
   "Move info files from the ELPA package directory to the info directory."
-- 
2.12.2

bug#26559: [PATCH] build: emacs: Install only a subset of files.

[tumashu]

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

What about  package-pkg.el  file?.








At 2017-04-19 15:35:25, "Arun Isaac" <arunisaac@systemreboot.net> wrote:
>* guix/build/emacs-build-system.scm (install): Install files matching
>  #:include while excluding files matching #:exclude.
>* guix/build-system/emacs.scm (emacs-build): Add keyword arguments #:include
>  and #:exclude.
>---
> guix/build-system/emacs.scm       |  6 ++++++
> guix/build/emacs-build-system.scm | 24 +++++++++++++++++++++---
> 2 files changed, 27 insertions(+), 3 deletions(-)
>
>diff --git a/guix/build-system/emacs.scm b/guix/build-system/emacs.scm
>index a7982002b..e6c021c7e 100644
>--- a/guix/build-system/emacs.scm
>+++ b/guix/build-system/emacs.scm
>@@ -83,6 +83,10 @@
>                       (phases '(@ (guix build emacs-build-system)
>                                   %standard-phases))
>                       (outputs '("out"))
>+                      (include ''(".*.el$" ".*.el.in$" "^dir$"
>+                                 ".*.info$" ".*.texi$" ".*.texinfo$"
>+                                 "doc/dir" "doc/*.info$" "doc/*.texi$" "doc/*.texinfo$"))
>+                      (exclude ''("^.dir-locals.el$" "^test.el$" "^tests.el$" ".*-test.el$" ".*-tests.el$"))
>                       (search-paths '())
>                       (system (%current-system))
>                       (guile #f)
>@@ -108,6 +112,8 @@
>                     #:tests? ,tests?
>                     #:phases ,phases
>                     #:outputs %outputs
>+                    #:include ,include
>+                    #:exclude ,exclude
>                     #:search-paths ',(map search-path-specification->sexp
>                                           search-paths)
>                     #:inputs %build-inputs)))
>diff --git a/guix/build/emacs-build-system.scm b/guix/build/emacs-build-system.scm
>index 44e8b0d31..579596d72 100644
>--- a/guix/build/emacs-build-system.scm
>+++ b/guix/build/emacs-build-system.scm
>@@ -28,6 +28,7 @@
>   #:use-module (ice-9 rdelim)
>   #:use-module (ice-9 regex)
>   #:use-module (ice-9 match)
>+  #:use-module (ice-9 ftw)
>   #:export (%standard-phases
>             emacs-build))
> 
>@@ -93,14 +94,31 @@ store in '.el' files."
>           (substitute-cmd))))
>     #t))
> 
>-(define* (install #:key outputs #:allow-other-keys)
>+(define* (install #:key outputs
>+                  (include '(".*.el$" ".*.el.in$" "^dir$"
>+                             ".*.info$" ".*.texi$" ".*.texinfo$"
>+                             "^doc/dir" "^doc/*.info$" "^doc/*.texi$" "^doc/*.texinfo$"))
>+                  (exclude '("^.dir-locals.el$" "^test.el$" "^tests.el$" ".*-test.el$" ".*-tests.el$"))
>+                  #:allow-other-keys)
>   "Install the package contents."
>+
>+  (define (include-file? file)
>+    (and (any (cut string-match <> file) include)
>+         (not (any (cut string-match <> file) exclude))))
>+
>   (let* ((out (assoc-ref outputs "out"))
>          (elpa-name-ver (store-directory->elpa-name-version out))
>          (src-dir (getcwd))
>          (tgt-dir (string-append out %install-suffix "/" elpa-name-ver)))
>-    (copy-recursively src-dir tgt-dir)
>-    #t))
>+    (ftw src-dir
>+         (lambda (file stat flag)
>+           (let ((stripped-file (substring file (string-length src-dir))))
>+             (when (and (eq? flag 'regular)
>+                        (include-file? (string-trim stripped-file #\/)))
>+               (format #t "`~a' -> `~a'~%"
>+                       file (string-append tgt-dir stripped-file))
>+               (install-file file tgt-dir)))
>+           #t))))
> 
> (define* (move-doc #:key outputs #:allow-other-keys)
>   "Move info files from the ELPA package directory to the info directory."
>-- 
>2.12.2
>
>
>
>

[-- Attachment #2: Type: text/html, Size: 4226 bytes --]

bug#26560: [PATCH] build: emacs: Install only a subset of files.

[Arun Isaac]

This is a work in progress patch for the discussion at
https://lists.gnu.org/archive/html/guix-devel/2017-04/msg00274.html

All feedback welcome!

Arun Isaac writes:

> +                      (include ''(".*.el$" ".*.el.in$" "^dir$"
> +                                 ".*.info$" ".*.texi$" ".*.texinfo$"
> +                                 "doc/dir" "doc/*.info$" "doc/*.texi$" "doc/*.texinfo$"))
> +                      (exclude ''("^.dir-locals.el$" "^test.el$" "^tests.el$" ".*-test.el$" ".*-tests.el$"))

I've copied all this from MELPA's default :files property described at
https://github.com/melpa/melpa . I have no idea what the rationale for
some of these regexes are.

Currently, include and exclude are a list of regexes that file names are
matched against. Should this be combined into one big regex?

Would it be a good idea to add a third keyword argument, say
#:documentation, that will select info documentation files and install
them separately in some other directory?

bug#26559: [PATCH] build: emacs: Install only a subset of files.

[Arun Isaac]

This is a work in progress patch for the discussion at
https://lists.gnu.org/archive/html/guix-devel/2017-04/msg00274.html

All feedback welcome!

Arun Isaac writes:

> +                      (include ''(".*.el$" ".*.el.in$" "^dir$"
> +                                 ".*.info$" ".*.texi$" ".*.texinfo$"
> +                                 "doc/dir" "doc/*.info$" "doc/*.texi$" "doc/*.texinfo$"))
> +                      (exclude ''("^.dir-locals.el$" "^test.el$" "^tests.el$" ".*-test.el$" ".*-tests.el$"))

I've copied all this from MELPA's default :files property described at
https://github.com/melpa/melpa . I have no idea what the rationale for
some of these regexes are.

Currently, include and exclude are a list of regexes that file names are
matched against. Should this be combined into one big regex?

Would it be a good idea to add a third keyword argument, say
#:documentation, that will select info documentation files and install
them separately in some other directory?

bug#26560: [PATCH] build: emacs: Install only a subset of files.

[Arun Isaac]

Sorry, I accidentally sent this mail to guix-patches@gnu.org instead of
26559@debbugs.gnu.org. Closing this.

bug#26559: [PATCH] build: emacs: Install only a subset of files.

[Arun Isaac]

tumashu writes:

> What about  package-pkg.el  file?.

What about it? I am not familiar with this file. Could you please
elaborate?

bug#26559: [PATCH] build: emacs: Install only a subset of files.

[tumashu]

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







*.pkg.el seem to be used by emacs  package.el,  which can not be compile without warn. I can not find the document.
this need to be verify.


At 2017-04-19 17:23:24, "Arun Isaac" <arunisaac@systemreboot.net> wrote:
>
>tumashu writes:
>
>> What about  package-pkg.el  file?.
>
>What about it? I am not familiar with this file. Could you please
>elaborate?
>
>
>

[-- Attachment #2: Type: text/html, Size: 621 bytes --]

bug#26559: [PATCH] build: emacs: Install only a subset of files.

[Arun Isaac]

tumashu writes:

> *.pkg.el seem to be used by emacs package.el, which can not be compile
> without warn. I can not find the document.  this need to be verify.

In the current implementation, *.pkg.el files will be installed due to
the ".*.el" regexp in #:include. So, this is not an issue.

bug#26559: [PATCH] build: emacs: Install only a subset of files.

[tumashu]

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

PACKAGE-pkg.el seem to *only* useful to package.el,   it is useless for guix emacs package.

The below is come from package.el's commentary


#+BEGIN_COMMENT

;; A package is described by its name and version.  The distribution
;; format is either  a tar file or a single .el file.

;; A tar file should be named "NAME-VERSION.tar".  The tar file must
;; unpack into a directory named after the package and version:
;; "NAME-VERSION".  It must contain a file named "PACKAGE-pkg.el"
;; which consists of a call to define-package.  It may also contain a
;; "dir" file and the info files it references.

;; A .el file is named "NAME-VERSION.el" in the remote archive, but is
;; installed as simply "NAME.el" in a directory named "NAME-VERSION".

#+END_COMMENT








At 2017-04-19 22:26:06, "Arun Isaac" <arunisaac@systemreboot.net> wrote:
>
>tumashu writes:
>
>> *.pkg.el seem to be used by emacs package.el, which can not be compile
>> without warn. I can not find the document.  this need to be verify.
>
>In the current implementation, *.pkg.el files will be installed due to
>the ".*.el" regexp in #:include. So, this is not an issue.
>
>
>

[-- Attachment #2: Type: text/html, Size: 1486 bytes --]

bug#26559: [PATCH] build: emacs: Install only a subset of files.

[Arun Isaac]

tumashu writes:

> it is useless for guix emacs package.

If it is unnecessary we can simply include one more regexp to the
#:exclude keyword argument so that it does not get installed.

bug#26559: [PATCH] build: emacs: Install only a subset of files.

[Alex Kost]

Arun Isaac (2017-04-19 13:25 +0530) wrote:

> This is a work in progress patch for the discussion at
> https://lists.gnu.org/archive/html/guix-devel/2017-04/msg00274.html
>
> All feedback welcome!

Great, thanks for working on it!

> Arun Isaac writes:
>
>> +                      (include ''(".*.el$" ".*.el.in$" "^dir$"
>> +                                 ".*.info$" ".*.texi$" ".*.texinfo$"
>> +                                 "doc/dir" "doc/*.info$" "doc/*.texi$" "doc/*.texinfo$"))
>> +                      (exclude ''("^.dir-locals.el$" "^test.el$" "^tests.el$" ".*-test.el$" ".*-tests.el$"))
>
> I've copied all this from MELPA's default :files property described at
> https://github.com/melpa/melpa . I have no idea what the rationale for
> some of these regexes are.

What regexps are not clear for you?

> Currently, include and exclude are a list of regexes that file names are
> matched against. Should this be combined into one big regex?

I think it is not needed, it is more clean to separate regexps.

> Would it be a good idea to add a third keyword argument, say
> #:documentation, that will select info documentation files and install
> them separately in some other directory?

Regarding documentation: note that it is already installed into the
proper place.  I mean if the package has ".info" files, they are
installed into "share/info" (look at 'move-doc' procedure in (guix build
emacs-build-system) module).  For example, 'emacs-debbugs' has the info
manual and it is installed appropriately.  So I think #:documentation is
not needed.

Also I would like to note that this patch (when it will be ready)
shouldn't be committed alone: there are some emacs packages that should
be adjusted to include additional files.  One that comes to mind is
'emacs-slime' - it *must* contain *.lisp files (and other files as
listed at <https://github.com/melpa/melpa/blob/master/recipes/slime>).
Otherwise, it wouldn't work at all.

So I think this patch should be committed with the according fixes for
such "complex" packages (not sure if there are other ones along with
'emacs-slime').

-- 
Alex

bug#26559: [PATCH] build: emacs: Install only a subset of files.

[Alex Kost]

Arun Isaac (2017-04-20 14:13 +0530) wrote:

> tumashu writes:
>
>> it is useless for guix emacs package.
>
> If it is unnecessary we can simply include one more regexp to the
> #:exclude keyword argument so that it does not get installed.

Yes, it is completely unnecessary for Guix, so it can be excluded.

-- 
Alex

bug#26559: [PATCH] build: emacs: Install only a subset of files.

[Arun Isaac]

>>> +                      (include ''(".*.el$" ".*.el.in$" "^dir$"
>>> +                                 ".*.info$" ".*.texi$" ".*.texinfo$"
>>> +                                 "doc/dir" "doc/*.info$" "doc/*.texi$" "doc/*.texinfo$"))
>>> +                      (exclude ''("^.dir-locals.el$" "^test.el$" "^tests.el$" ".*-test.el$" ".*-tests.el$"))
>>
>> I've copied all this from MELPA's default :files property described at
>> https://github.com/melpa/melpa . I have no idea what the rationale for
>> some of these regexes are.
>
> What regexps are not clear for you?

I don't understand what ".*.el.in$", "^dir$" and "doc/dir" are for.

>> Currently, include and exclude are a list of regexes that file names are
>> matched against. Should this be combined into one big regex?
>
> I think it is not needed, it is more clean to separate regexps.

Ok. Also, please suggest better names if #:include and #:exclude are not
clear. Perhaps #:include-files and #:exclude-files, or #:install-files
and #:no-install-files ? Any other style changes to the code are also
welcome.

> Regarding documentation: note that it is already installed into the
> proper place.  I mean if the package has ".info" files, they are
> installed into "share/info" (look at 'move-doc' procedure in (guix build
> emacs-build-system) module).  For example, 'emacs-debbugs' has the info
> manual and it is installed appropriately.  So I think #:documentation is
> not needed.

Yeah, move-doc is good. I forgot about it.

> Also I would like to note that this patch (when it will be ready)
> shouldn't be committed alone: there are some emacs packages that should
> be adjusted to include additional files.  One that comes to mind is
> 'emacs-slime' - it *must* contain *.lisp files (and other files as
> listed at <https://github.com/melpa/melpa/blob/master/recipes/slime>).
> Otherwise, it wouldn't work at all.

For specific packages, we can always override the #:include and
#:exclude keyword arguments. Or, even replace the 'install phase if it
comes to that.

> So I think this patch should be committed with the according fixes for
> such "complex" packages (not sure if there are other ones along with
> 'emacs-slime').

Is there some standard workflow for testing packages when there is a
build system change? There are currently 154 packages in
gnu/packages/emacs.scm. Should I test them all?

bug#26559: [PATCH] build: emacs: Install only a subset of files.

[Alex Kost]

Arun Isaac (2017-04-20 18:22 +0530) wrote:

>>>> +                      (include ''(".*.el$" ".*.el.in$" "^dir$"
>>>> +                                 ".*.info$" ".*.texi$" ".*.texinfo$"
>>>> +                                 "doc/dir" "doc/*.info$" "doc/*.texi$" "doc/*.texinfo$"))
>>>> +                      (exclude ''("^.dir-locals.el$" "^test.el$" "^tests.el$" ".*-test.el$" ".*-tests.el$"))
>>>
>>> I've copied all this from MELPA's default :files property described at
>>> https://github.com/melpa/melpa . I have no idea what the rationale for
>>> some of these regexes are.
>>
>> What regexps are not clear for you?
>
> I don't understand what ".*.el.in$", "^dir$" and "doc/dir" are for.

"dir" file is for documentation (to make the according manual entry
appear in the top Info directory).  These "dir" files are not needed for
Guix, as Guix generates "dir" after updating a profile
("<guix-profile>/share/info/dir").

Usually files with ".in" ending are used as templates to generate other
files from them (in this case ".el" from ".el.in").  It looks like there
are packages that have ".el.in" files but not ".el" files, and
package-build (used by melpa) just renames such files.  At least that's
what I understood from:

  https://github.com/melpa/package-build/commit/d1c217a7ef33807c4948853114133af72284031c

But I'm sure we don't have the packages with ".el.in" files, and if we
will, most likely gnu-build-system will be used for them, so I suggest
to remove ".el.in" regexp from the "include" list.

>>> Currently, include and exclude are a list of regexes that file names are
>>> matched against. Should this be combined into one big regex?
>>
>> I think it is not needed, it is more clean to separate regexps.
>
> Ok. Also, please suggest better names if #:include and #:exclude are not
> clear. Perhaps #:include-files and #:exclude-files, or #:install-files
> and #:no-install-files ?

I think include/exclude is OK.  But maybe other people (if anyone else
reads this thread :-)) have other opinions.

[...]
>> Also I would like to note that this patch (when it will be ready)
>> shouldn't be committed alone: there are some emacs packages that should
>> be adjusted to include additional files.  One that comes to mind is
>> 'emacs-slime' - it *must* contain *.lisp files (and other files as
>> listed at <https://github.com/melpa/melpa/blob/master/recipes/slime>).
>> Otherwise, it wouldn't work at all.
>
> For specific packages, we can always override the #:include and
> #:exclude keyword arguments. Or, even replace the 'install phase if it
> comes to that.

Yes, that's what I meant.

>> So I think this patch should be committed with the according fixes for
>> such "complex" packages (not sure if there are other ones along with
>> 'emacs-slime').
>
> Is there some standard workflow for testing packages when there is a
> build system change?

AFAIK, there is no standard workflow :-)

> There are currently 154 packages in
> gnu/packages/emacs.scm. Should I test them all?

I think it would be too much work.  I quickly looked at the emacs
packages, and I believe that only slime, auctex and yasnippet need to be
adjusted to include non-standard files.  Of course, there may be other
packages that I'm not aware of, but they can be fixed later.

-- 
Alex

bug#26559: [PATCH] build: emacs: Install only a subset of files.

[Alex Kost]

Arun Isaac (2017-04-19 13:05 +0530) wrote:

> * guix/build/emacs-build-system.scm (install): Install files matching
>   #:include while excluding files matching #:exclude.
> * guix/build-system/emacs.scm (emacs-build): Add keyword arguments #:include
>   and #:exclude.
> ---
>  guix/build-system/emacs.scm       |  6 ++++++
>  guix/build/emacs-build-system.scm | 24 +++++++++++++++++++++---
>  2 files changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/guix/build-system/emacs.scm b/guix/build-system/emacs.scm
> index a7982002b..e6c021c7e 100644
> --- a/guix/build-system/emacs.scm
> +++ b/guix/build-system/emacs.scm
> @@ -83,6 +83,10 @@
>                        (phases '(@ (guix build emacs-build-system)
>                                    %standard-phases))
>                        (outputs '("out"))
> +                      (include ''(".*.el$" ".*.el.in$" "^dir$"
> +                                 ".*.info$" ".*.texi$" ".*.texinfo$"
> +                                 "doc/dir" "doc/*.info$" "doc/*.texi$" "doc/*.texinfo$"))

I think only ".*.el$", ".*.info$", and probably "doc/*.info$" are needed
here.  The rest regexps look useless to me.

Note, however, that 'move-doc' procedure should be adjusted to find info
in "doc" subdir (for "doc/*.info$" regex).

> +                      (exclude ''("^.dir-locals.el$" "^test.el$" "^tests.el$" ".*-test.el$" ".*-tests.el$"))
Don't forget to add ".*-pkg.el" here.

[...]
> -(define* (install #:key outputs #:allow-other-keys)
> +(define* (install #:key outputs
> +                  (include '(".*.el$" ".*.el.in$" "^dir$"
> +                             ".*.info$" ".*.texi$" ".*.texinfo$"
> +                             "^doc/dir" "^doc/*.info$" "^doc/*.texi$" "^doc/*.texinfo$"))
> +                  (exclude '("^.dir-locals.el$" "^test.el$" "^tests.el$" ".*-test.el$" ".*-tests.el$"))

It doesn't look right that these regexps are duplicated in 2 places.
I'm not very familiar with build systems, but what if the
'include'/'exclude' arguments of 'install' procedure would simply be
empty lists?  I think it wouldn't do harm if you leave these regexps
only in 'emacs-build' procedure or would it?

-- 
Alex

bug#26559: [PATCH 1/3] build: emacs: Install only a subset of files.

[Arun Isaac]

* guix/build/emacs-build-system.scm (install): Install files matching
  #:include while excluding files matching #:exclude.
* guix/build-system/emacs.scm (emacs-build): Add keyword arguments #:include
  and #:exclude.
---
 guix/build-system/emacs.scm       |  4 ++++
 guix/build/emacs-build-system.scm | 25 +++++++++++++++++++++----
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/guix/build-system/emacs.scm b/guix/build-system/emacs.scm
index a7982002b..9a46ecfd2 100644
--- a/guix/build-system/emacs.scm
+++ b/guix/build-system/emacs.scm
@@ -83,6 +83,8 @@
                       (phases '(@ (guix build emacs-build-system)
                                   %standard-phases))
                       (outputs '("out"))
+                      (include ''("^[^/]*\\.el$" "^[^/]*\\.info$" "^doc/.*\\.info$"))
+                      (exclude ''("^\\.dir-locals\\.el$" "-pkg\\.el$" "^[^/]*tests?\\.el$"))
                       (search-paths '())
                       (system (%current-system))
                       (guile #f)
@@ -108,6 +110,8 @@
                     #:tests? ,tests?
                     #:phases ,phases
                     #:outputs %outputs
+                    #:include ,include
+                    #:exclude ,exclude
                     #:search-paths ',(map search-path-specification->sexp
                                           search-paths)
                     #:inputs %build-inputs)))
diff --git a/guix/build/emacs-build-system.scm b/guix/build/emacs-build-system.scm
index 44e8b0d31..fefdbb96e 100644
--- a/guix/build/emacs-build-system.scm
+++ b/guix/build/emacs-build-system.scm
@@ -28,6 +28,7 @@
   #:use-module (ice-9 rdelim)
   #:use-module (ice-9 regex)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 ftw)
   #:export (%standard-phases
             emacs-build))
 
@@ -93,14 +94,30 @@ store in '.el' files."
           (substitute-cmd))))
     #t))
 
-(define* (install #:key outputs #:allow-other-keys)
+(define* (install #:key outputs
+                  (include '("^[^/]*\\.el$" "^[^/]*\\.info$" "^doc/.*\\.info$"))
+                  (exclude '("^\\.dir-locals\\.el$" "-pkg\\.el$" "^[^/]*tests?\\.el$"))
+                  #:allow-other-keys)
   "Install the package contents."
+
+  (define src-dir (getcwd))
+
+  (define (install-file? file stat)
+    (let ((stripped-file (string-trim (substring file (string-length src-dir)) #\/)))
+      (and (any (cut string-match <> stripped-file) include)
+           (not (any (cut string-match <> stripped-file) exclude)))))
+
   (let* ((out (assoc-ref outputs "out"))
          (elpa-name-ver (store-directory->elpa-name-version out))
-         (src-dir (getcwd))
          (tgt-dir (string-append out %install-suffix "/" elpa-name-ver)))
-    (copy-recursively src-dir tgt-dir)
-    #t))
+    (for-each
+     (lambda (file)
+       (let* ((stripped-file (substring file (string-length src-dir)))
+              (tgt-file (string-append tgt-dir stripped-file)))
+         (format #t "`~a' -> `~a'~%" file tgt-file)
+         (install-file file (dirname tgt-file))))
+     (find-files src-dir install-file?)))
+  #t)
 
 (define* (move-doc #:key outputs #:allow-other-keys)
   "Move info files from the ELPA package directory to the info directory."
-- 
2.12.2

bug#26559: [PATCH] build: emacs: Install only a subset of files.

[Arun Isaac]

In the new patchset, I have cleaned up and removed some of the regexps,
as you suggested. I have also rewritten the logic using `find-files'
rather than with `ftw'.

> Note, however, that 'move-doc' procedure should be adjusted to find
> info in "doc" subdir (for "doc/*.info$" regex).

I don't think the `move-doc' phase needs to be changed. It finds .info
files with `find-files' which is recursive.

> It doesn't look right that these regexps are duplicated in 2 places.
> I'm not very familiar with build systems, but what if the
> 'include'/'exclude' arguments of 'install' procedure would simply be
> empty lists?  I think it wouldn't do harm if you leave these regexps
> only in 'emacs-build' procedure or would it?

I am not too familiar with build systems, but I think the
include/exclude arguments need to be duplicated in two places. For
example, look at arguments #:strip-flags and #:strip-directories in the
`strip' phase of the gnu-build-system. Even there, the default values of
the arguments are repeated in two places.

> I think it would be too much work.  I quickly looked at the emacs
> packages, and I believe that only slime, auctex and yasnippet need to be
> adjusted to include non-standard files.  Of course, there may be other
> packages that I'm not aware of, but they can be fixed later.

I've provided patches for emacs-slime and emacs-auctex packages as
well. I believe I got all the required files, but please check.

As it stands, emacs-yasnippet does not seem to need any changes. But, I
think the package is already broken. It needs a snippets directory
pulled from a git submodule. And, even the current package does not pull
this submodule. So, I believe the yasnippet package is already
broken. But, I don't use yasnippet, and I'm not too sure.

bug#26559: [PATCH] build: emacs: Install only a subset of files.

[Alex Kost]

Arun Isaac (2017-04-26 19:50 +0530) wrote:

> In the new patchset, I have cleaned up and removed some of the regexps,
> as you suggested. I have also rewritten the logic using `find-files'
> rather than with `ftw'.

Great, thanks!

>> Note, however, that 'move-doc' procedure should be adjusted to find
>> info in "doc" subdir (for "doc/*.info$" regex).
>
> I don't think the `move-doc' phase needs to be changed. It finds .info
> files with `find-files' which is recursive.

Oh, right.

>> It doesn't look right that these regexps are duplicated in 2 places.
>> I'm not very familiar with build systems, but what if the
>> 'include'/'exclude' arguments of 'install' procedure would simply be
>> empty lists?  I think it wouldn't do harm if you leave these regexps
>> only in 'emacs-build' procedure or would it?
>
> I am not too familiar with build systems, but I think the
> include/exclude arguments need to be duplicated in two places. For
> example, look at arguments #:strip-flags and #:strip-directories in the
> `strip' phase of the gnu-build-system. Even there, the default values of
> the arguments are repeated in two places.

Hm, ok, although I still think that these arguments are not needed to be
duplicated in 'install' procedure, but I'm not the one to judge about it.

>> I think it would be too much work.  I quickly looked at the emacs
>> packages, and I believe that only slime, auctex and yasnippet need to be
>> adjusted to include non-standard files.  Of course, there may be other
>> packages that I'm not aware of, but they can be fixed later.
>
> I've provided patches for emacs-slime and emacs-auctex packages as
> well. I believe I got all the required files, but please check.

I personally don't use these packages, but I think they should be OK
now, thank you!

> As it stands, emacs-yasnippet does not seem to need any changes. But, I
> think the package is already broken. It needs a snippets directory
> pulled from a git submodule.

Oh, right; last time I checked "yasnippet" (several years ago) this
"snippets" directory was a part of the repo.

> And, even the current package does not pull
> this submodule. So, I believe the yasnippet package is already
> broken. But, I don't use yasnippet, and I'm not too sure.

Yeah, you are probably right, without snippets, yasnippet is… well,
let's say limited.  But let's leave this problem for those who use
yasnippet :-)

Thank you for this work!  I hope someone else will look at this thread
and will say "OK".  As for me, I don't have any further comments and I
think it is ready to be committed.  In the worst case there would be a
couple of broken emacs packages but they could be easily fixed.

-- 
Alex

bug#26559: [PATCH 1/3] build: emacs: Install only a subset of files.

[Arun Isaac]

* guix/build/emacs-build-system.scm (install): Install files matching
  #:include while excluding files matching #:exclude.
* guix/build-system/emacs.scm (emacs-build): Add keyword arguments #:include
  and #:exclude.
---
 guix/build-system/emacs.scm       |  4 ++++
 guix/build/emacs-build-system.scm | 25 +++++++++++++++++++++----
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/guix/build-system/emacs.scm b/guix/build-system/emacs.scm
index a7982002b..9a46ecfd2 100644
--- a/guix/build-system/emacs.scm
+++ b/guix/build-system/emacs.scm
@@ -83,6 +83,8 @@
                       (phases '(@ (guix build emacs-build-system)
                                   %standard-phases))
                       (outputs '("out"))
+                      (include ''("^[^/]*\\.el$" "^[^/]*\\.info$" "^doc/.*\\.info$"))
+                      (exclude ''("^\\.dir-locals\\.el$" "-pkg\\.el$" "^[^/]*tests?\\.el$"))
                       (search-paths '())
                       (system (%current-system))
                       (guile #f)
@@ -108,6 +110,8 @@
                     #:tests? ,tests?
                     #:phases ,phases
                     #:outputs %outputs
+                    #:include ,include
+                    #:exclude ,exclude
                     #:search-paths ',(map search-path-specification->sexp
                                           search-paths)
                     #:inputs %build-inputs)))
diff --git a/guix/build/emacs-build-system.scm b/guix/build/emacs-build-system.scm
index 44e8b0d31..fefdbb96e 100644
--- a/guix/build/emacs-build-system.scm
+++ b/guix/build/emacs-build-system.scm
@@ -28,6 +28,7 @@
   #:use-module (ice-9 rdelim)
   #:use-module (ice-9 regex)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 ftw)
   #:export (%standard-phases
             emacs-build))
 
@@ -93,14 +94,30 @@ store in '.el' files."
           (substitute-cmd))))
     #t))
 
-(define* (install #:key outputs #:allow-other-keys)
+(define* (install #:key outputs
+                  (include '("^[^/]*\\.el$" "^[^/]*\\.info$" "^doc/.*\\.info$"))
+                  (exclude '("^\\.dir-locals\\.el$" "-pkg\\.el$" "^[^/]*tests?\\.el$"))
+                  #:allow-other-keys)
   "Install the package contents."
+
+  (define src-dir (getcwd))
+
+  (define (install-file? file stat)
+    (let ((stripped-file (string-trim (substring file (string-length src-dir)) #\/)))
+      (and (any (cut string-match <> stripped-file) include)
+           (not (any (cut string-match <> stripped-file) exclude)))))
+
   (let* ((out (assoc-ref outputs "out"))
          (elpa-name-ver (store-directory->elpa-name-version out))
-         (src-dir (getcwd))
          (tgt-dir (string-append out %install-suffix "/" elpa-name-ver)))
-    (copy-recursively src-dir tgt-dir)
-    #t))
+    (for-each
+     (lambda (file)
+       (let* ((stripped-file (substring file (string-length src-dir)))
+              (tgt-file (string-append tgt-dir stripped-file)))
+         (format #t "`~a' -> `~a'~%" file tgt-file)
+         (install-file file (dirname tgt-file))))
+     (find-files src-dir install-file?)))
+  #t)
 
 (define* (move-doc #:key outputs #:allow-other-keys)
   "Move info files from the ELPA package directory to the info directory."
-- 
2.12.2

bug#26559: [PATCH] build: emacs: Install only a subset of files.

[Arun Isaac]

I've sent a new patchset. Now, we just have to wait for someone else to
look at this thread...

bug#26559: [PATCH] build: emacs: Install only a subset of files.

[Arun Isaac]

Ludo said (in another thread) that we should prefer `string-drop' to
`substring'. I'll make that change along with any others suggested by
others who review this patch.

bug#26559: [PATCH] build: emacs: Install only a subset of files.

[Alex Kost]

Arun Isaac (2017-05-05 00:09 +0530) wrote:

> I've sent a new patchset. Now, we just have to wait for someone else to
> look at this thread...

This thread is already 1 month old, IMO it's not necessary to wait any
longer.  I think you can just commit these patches.  If there will be
some breakage caused by this change, we will fix it.

-- 
Alex

bug#26559: [PATCH] build: emacs: Install only a subset of files.

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> Arun Isaac (2017-05-05 00:09 +0530) wrote:
>
>> I've sent a new patchset. Now, we just have to wait for someone else to
>> look at this thread...
>
> This thread is already 1 month old, IMO it's not necessary to wait any
> longer.  I think you can just commit these patches.  If there will be
> some breakage caused by this change, we will fix it.

I agree with this strategy!

Thanks,
Ludo’.

bug#26559: [PATCH] build: emacs: Install only a subset of files.

[Arun Isaac]

Pushed with a few minor changes!