all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#27222: emacs-build-system install phase doesn't honor directory hierarchy
@ 2017-06-03 23:02 Maxim Cournoyer
  2017-06-04  6:53 ` bug#27222: [PATCH] " Maxim Cournoyer
  2017-06-06 16:35 ` bug#27222: [PATCH] Fix ert-runner regression (was: emacs-build-system install phase doesn't honor directory hierarchy) Maxim Cournoyer
  0 siblings, 2 replies; 18+ messages in thread
From: Maxim Cournoyer @ 2017-06-03 23:02 UTC (permalink / raw)
  To: 27222

It seems the recent changes made to the emacs-build-system broke
ert-runner; it always output the following:

Invalid reporter: dot

A quick investigation revealed that ert-runner.el is looking for
reporters under the reporters/ sudbdirectory. Here's the arborescence in
the original source:

--8<---------------cut here---------------start------------->8---
find . -name '*.el'
./reporters/ert-runner-reporter-ert.el
./reporters/ert-runner-reporter-dot.el
./features/step-definitions/ert-runner-steps.el
./features/support/env.el
./ert-runner.el
./ert-compat.el
--8<---------------cut here---------------end--------------->8---

And here's what got installed in the store:

--8<---------------cut here---------------start------------->8---
find . -name '*.el'
./ert-runner-autoloads.el
./ert-runner.el
./ert-compat.el
--8<---------------cut here---------------end--------------->8---

