diff mbox

[1/2] build: emacs: Handle sources that are a single elisp file.

Message ID 1464358215-4416-1-git-send-email-dthompson2@worcester.edu
State New
Headers show

Commit Message

David Thompson May 27, 2016, 2:10 p.m. UTC
* guix/build/emacs-build-system.scm (gnu:unpack)
(store-file->elisp-source-file, unpack): New procedures.
(%standard-phases): Use the new unpack procedure.
---
 guix/build/emacs-build-system.scm | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Comments

Alex Kost May 28, 2016, 1:59 p.m. UTC | #1
David Thompson (2016-05-27 17:10 +0300) wrote:

> * guix/build/emacs-build-system.scm (gnu:unpack)
> (store-file->elisp-source-file, unpack): New procedures.
> (%standard-phases): Use the new unpack procedure.

I think there is no need to improve emacs-build-system to handle single
file sources: there is 'uncompressed-file-fetch' method for this
purpose.  Look in (gnu packages emacs) for this procedure and how it is
used (for example, by 'emacs-rfcview' package).

I Cc-ed Federico as he added 'uncompressed-file-fetch'.
Ludovic Courtès May 28, 2016, 3:36 p.m. UTC | #2
David Thompson <dthompson2@worcester.edu> skribis:

> * guix/build/emacs-build-system.scm (gnu:unpack)
> (store-file->elisp-source-file, unpack): New procedures.
> (%standard-phases): Use the new unpack procedure.

Good idea, LGTM!

Could you adjust users of ‘uncompressed-file-fetch’ in a subsequent
commit, and remove ‘uncompressed-file-fetch’?

Thanks,
Ludo’.
Alex Kost May 28, 2016, 7:05 p.m. UTC | #3
Ludovic Courtès (2016-05-28 18:36 +0300) wrote:

> David Thompson <dthompson2@worcester.edu> skribis:
>
>> * guix/build/emacs-build-system.scm (gnu:unpack)
>> (store-file->elisp-source-file, unpack): New procedures.
>> (%standard-phases): Use the new unpack procedure.
>
> Good idea, LGTM!
>
> Could you adjust users of ‘uncompressed-file-fetch’ in a subsequent
> commit, and remove ‘uncompressed-file-fetch’?

I object!  I mean this should be discussed at least.  I would really
prefer to keep (and document) 'uncompressed-file-fetch' and not to
update emacs-build-system for this case.  It is possible, that once
we'll need to handle non-compressed files for another build system.  So
it should also be adjusted in the same way.  But if we keep
'uncompressed-file-fetch', it will be a general decision as it can be
used for any build system.
Ludovic Courtès May 29, 2016, 9:50 p.m. UTC | #4
Alex Kost <alezost@gmail.com> skribis:

> Ludovic Courtès (2016-05-28 18:36 +0300) wrote:
>
>> David Thompson <dthompson2@worcester.edu> skribis:
>>
>>> * guix/build/emacs-build-system.scm (gnu:unpack)
>>> (store-file->elisp-source-file, unpack): New procedures.
>>> (%standard-phases): Use the new unpack procedure.
>>
>> Good idea, LGTM!
>>
>> Could you adjust users of ‘uncompressed-file-fetch’ in a subsequent
>> commit, and remove ‘uncompressed-file-fetch’?
>
> I object!

Damn it, sorry, I thought this would be uncontroversial.

> I mean this should be discussed at least.  I would really prefer to
> keep (and document) 'uncompressed-file-fetch' and not to update
> emacs-build-system for this case.  It is possible, that once we'll
> need to handle non-compressed files for another build system.  So it
> should also be adjusted in the same way.  But if we keep
> 'uncompressed-file-fetch', it will be a general decision as it can be
> used for any build system.

In my view, ‘uncompressed-file-fetch’ and the ‘emacs-build-system’
change that Dave proposes are equally good hacks, but the latter has the
advantage that people won’t have to think about it: they can just use
‘url-fetch’ and ‘emacs-build-system’ as usual and things will just work.

Of course, perhaps we should consider handling flat files closer to the
core, but so far the only use case we have, AFAIK, is .el files.  Thus,
it doesn’t seem that bad to add a special case in ‘emacs-build-system’.
Pragmatic approach I suppose.  ;-)

WDYT?

