diff mbox series

[bug#50286,RFC] Let 'package-location' returns location of surrounding 'let'.

Message ID 0b61652d751633f78e876a27be88ed14e47527b6.camel@telenet.be
State Accepted
Headers show
Series [bug#50286,RFC] Let 'package-location' returns location of surrounding 'let'. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

M Aug. 30, 2021, 9:26 p.m. UTC 
X-Debbugs-CC: ludo@gnu.org
X-Debbugs-CC: iskarian@mgsn.dev

Hi guix,

These three patches allows (guix upstream) to replace the values in the surrounding 'let'
form, if any.  It's important for constructs like:

(define-public gnash
   (let ((version "0.8.11")
         (commit "583ccbc1275c7701dc4843ec12142ff86bb305b4")
         (revision "0"))
     (package
       (name "gnash")
       (version (git-version version revision commit))
       (source (git-reference
                 (url "https://example.org")
                 (commit commit)))
       [...])))

such that it can update the version, commit, revision. (Currently only the
version will be updatable, but see <https://issues.guix.gnu.org/50072#0>
and <https://issues.guix.gnu.org/50072#9> for work on making 'commit' updatable).

More details in the patches themselves.

Greetings,
Maxime

Comments

Ludovic Courtès Sept. 6, 2021, 10:07 a.m. UTC | #1 
Hello,

Maxime Devos <maximedevos@telenet.be> skribis:

> These three patches allows (guix upstream) to replace the values in the surrounding 'let'
> form, if any.  It's important for constructs like:
>
> (define-public gnash
>    (let ((version "0.8.11")
>          (commit "583ccbc1275c7701dc4843ec12142ff86bb305b4")
>          (revision "0"))
>      (package
>        (name "gnash")
>        (version (git-version version revision commit))
>        (source (git-reference
>                  (url "https://example.org")
>                  (commit commit)))
>        [...])))
>
> such that it can update the version, commit, revision. (Currently only the
> version will be updatable, but see <https://issues.guix.gnu.org/50072#0>
> and <https://issues.guix.gnu.org/50072#9> for work on making 'commit' updatable).

This is smart!

I wonder if we’re going overboard, though.  Intuitively, I would rather
leave ‘location’ fields dumb, and instead add editing features to do
things like getting the location of the parent sexp.  It does add some
overhead, but it also makes things more explicit and preserves
separation of concern.  (Also, in ‘core-updates-frozen’,
‘go-to-location’ uses a location cache that makes it less expensive than
on ‘master’.)  But yeah, it’s trickier…

Hmm, thinking out loud, what about this: use the same trick as you did,
but replace ‘define-public’ instead of ‘let’ & co., so as to be less
intrusive.

  (define-syntax-parameter current-definition-location
    (identifier-syntax #f))

  (define-syntax define-public*
    (syntax-rules ()
      ((_ prototype body)
       (define-public prototype
         (syntax-parameterize ((current-definition-location
                                (identifier-syntax (current-source-location))))
           body)))))

Since there’s code that assumes ‘package-location’ returns the location
of the (package …) sexp, we could add a ‘definition-location’ field in
<package>, defaulting to ‘current-definition-location’, or tweak
‘location’ to include both.

WDYT?

Thanks,
Ludo’.
diff mbox series

Patch

From fd716c2924c96a0bf908f615adaa404a3e382e7c Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Mon, 30 Aug 2021 20:31:00 +0200
Subject: [PATCH 3/3] guix: Find 'let' binding when using guile@3.0.0.

Without this patch, errors like this result:

[ 90%] LOAD     gnu/services/nfs.scm
WARNING: (gnu services nfs): imported module (guix) overrides core binding `let'
WARNING: (gnu services nfs): `let' imported from both (guile) and (guix)
WARNING: (gnu services nfs): imported module (guix) overrides core binding `let'
WARNING: (gnu services nfs): `let' imported from both (guile) and (guix)
ice-9/eval.scm:293:34: error: let: unbound variable
hint: Did you forget `(use-modules (srfi srfi-71))'?

I don't know why this happens, but this patch stops this error.

* guix.scm: Hide 'let' and 'let*' when importing (guix packages).
---
 guix.scm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/guix.scm b/guix.scm
index 42bc8c8818..7e1e5fb109 100644
--- a/guix.scm
+++ b/guix.scm
@@ -36,5 +36,10 @@ 
 
     (for-each (let ((i (module-public-interface (current-module))))
                 (lambda (m)
-                  (module-use! i (resolve-interface `(guix ,m)))))
+                  (module-use! i (resolve-interface `(guix ,m)
+                                  ;; XXX: why is this required with Guile 3.0.2
+                                  ;; to allow (gnu services nfs) to compile?
+                                  #:hide (if (eq? m 'packages)
+                                             '(let let*)
+                                             '())))))
               %public-modules)))
-- 
2.33.0