[bug#28832] [PATCH 0/3] gnu: Add emacs-json-mode.

[bug#28832] [PATCH 0/3] gnu: Add emacs-json-mode.

[Oleg Pykhalov]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: [PATCH 0/3] gnu: Add emacs-json-mode. --]
[-- Type: text/x-patch, Size: 476 bytes --]

From b3b7b79f85e3b2aca6322e9c994b0fe0a666825d Mon Sep 17 00:00:00 2001
From: Oleg Pykhalov <go.wigust@gmail.com>
Date: Sat, 14 Oct 2017 12:50:21 +0300
Subject: [PATCH 0/3] gnu: Add emacs-json-mode.

2 tests fails in emacs-json-reformat.

Oleg Pykhalov (3):
  gnu: Add emacs-json-reformat.
  gnu: Add emacs-json-snatcher.
  gnu: Add emacs-json-mode.

 gnu/packages/emacs.scm | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

-- 
2.14.2

[bug#28832] [PATCH 1/3] gnu: Add emacs-json-reformat.

[Oleg Pykhalov]

* gnu/packages/emacs.scm (emacs-json-reformat): New variable.
---
 gnu/packages/emacs.scm | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
index 66f0a2ebe..8d94341ab 100644
--- a/gnu/packages/emacs.scm
+++ b/gnu/packages/emacs.scm
@@ -5569,6 +5569,55 @@ pair of minor modes which suppress all mouse events by intercepting them and
 running a customisable handler command (@code{ignore} by default). ")
     (license license:gpl3+)))
 
