* [PATCH] Elpa importer improvements
@ 2016-12-14 2:04 Carlo Zancanaro
2016-12-23 1:57 ` Carlo Zancanaro
2016-12-27 8:54 ` Alex Kost
0 siblings, 2 replies; 5+ messages in thread
From: Carlo Zancanaro @ 2016-12-14 2:04 UTC (permalink / raw)
To: Guix-devel
[-- Attachment #1.1: Type: text/plain, Size: 402 bytes --]
I was trying to import some elpa packages recently and found some
issues, so here are some patches to fix them.
1. call-with-downloaded-file had behaviour different to how it was
documented to behave. This was primarily a problem when trying to
import packages with no description (eg. color-theme-solarized on
melpa).
2. Package dependencies need to be propagated so that emacs can
find them.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-import-elpa-Fix-call-with-downloaded-file.patch --]
[-- Type: text/x-diff, Size: 1201 bytes --]
From 0e561aee91b5d389d72906d059ddf486a322f20a Mon Sep 17 00:00:00 2001
From: Carlo Zancanaro <carlo@zancanaro.id.au>
Date: Wed, 14 Dec 2016 12:31:12 +1100
Subject: [PATCH 1/2] import: elpa: Fix call-with-downloaded-file
* guix/import/elpa.scm (call-with-downloaded-file): Make function behaviour
match documentation string.
---
guix/import/elpa.scm | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/guix/import/elpa.scm b/guix/import/elpa.scm
index 320a09e8c..5f8b7a9e5 100644
--- a/guix/import/elpa.scm
+++ b/guix/import/elpa.scm
@@ -89,7 +89,13 @@ NAMES (strings)."
"Fetch URL, store the content in a temporary file and call PROC with that
file. Returns the value returned by PROC. On error call ERROR-THUNK and
return its value or leave if it's false."
- (proc (http-fetch/cached (string->uri url))))
+ (catch #t
+ (lambda ()
+ (proc (http-fetch/cached (string->uri url))))
+ (lambda (key . args)
+ (if error-thunk
+ (error-thunk)
+ (apply throw key args)))))
(define (is-elpa-package? name elpa-pkg-spec)
"Return true if the string NAME corresponds to the name of the package
--
2.11.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.3: 0002-import-elpa-Import-dependencies-as-propagated-inputs.patch --]
[-- Type: text/x-diff, Size: 1063 bytes --]
From 88f23b9c369841837a83225b52e19f4c029906ab Mon Sep 17 00:00:00 2001
From: Carlo Zancanaro <carlo@zancanaro.id.au>
Date: Wed, 14 Dec 2016 12:34:15 +1100
Subject: [PATCH 2/2] import: elpa: Import dependencies as propagated-inputs
* guix/import/elpa.scm (elpa-package->sexp): Import dependencies as
propagated-inputs.
---
guix/import/elpa.scm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/guix/import/elpa.scm b/guix/import/elpa.scm
index 5f8b7a9e5..897ce6a69 100644
--- a/guix/import/elpa.scm
+++ b/guix/import/elpa.scm
@@ -228,7 +228,7 @@ type '<elpa-package>'."
(bytevector->nix-base32-string (file-sha256 tarball))
"failed to download package")))))
(build-system emacs-build-system)
- ,@(maybe-inputs 'inputs dependencies)
+ ,@(maybe-inputs 'propagated-inputs dependencies)
(home-page ,(elpa-package-home-page pkg))
(synopsis ,(elpa-package-synopsis pkg))
(description ,(elpa-package-description pkg))
--
2.11.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Elpa importer improvements
2016-12-14 2:04 [PATCH] Elpa importer improvements Carlo Zancanaro
@ 2016-12-23 1:57 ` Carlo Zancanaro
2016-12-27 8:54 ` Alex Kost
1 sibling, 0 replies; 5+ messages in thread
From: Carlo Zancanaro @ 2016-12-23 1:57 UTC (permalink / raw)
To: Guix-devel
[-- Attachment #1: Type: text/plain, Size: 601 bytes --]
On Wed, Dec 14 2016, Carlo Zancanaro wrote
> I was trying to import some elpa packages recently and found
> some issues, so here are some patches to fix them.
>
> 1. call-with-downloaded-file had behaviour different to how it
> was documented to behave. This was primarily a problem when
> trying to import packages with no description (eg.
> color-theme-solarized on melpa).
>
> 2. Package dependencies need to be propagated so that emacs can
> find them.
Has anyone looked at this? This is relatively minor, but now that the
0.12.0 release is done I thought a ping might be appropriate.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Elpa importer improvements
2016-12-14 2:04 [PATCH] Elpa importer improvements Carlo Zancanaro
2016-12-23 1:57 ` Carlo Zancanaro
@ 2016-12-27 8:54 ` Alex Kost
2016-12-27 10:20 ` Carlo Zancanaro
2016-12-29 17:16 ` Ludovic Courtès
1 sibling, 2 replies; 5+ messages in thread
From: Alex Kost @ 2016-12-27 8:54 UTC (permalink / raw)
To: Carlo Zancanaro; +Cc: Guix-devel, Federico Beffa
Carlo Zancanaro (2016-12-14 13:04 +1100) wrote:
> I was trying to import some elpa packages recently and found some
> issues, so here are some patches to fix them.
>
> 1. call-with-downloaded-file had behaviour different to how it was
> documented to behave. This was primarily a problem when trying to import
> packages with no description (eg. color-theme-solarized on melpa).
>
> 2. Package dependencies need to be propagated so that emacs can find
> them.
> Has anyone looked at this? This is relatively minor, but now that the
> 0.12.0 release is done I thought a ping might be appropriate.
Thanks for pinging! I've also Cc-ed Federico and Ludovic who wrote and
changed this part of code.
> From 0e561aee91b5d389d72906d059ddf486a322f20a Mon Sep 17 00:00:00 2001
> From: Carlo Zancanaro <carlo@zancanaro.id.au>
> Date: Wed, 14 Dec 2016 12:31:12 +1100
> Subject: [PATCH 1/2] import: elpa: Fix call-with-downloaded-file
>
> * guix/import/elpa.scm (call-with-downloaded-file): Make function behaviour
> match documentation string.
> ---
> guix/import/elpa.scm | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/guix/import/elpa.scm b/guix/import/elpa.scm
> index 320a09e8c..5f8b7a9e5 100644
> --- a/guix/import/elpa.scm
> +++ b/guix/import/elpa.scm
> @@ -89,7 +89,13 @@ NAMES (strings)."
> "Fetch URL, store the content in a temporary file and call PROC with that
> file. Returns the value returned by PROC. On error call ERROR-THUNK and
> return its value or leave if it's false."
> - (proc (http-fetch/cached (string->uri url))))
> + (catch #t
> + (lambda ()
> + (proc (http-fetch/cached (string->uri url))))
> + (lambda (key . args)
> + (if error-thunk
> + (error-thunk)
> + (apply throw key args)))))
So here you fixed a regression introduced by commit 218622a73¹, thanks!
I think it's better to call 'leave' on the last line as it was done
originally.
¹ http://git.savannah.gnu.org/cgit/guix.git/commit/?id=218622a73794c3b0d0d81db9176a59125c58df41
> From 88f23b9c369841837a83225b52e19f4c029906ab Mon Sep 17 00:00:00 2001
> From: Carlo Zancanaro <carlo@zancanaro.id.au>
> Date: Wed, 14 Dec 2016 12:34:15 +1100
> Subject: [PATCH 2/2] import: elpa: Import dependencies as propagated-inputs
>
> * guix/import/elpa.scm (elpa-package->sexp): Import dependencies as
> propagated-inputs.
> ---
> guix/import/elpa.scm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/guix/import/elpa.scm b/guix/import/elpa.scm
> index 5f8b7a9e5..897ce6a69 100644
> --- a/guix/import/elpa.scm
> +++ b/guix/import/elpa.scm
> @@ -228,7 +228,7 @@ type '<elpa-package>'."
> (bytevector->nix-base32-string (file-sha256 tarball))
> "failed to download package")))))
> (build-system emacs-build-system)
> - ,@(maybe-inputs 'inputs dependencies)
> + ,@(maybe-inputs 'propagated-inputs dependencies)
> (home-page ,(elpa-package-home-page pkg))
> (synopsis ,(elpa-package-synopsis pkg))
> (description ,(elpa-package-description pkg))
Using 'propagated-inputs' looks right to me.
--
Alex
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Elpa importer improvements
2016-12-27 8:54 ` Alex Kost
@ 2016-12-27 10:20 ` Carlo Zancanaro
2016-12-29 17:16 ` Ludovic Courtès
1 sibling, 0 replies; 5+ messages in thread
From: Carlo Zancanaro @ 2016-12-27 10:20 UTC (permalink / raw)
To: Alex Kost; +Cc: Guix-devel, Federico Beffa
[-- Attachment #1.1: Type: text/plain, Size: 700 bytes --]
On Tue, Dec 27 2016, Alex Kost wrote
> [...]
>
>> - (proc (http-fetch/cached (string->uri url)))) + (catch #t +
>> (lambda () + (proc (http-fetch/cached (string->uri url))))
>> + (lambda (key . args) + (if error-thunk +
>> (error-thunk) + (apply throw key args)))))
>
> So here you fixed a regression introduced by commit 218622a73¹,
> thanks! I think it's better to call 'leave' on the last line as
> it was done originally.
Ah, so that's what it meant by "leave"! I misunderstood. I had
interpreted it as "leave the error alone", so thought re-throwing
was consistent with that. I've attached an updated patch to use
the original "leave" line.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-import-elpa-Fix-call-with-downloaded-file.patch --]
[-- Type: text/x-diff, Size: 1218 bytes --]
From 35da0d4977f217be6aefb24ab062b646d17de671 Mon Sep 17 00:00:00 2001
From: Carlo Zancanaro <carlo@zancanaro.id.au>
Date: Wed, 14 Dec 2016 12:31:12 +1100
Subject: [PATCH 1/2] import: elpa: Fix call-with-downloaded-file
* guix/import/elpa.scm (call-with-downloaded-file): Make function behaviour
match documentation string.
---
guix/import/elpa.scm | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/guix/import/elpa.scm b/guix/import/elpa.scm
index 320a09e8c..ec232cd8a 100644
--- a/guix/import/elpa.scm
+++ b/guix/import/elpa.scm
@@ -89,7 +89,13 @@ NAMES (strings)."
"Fetch URL, store the content in a temporary file and call PROC with that
file. Returns the value returned by PROC. On error call ERROR-THUNK and
return its value or leave if it's false."
- (proc (http-fetch/cached (string->uri url))))
+ (catch #t
+ (lambda ()
+ (proc (http-fetch/cached (string->uri url))))
+ (lambda (key . args)
+ (if error-thunk
+ (error-thunk)
+ (leave (_ "~A: download failed~%") url)))))
(define (is-elpa-package? name elpa-pkg-spec)
"Return true if the string NAME corresponds to the name of the package
--
2.11.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Elpa importer improvements
2016-12-27 8:54 ` Alex Kost
2016-12-27 10:20 ` Carlo Zancanaro
@ 2016-12-29 17:16 ` Ludovic Courtès
1 sibling, 0 replies; 5+ messages in thread
From: Ludovic Courtès @ 2016-12-29 17:16 UTC (permalink / raw)
To: Alex Kost; +Cc: Guix-devel, Carlo Zancanaro, Federico Beffa
Hello Guix!
Alex Kost <alezost@gmail.com> skribis:
> Carlo Zancanaro (2016-12-14 13:04 +1100) wrote:
>
>> I was trying to import some elpa packages recently and found some
>> issues, so here are some patches to fix them.
>>
>> 1. call-with-downloaded-file had behaviour different to how it was
>> documented to behave. This was primarily a problem when trying to import
>> packages with no description (eg. color-theme-solarized on melpa).
>>
>> 2. Package dependencies need to be propagated so that emacs can find
>> them.
>
>> Has anyone looked at this? This is relatively minor, but now that the
>> 0.12.0 release is done I thought a ping might be appropriate.
>
> Thanks for pinging! I've also Cc-ed Federico and Ludovic who wrote and
> changed this part of code.
Sorry for the delay!
>> From 0e561aee91b5d389d72906d059ddf486a322f20a Mon Sep 17 00:00:00 2001
>> From: Carlo Zancanaro <carlo@zancanaro.id.au>
>> Date: Wed, 14 Dec 2016 12:31:12 +1100
>> Subject: [PATCH 1/2] import: elpa: Fix call-with-downloaded-file
>>
>> * guix/import/elpa.scm (call-with-downloaded-file): Make function behaviour
>> match documentation string.
>> ---
>> guix/import/elpa.scm | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/guix/import/elpa.scm b/guix/import/elpa.scm
>> index 320a09e8c..5f8b7a9e5 100644
>> --- a/guix/import/elpa.scm
>> +++ b/guix/import/elpa.scm
>> @@ -89,7 +89,13 @@ NAMES (strings)."
>> "Fetch URL, store the content in a temporary file and call PROC with that
>> file. Returns the value returned by PROC. On error call ERROR-THUNK and
>> return its value or leave if it's false."
>> - (proc (http-fetch/cached (string->uri url))))
>> + (catch #t
>> + (lambda ()
>> + (proc (http-fetch/cached (string->uri url))))
>> + (lambda (key . args)
>> + (if error-thunk
>> + (error-thunk)
>> + (apply throw key args)))))
>
> So here you fixed a regression introduced by commit 218622a73¹, thanks!
> I think it's better to call 'leave' on the last line as it was done
> originally.
>
> ¹ http://git.savannah.gnu.org/cgit/guix.git/commit/?id=218622a73794c3b0d0d81db9176a59125c58df41
‘http-fetch/cached’ raises an &http-get-error condition (or a
‘getaddrinfo-error’ Guile exception), which I thought would be handled
at the top level, but apparently they’re not.
So I’ve applied the latest version of this change.
>> From 88f23b9c369841837a83225b52e19f4c029906ab Mon Sep 17 00:00:00 2001
>> From: Carlo Zancanaro <carlo@zancanaro.id.au>
>> Date: Wed, 14 Dec 2016 12:34:15 +1100
>> Subject: [PATCH 2/2] import: elpa: Import dependencies as propagated-inputs
>>
>> * guix/import/elpa.scm (elpa-package->sexp): Import dependencies as
>> propagated-inputs.
>> ---
>> guix/import/elpa.scm | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/guix/import/elpa.scm b/guix/import/elpa.scm
>> index 5f8b7a9e5..897ce6a69 100644
>> --- a/guix/import/elpa.scm
>> +++ b/guix/import/elpa.scm
>> @@ -228,7 +228,7 @@ type '<elpa-package>'."
>> (bytevector->nix-base32-string (file-sha256 tarball))
>> "failed to download package")))))
>> (build-system emacs-build-system)
>> - ,@(maybe-inputs 'inputs dependencies)
>> + ,@(maybe-inputs 'propagated-inputs dependencies)
>> (home-page ,(elpa-package-home-page pkg))
>> (synopsis ,(elpa-package-synopsis pkg))
>> (description ,(elpa-package-description pkg))
>
> Using 'propagated-inputs' looks right to me.
Applied.
Thank you!
Ludo’.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-12-29 17:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-14 2:04 [PATCH] Elpa importer improvements Carlo Zancanaro
2016-12-23 1:57 ` Carlo Zancanaro
2016-12-27 8:54 ` Alex Kost
2016-12-27 10:20 ` Carlo Zancanaro
2016-12-29 17:16 ` Ludovic Courtès
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).