Thanks,
Ludo’.
Alex Kost May 30, 2016, 10:14 a.m. UTC | #5
Ludovic Courtès (2016-05-30 00:50 +0300) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> Ludovic Courtès (2016-05-28 18:36 +0300) wrote:
>>
>>> David Thompson <dthompson2@worcester.edu> skribis:
>>>
>>>> * guix/build/emacs-build-system.scm (gnu:unpack)
>>>> (store-file->elisp-source-file, unpack): New procedures.
>>>> (%standard-phases): Use the new unpack procedure.
>>>
>>> Good idea, LGTM!
>>>
>>> Could you adjust users of ‘uncompressed-file-fetch’ in a subsequent
>>> commit, and remove ‘uncompressed-file-fetch’?
>>
>> I object!
>
> Damn it, sorry, I thought this would be uncontroversial.
>
>> I mean this should be discussed at least.  I would really prefer to
>> keep (and document) 'uncompressed-file-fetch' and not to update
>> emacs-build-system for this case.  It is possible, that once we'll
>> need to handle non-compressed files for another build system.  So it
>> should also be adjusted in the same way.  But if we keep
>> 'uncompressed-file-fetch', it will be a general decision as it can be
>> used for any build system.
>
> In my view, ‘uncompressed-file-fetch’ and the ‘emacs-build-system’
> change that Dave proposes are equally good hacks, but the latter has the
> advantage that people won’t have to think about it: they can just use
> ‘url-fetch’ and ‘emacs-build-system’ as usual and things will just work.
>
> Of course, perhaps we should consider handling flat files closer to the
> core, but so far the only use case we have, AFAIK, is .el files.  Thus,
> it doesn’t seem that bad to add a special case in ‘emacs-build-system’.
> Pragmatic approach I suppose.  ;-)
>
> WDYT?

<cough>, OK, I wanted to write verbosely why I prefer
uncompressed-file-fetch, and why we should still use it, etc.;

