Hello, Arun Isaac <arunis...@systemreboot.net> writes:
>> As far as I understand it, it was done for purpose: some packages >> include "uninteresting" (for tests, maintenance, etc.) *.el files in >> subdirs, that's why they are excluded by default. So probably a better >> solution would be to fix 'ert-runner' package (as it is done in commit >> b1d32ec0e23bfec1dab4c56909228a494b2b0d60, for example). WDYT? > > I agree. The solution is to fix the ert-runner package, not the > emacs-build-system. > >> This change also doesn't prevent excluding subfolders if they are truly >> unnecessary (such as tests subfolder), but this should happen due to >> explicit regexp in the exclude option, not because *all* subfolders are >> excluded. > > We adopted the policy of excluding *all* subfolders from MELPA. From > their "Recipe Format" section at https://github.com/melpa/melpa > > "Note that elisp in subdirectories is never included by default, so you > might find it convenient to separate auxiliiary files such as tests into > subdirectories to keep packaging simple." Oh. I didn't know MELPA had such a policy. This is a good point. It's nice to try to stick to whatever MELPA does, as they've pretty much become the authority in Elisp packaging IIUC. > I think this is a good policy. If we include subfolders by default, > we'll have to modify many packages with #:exclude arguments to get rid > of unnecessary subfolders. However, if we exclude subfolders by default, > we'll only have to modify fewer packages with #:include arguments. Although, for the sake of cleanliness, they enforce a project layout that discourage the organization of a project in subdirectories, which I find a bit strange. But if the lack of complaints about it in the MELPA issues tracker tells anything, it doesn't seem to be much of a bother for most projects (this might have to do with the fact that until recently most Elisp projects were organized as a single file of thousands of lines of code ;). >> I also think these arguments are redundant! I suggested to remove this >> duplication at: >> >> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=26559#41 > > And, I did respond at > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=26559#53 > >> ... 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. > Do you know of some way in which we can avoid duplication of the > arguments? Even the gnu-build-system duplicates default values of > arguments. I've decided to go with the flow and modify ert-runner so that it includes the elisp files under the 'reporters' subdirectory. I've also factorized out the default args of the include and exclude option of the emacs-build-system install phase. Please see the two patches attached. Maxim
From 626eb2b0551aee16836b7ec796a8ad1759be5a52 Mon Sep 17 00:00:00 2001 From: Maxim Cournoyer <maxim.courno...@gmail.com> Date: Sat, 3 Jun 2017 23:43:02 -0700 Subject: [PATCH 1/2] build-system: emacs: Factorize include/exclude default args The `install' phase of the emac-build-system builder side contained arguments duplicated from the higer level `emacs-build' procedure. This change factorizes them so that: 1. They are not duplicated; 2. They can be reused and extended easily when defining Emacs packages. * guix/build/emacs-build-system.scm (%default-include): New symbol. (%default-exclude): Likewise. (install)[include]: Use %default-include variable. [exclude]: Use %default exclude variable. --- guix/build-system/emacs.scm | 11 ++++++++--- guix/build/emacs-build-system.scm | 11 +++++++++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/guix/build-system/emacs.scm b/guix/build-system/emacs.scm index 9a46ecfd2..02296829c 100644 --- a/guix/build-system/emacs.scm +++ b/guix/build-system/emacs.scm @@ -17,6 +17,8 @@ ;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>. (define-module (guix build-system emacs) + #:use-module ((guix build emacs-build-system) + #:select (%default-include %default-exclude)) #:use-module (guix store) #:use-module (guix utils) #:use-module (guix packages) @@ -28,7 +30,10 @@ #:use-module (srfi srfi-26) #:export (%emacs-build-system-modules emacs-build - emacs-build-system)) + emacs-build-system) + #:re-export (%default-include ;for convenience + %default-exclude)) + ;; Commentary: ;; @@ -83,8 +88,8 @@ (phases '(@ (guix build emacs-build-system) %standard-phases)) (outputs '("out")) - (include ''("^[^/]*\\.el$" "^[^/]*\\.info$" "^doc/.*\\.info$")) - (exclude ''("^\\.dir-locals\\.el$" "-pkg\\.el$" "^[^/]*tests?\\.el$")) + (include (quote %default-include)) + (exclude (quote %default-exclude)) (search-paths '()) (system (%current-system)) (guile #f) diff --git a/guix/build/emacs-build-system.scm b/guix/build/emacs-build-system.scm index 50af4be36..bda699ddf 100644 --- a/guix/build/emacs-build-system.scm +++ b/guix/build/emacs-build-system.scm @@ -29,6 +29,8 @@ #:use-module (ice-9 regex) #:use-module (ice-9 match) #:export (%standard-phases + %default-include + %default-exclude emacs-build)) ;; Commentary: @@ -42,6 +44,11 @@ ;; archive signature. (define %install-suffix "/share/emacs/site-lisp/guix.d") +;; These are the default inclusion/exclusion regexps for the install phase. +(define %default-include '("^[^/]*\\.el$" "^[^/]*\\.info$" "^doc/.*\\.info$")) +(define %default-exclude '("^\\.dir-locals\\.el$" "-pkg\\.el$" + "^[^/]*tests?\\.el$")) + (define gnu:unpack (assoc-ref gnu:%standard-phases 'unpack)) (define (store-file->elisp-source-file file) @@ -96,8 +103,8 @@ store in '.el' files." #t)) (define* (install #:key outputs - (include '("^[^/]*\\.el$" "^[^/]*\\.info$" "^doc/.*\\.info$")) - (exclude '("^\\.dir-locals\\.el$" "-pkg\\.el$" "^[^/]*tests?\\.el$")) + (include %default-include) + (exclude %default-exclude) #:allow-other-keys) "Install the package contents." -- 2.13.0
From ffb86810fe945a6cded4b5363ef0b8ce4ea58d02 Mon Sep 17 00:00:00 2001
From: Maxim Cournoyer <maxim.courno...@gmail.com>
Date: Sun, 4 Jun 2017 20:57:03 -0700
Subject: [PATCH 2/2] gnu: emacs: Fix ert-runner by adding 'reporters' subdir
Previous this change, ert-runner would fail with error: "Invalid reporter: dot".
* gnu/packages/emacs.scm (ert-runner)[include]: Add regexp to match elisp
files under the 'reporters' subdirectory.
---
gnu/packages/emacs.scm | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
index 81a74d1fb..52118099e 100644
--- a/gnu/packages/emacs.scm
+++ b/gnu/packages/emacs.scm
@@ -4814,7 +4814,8 @@ Emacs.")
;; determined by emacs' standard initialization
;; procedure
(list ""))))
- #t))))))
+ #t))))
+ #:include (cons* "^reporters/.*\\.el$" %default-include)))
(home-page "https://github.com/rejeep/ert-runner.el";)
(synopsis "Opinionated Ert testing workflow")
(description "@code{ert-runner} is a tool for Emacs projects tested
--
2.13.0
signature.asc
Description: PGP signature