I'm guessing the current inclusion regexp:
(include '("^[^/]*\\.el$" "^[^/]*\\.info$" "^doc/.*\\.info$"))

must be extended/adapted.

What should "^[^/]*\\.el$" become?

I've tried ".*\\.el" without success.

Ideas?

Maxim

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#27222: [PATCH] emacs-build-system install phase doesn't honor directory hierarchy
  2017-06-03 23:02 bug#27222: emacs-build-system install phase doesn't honor directory hierarchy Maxim Cournoyer
@ 2017-06-04  6:53 ` Maxim Cournoyer
  2017-06-04 12:59   ` Alex Kost
  2017-06-06 16:35 ` bug#27222: [PATCH] Fix ert-runner regression (was: emacs-build-system install phase doesn't honor directory hierarchy) Maxim Cournoyer
  1 sibling, 1 reply; 18+ messages in thread
From: Maxim Cournoyer @ 2017-06-04  6:53 UTC (permalink / raw)
  To: 27222

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

Hello,

The previous regexp would have worked, but what got me was that the
default keyword arguments values were duplicated (and I was fixing the
useless version in build/emac-build-system.scm instead of the one used
in build-system/emacs.scm).

The attached patch fixes this particular problem (tested with
ert-runner).

Thanks,

Maxim


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-build-system-emacs-Install-elisp-files-from-subdirec.patch --]
[-- Type: text/x-patch, Size: 2028 bytes --]

From a035d07dfa6cbddccfa0476e2009d19bdf296941 Mon Sep 17 00:00:00 2001
From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Date: Sat, 3 Jun 2017 23:43:02 -0700
Subject: [PATCH] build-system: emacs: Install elisp files from subdirectories

* guix/build/emacs-build-system.scm (install)[include]: Get rid of default
value.
[exclude]: Likewise.
* guix/build/emacs-build-system.scm (emacs-build)[include]: Modify default
regexp value so that elisp files get matched (and installed) for any directory
depth level.
---
 guix/build-system/emacs.scm       | 2 +-
 guix/build/emacs-build-system.scm | 5 +----
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/guix/build-system/emacs.scm b/guix/build-system/emacs.scm
index 9a46ecfd2..a97fcedc3 100644
--- a/guix/build-system/emacs.scm
+++ b/guix/build-system/emacs.scm
@@ -83,7 +83,7 @@
                       (phases '(@ (guix build emacs-build-system)
                                   %standard-phases))
                       (outputs '("out"))
-                      (include ''("^[^/]*\\.el$" "^[^/]*\\.info$" "^doc/.*\\.info$"))
+                      (include ''("\\.el$" "^[^/]*\\.info$" "^doc/.*\\.info$"))
                       (exclude ''("^\\.dir-locals\\.el$" "-pkg\\.el$" "^[^/]*tests?\\.el$"))
                       (search-paths '())
                       (system (%current-system))
diff --git a/guix/build/emacs-build-system.scm b/guix/build/emacs-build-system.scm
index 50af4be36..1373cb6f7 100644
--- a/guix/build/emacs-build-system.scm
+++ b/guix/build/emacs-build-system.scm
@@ -95,10 +95,7 @@ store in '.el' files."
           (substitute-cmd))))
     #t))
 
-(define* (install #:key outputs
-                  (include '("^[^/]*\\.el$" "^[^/]*\\.info$" "^doc/.*\\.info$"))
-                  (exclude '("^\\.dir-locals\\.el$" "-pkg\\.el$" "^[^/]*tests?\\.el$"))
-                  #:allow-other-keys)
+(define* (install #:key outputs include exclude #:allow-other-keys)
   "Install the package contents."
 
   (define source (getcwd))
-- 
2.13.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* bug#27222: [PATCH] emacs-build-system install phase doesn't honor directory hierarchy
  2017-06-04  6:53 ` bug#27222: [PATCH] " Maxim Cournoyer
@ 2017-06-04 12:59   ` Alex Kost
  2017-06-04 16:44     ` Maxim Cournoyer
                       ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Alex Kost @ 2017-06-04 12:59 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 27222

I Cc-ed Arun, the author of the mentioned change (commit
d879685176d23c111f4fc665698251b25cdf9124).

[...]
> From a035d07dfa6cbddccfa0476e2009d19bdf296941 Mon Sep 17 00:00:00 2001
> From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
> Date: Sat, 3 Jun 2017 23:43:02 -0700
> Subject: [PATCH] build-system: emacs: Install elisp files from subdirectories
>
> * guix/build/emacs-build-system.scm (install)[include]: Get rid of default
> value.
> [exclude]: Likewise.
> * guix/build/emacs-build-system.scm (emacs-build)[include]: Modify default
> regexp value so that elisp files get matched (and installed) for any directory
> depth level.
> ---
>  guix/build-system/emacs.scm       | 2 +-
>  guix/build/emacs-build-system.scm | 5 +----
>  2 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/guix/build-system/emacs.scm b/guix/build-system/emacs.scm
> index 9a46ecfd2..a97fcedc3 100644
> --- a/guix/build-system/emacs.scm
> +++ b/guix/build-system/emacs.scm
> @@ -83,7 +83,7 @@
>                        (phases '(@ (guix build emacs-build-system)
>                                    %standard-phases))
>                        (outputs '("out"))
> -                      (include ''("^[^/]*\\.el$" "^[^/]*\\.info$" "^doc/.*\\.info$"))
> +                      (include ''("\\.el$" "^[^/]*\\.info$" "^doc/.*\\.info$"))

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?

>                        (exclude ''("^\\.dir-locals\\.el$" "-pkg\\.el$" "^[^/]*tests?\\.el$"))
>                        (search-paths '())
>                        (system (%current-system))
> diff --git a/guix/build/emacs-build-system.scm b/guix/build/emacs-build-system.scm
> index 50af4be36..1373cb6f7 100644
> --- a/guix/build/emacs-build-system.scm
> +++ b/guix/build/emacs-build-system.scm
> @@ -95,10 +95,7 @@ store in '.el' files."
>            (substitute-cmd))))
>      #t))
>  
> -(define* (install #:key outputs
> -                  (include '("^[^/]*\\.el$" "^[^/]*\\.info$" "^doc/.*\\.info$"))
> -                  (exclude '("^\\.dir-locals\\.el$" "-pkg\\.el$" "^[^/]*tests?\\.el$"))
> -                  #:allow-other-keys)
> +(define* (install #:key outputs include exclude #:allow-other-keys)
>    "Install the package contents."

I also think these arguments are redundant!  I suggested to remove this
duplication at:

  https://debbugs.gnu.org/cgi/bugreport.cgi?bug=26559#41

-- 
Alex

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#27222: [PATCH] emacs-build-system install phase doesn't honor directory hierarchy
  2017-06-04 12:59   ` Alex Kost
@ 2017-06-04 16:44     ` Maxim Cournoyer
  2017-06-04 19:41       ` Alex Kost
  2017-06-04 19:25     ` Arun Isaac
       [not found]     ` <0efe58d4.AEUAK47aAUsAAAAAAAAAAAO2CsUAAAACwQwAAAAAAAW9WABZNF6l@mailjet.com>
  2 siblings, 1 reply; 18+ messages in thread
From: Maxim Cournoyer @ 2017-06-04 16:44 UTC (permalink / raw)
  To: Alex Kost; +Cc: 27222

Hello,

Alex Kost <alezost@gmail.com> writes:

> I Cc-ed Arun, the author of the mentioned change (commit
> d879685176d23c111f4fc665698251b25cdf9124).
>
> [...]
>> From a035d07dfa6cbddccfa0476e2009d19bdf296941 Mon Sep 17 00:00:00 2001
>> From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
>> Date: Sat, 3 Jun 2017 23:43:02 -0700
>> Subject: [PATCH] build-system: emacs: Install elisp files from subdirectories
>>
>> * guix/build/emacs-build-system.scm (install)[include]: Get rid of default
>> value.
>> [exclude]: Likewise.
>> * guix/build/emacs-build-system.scm (emacs-build)[include]: Modify default
>> regexp value so that elisp files get matched (and installed) for any directory
>> depth level.
>> ---
>>  guix/build-system/emacs.scm       | 2 +-
>>  guix/build/emacs-build-system.scm | 5 +----
>>  2 files changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/guix/build-system/emacs.scm b/guix/build-system/emacs.scm
>> index 9a46ecfd2..a97fcedc3 100644
>> --- a/guix/build-system/emacs.scm
>> +++ b/guix/build-system/emacs.scm
>> @@ -83,7 +83,7 @@
>>                        (phases '(@ (guix build emacs-build-system)
>>                                    %standard-phases))
>>                        (outputs '("out"))
>> -                      (include ''("^[^/]*\\.el$" "^[^/]*\\.info$" "^doc/.*\\.info$"))
>> +                      (include ''("\\.el$" "^[^/]*\\.info$" "^doc/.*\\.info$"))
>
> 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 acknowledge the intent, but I think the default set of rexgeps
should be more lenient; filtering (inoffensive) files is desirable, but
not at the cost of breaking perfectly valid packages. That's one extra
hurdle the packagers shouldn't have to bear in my opinion.

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.

What do you think?

Maxim

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#27222: [PATCH] emacs-build-system install phase doesn't honor directory hierarchy
  2017-06-04 12:59   ` Alex Kost
  2017-06-04 16:44     ` Maxim Cournoyer
@ 2017-06-04 19:25     ` Arun Isaac
  2017-06-05  5:07       ` Maxim Cournoyer
       [not found]     ` <0efe58d4.AEUAK47aAUsAAAAAAAAAAAO2CsUAAAACwQwAAAAAAAW9WABZNF6l@mailjet.com>
  2 siblings, 1 reply; 18+ messages in thread
From: Arun Isaac @ 2017-06-04 19:25 UTC (permalink / raw)
  To: Alex Kost; +Cc: Maxim Cournoyer, 27222


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

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.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#27222: [PATCH] emacs-build-system install phase doesn't honor directory hierarchy
  2017-06-04 16:44     ` Maxim Cournoyer
@ 2017-06-04 19:41       ` Alex Kost
  0 siblings, 0 replies; 18+ messages in thread
From: Alex Kost @ 2017-06-04 19:41 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 27222

Maxim Cournoyer (2017-06-04 09:44 -0700) wrote:

[...]
>>>                        (phases '(@ (guix build emacs-build-system)
>>>                                    %standard-phases))
>>>                        (outputs '("out"))
>>> -                      (include ''("^[^/]*\\.el$" "^[^/]*\\.info$" "^doc/.*\\.info$"))
>>> +                      (include ''("\\.el$" "^[^/]*\\.info$" "^doc/.*\\.info$"))
>>
>> 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 acknowledge the intent, but I think the default set of rexgeps
> should be more lenient; filtering (inoffensive) files is desirable, but
> not at the cost of breaking perfectly valid packages. That's one extra
> hurdle the packagers shouldn't have to bear in my opinion.
>
> 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.
>
> What do you think?

I think my view is not what most people would like: I am for excluding
as much as possible (as it is now), and for manual adjusting packages
when it is needed.

-- 
Alex

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#27222: [PATCH] emacs-build-system install phase doesn't honor directory hierarchy
  2017-06-04 19:25     ` Arun Isaac
@ 2017-06-05  5:07       ` Maxim Cournoyer
  2017-06-05 10:03         ` Arun Isaac
                           ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Maxim Cournoyer @ 2017-06-05  5:07 UTC (permalink / raw)
  To: Arun Isaac; +Cc: Alex Kost, 27222


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

Hello,

Arun Isaac <arunisaac@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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-build-system-emacs-Factorize-include-exclude-default.patch --]
[-- Type: text/x-patch, Size: 3709 bytes --]

From 626eb2b0551aee16836b7ec796a8ad1759be5a52 Mon Sep 17 00:00:00 2001
From: Maxim Cournoyer <maxim.cournoyer@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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.3: 0002-gnu-emacs-Fix-ert-runner-by-adding-reporters-subdir.patch --]
[-- Type: text/x-patch, Size: 1210 bytes --]

From ffb86810fe945a6cded4b5363ef0b8ce4ea58d02 Mon Sep 17 00:00:00 2001
From: Maxim Cournoyer <maxim.cournoyer@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


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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* bug#27222: [PATCH] emacs-build-system install phase doesn't honor directory hierarchy
  2017-06-05  5:07       ` Maxim Cournoyer
@ 2017-06-05 10:03         ` Arun Isaac
       [not found]         ` <73a30871.AEQALMWm9gcAAAAAAAAAAAPHPfsAAAACwQwAAAAAAAW9WABZNSx9@mailjet.com>
       [not found]         ` <a444cf1b.AEQALMWm9gYAAAAAAAAAAAO2CsUAAAACwQwAAAAAAAW9WABZNSx9@mailjet.com>
  2 siblings, 0 replies; 18+ messages in thread
From: Arun Isaac @ 2017-06-05 10:03 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: Alex Kost, 27222


> 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
> ;).

Exactly! :-)

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

Thanks! Both patches LGTM. I just have a few doubts and minor comments.

>  (define-module (guix build-system emacs)
> +  #:use-module ((guix build emacs-build-system)
> +                #:select (%default-include %default-exclude))

Interesting solution to the problem. I didn't think of this
earlier. Importing host side to build side would be a problem, but you
are importing build side to host side, and I think that's ok.

>    #: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))

What is this re-export for? ert-runner seems to build fine without
this. I don't understand what you mean by "convenience" here.

Also, perhaps we should deduplicate default values of arguments in the
gnu-build-system as well. But, I don't know if it is worth deduplicating
simple default values like #t. WDYT?

@Alex Kost:

Please make any other changes you think are necessary, and push. Thanks!

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#27222: [PATCH] emacs-build-system install phase doesn't honor directory hierarchy
       [not found]         ` <73a30871.AEQALMWm9gcAAAAAAAAAAAPHPfsAAAACwQwAAAAAAAW9WABZNSx9@mailjet.com>
@ 2017-06-05 14:54           ` Maxim Cournoyer
  0 siblings, 0 replies; 18+ messages in thread
From: Maxim Cournoyer @ 2017-06-05 14:54 UTC (permalink / raw)
  To: Arun Isaac; +Cc: Alex Kost, 27222

Hello Arun,

Arun Isaac <arunisaac@systemreboot.net> writes:

>> 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
>> ;).
>
> Exactly! :-)
>
>> 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.
>
> Thanks! Both patches LGTM. I just have a few doubts and minor comments.
>

Thanks for the prompt review!

>>  (define-module (guix build-system emacs)
>> +  #:use-module ((guix build emacs-build-system)
>> +                #:select (%default-include %default-exclude))
>
> Interesting solution to the problem. I didn't think of this
> earlier. Importing host side to build side would be a problem, but you
> are importing build side to host side, and I think that's ok.
>
>>    #: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))
>
> What is this re-export for? ert-runner seems to build fine without
> this. I don't understand what you mean by "convenience" here.

If you take another look at the second patch I sent, you'll see I build
the include argument value of the emacs-build-sysltem by consing the new
regexp in front of %default-include instead of building it from
scratch. That's what these re-exports are for: they enable their use in
gnu/packages/emacs.scm (and anywhere we've imported (guix build-system emacs).

> Also, perhaps we should deduplicate default values of arguments in the
> gnu-build-system as well. But, I don't know if it is worth deduplicating
> simple default values like #t. WDYT?

There's not much value in deduplicating simple things such as a boolean
flag, but for more complicated ones it could be useful; although it
would be very nice if instead of having to do this Scheme/Guile offered
something for "querying" the defaults values of function. One could then
go and extend the defaults rather than override them them in a similar
way to what we can do with records.

Maxim

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#27222: [PATCH] emacs-build-system install phase doesn't honor directory hierarchy
       [not found]     ` <0efe58d4.AEUAK47aAUsAAAAAAAAAAAO2CsUAAAACwQwAAAAAAAW9WABZNF6l@mailjet.com>
@ 2017-06-05 20:07       ` Alex Kost
  2017-06-06 17:44         ` Arun Isaac
  0 siblings, 1 reply; 18+ messages in thread
From: Alex Kost @ 2017-06-05 20:07 UTC (permalink / raw)
  To: Arun Isaac; +Cc: Maxim Cournoyer, 27222

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

[...]
>> 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

Sure, I didn't mean I was ignored, I just wanted to say that I got the
same thought about those arguments as Maxim.

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

As I said, I think this duplication can be avoided simply by removing
the values of 'include'/'exclude' arguments from 'install' procedures of
(guix build emacs-build-system) module.

> Even the gnu-build-system duplicates default values of arguments.

I don't know about gnu-build-system, but I don't see a reason to have
these values in 'install' procedure of emacs-build-system.

-- 
Alex

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#27222: [PATCH] emacs-build-system install phase doesn't honor directory hierarchy
       [not found]         ` <a444cf1b.AEQALMWm9gYAAAAAAAAAAAO2CsUAAAACwQwAAAAAAAW9WABZNSx9@mailjet.com>
@ 2017-06-05 20:13           ` Alex Kost
  2017-06-08 14:31             ` Arun Isaac
       [not found]             ` <ad8c9523.AEUALD4wqa8AAAAAAAAAAAPHPfsAAAACwQwAAAAAAAW9WABZOV_H@mailjet.com>
  0 siblings, 2 replies; 18+ messages in thread
From: Alex Kost @ 2017-06-05 20:13 UTC (permalink / raw)
  To: Arun Isaac; +Cc: Maxim Cournoyer, 27222

Arun Isaac (2017-06-05 15:33 +0530) wrote:

> Please make any other changes you think are necessary, and push. Thanks!

Thanks, but I'm afraid I'm not competent to judge about the first
patch.  I don't really know what is used on build side and on host side,
and whether these things can be mixed like that.

-- 
Alex

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#27222: [PATCH] Fix ert-runner regression (was: emacs-build-system install phase doesn't honor directory hierarchy)
  2017-06-03 23:02 bug#27222: emacs-build-system install phase doesn't honor directory hierarchy Maxim Cournoyer
  2017-06-04  6:53 ` bug#27222: [PATCH] " Maxim Cournoyer
@ 2017-06-06 16:35 ` Maxim Cournoyer
  2017-06-06 23:02   ` bug#27222: [PATCH] Fix ert-runner regression Ludovic Courtès
  1 sibling, 1 reply; 18+ messages in thread
From: Maxim Cournoyer @ 2017-06-06 16:35 UTC (permalink / raw)
  To: ludo; +Cc: alezost, 27222

Hello Ludovic,

Alex Kost (2017-06-05) wrote:

> Arun Isaac (2017-06-05) wrote:

> > Please make any other changes you think are necessary, and push. Thanks!

> Thanks, but I'm afraid I'm not competent to judge about the first
> patch.  I don't really know what is used on build side and on host side,
> and whether these things can be mixed like that.

> -- 
> Alex

Could you please have a quick look at the small change I did in the
first patch at https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27222#23?
If it is sane, feel free to merge those (as suggested by Arun) and close
this bug :)

Thanks,

Maxim

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#27222: [PATCH] emacs-build-system install phase doesn't honor directory hierarchy
  2017-06-05 20:07       ` Alex Kost
@ 2017-06-06 17:44         ` Arun Isaac
  0 siblings, 0 replies; 18+ messages in thread
From: Arun Isaac @ 2017-06-06 17:44 UTC (permalink / raw)
  To: Alex Kost; +Cc: Maxim Cournoyer, 27222


Alex Kost writes:

> Sure, I didn't mean I was ignored, I just wanted to say that I got the
> same thought about those arguments as Maxim.

No hard feelings! :-) debbugs can be a pain to read. I thought you
missed that message from me.

> As I said, I think this duplication can be avoided simply by removing
> the values of 'include'/'exclude' arguments from 'install' procedures of
> (guix build emacs-build-system) module.

I tried something similar when I wrote the original patch, but I
couldn't get it working. Probably something to do with all the quotes
and build side/host side stuff. I didn't pursue it further when I
noticed that even the gnu-build-system duplicates default arguments.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#27222: [PATCH] Fix ert-runner regression
  2017-06-06 16:35 ` bug#27222: [PATCH] Fix ert-runner regression (was: emacs-build-system install phase doesn't honor directory hierarchy) Maxim Cournoyer
@ 2017-06-06 23:02   ` Ludovic Courtès
  2017-06-07 10:28     ` Arun Isaac
  0 siblings, 1 reply; 18+ messages in thread
From: Ludovic Courtès @ 2017-06-06 23:02 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: alezost, 27222

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> Alex Kost (2017-06-05) wrote:
>
>> Arun Isaac (2017-06-05) wrote:
>
>> > Please make any other changes you think are necessary, and push. Thanks!
>
>> Thanks, but I'm afraid I'm not competent to judge about the first
>> patch.  I don't really know what is used on build side and on host side,
>> and whether these things can be mixed like that.
>
>> -- 
>> Alex
>
> Could you please have a quick look at the small change I did in the
> first patch at https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27222#23?
> If it is sane, feel free to merge those (as suggested by Arun) and close
> this bug :)

Sorry I haven’t closely followed this discussion.  Arun: Could you merge
it if that looks good to you?  I trust your judgment, comrades.  :-)

TIA!

Ludo’.

PS: Apologies, I’ll be less responsive over the next few days.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#27222: [PATCH] Fix ert-runner regression
  2017-06-06 23:02   ` bug#27222: [PATCH] Fix ert-runner regression Ludovic Courtès
@ 2017-06-07 10:28     ` Arun Isaac
  2017-06-07 20:11       ` Alex Kost
  0 siblings, 1 reply; 18+ messages in thread
From: Arun Isaac @ 2017-06-07 10:28 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: alezost, Maxim Cournoyer, 27222


> Sorry I haven’t closely followed this discussion.  Arun: Could you merge
> it if that looks good to you?  I trust your judgment, comrades.  :-)

The patches are fine by me. Alex Kost is reviewing this patch. So, I
think he should push. Thanks!

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#27222: [PATCH] Fix ert-runner regression
  2017-06-07 10:28     ` Arun Isaac
@ 2017-06-07 20:11       ` Alex Kost
  0 siblings, 0 replies; 18+ messages in thread
From: Alex Kost @ 2017-06-07 20:11 UTC (permalink / raw)
  To: Arun Isaac; +Cc: 27222, Maxim Cournoyer

Arun Isaac (2017-06-07 15:58 +0530) wrote:

>> Sorry I haven’t closely followed this discussion.  Arun: Could you merge
>> it if that looks good to you?  I trust your judgment, comrades.  :-)
>
> The patches are fine by me. Alex Kost is reviewing this patch. So, I
> think he should push. Thanks!

Wow, so if someone replies to some message, (s)he is considered to be a
reviewer?  I didn't realize this is so strict :-)

I just replied to the original Maxim's proposal to include elisp files
from subdirs.  After that, the patch changed completely, and I'm not
confident in the potential consequences, so I let other people judge on
that patch.

Arun, please commit it, if it looks good to you.

-- 
Alex

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#27222: [PATCH] emacs-build-system install phase doesn't honor directory hierarchy
  2017-06-05 20:13           ` Alex Kost
@ 2017-06-08 14:31             ` Arun Isaac
       [not found]             ` <ad8c9523.AEUALD4wqa8AAAAAAAAAAAPHPfsAAAACwQwAAAAAAAW9WABZOV_H@mailjet.com>
  1 sibling, 0 replies; 18+ messages in thread
From: Arun Isaac @ 2017-06-08 14:31 UTC (permalink / raw)
  To: Alex Kost; +Cc: Maxim Cournoyer, 27222-done


Pushed! :-)

Made a few minor modifications to the commit message...

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#27222: [PATCH] emacs-build-system install phase doesn't honor directory hierarchy
       [not found]             ` <ad8c9523.AEUALD4wqa8AAAAAAAAAAAPHPfsAAAACwQwAAAAAAAW9WABZOV_H@mailjet.com>
@ 2017-06-08 14:58               ` Maxim Cournoyer
  0 siblings, 0 replies; 18+ messages in thread
From: Maxim Cournoyer @ 2017-06-08 14:58 UTC (permalink / raw)
  To: Arun Isaac; +Cc: Alex Kost, 27222-done

Arun Isaac <arunisaac@systemreboot.net> writes:

> Pushed! :-)
>
> Made a few minor modifications to the commit message...

Thank you, Arun!

Maxim

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2017-06-08 14:59 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-03 23:02 bug#27222: emacs-build-system install phase doesn't honor directory hierarchy Maxim Cournoyer
2017-06-04  6:53 ` bug#27222: [PATCH] " Maxim Cournoyer
2017-06-04 12:59   ` Alex Kost
2017-06-04 16:44     ` Maxim Cournoyer
2017-06-04 19:41       ` Alex Kost
2017-06-04 19:25     ` Arun Isaac
2017-06-05  5:07       ` Maxim Cournoyer
2017-06-05 10:03         ` Arun Isaac
     [not found]         ` <73a30871.AEQALMWm9gcAAAAAAAAAAAPHPfsAAAACwQwAAAAAAAW9WABZNSx9@mailjet.com>
2017-06-05 14:54           ` Maxim Cournoyer
     [not found]         ` <a444cf1b.AEQALMWm9gYAAAAAAAAAAAO2CsUAAAACwQwAAAAAAAW9WABZNSx9@mailjet.com>
2017-06-05 20:13           ` Alex Kost
2017-06-08 14:31             ` Arun Isaac
     [not found]             ` <ad8c9523.AEUALD4wqa8AAAAAAAAAAAPHPfsAAAACwQwAAAAAAAW9WABZOV_H@mailjet.com>
2017-06-08 14:58               ` Maxim Cournoyer
     [not found]     ` <0efe58d4.AEUAK47aAUsAAAAAAAAAAAO2CsUAAAACwQwAAAAAAAW9WABZNF6l@mailjet.com>
2017-06-05 20:07       ` Alex Kost
2017-06-06 17:44         ` Arun Isaac
2017-06-06 16:35 ` bug#27222: [PATCH] Fix ert-runner regression (was: emacs-build-system install phase doesn't honor directory hierarchy) Maxim Cournoyer
2017-06-06 23:02   ` bug#27222: [PATCH] Fix ert-runner regression Ludovic Courtès
2017-06-07 10:28     ` Arun Isaac
2017-06-07 20:11       ` Alex Kost

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.