[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#27222: [PATCH] emacs-build-system install phase doesn't honor direct
From: |
Maxim Cournoyer |
Subject: |
bug#27222: [PATCH] emacs-build-system install phase doesn't honor directory hierarchy |
Date: |
Sun, 04 Jun 2017 22:07:16 -0700 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) |
Hello,
Arun Isaac <address@hidden> 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 <address@hidden>
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 <address@hidden>
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
bug#27222: [PATCH] Fix ert-runner regression (was: emacs-build-system install phase doesn't honor directory hierarchy), Maxim Cournoyer, 2017/06/06