but I've just noticed an unpleasant downside with
‘uncompressed-file-fetch’: for example, if you build the recently added
"emacs-queue" package, you'll get "queue-0.1.1.el" file, which makes it
impossible to do (require 'queue).  With David's solution, it will be a
proper "queue.el" file.

So I agree now.  David's patch is definitely a better solution :-)
Catonano May 30, 2016, 11:55 a.m. UTC | #6
2016-05-30 12:14 GMT+02:00 Alex Kost <alezost@gmail.com>:

>
> <cough>, OK, I wanted to write verbosely why I prefer
> uncompressed-file-fetch, and why we should still use it, etc.;
>
> but I've just noticed an unpleasant downside with
> ‘uncompressed-file-fetch’: for example, if you build the recently added
> "emacs-queue" package, you'll get "queue-0.1.1.el" file, which makes it
> impossible to do (require 'queue).  With David's solution, it will be a
> proper "queue.el" file.
>
> So I agree now.  David's patch is definitely a better solution :-)
>
>
Ah ! This is probably why I couldn't manage emacs-cider to require
emacs-spinner and emacs-queue. They both are packaged with
uncompressed-file-fetch.

In fact queue and spinner work if I installl them on their own.

After I attempt with cider, they not only are not requirable but they are
also not autoloaded anymore.

So now I'll try stick with more tested download methods and see if cider
works with those.
Catonano May 30, 2016, 12:42 p.m. UTC | #7
2016-05-30 13:55 GMT+02:00 Catonano <catonano@gmail.com>:

> 2016-05-30 12:14 GMT+02:00 Alex Kost <alezost@gmail.com>:
>
>>
>> <cough>, OK, I wanted to write verbosely why I prefer
>> uncompressed-file-fetch, and why we should still use it, etc.;
>>
>> but I've just noticed an unpleasant downside with
>> ‘uncompressed-file-fetch’: for example, if you build the recently added
>> "emacs-queue" package, you'll get "queue-0.1.1.el" file, which makes it
>> impossible to do (require 'queue).  With David's solution, it will be a
>> proper "queue.el" file.
>>
>> So I agree now.  David's patch is definitely a better solution :-)
>>
>>
> Ah ! This is probably why I couldn't manage emacs-cider to require
> emacs-spinner and emacs-queue. They both are packaged with
> uncompressed-file-fetch.
>
> In fact queue and spinner work if I installl them on their own.
>
> After I attempt with cider, they not only are not requirable but they are
> also not autoloaded anymore.
>
> So now I'll try stick with more tested download methods and see if cider
> works with those.
>
>

queue is available only as an uncompresse file

So I'll wait for this patch to be merged and then I'll try with the new
emacs-build-system
David Thompson May 30, 2016, 3:12 p.m. UTC | #8
On Mon, May 30, 2016 at 6:14 AM, Alex Kost <alezost@gmail.com> wrote:
> Ludovic Courtès (2016-05-30 00:50 +0300) wrote:
>
>> Alex Kost <alezost@gmail.com> skribis:
>>
>>> Ludovic Courtès (2016-05-28 18:36 +0300) wrote:
>>>
>>>> David Thompson <dthompson2@worcester.edu> skribis:
>>>>
>>>>> * guix/build/emacs-build-system.scm (gnu:unpack)
>>>>> (store-file->elisp-source-file, unpack): New procedures.
>>>>> (%standard-phases): Use the new unpack procedure.
>>>>
>>>> Good idea, LGTM!
>>>>
>>>> Could you adjust users of ‘uncompressed-file-fetch’ in a subsequent
>>>> commit, and remove ‘uncompressed-file-fetch’?
>>>
>>> I object!
>>
>> Damn it, sorry, I thought this would be uncontroversial.
>>
>>> I mean this should be discussed at least.  I would really prefer to
>>> keep (and document) 'uncompressed-file-fetch' and not to update
>>> emacs-build-system for this case.  It is possible, that once we'll
>>> need to handle non-compressed files for another build system.  So it
>>> should also be adjusted in the same way.  But if we keep
>>> 'uncompressed-file-fetch', it will be a general decision as it can be
>>> used for any build system.
>>
>> In my view, ‘uncompressed-file-fetch’ and the ‘emacs-build-system’
>> change that Dave proposes are equally good hacks, but the latter has the
>> advantage that people won’t have to think about it: they can just use
>> ‘url-fetch’ and ‘emacs-build-system’ as usual and things will just work.
>>
>> Of course, perhaps we should consider handling flat files closer to the
>> core, but so far the only use case we have, AFAIK, is .el files.  Thus,
>> it doesn’t seem that bad to add a special case in ‘emacs-build-system’.
>> Pragmatic approach I suppose.  ;-)
>>
>> WDYT?
>
> <cough>, OK, I wanted to write verbosely why I prefer
> uncompressed-file-fetch, and why we should still use it, etc.;
>
> but I've just noticed an unpleasant downside with
> ‘uncompressed-file-fetch’: for example, if you build the recently added
> "emacs-queue" package, you'll get "queue-0.1.1.el" file, which makes it
> impossible to do (require 'queue).  With David's solution, it will be a
> proper "queue.el" file.
>
> So I agree now.  David's patch is definitely a better solution :-)

Hehe.  Thanks for the discussion.  Pushed!

- Dave
Mathieu Lirzin May 30, 2016, 4:27 p.m. UTC | #9
Hi,

I am a bit late.

David Thompson <dthompson2@worcester.edu> writes:

> * guix/build/emacs-build-system.scm (gnu:unpack)
> (store-file->elisp-source-file, unpack): New procedures.
> (%standard-phases): Use the new unpack procedure.
> ---
>  guix/build/emacs-build-system.scm | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/guix/build/emacs-build-system.scm b/guix/build/emacs-build-system.scm
> index f0a9a6e..4fd36d1 100644
> --- a/guix/build/emacs-build-system.scm
> +++ b/guix/build/emacs-build-system.scm
> @@ -21,6 +21,7 @@
>    #:use-module (guix build utils)
>    #:use-module (guix build emacs-utils)
>    #:use-module (srfi srfi-1)
> +  #:use-module (srfi srfi-11)
>    #:use-module (srfi srfi-26)
>    #:use-module (ice-9 rdelim)
>    #:use-module (ice-9 regex)
> @@ -39,6 +40,27 @@
>  ;; archive signature.
>  (define %install-suffix "/share/emacs/site-lisp/guix.d")
>  
> +(define gnu:unpack (assoc-ref gnu:%standard-phases 'unpack))
> +
> +(define (store-file->elisp-source-file file)
> +  "Convert file, a store file name for an Emacs Lisp source file, into a file
> +name that has been stripped of the hash and version number."
> +  (let-values (((name version)
> +                (package-name->name+version
                    ^^^

This is the old ‘package-name->name+version’ from (guix build utils)
which has been replaced when possible by a new one in (guix utils) using
'@' as a delimiter.  While I think it was OK to use it to fix previously
written code, I don't want Guix to build upon the old one.

Time has come to resolve this ugly and confusing name conflict.  The
problem is that I don't fully understand the rationale behind this
temporary solution, so I can't help much.

Ludo: Since you are the mind behind it, I think you are in the best
position to figure this out or at least explain to us “mere mortals”
what is possible and what is not.  :)
Ludovic Courtès June 8, 2016, 12:44 p.m. UTC | #10
Mathieu Lirzin <mthl@gnu.org> skribis:

> David Thompson <dthompson2@worcester.edu> writes:
>
>> * guix/build/emacs-build-system.scm (gnu:unpack)
>> (store-file->elisp-source-file, unpack): New procedures.
>> (%standard-phases): Use the new unpack procedure.
>> ---
>>  guix/build/emacs-build-system.scm | 23 +++++++++++++++++++++++
>>  1 file changed, 23 insertions(+)
>>
>> diff --git a/guix/build/emacs-build-system.scm b/guix/build/emacs-build-system.scm
>> index f0a9a6e..4fd36d1 100644
>> --- a/guix/build/emacs-build-system.scm
>> +++ b/guix/build/emacs-build-system.scm
>> @@ -21,6 +21,7 @@
>>    #:use-module (guix build utils)
>>    #:use-module (guix build emacs-utils)
>>    #:use-module (srfi srfi-1)
>> +  #:use-module (srfi srfi-11)
>>    #:use-module (srfi srfi-26)
>>    #:use-module (ice-9 rdelim)
>>    #:use-module (ice-9 regex)
>> @@ -39,6 +40,27 @@
>>  ;; archive signature.
>>  (define %install-suffix "/share/emacs/site-lisp/guix.d")
>>  
>> +(define gnu:unpack (assoc-ref gnu:%standard-phases 'unpack))
>> +
>> +(define (store-file->elisp-source-file file)
>> +  "Convert file, a store file name for an Emacs Lisp source file, into a file
>> +name that has been stripped of the hash and version number."
>> +  (let-values (((name version)
>> +                (package-name->name+version
>                     ^^^
>
> This is the old ‘package-name->name+version’ from (guix build utils)
> which has been replaced when possible by a new one in (guix utils) using
> '@' as a delimiter.  While I think it was OK to use it to fix previously
> written code, I don't want Guix to build upon the old one.

We kept this procedure in (guix build utils) because it’s still used.
In this case, we have “foo-1.2”, and we want to get the values “foo” and
“1.2”, so using this procedure is the right thing to do.

> Time has come to resolve this ugly and confusing name conflict.  The
> problem is that I don't fully understand the rationale behind this
> temporary solution, so I can't help much.
>
> Ludo: Since you are the mind behind it, I think you are in the best
> position to figure this out or at least explain to us “mere mortals”
> what is possible and what is not.  :)

Naming is one of the hardest problems in programming, you know!  :-)

So we have:

  (guix build utils):package-name->name+version
    which expects “foo-1.2”

  (guix utils):package-name->name+version
    which expects “foo@1.2”

There’s no doubt we need to keep both, so what we can do is rename one
of them.

What about renaming the one in (guix utils) to
‘package-specification->name+version’?

Bonus points if you provide a patch!  :-)

Thanks,
Ludo’.
diff mbox

Patch

diff --git a/guix/build/emacs-build-system.scm b/guix/build/emacs-build-system.scm
index f0a9a6e..4fd36d1 100644
--- a/guix/build/emacs-build-system.scm
+++ b/guix/build/emacs-build-system.scm
@@ -21,6 +21,7 @@ 
   #:use-module (guix build utils)
   #:use-module (guix build emacs-utils)
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-26)
   #:use-module (ice-9 rdelim)
   #:use-module (ice-9 regex)
@@ -39,6 +40,27 @@ 
 ;; archive signature.
 (define %install-suffix "/share/emacs/site-lisp/guix.d")
 
+(define gnu:unpack (assoc-ref gnu:%standard-phases 'unpack))
+
+(define (store-file->elisp-source-file file)
+  "Convert file, a store file name for an Emacs Lisp source file, into a file
+name that has been stripped of the hash and version number."
+  (let-values (((name version)
+                (package-name->name+version
+                 (strip-store-file-name file))))
+    (string-append name ".el")))
+
+(define* (unpack #:key source #:allow-other-keys)
+  "Unpack SOURCE into the build directory.  SOURCE may be a compressed
+archive, a directory, or an Emacs Lisp file."
+  (if (string-suffix? ".el" source)
+      (begin
+        (mkdir "source")
+        (chdir "source")
+        (copy-file source (store-file->elisp-source-file source))
+        #t)
+      (gnu:unpack #:source source)))
+
 (define* (build #:key outputs inputs #:allow-other-keys)
   "Compile .el files."
   (let* ((emacs (string-append (assoc-ref inputs "emacs") "/bin/emacs"))
@@ -151,6 +173,7 @@  second hyphen.  This corresponds to 'name-version' as used in ELPA packages."
 
 (define %standard-phases
   (modify-phases gnu:%standard-phases
+    (replace 'unpack unpack)
     (delete 'configure)
     (delete 'check)
     (delete 'install)