+(define-public emacs-json-reformat
+  (package
+    (name "emacs-json-reformat")
+    (version "0.0.6")
+    (source
+     (origin
+       (method url-fetch)
+       (uri (string-append "https://github.com/gongo/json-reformat/archive/"
+                           version ".tar.gz"))
+       (file-name (string-append name "-" version ".tar.gz"))
+       (sha256
+        (base32
+         "11fbq4scrgr7m0iwnzcrn2g7xvqwm2gf82sa7zy1l0nil7265p28"))))
+    (build-system emacs-build-system)
+    (propagated-inputs `(("emacs-undercover" ,emacs-undercover)))
+    (inputs
+     `(("emacs-dash" ,emacs-dash)         ; for tests
+       ("emacs-shut-up" ,emacs-shut-up))) ; for tests
+    (arguments
+     `(#:phases
+       (modify-phases %standard-phases
+         (add-before 'install 'check
+           (lambda* (#:key inputs #:allow-other-keys)
+             (zero? (system* "emacs" "--batch" "-L" "."
+                             "-L" (string-append
+                                   (assoc-ref inputs "emacs-undercover")
+                                   "/share/emacs/site-lisp/guix.d/undercover-"
+                                   ,(package-version emacs-undercover))
+                             "-L" (string-append
+                                   (assoc-ref inputs "emacs-dash")
+                                   "/share/emacs/site-lisp/guix.d/dash-"
+                                   ,(package-version emacs-dash))
+                             "-L" (string-append
+                                   (assoc-ref inputs "emacs-shut-up")
+                                   "/share/emacs/site-lisp/guix.d/shut-up-"
+                                   ,(package-version emacs-shut-up))
+                             "-l" "test/test-helper.el"
+                             "-l" "test/json-reformat-test.el"
+                             "-f" "ert-run-tests-batch-and-exit"))
+             ;; Fails
+             ;; json-reformat-test:json-reformat-region-occur-error
+             ;; json-reformat-test:string-to-string
+             #t)))))
+    (home-page "https://github.com/gongo/json-reformat")
+    (synopsis "Reformatting tool for JSON")
+    (description "@code{json-reformat} provides a reformatting tool for
+@url{http://json.org/, JSON}.")
+    (license license:gpl3+)))
+
 (define-public emacs-restclient
   (let ((commit "07a3888bb36d0e29608142ebe743b4362b800f40")
         (revision "1"))                 ;Guix package revision,
-- 
2.14.2

[bug#28832] [PATCH 2/3] gnu: Add emacs-json-snatcher.

[Oleg Pykhalov]

* gnu/packages/emacs.scm (emacs-json-snatcher): New variable.
---
 gnu/packages/emacs.scm | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
index 8d94341ab..9bf7acf0b 100644
--- a/gnu/packages/emacs.scm
+++ b/gnu/packages/emacs.scm
@@ -5618,6 +5618,26 @@ running a customisable handler command (@code{ignore} by default). ")
 @url{http://json.org/, JSON}.")
     (license license:gpl3+)))
 
+(define-public emacs-json-snatcher
+  (package
+    (name "emacs-json-snatcher")
+    (version "1.0.0")
+    (source
+     (origin
+       (method url-fetch)
+       (uri (string-append "https://github.com/Sterlingg/json-snatcher/archive/"
+                           version ".tar.gz"))
+       (file-name (string-append name "-" version ".tar.gz"))
+       (sha256
+        (base32
+         "1nfiwsifpdiz0lbrqa77nl0crnfrv5h85ans9b0g5rggnmyshcfb"))))
+    (build-system emacs-build-system)
+    (home-page "https://github.com/sterlingg/json-snatcher")
+    (synopsis "Grabs the path to JSON values in a JSON file")
+    (description "@code{emacs-json-snatcher} grabs the path to JSON values in
+a @url{http://json.org/, JSON} file.")
+    (license license:gpl3+)))
+
 (define-public emacs-restclient
   (let ((commit "07a3888bb36d0e29608142ebe743b4362b800f40")
         (revision "1"))                 ;Guix package revision,
-- 
2.14.2

[bug#28832] [PATCH 2/3] gnu: Add emacs-json-snatcher.

[Ludovic Courtès]

Oleg Pykhalov <go.wigust@gmail.com> skribis:

> * gnu/packages/emacs.scm (emacs-json-snatcher): New variable.

Applied, thanks!

[bug#28832] [PATCH 3/3] gnu: Add emacs-json-mode.

[Oleg Pykhalov]

* gnu/packages/emacs.scm (emacs-json-mode): New variable.
---
 gnu/packages/emacs.scm | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
index 9bf7acf0b..78a950900 100644
--- a/gnu/packages/emacs.scm
+++ b/gnu/packages/emacs.scm
@@ -5638,6 +5638,29 @@ running a customisable handler command (@code{ignore} by default). ")
 a @url{http://json.org/, JSON} file.")
     (license license:gpl3+)))
 
+(define-public emacs-json-mode
+  (package
+    (name "emacs-json-mode")
+    (version "1.7.0")
+    (source
+     (origin
+       (method url-fetch)
+       (uri (string-append "https://github.com/joshwnj/json-mode/archive/"
+                           "v" version ".tar.gz"))
+       (file-name (string-append name "-" version ".tar.gz"))
+       (sha256
+        (base32
+         "06h45p4cn767pk9sqi2zb1c65wy5gyyijqxzpglp80zwxhvajdz5"))))
+    (build-system emacs-build-system)
+    (propagated-inputs
+     `(("emacs-json-reformat" ,emacs-json-reformat)
+       ("emacs-json-snatcher" ,emacs-json-snatcher)))
+    (home-page "https://github.com/joshwnj/json-mode")
+    (synopsis "Major mode for editing JSON files")
+    (description "@code{json-mode} extends the builtin js-mode syntax
+highlighting.")
+    (license license:gpl3+)))
+
 (define-public emacs-restclient
   (let ((commit "07a3888bb36d0e29608142ebe743b4362b800f40")
         (revision "1"))                 ;Guix package revision,
-- 
2.14.2

[bug#28832] [PATCH 1/3] gnu: Add emacs-json-reformat.

[Ludovic Courtès]

Hello,

Oleg Pykhalov <go.wigust@gmail.com> skribis:

> * gnu/packages/emacs.scm (emacs-json-reformat): New variable.

[...]

> +       (modify-phases %standard-phases
> +         (add-before 'install 'check
> +           (lambda* (#:key inputs #:allow-other-keys)
> +             (zero? (system* "emacs" "--batch" "-L" "."
> +                             "-L" (string-append
> +                                   (assoc-ref inputs "emacs-undercover")
> +                                   "/share/emacs/site-lisp/guix.d/undercover-"
> +                                   ,(package-version emacs-undercover))
> +                             "-L" (string-append
> +                                   (assoc-ref inputs "emacs-dash")
> +                                   "/share/emacs/site-lisp/guix.d/dash-"
> +                                   ,(package-version emacs-dash))
> +                             "-L" (string-append
> +                                   (assoc-ref inputs "emacs-shut-up")
> +                                   "/share/emacs/site-lisp/guix.d/shut-up-"
> +                                   ,(package-version emacs-shut-up))
> +                             "-l" "test/test-helper.el"
> +                             "-l" "test/json-reformat-test.el"
> +                             "-f" "ert-run-tests-batch-and-exit"))
> +             ;; Fails
> +             ;; json-reformat-test:json-reformat-region-occur-error
> +             ;; json-reformat-test:string-to-string
> +             #t)))))

Did you have a chance to investigate the test failures?  It’s not
confidence-inspiring ;-), so it would be good to at least have a link to
an upstream bug report.

Besides, I wonder: shouldn’t ‘emacs-build-system’ define the
‘EMACSLOADPATH’ env. var. so we don’t have to carry all these -L flags?
Is there any downside?

Thanks,
Ludo’.

[bug#28832] [PATCH 1/3] gnu: Add emacs-json-reformat.

[Ludovic Courtès]

Hi Oleg,

Ping!  :-)

ludo@gnu.org (Ludovic Courtès) skribis:

> Oleg Pykhalov <go.wigust@gmail.com> skribis:
>
>> * gnu/packages/emacs.scm (emacs-json-reformat): New variable.
>
> [...]
>
>> +       (modify-phases %standard-phases
>> +         (add-before 'install 'check
>> +           (lambda* (#:key inputs #:allow-other-keys)
>> +             (zero? (system* "emacs" "--batch" "-L" "."
>> +                             "-L" (string-append
>> +                                   (assoc-ref inputs "emacs-undercover")
>> +                                   "/share/emacs/site-lisp/guix.d/undercover-"
>> +                                   ,(package-version emacs-undercover))
>> +                             "-L" (string-append
>> +                                   (assoc-ref inputs "emacs-dash")
>> +                                   "/share/emacs/site-lisp/guix.d/dash-"
>> +                                   ,(package-version emacs-dash))
>> +                             "-L" (string-append
>> +                                   (assoc-ref inputs "emacs-shut-up")
>> +                                   "/share/emacs/site-lisp/guix.d/shut-up-"
>> +                                   ,(package-version emacs-shut-up))
>> +                             "-l" "test/test-helper.el"
>> +                             "-l" "test/json-reformat-test.el"
>> +                             "-f" "ert-run-tests-batch-and-exit"))
>> +             ;; Fails
>> +             ;; json-reformat-test:json-reformat-region-occur-error
>> +             ;; json-reformat-test:string-to-string
>> +             #t)))))
>
> Did you have a chance to investigate the test failures?  It’s not
> confidence-inspiring ;-), so it would be good to at least have a link to
> an upstream bug report.
>
> Besides, I wonder: shouldn’t ‘emacs-build-system’ define the
> ‘EMACSLOADPATH’ env. var. so we don’t have to carry all these -L flags?
> Is there any downside?
>
> Thanks,
> Ludo’.

[bug#28832] [PATCH 1/3] gnu: Add emacs-json-reformat.

[Oleg Pykhalov]

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

Hello Ludovic,

Apologies for late reply and thank you for review.

ludo@gnu.org (Ludovic Courtès) writes:

[...]

>>> +       (modify-phases %standard-phases
>>> +         (add-before 'install 'check
>>> +           (lambda* (#:key inputs #:allow-other-keys)
>>> +             (zero? (system* "emacs" "--batch" "-L" "."
>>> +                             "-L" (string-append
>>> +                                   (assoc-ref inputs "emacs-undercover")
>>> +                                   "/share/emacs/site-lisp/guix.d/undercover-"
>>> +                                   ,(package-version emacs-undercover))
>>> +                             "-L" (string-append
>>> +                                   (assoc-ref inputs "emacs-dash")
>>> +                                   "/share/emacs/site-lisp/guix.d/dash-"
>>> +                                   ,(package-version emacs-dash))
>>> +                             "-L" (string-append
>>> +                                   (assoc-ref inputs "emacs-shut-up")
>>> +                                   "/share/emacs/site-lisp/guix.d/shut-up-"
>>> +                                   ,(package-version emacs-shut-up))
>>> +                             "-l" "test/test-helper.el"
>>> +                             "-l" "test/json-reformat-test.el"
>>> +                             "-f" "ert-run-tests-batch-and-exit"))
>>> +             ;; Fails
>>> +             ;; json-reformat-test:json-reformat-region-occur-error
>>> +             ;; json-reformat-test:string-to-string
>>> +             #t)))))
>>
>> Did you have a chance to investigate the test failures?  It’s not
>> confidence-inspiring ;-), so it would be good to at least have a link to
>> an upstream bug report.

Yes I looked at those tests.  Here is a little review of them.

json-reformat-test:string-to-string basically just calls
a reimplemented in Emacs 25 json-encode-string function.

json-reformat-test:json-reformat-region-occur-error emulates error and
produce a message.  This message is differ from Emacs 24 in symbol '`'.

So, I think those test fails are harmless.  I attach the new patch with
fixing those failing tests.


[-- Attachment #1.2: [PATCH] gnu: Add emacs-json-reformat. --]
[-- Type: text/x-patch, Size: 5700 bytes --]

From d589de21acb02d0fba7aee3b48e5f42d7bd8957e Mon Sep 17 00:00:00 2001
From: Oleg Pykhalov <go.wigust@gmail.com>
Date: Tue, 12 Dec 2017 01:41:08 +0300
Subject: [PATCH] gnu: Add emacs-json-reformat.

* gnu/packages/patches/emacs-json-reformat-fix-tests.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add this.
* gnu/packages/emacs.scm (emacs-json-reformat): New variable.
---
 gnu/local.mk                                       |  1 +
 gnu/packages/emacs.scm                             | 47 ++++++++++++++++++++++
 .../patches/emacs-json-reformat-fix-tests.patch    | 28 +++++++++++++
 3 files changed, 76 insertions(+)
 create mode 100644 gnu/packages/patches/emacs-json-reformat-fix-tests.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 46829756b..ec5ba06d3 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -613,6 +613,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/einstein-build.patch			\
   %D%/packages/patches/emacs-exec-path.patch			\
   %D%/packages/patches/emacs-fix-scheme-indent-function.patch	\
+  %D%/packages/patches/emacs-json-reformat-fix-tests.patch	\
   %D%/packages/patches/emacs-highlight-stages-add-gexp.patch	\
   %D%/packages/patches/emacs-scheme-complete-scheme-r5rs-info.patch	\
   %D%/packages/patches/emacs-source-date-epoch.patch		\
diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
index fd0305629..1aa139c92 100644
--- a/gnu/packages/emacs.scm
+++ b/gnu/packages/emacs.scm
@@ -5914,6 +5914,53 @@ pair of minor modes which suppress all mouse events by intercepting them and
 running a customisable handler command (@code{ignore} by default). ")
     (license license:gpl3+)))
 
+(define-public emacs-json-reformat
+  (package
+    (name "emacs-json-reformat")
+    (version "0.0.6")
+    (source
+     (origin
+       (method url-fetch)
+       (uri (string-append "https://github.com/gongo/json-reformat/archive/"
+                           version ".tar.gz"))
+       (file-name (string-append name "-" version ".tar.gz"))
+       (sha256
+        (base32
+         "11fbq4scrgr7m0iwnzcrn2g7xvqwm2gf82sa7zy1l0nil7265p28"))
+       (patches (search-patches "emacs-json-reformat-fix-tests.patch"))))
+    (build-system emacs-build-system)
+    (propagated-inputs `(("emacs-undercover" ,emacs-undercover)))
+    (inputs
+     `(("emacs-dash" ,emacs-dash)         ; for tests
+       ("emacs-shut-up" ,emacs-shut-up))) ; for tests
+    (arguments
+     `(#:phases
+       (modify-phases %standard-phases
+         (add-before 'install 'check
+           (lambda* (#:key inputs #:allow-other-keys)
+             (zero? (system* "emacs" "--batch" "-L" "."
+                             "-L" (string-append
+                                   (assoc-ref inputs "emacs-undercover")
+                                   "/share/emacs/site-lisp/guix.d/undercover-"
+                                   ,(package-version emacs-undercover))
+                             "-L" (string-append
+                                   (assoc-ref inputs "emacs-dash")
+                                   "/share/emacs/site-lisp/guix.d/dash-"
+                                   ,(package-version emacs-dash))
+                             "-L" (string-append
+                                   (assoc-ref inputs "emacs-shut-up")
+                                   "/share/emacs/site-lisp/guix.d/shut-up-"
+                                   ,(package-version emacs-shut-up))
+                             "-l" "test/test-helper.el"
+                             "-l" "test/json-reformat-test.el"
+                             "-f" "ert-run-tests-batch-and-exit"))
+             #t)))))
+    (home-page "https://github.com/gongo/json-reformat")
+    (synopsis "Reformatting tool for JSON")
+    (description "@code{json-reformat} provides a reformatting tool for
+@url{http://json.org/, JSON}.")
+    (license license:gpl3+)))
+
 (define-public emacs-json-snatcher
   (package
     (name "emacs-json-snatcher")
diff --git a/gnu/packages/patches/emacs-json-reformat-fix-tests.patch b/gnu/packages/patches/emacs-json-reformat-fix-tests.patch
new file mode 100644
index 000000000..23a239582
--- /dev/null
+++ b/gnu/packages/patches/emacs-json-reformat-fix-tests.patch
@@ -0,0 +1,28 @@
+Copyright © 2017 Oleg Pykhalov <go.wigust@gmail.com>
+
+This patch fixes tests for Emacs 25.
+
+diff --git a/test/json-reformat-test.el b/test/json-reformat-test.el
+index 7de3be1..b4a4dde 100644
+--- a/test/json-reformat-test.el
++++ b/test/json-reformat-test.el
+@@ -58,7 +58,7 @@
+ (ert-deftest json-reformat-test:string-to-string ()
+   (should (string= "\"foobar\"" (json-reformat:string-to-string "foobar")))
+   (should (string= "\"fo\\\"o\\nbar\"" (json-reformat:string-to-string "fo\"o\nbar")))
+-  (should (string= "\"\\u2661\"" (json-reformat:string-to-string "\u2661")))
++  (should (string= "\"♡\"" (json-reformat:string-to-string "\u2661")))
+ 
+   (should (string= "\"^(amq\\\\.gen.*|amq\\\\.default)$\"" (json-reformat:string-to-string "^(amq\\.gen.*|amq\\.default)$")))
+   )
+@@ -148,6 +148,6 @@ bar\"" (json-reformat:string-to-string "fo\"o\nbar")))
+ [{ foo : \"bar\" }, { \"foo\" : \"baz\" }]") ;; At 3 (line)
+         (json-reformat-region (point-min) (point-max)))
+       (should (string=
+-               "JSON parse error [Reason] Bad string format: \"doesn't start with '\\\"'!\" [Position] In buffer, line 3 (char 6)"
++               "JSON parse error [Reason] Bad string format: \"doesn't start with \`\\\"'!\" [Position] In buffer, line 3 (char 6)"
+                message-string))
+       )))
+-- 
+2.15.1
+
-- 
2.15.1


[-- Attachment #1.3: Type: text/plain, Size: 566 bytes --]



The upstream is quite: https://github.com/gongo/json-reformat/issues/33

>> Besides, I wonder: shouldn’t ‘emacs-build-system’ define the
>> ‘EMACSLOADPATH’ env. var. so we don’t have to carry all these -L flags?
>> Is there any downside?

As I see from the documentation¹ EMACSLOADPATH is a list of directories
with *.el files in it.  If we will use it, then it will be almost the
same carring bunch of directories in package recipes, will it?

¹ https://www.gnu.org/software/emacs/manual/html_node/emacs/General-Variables.html

Oleg.

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

[bug#28832] [PATCH 1/3] gnu: Add emacs-json-reformat.

[Ludovic Courtès]

Hi Oleg,

Oleg Pykhalov <go.wigust@gmail.com> skribis:

> ludo@gnu.org (Ludovic Courtès) writes:

[...]

>>> Did you have a chance to investigate the test failures?  It’s not
>>> confidence-inspiring ;-), so it would be good to at least have a link to
>>> an upstream bug report.
>
> Yes I looked at those tests.  Here is a little review of them.
>
> json-reformat-test:string-to-string basically just calls
> a reimplemented in Emacs 25 json-encode-string function.
>
> json-reformat-test:json-reformat-region-occur-error emulates error and
> produce a message.  This message is differ from Emacs 24 in symbol '`'.
>
> So, I think those test fails are harmless.  I attach the new patch with
> fixing those failing tests.

Indeed.

> From d589de21acb02d0fba7aee3b48e5f42d7bd8957e Mon Sep 17 00:00:00 2001
> From: Oleg Pykhalov <go.wigust@gmail.com>
> Date: Tue, 12 Dec 2017 01:41:08 +0300
> Subject: [PATCH] gnu: Add emacs-json-reformat.
>
> * gnu/packages/patches/emacs-json-reformat-fix-tests.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Add this.
> * gnu/packages/emacs.scm (emacs-json-reformat): New variable.

[...]

> diff --git a/gnu/packages/patches/emacs-json-reformat-fix-tests.patch b/gnu/packages/patches/emacs-json-reformat-fix-tests.patch
> new file mode 100644
> index 000000000..23a239582
> --- /dev/null
> +++ b/gnu/packages/patches/emacs-json-reformat-fix-tests.patch
> @@ -0,0 +1,28 @@
> +Copyright © 2017 Oleg Pykhalov <go.wigust@gmail.com>
> +
> +This patch fixes tests for Emacs 25.

Please mention <https://github.com/gongo/json-reformat/issues/33> here.

OK with these changes!

> >> Besides, I wonder: shouldn’t ‘emacs-build-system’ define the
> >> ‘EMACSLOADPATH’ env. var. so we don’t have to carry all these -L flags?
> >> Is there any downside?
> 
> As I see from the documentation¹ EMACSLOADPATH is a list of directories
> with *.el files in it.  If we will use it, then it will be almost the
> same carring bunch of directories in package recipes, will it?

If ‘emacs-build-system’ sets ‘EMACSLOADPATH’ automatically, then
individual package definitions won’t need those -L flags.  Dunno if
there are good reasons not to do so.  Maybe Alex has an opinion?

Thanks,
Ludo’.

[bug#28832] [PATCH 1/3] gnu: Add emacs-json-reformat.

[Alex Kost]

Ludovic Courtès (2017-12-12 10:17 +0100) wrote:

> Oleg Pykhalov <go.wigust@gmail.com> skribis:
>
>> ludo@gnu.org (Ludovic Courtès) writes:
>>
>>> Besides, I wonder: shouldn’t ‘emacs-build-system’ define the
>>> ‘EMACSLOADPATH’ env. var. so we don’t have to carry all these -L flags?
>>> Is there any downside?
>>
>> As I see from the documentation¹ EMACSLOADPATH is a list of directories
>> with *.el files in it.  If we will use it, then it will be almost the
>> same carring bunch of directories in package recipes, will it?
>
> If ‘emacs-build-system’ sets ‘EMACSLOADPATH’ automatically, then
> individual package definitions won’t need those -L flags.  Dunno if
> there are good reasons not to do so.  Maybe Alex has an opinion?

I would rather ask Federico who wrote ‘emacs-build-system’ :)

Yeah, maybe ‘emacs-build-system’ could benefit from using EMACSLOADPATH,
I don't know, someone should probably give it a try ;-)

BTW, Oleg, did you try to use 'ert-runner' instead of running emacs for
tests manually?  Perhaps, it will work; look at commit
8505d34829b99744a36d72dd583768f1e49210a6 for example.

-- 
Alex

[bug#28832] [PATCH 1/3] gnu: Add emacs-json-reformat.

[Oleg Pykhalov]

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

Hello Alex and Ludovic,

Alex Kost <alezost@gmail.com> writes:

[...]

>> If ‘emacs-build-system’ sets ‘EMACSLOADPATH’ automatically, then
>> individual package definitions won’t need those -L flags.  Dunno if
>> there are good reasons not to do so.  Maybe Alex has an opinion?
>
> I would rather ask Federico who wrote ‘emacs-build-system’ :)

Do you mean Federico Beffa <beffa@fbengineering.ch>?  He is quite in
Guix git repository for 9 months as I see.  Is it a good idea to CC him?

> Yeah, maybe ‘emacs-build-system’ could benefit from using EMACSLOADPATH,
> I don't know, someone should probably give it a try ;-)

I succeeded to implement this, but the patch needs some more love.  The
biggest issue is how to get an Emacs version in setup-environment.

You could test it on emacs-git-messenger or emacs-json-reformat.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: EMACSLOADPATH --]
[-- Type: text/x-patch, Size: 2575 bytes --]

diff --git a/guix/build/emacs-build-system.scm b/guix/build/emacs-build-system.scm
index bd0d2e026..269038744 100644
--- a/guix/build/emacs-build-system.scm
+++ b/guix/build/emacs-build-system.scm
@@ -183,6 +183,10 @@ store in '.el' files."
   "Check if NAME correspond to the name of an Emacs package."
   (string-prefix? "emacs-" name))
 
+(define (string-drop-emacs x)
+  "Drops `emacs-' from a string."
+  (string-drop x 6))
+
 (define (emacs-inputs inputs)
   "Retrieve the list of Emacs packages from INPUTS."
   (filter (match-lambda
@@ -222,6 +226,38 @@ second hyphen.  This corresponds to 'name-version' as used in ELPA packages."
             strip-store-file-name)
    store-dir))
 
+;; Copied from haskell-build-system.scm
+(define (package-name-version store-dir)
+  "Given a store directory STORE-DIR return 'name-version' of the package."
+  (let* ((base (basename store-dir)))
+    (string-drop base (+ 1 (string-index base #\-)))))
+
+(define* (setup-environment #:key inputs outputs #:allow-other-keys)
+  "Export the variable EMACSLOADPATH, which are based on INPUTS and OUTPUTS,
+respectively."
+  (let ((out (assoc-ref outputs "out")))
+    ;; EMACSLOADPATH is where Emacs looks for the source code of the build's
+    ;; dependencies.
+    (set-path-environment-variable
+     "EMACSLOADPATH"
+     ;; XXX Matching "." hints that we could do
+     ;; something simpler here...
+     (list ".")
+     (cons (let ((store-item (cdr (assoc "emacs" (emacs-inputs inputs)))))
+             ;; TODO: Get a version from inputs
+             (string-append store-item "/share/emacs/25.3/lisp"))
+           (map
+            (lambda (foobar)
+              (let ((store-item (cdr foobar)))
+                (string-append store-item
+                               %install-suffix "/"
+                               (string-drop-emacs
+                                (package-name-version store-item)))))
+            (alist-delete "emacs"
+                          (alist-delete "source"
+                                        (emacs-inputs inputs))))))
+    #t))
+
 (define %standard-phases
   (modify-phases gnu:%standard-phases
     (replace 'unpack unpack)
@@ -229,6 +265,7 @@ second hyphen.  This corresponds to 'name-version' as used in ELPA packages."
     (delete 'check)
     (delete 'install)
     (replace 'build build)
+    (add-before 'build 'setup-environment setup-environment)
     (add-before 'build 'install install)
     (add-after 'install 'make-autoloads make-autoloads)
     (add-after 'make-autoloads 'patch-el-files patch-el-files)

[-- Attachment #1.3: Type: text/plain, Size: 250 bytes --]


> BTW, Oleg, did you try to use 'ert-runner' instead of running emacs for
> tests manually?  Perhaps, it will work; look at commit
> 8505d34829b99744a36d72dd583768f1e49210a6 for example.

Nope, but I will.  Thank you for notice this!

Thanks,
Oleg.

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

[bug#28832] [PATCH 1/3] gnu: Add emacs-json-reformat.

[Alex Kost]

Oleg Pykhalov (2017-12-13 07:55 +0300) wrote:

> Hello Alex and Ludovic,
>
> Alex Kost <alezost@gmail.com> writes:
>
> [...]
>
>>> If ‘emacs-build-system’ sets ‘EMACSLOADPATH’ automatically, then
>>> individual package definitions won’t need those -L flags.  Dunno if
>>> there are good reasons not to do so.  Maybe Alex has an opinion?
>>
>> I would rather ask Federico who wrote ‘emacs-build-system’ :)
>
> Do you mean Federico Beffa <beffa@fbengineering.ch>?  He is quite in
> Guix git repository for 9 months as I see.  Is it a good idea to CC him?

Yeah, I meant this Federico.  I'm not sure if it is a good idea...,
probably..., I don't know :-)

-- 
Alex

[bug#28832] [PATCH 1/3] gnu: Add emacs-json-reformat.

[Oleg Pykhalov]

[-- Attachment #1.1: [PATCH] emacs-build-system: Add EMACSLOADPATH. --]
[-- Type: text/x-patch, Size: 3790 bytes --]

From adda59022a61dfcf2add1d9f01d02df4be90a0f1 Mon Sep 17 00:00:00 2001
From: Oleg Pykhalov <go.wigust@gmail.com>
Date: Wed, 13 Dec 2017 08:10:21 +0300
Subject: [PATCH] emacs-build-system: Add EMACSLOADPATH.

* guix/build/emacs-build-system.scm: Add EMACSLOADPATH.
---
 guix/build/emacs-build-system.scm | 47 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/guix/build/emacs-build-system.scm b/guix/build/emacs-build-system.scm
index bd0d2e026..06479bf6b 100644
--- a/guix/build/emacs-build-system.scm
+++ b/guix/build/emacs-build-system.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2015 Federico Beffa <beffa@fbengineering.ch>
 ;;; Copyright © 2016 David Thompson <davet@gnu.org>
 ;;; Copyright © 2016 Alex Kost <alezost@gmail.com>
+;;; Copyright © 2017 Oleg Pykhalov <go.wigust@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -183,6 +184,10 @@ store in '.el' files."
   "Check if NAME correspond to the name of an Emacs package."
   (string-prefix? "emacs-" name))
 
+(define (string-drop-emacs value)
+  "Drops `emacs-' from a string."
+  (string-drop value 6))
+
 (define (emacs-inputs inputs)
   "Retrieve the list of Emacs packages from INPUTS."
   (filter (match-lambda
@@ -222,6 +227,47 @@ second hyphen.  This corresponds to 'name-version' as used in ELPA packages."
             strip-store-file-name)
    store-dir))
 
+(define (store-directory->package-name store-dir)
+  "Extract package name from STORE-DIR."
+  (let-values (((name _) ((compose package-name->name+version
+                                   strip-store-file-name)
+                          store-dir)))
+    name))
+
+(define (store-directory->package-version store-dir)
+  "Extract package version from STORE-DIR."
+  (let-values (((_ version) ((compose package-name->name+version
+                                      strip-store-file-name)
+                             store-dir)))
+    version))
+
+(define* (setup-environment #:key inputs #:allow-other-keys)
+  "Export the variable EMACSLOADPATH, which are based on INPUTS respectively."
+  (let* ((filtered-inputs (emacs-inputs inputs))
+         (emacs-input-dir (cdr (assoc "emacs" filtered-inputs)))
+         (inputs-dirs (cdr filtered-inputs)))
+    ;; EMACSLOADPATH is where Emacs looks for the source code of the build's
+    ;; dependencies.
+    (setenv "EMACSLOADPATH"
+            (string-append emacs-input-dir "/share/emacs/"
+                           (store-directory->package-version emacs-input-dir)
+                           "/lisp"))
+    (for-each
+     (lambda (input)
+       (let ((store-item (cdr input)))
+         (setenv "EMACSLOADPATH"
+                 (string-append
+                  (or (getenv "EMACSLOADPATH") "")
+                  ":" store-item %install-suffix "/"
+                  ((compose string-drop-emacs store-directory->package-name)
+                   store-item)))))
+     ((compose (lambda (inputs) (alist-delete "emacs" inputs))
+               (lambda (inputs) (alist-delete "source" inputs)))
+      (delete-duplicates inputs-dirs)))
+    (format #t "environment variable EMACSLOADPATH set to `~a'~%"
+            (getenv "EMACSLOADPATH"))
+    #t))
+
 (define %standard-phases
   (modify-phases gnu:%standard-phases
     (replace 'unpack unpack)
@@ -229,6 +275,7 @@ second hyphen.  This corresponds to 'name-version' as used in ELPA packages."
     (delete 'check)
     (delete 'install)
     (replace 'build build)
+    (add-before 'build 'setup-environment setup-environment)
     (add-before 'build 'install install)
     (add-after 'install 'make-autoloads make-autoloads)
     (add-after 'make-autoloads 'patch-el-files patch-el-files)
-- 
2.15.1


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

[bug#28832] [PATCH 1/3] gnu: Add emacs-json-reformat.

[Ludovic Courtès]

Hi Oleg,

I think it would have been better to open a separate issue for this.
Here are some superficial comments.  I hope someone more knowledgeable
about Emacs can comment.

Oleg Pykhalov <go.wigust@gmail.com> skribis:

> From adda59022a61dfcf2add1d9f01d02df4be90a0f1 Mon Sep 17 00:00:00 2001
> From: Oleg Pykhalov <go.wigust@gmail.com>
> Date: Wed, 13 Dec 2017 08:10:21 +0300
> Subject: [PATCH] emacs-build-system: Add EMACSLOADPATH.
>
> * guix/build/emacs-build-system.scm: Add EMACSLOADPATH.

Please see ‘C-x v l’ in that file for the syntax of commit logs.  :-)

> +(define (store-directory->package-name store-dir)
> +  "Extract package name from STORE-DIR."
> +  (let-values (((name _) ((compose package-name->name+version
> +                                   strip-store-file-name)
> +                          store-dir)))
> +    name))

It’s enough to write it like this:

  (define store-directory->package-name
    (compose package-name->name+version
             strip-store-file-name))

Guile does automatic “multiple-value truncation”, which means that the
second value that the procedure returns can be ignored by the caller.

> +(define (store-directory->package-version store-dir)
> +  "Extract package version from STORE-DIR."
> +  (let-values (((_ version) ((compose package-name->name+version
> +                                      strip-store-file-name)
> +                             store-dir)))
> +    version))

Likewise.

> +(define* (setup-environment #:key inputs #:allow-other-keys)
> +  "Export the variable EMACSLOADPATH, which are based on INPUTS respectively."
> +  (let* ((filtered-inputs (emacs-inputs inputs))
> +         (emacs-input-dir (cdr (assoc "emacs" filtered-inputs)))
> +         (inputs-dirs (cdr filtered-inputs)))

Nitpick: Please see the guidelines mentioned in
<https://www.gnu.org/software/guix/manual/html_node/Formatting-Code.html>
on how to choose identifiers.  I’d write:

  (let* ((inputs (emacs-inputs inputs))
         (emacs  (assoc-ref inputs "emacs")))
    …)

‘inputs-dirs’ is unneeded AIUI (see below).

> +    ;; EMACSLOADPATH is where Emacs looks for the source code of the build's
> +    ;; dependencies.
> +    (setenv "EMACSLOADPATH"
> +            (string-append emacs-input-dir "/share/emacs/"
> +                           (store-directory->package-version emacs-input-dir)
> +                           "/lisp"))
> +    (for-each
> +     (lambda (input)
> +       (let ((store-item (cdr input)))

Rather:

  (for-each (match-lambda
              ((name . input)
               …))
            …)

> +         (setenv "EMACSLOADPATH"
> +                 (string-append
> +                  (or (getenv "EMACSLOADPATH") "")
> +                  ":" store-item %install-suffix "/"
> +                  ((compose string-drop-emacs store-directory->package-name)
> +                   store-item)))))

Rather:

  (string-drop-emacs (store-directory->package-name item))

IMO ‘compose’ makes things less readable in this case.

> +     ((compose (lambda (inputs) (alist-delete "emacs" inputs))
> +               (lambda (inputs) (alist-delete "source" inputs)))
> +      (delete-duplicates inputs-dirs)))

Rather:

  (fold alist-delete inputs '("emacs" "source"))

Did you try rebuilding Emacs packages, and to simplify those that
explicitly pass -L flags?

Thanks a lot for working on it!

Ludo’.

[bug#28832] [PATCH 1/3] gnu: Add emacs-json-reformat.

[Oleg Pykhalov]

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

Hello Ludovic,

Thank you for refactoring notes!

ludo@gnu.org (Ludovic Courtès) writes:

> I think it would have been better to open a separate issue for this.
> Here are some superficial comments.  I hope someone more knowledgeable
> about Emacs can comment.

OK, I CC bug-guix@gnu.org in this message.

Previos discussion: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28832#17

> Oleg Pykhalov <go.wigust@gmail.com> skribis:

[...]

> Please see ‘C-x v l’ in that file for the syntax of commit logs.  :-)

Oh, nice tip.  Thanks!

[...]

> Did you try rebuilding Emacs packages, and to simplify those that
> explicitly pass -L flags?

I tried it on emacs-json-reformat which wants many -L flags and
succeeded build a manifest with following packages and patch:

  emacs-aggressive-indent emacs-company emacs-company-quickhelp
  emacs-debbugs emacs-elfeed emacs-erc-hl-nicks emacs-default-encrypt
  emacs-god-mode emacs-ggtags emacs-git-gutter emacs-gitpatch emacs-guix
  emacs-helm emacs-helm-make emacs-helm-projectile
  emacs-highlight-stages emacs-ivy emacs-markdown-mode
  emacs-multiple-cursors emacs-nix-mode emacs-org emacs-projectile
  emacs-rainbow-delimiters emacs-rainbow-mode emacs-slime
  emacs-smartparens emacs-transmission emacs-transpose-frame
  emacs-use-package emacs-w3m emacs-which-key emacs-yasnippet
  emacs-yasnippet-snippets flycheck geiser magit


[-- Attachment #1.2: [PATCH] emacs-build-system: Add EMACSLOADPATH. --]
[-- Type: text/x-patch, Size: 3217 bytes --]

From c66f03c09517571a03fdf97b86bc571a71e969bf Mon Sep 17 00:00:00 2001
From: Oleg Pykhalov <go.wigust@gmail.com>
Date: Wed, 13 Dec 2017 08:10:21 +0300
Subject: [PATCH] emacs-build-system: Handle EMACSLOADPATH environment
 variable.

Define the 'EMACSLOADPATH' environment variable to carry all inputs in tests.

* guix/build/emacs-build-system.scm (setup-environment): New procedure.
  (%standard-phases): Use the new setup-environment procedure.
---
 guix/build/emacs-build-system.scm | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/guix/build/emacs-build-system.scm b/guix/build/emacs-build-system.scm
index bd0d2e026..9897abf9e 100644
--- a/guix/build/emacs-build-system.scm
+++ b/guix/build/emacs-build-system.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2015 Federico Beffa <beffa@fbengineering.ch>
 ;;; Copyright © 2016 David Thompson <davet@gnu.org>
 ;;; Copyright © 2016 Alex Kost <alezost@gmail.com>
+;;; Copyright © 2017 Oleg Pykhalov <go.wigust@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -222,6 +223,37 @@ second hyphen.  This corresponds to 'name-version' as used in ELPA packages."
             strip-store-file-name)
    store-dir))
 
+(define* (setup-environment #:key inputs #:allow-other-keys)
+  "Export the variable EMACSLOADPATH, which are based on INPUTS respectively."
+  (define (name+version input)
+    (package-name->name+version (strip-store-file-name input)))
+  (match (assoc "emacs" inputs)
+    ((name . input)
+     (setenv "EMACSLOADPATH"
+             (string-join (list input "share" name
+                                ((compose (lambda (_ version) version)
+                                          name+version)
+                                 input)
+                                "lisp")
+                          "/"))))
+  (for-each (match-lambda
+              ((name . input)
+               (setenv "EMACSLOADPATH"
+                       (string-append
+                        (or (getenv "EMACSLOADPATH") "")
+                        ":" input %install-suffix "/"
+                        ((compose (lambda (name version)
+                                    (string-append
+                                     (string-drop name
+                                                  (string-length "emacs-"))
+                                     "-" version))
+                                  name+version)
+                         input)))))
+            (fold alist-delete (emacs-inputs inputs) '("emacs" "source")))
+  (format #t "environment variable EMACSLOADPATH set to `~a'~%"
+          (getenv "EMACSLOADPATH"))
+  #t)
+
 (define %standard-phases
   (modify-phases gnu:%standard-phases
     (replace 'unpack unpack)
@@ -229,6 +261,7 @@ second hyphen.  This corresponds to 'name-version' as used in ELPA packages."
     (delete 'check)
     (delete 'install)
     (replace 'build build)
+    (add-before 'build 'setup-environment setup-environment)
     (add-before 'build 'install install)
     (add-after 'install 'make-autoloads make-autoloads)
     (add-after 'make-autoloads 'patch-el-files patch-el-files)
-- 
2.15.1


[-- Attachment #1.3: Type: text/plain, Size: 7 bytes --]


Oleg.

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

[bug#28832] [PATCH 1/3] gnu: Add emacs-json-reformat.

[Alex Kost]

Oleg Pykhalov (2017-12-19 13:46 +0300) wrote:

[...]
> +  (for-each (match-lambda
> +              ((name . input)
> +               (setenv "EMACSLOADPATH"
> +                       (string-append
> +                        (or (getenv "EMACSLOADPATH") "")
> +                        ":" input %install-suffix "/"
> +                        ((compose (lambda (name version)
> +                                    (string-append
> +                                     (string-drop name
> +                                                  (string-length "emacs-"))

I would move this code into its own 'string-drop-emacs' function (as you
did in the previous patch) and I would make it more robust: there is a
problem with this code: (string-drop "geiser" 6) does not return what
you mean, and (string-drop "dash" 6) errors!  I think we shouldn't rely
on the assumption that all emacs inputs have "emacs-" prefix, so I think
this procedure should check whether the input name begins with "emacs-"
before trying to remove this substring.

> +                                     "-" version))
> +                                  name+version)
> +                         input)))))
> +            (fold alist-delete (emacs-inputs inputs) '("emacs" "source")))

Since you already take only emacs inputs, is it really needed to remove
"emacs" and "source"?  I mean emacs inputs do not contain "emacs" and
"source" anyway, right?

-- 
Alex

[bug#28832] [PATCH 1/3] gnu: Add emacs-json-reformat.

[Oleg Pykhalov]

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

Alex Kost <alezost@gmail.com> writes:

> and I would make it more robust: there is a
> problem with this code: (string-drop "geiser" 6) does not return what
> you mean

Yes, because "geiser" differs from most of Emacs packages:

    ls /gnu/store/7rl2k8ismmyq9ic6ha6vzv38a3hrrni2-emacs-dash-2.13.0/share/emacs/site-lisp/guix.d/dash-2.13.0/
    dash-autoloads.el  dash.el  dash.elc  dash-functional.el  dash-functional.elc

    ls /gnu/store/7lh77fmapmjjv3kj2q69dy58kjniw9am-geiser-0.9/share/emacs/site-lisp/
    geiser-autodoc.el    geiser-chibi.elc	…

Maybe we just need to fix "geiser"?

> , and (string-drop "dash" 6) errors!

Do you really mean emacs-dash or dash?  For me both succeeded.

As I see emacs-dash is OK, except "emacs-minimal" in EMACSLOADPATH.
Here is a log-file with DEBUG-INPUTS:

[-- Attachment #1.2: emacs-dash build log-file --]
[-- Type: application/octet-stream, Size: 3697 bytes --]

[-- Attachment #1.3: Type: text/plain, Size: 1891 bytes --]


> I think we shouldn't rely on the assumption that all emacs inputs have
> "emacs-" prefix

Then, how to determine that a package is Emacs package?

> , so I think
> this procedure should check whether the input name begins with "emacs-"
> before trying to remove this substring.
>> +                                     "-" version))
>> +                                  name+version)
>> +                         input)))))
>> +            (fold alist-delete (emacs-inputs inputs) '("emacs" "source")))
>
> Since you already take only emacs inputs, is it really needed to remove
> "emacs" and "source"?  I mean emacs inputs do not contain "emacs" and
> "source" anyway, right?

emacs inputs contain "emacs-minimal" and "source".  So we actually need
to remove "emacs-minimal" instead "emacs".

    (emacs-inputs '(("emacs" . "/gnu/store/g1ldcr600kmdf2n1gsphk04hm30jr4bn-emacs-25.3")
                    ("emacs-minimal" . "/gnu/store/p4smq1mw13lmpkdbs59d7w827hy7mvgy-emacs-minimal-25.3")
                    ("emacs-dash" . "/gnu/store/dn7mygbi0pm985lz6qc64fsaz9f8zmfi-emacs-dash-2.13.0")
                    ("emacs-shut-up" . "/gnu/store/k0zddbwfwpdgj1ih2ypl50n09dfxhq1f-emacs-shut-up-0.3.2")
                    ("emacs-undercover" . "/gnu/store/ypcyxb3wpqlnf962k8ygp5csr6cmi6w3-emacs-undercover-0.6.0")
                    ("source" . "/gnu/store/gyxjrmhk4xqd8r78blxb92f9xc1z92fr-emacs-pos-tip-0.4.6.tar.gz")))

    (("emacs-minimal" . "/gnu/store/p4smq1mw13lmpkdbs59d7w827hy7mvgy-emacs-minimal-25.3")
     ("emacs-dash" . "/gnu/store/dn7mygbi0pm985lz6qc64fsaz9f8zmfi-emacs-dash-2.13.0")
     ("emacs-shut-up" . "/gnu/store/k0zddbwfwpdgj1ih2ypl50n09dfxhq1f-emacs-shut-up-0.3.2")
     ("emacs-undercover" . "/gnu/store/ypcyxb3wpqlnf962k8ygp5csr6cmi6w3-emacs-undercover-0.6.0")
     ("source" . "/gnu/store/gyxjrmhk4xqd8r78blxb92f9xc1z92fr-emacs-pos-tip-0.4.6.tar.gz"))

Thanks,
Oleg.

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

[bug#28832] [PATCH 1/3] gnu: Add emacs-json-reformat.

[Alex Kost]

Oleg Pykhalov (2017-12-20 06:26 +0300) wrote:

> Alex Kost <alezost@gmail.com> writes:
>
>> and I would make it more robust: there is a
>> problem with this code: (string-drop "geiser" 6) does not return what
>> you mean
>
> Yes, because "geiser" differs from most of Emacs packages:
>
>     ls
> /gnu/store/7rl2k8ismmyq9ic6ha6vzv38a3hrrni2-emacs-dash-2.13.0/share/emacs/site-lisp/guix.d/dash-2.13.0/
>     dash-autoloads.el  dash.el  dash.elc  dash-functional.el  dash-functional.elc
>
>     ls /gnu/store/7lh77fmapmjjv3kj2q69dy58kjniw9am-geiser-0.9/share/emacs/site-lisp/
>     geiser-autodoc.el    geiser-chibi.elc	…
>
> Maybe we just need to fix "geiser"?

Sorry, I don't understand what you mean.  What is wrong with geiser and
why/how should it be fixed?

Also do other non-"emacs-" packages (magit, emms) have the same problem?

>> , and (string-drop "dash" 6) errors!
>
> Do you really mean emacs-dash or dash?  For me both succeeded.

I meant just "dash" name.

> As I see emacs-dash is OK, except "emacs-minimal" in EMACSLOADPATH.
> Here is a log-file with DEBUG-INPUTS:

No-no, I didn't mean that the build process of 'emacs-dash' would fail,
I meant that your code uses 'string-drop' and it may fail:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> (string-drop "dash" 6)
ERROR: In procedure string-drop:
Value out of range 0 to 4: 6

Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
scheme@(guile-user) [1]> 
--8<---------------cut here---------------end--------------->8---

All I wanted to say is that emacs inputs do not necessarily have
"emacs-" prefix.  For example, 'emacs-direnv' and 'emacs-ag' packages
have "dash" input, but not "emacs-dash".

But now I see that it's not a problem: I mistakenly thought that the
names are taken from the input names, but now I see they are extracted
from the store file names, so my above concern is not relevant :-)

>> I think we shouldn't rely on the assumption that all emacs inputs have
>> "emacs-" prefix
>
> Then, how to determine that a package is Emacs package?

I don't know :-)  'emacs-inputs' is probably the best way.

>> , so I think
>> this procedure should check whether the input name begins with "emacs-"
>> before trying to remove this substring.
>>> +                                     "-" version))
>>> +                                  name+version)
>>> +                         input)))))
>>> +            (fold alist-delete (emacs-inputs inputs) '("emacs" "source")))
>>
>> Since you already take only emacs inputs, is it really needed to remove
>> "emacs" and "source"?  I mean emacs inputs do not contain "emacs" and
>> "source" anyway, right?
>
> emacs inputs contain "emacs-minimal" and "source".
> So we actually need to remove "emacs-minimal" instead "emacs".

or maybe both? since some packages uses 'emacs' instead of
'emacs-minimal' (emacs-auctex, emacs-exwm, etc.).

-- 
Alex

[bug#28832] [PATCH 1/3] gnu: Add emacs-json-reformat.

[Oleg Pykhalov]

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

Hello Alex,

Alex Kost <alezost@gmail.com> writes:

> Oleg Pykhalov (2017-12-20 06:26 +0300) wrote:

>> Yes, because "geiser" differs from most of Emacs packages:
>>
>>     ls
>> /gnu/store/7rl2k8ismmyq9ic6ha6vzv38a3hrrni2-emacs-dash-2.13.0/share/emacs/site-lisp/guix.d/dash-2.13.0/
>>     dash-autoloads.el  dash.el  dash.elc  dash-functional.el  dash-functional.elc
>>
>>     ls /gnu/store/7lh77fmapmjjv3kj2q69dy58kjniw9am-geiser-0.9/share/emacs/site-lisp/
>>     geiser-autodoc.el    geiser-chibi.elc	…
>>
>> Maybe we just need to fix "geiser"?
>
> Sorry, I don't understand what you mean.  What is wrong with geiser and
> why/how should it be fixed?

Elisp files of Geiser are in different place than others Emacs
packages.  There is no 'guix.d/geiser-0.9/'.

(for-each (match-lambda …) …) in 'setup-environment' will failed.

Either we need to handle this case specific for Geiser or just
change where it need to store Elisp files in 'geiser' package recipe.

> Also do other non-"emacs-" packages (magit, emms) have the same problem?

Hm,

    /gnu/store/k9zrrzpdw0mld0lqyackba3kwbw41ipr-emacs-emms-4.3/share/emacs/site-lisp/
    /gnu/store/zihybmvkccjb310fsxc2sad5j0w5vdi1-magit-2.11.0/share/emacs/stie-lisp/

it seems that it will be easier to handle a case without
'guix.d/PACKAGE-VERSION/'.

But I don't see a way to determine is magit an Emacs package, because
there is no "emacs-" prefix in "/gnu/store/…-magit-2.11.0".

'emacs-inputs' will not help.  See below.

>>> I think we shouldn't rely on the assumption that all emacs inputs have
>>> "emacs-" prefix
>>
>> Then, how to determine that a package is Emacs package?
>
> I don't know :-)  'emacs-inputs' is probably the best way.

No :-), it only relies on "emacs-" prefix in store.

emacs-inputs -> emacs-package? -> (string-prefix? "emacs-" name)

>> emacs inputs contain "emacs-minimal" and "source".
>> So we actually need to remove "emacs-minimal" instead "emacs".
>
> or maybe both? since some packages uses 'emacs' instead of
> 'emacs-minimal' (emacs-auctex, emacs-exwm, etc.).

Not both, because 'emacs-inputs' removes all inputs without "emacs-"
prefix, so 'emacs' too.

(emacs-inputs '(("emacs" . "/gnu/store/g1ldcr600kmdf2n1gsphk04hm30jr4bn-emacs-25.3")
                ("emacs-minimal" . "/gnu/store/p4smq1mw13lmpkdbs59d7w827hy7mvgy-emacs-minimal-25.3")))
$3 = (("emacs-minimal" . "/gnu/store/p4smq1mw13lmpkdbs59d7w827hy7mvgy-emacs-minimal-25.3"))

Thanks,
Oleg.

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

[bug#28832] [PATCH 1/3] gnu: Add emacs-json-reformat.

[Alex Kost]

Oleg Pykhalov (2017-12-21 07:48 +0300) wrote:

> Hello Alex,
>
> Alex Kost <alezost@gmail.com> writes:
>
>> Oleg Pykhalov (2017-12-20 06:26 +0300) wrote:
[...]
>>> Maybe we just need to fix "geiser"?
>>
>> Sorry, I don't understand what you mean.  What is wrong with geiser and
>> why/how should it be fixed?
>
> Elisp files of Geiser are in different place than others Emacs
> packages.  There is no 'guix.d/geiser-0.9/'.

Oh, now I see what you mean.

> (for-each (match-lambda …) …) in 'setup-environment' will failed.
>
> Either we need to handle this case specific for Geiser or just
> change where it need to store Elisp files in 'geiser' package recipe.

It is not a specific Geiser case: installing *.el files in
"share/emacs/site-lisp/" is a common practice.  Actually, it is the
default place where automake wants to install elisp files (using
AM_PATH_LISPDIR macro), so whenever a package uses gnu-build-system,
most likely it will install elisp files to the site-lisp directory.  For
example, the following packages do this: bbdb, gettext, emms, magit,
emacs-wget, emacs-w3m, emacs-mmm-mode,...

>> Also do other non-"emacs-" packages (magit, emms) have the same problem?
>
> Hm,
>
>     /gnu/store/k9zrrzpdw0mld0lqyackba3kwbw41ipr-emacs-emms-4.3/share/emacs/site-lisp/
>     /gnu/store/zihybmvkccjb310fsxc2sad5j0w5vdi1-magit-2.11.0/share/emacs/stie-lisp/
>
> it seems that it will be easier to handle a case without
> 'guix.d/PACKAGE-VERSION/'.

I also think so, for example, 'emacs-inputs-el-directories' procedure
simply adds "/share/emacs/site-lisp" along with the "guix.d/..." directory.

[...]
>>>> I think we shouldn't rely on the assumption that all emacs inputs have
>>>> "emacs-" prefix
>>>
>>> Then, how to determine that a package is Emacs package?
>>
>> I don't know :-)  'emacs-inputs' is probably the best way.
>
> No :-), it only relies on "emacs-" prefix in store.
> emacs-inputs -> emacs-package? -> (string-prefix? "emacs-" name)

Yeah, I understand this.  I meant this is the best way we have at our
disposal.  I also don't know how to determine emacs packages without
"emacs-" prefix (well, maybe by looking for *.el files inside the
package store dir, not sure if it's suitable though).

>>> emacs inputs contain "emacs-minimal" and "source".
>>> So we actually need to remove "emacs-minimal" instead "emacs".
>>
>> or maybe both? since some packages uses 'emacs' instead of
>> 'emacs-minimal' (emacs-auctex, emacs-exwm, etc.).
>
> Not both, because 'emacs-inputs' removes all inputs without "emacs-"
> prefix, so 'emacs' too.

Oh right, sorry.

So to clarify the current situation, we have 2 problems:

1. 'emacs-package?' defines emacs package simply by checking "emacs-"
prefix, so it doesn't find such packages as magit or geiser.  This
problem does not relate directly to your patch; rather it is the problem
of the current 'emacs-build-system': if some emacs package depends on
'magit' or 'geiser' (currently there are no such packages),
emacs-build-system will not compile *.el files (because it will not find
'magit'/'geiser' needed for compilation).

2. Your patch handles only "/share/emacs/site-lisp/guix.d/<foo>/" but
not "/share/emacs/site-lisp/".

-- 
Alex

[bug#28832] [PATCH 1/3] gnu: Add emacs-json-reformat.

[Alex Kost]

Hello, I have only one addition to the Ludovic's comments.

Oleg Pykhalov (2017-12-15 12:36 +0300) wrote:

[...]
> +(define* (setup-environment #:key inputs #:allow-other-keys)
> +  "Export the variable EMACSLOADPATH, which are based on INPUTS respectively."
> +  (let* ((filtered-inputs (emacs-inputs inputs))
> +         (emacs-input-dir (cdr (assoc "emacs" filtered-inputs)))
> +         (inputs-dirs (cdr filtered-inputs)))
> +    ;; EMACSLOADPATH is where Emacs looks for the source code of the build's
> +    ;; dependencies.
> +    (setenv "EMACSLOADPATH"
> +            (string-append emacs-input-dir "/share/emacs/"
> +                           (store-directory->package-version emacs-input-dir)
> +                           "/lisp"))
> +    (for-each
> +     (lambda (input)
> +       (let ((store-item (cdr input)))
> +         (setenv "EMACSLOADPATH"
> +                 (string-append
> +                  (or (getenv "EMACSLOADPATH") "")
> +                  ":" store-item %install-suffix "/"
> +                  ((compose string-drop-emacs store-directory->package-name)
> +                   store-item)))))
> +     ((compose (lambda (inputs) (alist-delete "emacs" inputs))
> +               (lambda (inputs) (alist-delete "source" inputs)))

"emacs" and "source" may not be the only inputs that should be deleted.
For example, 'emacs-exwm-x' package also has "xhost" and "dbus" inputs.
And your 'string-drop-emacs' will fail on these names, right?

Since there is 'emacs-package?' function in this module, perhaps it
would be better to use it instead (to add only emacs packages to
EMACSLOADPATH), WDYT?

P.S.  Actually, I didn't test your patch, so maybe I just didn't
understand the code properly :-)

-- 
Alex

[bug#28832] [PATCH 1/3] gnu: Add emacs-json-reformat.

[Oleg Pykhalov]

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

Hello Alex,

Alex Kost <alezost@gmail.com> writes:

[...]

> "emacs" and "source" may not be the only inputs that should be deleted.
> For example, 'emacs-exwm-x' package also has "xhost" and "dbus" inputs.
> And your 'string-drop-emacs' will fail on these names, right?
>
> Since there is 'emacs-package?' function in this module, perhaps it
> would be better to use it instead (to add only emacs packages to
> EMACSLOADPATH), WDYT?

Yes, this is why I used 'emacs-inputs'.  It evaluates 'emacs-package?'
as I see.  So we peek only "/gnu/store/…-emacs-" packages.

[...]

Thanks,
Oleg.

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

[bug#28832] [PATCH 1/3] gnu: Add emacs-json-reformat.

[Ludovic Courtès]

Hi Oleg,

ludo@gnu.org (Ludovic Courtès) skribis:

> Oleg Pykhalov <go.wigust@gmail.com> skribis:
>
>> ludo@gnu.org (Ludovic Courtès) writes:

[...]

>> From d589de21acb02d0fba7aee3b48e5f42d7bd8957e Mon Sep 17 00:00:00 2001
>> From: Oleg Pykhalov <go.wigust@gmail.com>
>> Date: Tue, 12 Dec 2017 01:41:08 +0300
>> Subject: [PATCH] gnu: Add emacs-json-reformat.
>>
>> * gnu/packages/patches/emacs-json-reformat-fix-tests.patch: New file.
>> * gnu/local.mk (dist_patch_DATA): Add this.
>> * gnu/packages/emacs.scm (emacs-json-reformat): New variable.
>
> [...]
>
>> diff --git a/gnu/packages/patches/emacs-json-reformat-fix-tests.patch b/gnu/packages/patches/emacs-json-reformat-fix-tests.patch
>> new file mode 100644
>> index 000000000..23a239582
>> --- /dev/null
>> +++ b/gnu/packages/patches/emacs-json-reformat-fix-tests.patch
>> @@ -0,0 +1,28 @@
>> +Copyright © 2017 Oleg Pykhalov <go.wigust@gmail.com>
>> +
>> +This patch fixes tests for Emacs 25.
>
> Please mention <https://github.com/gongo/json-reformat/issues/33> here.
>
> OK with these changes!

We went on discussing other things and forgot about the patch.  :-)
Could you commit it with these changes, Oleg?

Thanks,
Ludo’.

bug#28832: [PATCH 1/3] gnu: Add emacs-json-reformat.

[Oleg Pykhalov]

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

Hello Ludovic,

ludo@gnu.org (Ludovic Courtès) writes:

[...]

>>> From d589de21acb02d0fba7aee3b48e5f42d7bd8957e Mon Sep 17 00:00:00 2001
>>> From: Oleg Pykhalov <go.wigust@gmail.com>
>>> Date: Tue, 12 Dec 2017 01:41:08 +0300
>>> Subject: [PATCH] gnu: Add emacs-json-reformat.
>>>
>>> * gnu/packages/patches/emacs-json-reformat-fix-tests.patch: New file.
>>> * gnu/local.mk (dist_patch_DATA): Add this.
>>> * gnu/packages/emacs.scm (emacs-json-reformat): New variable.
>>
>> [...]
>>
>>> diff --git a/gnu/packages/patches/emacs-json-reformat-fix-tests.patch b/gnu/packages/patches/emacs-json-reformat-fix-tests.patch
>>> new file mode 100644
>>> index 000000000..23a239582
>>> --- /dev/null
>>> +++ b/gnu/packages/patches/emacs-json-reformat-fix-tests.patch
>>> @@ -0,0 +1,28 @@
>>> +Copyright © 2017 Oleg Pykhalov <go.wigust@gmail.com>
>>> +
>>> +This patch fixes tests for Emacs 25.
>>
>> Please mention <https://github.com/gongo/json-reformat/issues/33> here.
>>
>> OK with these changes!
>
> We went on discussing other things and forgot about the patch.  :-)
> Could you commit it with these changes, Oleg?

I thought we suspend this until automation of "-L" flags.
Seems like a similar discussion[1] gets stuck too.
Pushed as b0912e9fdbffab15d9a754b2922778cfbd1fac2a

Also pushed emacs-json-mode as dd72837dff128dbb1258826fe39467d1ef000ac1

I'll close current bug report.

Footnotes: 
[1]  https://lists.nongnu.org/archive/html/guix-patches/2017-12/msg00640.html

Thanks,
Oleg.

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

[bug#28832] [PATCH 1/3] gnu: Add emacs-json-reformat.

[Ludovic Courtès]

Hi Oleg,

Oleg Pykhalov <go.wigust@gmail.com> skribis:

> ludo@gnu.org (Ludovic Courtès) writes:
>
> [...]
>
>>>> From d589de21acb02d0fba7aee3b48e5f42d7bd8957e Mon Sep 17 00:00:00 2001
>>>> From: Oleg Pykhalov <go.wigust@gmail.com>
>>>> Date: Tue, 12 Dec 2017 01:41:08 +0300
>>>> Subject: [PATCH] gnu: Add emacs-json-reformat.
>>>>
>>>> * gnu/packages/patches/emacs-json-reformat-fix-tests.patch: New file.
>>>> * gnu/local.mk (dist_patch_DATA): Add this.
>>>> * gnu/packages/emacs.scm (emacs-json-reformat): New variable.
>>>
>>> [...]
>>>
>>>> diff --git a/gnu/packages/patches/emacs-json-reformat-fix-tests.patch b/gnu/packages/patches/emacs-json-reformat-fix-tests.patch
>>>> new file mode 100644
>>>> index 000000000..23a239582
>>>> --- /dev/null
>>>> +++ b/gnu/packages/patches/emacs-json-reformat-fix-tests.patch
>>>> @@ -0,0 +1,28 @@
>>>> +Copyright © 2017 Oleg Pykhalov <go.wigust@gmail.com>
>>>> +
>>>> +This patch fixes tests for Emacs 25.
>>>
>>> Please mention <https://github.com/gongo/json-reformat/issues/33> here.
>>>
>>> OK with these changes!
>>
>> We went on discussing other things and forgot about the patch.  :-)
>> Could you commit it with these changes, Oleg?
>
> I thought we suspend this until automation of "-L" flags.

No no, the “OK with these changes” above really means you can go ahead.
:-)

The “-L” discussion is about an improvement we should make, but it
should not block this patch.

> Seems like a similar discussion[1] gets stuck too.
> Pushed as b0912e9fdbffab15d9a754b2922778cfbd1fac2a

Yeah, though I don’t think you should wait for Alex or anyone else to
make a decision: if you have an idea on how to improve this, based on
these discussions, then you’re probably in a good position to do it and
everyone will be happy.  ;-)

> Also pushed emacs-json-mode as dd72837dff128dbb1258826fe39467d1ef000ac1

Great, thank you!

Ludo’.

[bug#28832] [PATCH 1/3] gnu: Add emacs-json-reformat.

[Alex Kost]

Ludovic Courtès (2018-01-15 14:33 +0100) wrote:

> Oleg Pykhalov <go.wigust@gmail.com> skribis:
[...]
>> I thought we suspend this until automation of "-L" flags.
>
> No no, the “OK with these changes” above really means you can go ahead.
> :-)
>
> The “-L” discussion is about an improvement we should make, but it
> should not block this patch.
>
>> Seems like a similar discussion[1] gets stuck too.
>> Pushed as b0912e9fdbffab15d9a754b2922778cfbd1fac2a
>
> Yeah, though I don’t think you should wait for Alex or anyone else to
> make a decision: if you have an idea on how to improve this, based on
> these discussions, then you’re probably in a good position to do it and
> everyone will be happy.  ;-)

I didn't think someone waits for me.  My last message was:

  https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28832#71

and I wasn't going to write anything else until a reply.  To make it
clear: I don't mind any decision you make, I participated in this
discussion just because I was CC-ed.  Please, go on with whatever seems
appropriate to you.

-- 
Alex