[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#38408] [PATCH v9 3/8] Added Guile-Semver as a dependency to guix

From: Martin Becze
Subject: [bug#38408] [PATCH v9 3/8] Added Guile-Semver as a dependency to guix
Date: Tue, 24 Mar 2020 15:00:01 -0400

Ok Ludo, so I think I have worked through everything that you mentioned and attached is a new patch set.

On 3/24/20 6:18 AM, Ludovic Courtès wrote:
+(define mem-lookup-crate (memoize lookup-crate))
  (define (crate-version-dependencies version)
    "Return the list of <crate-dependency> records of VERSION, a
@@ -216,7 +219,7 @@ latest version of CRATE-NAME."
          (eq? (crate-dependency-kind dependency) 'normal)))
(define crate
-    (lookup-crate crate-name))
+    (mem-lookup-crate crate-name))

I’d suggest calling ‘mem-lookup-crate’ ‘lookup-crate*’ for instance.
Can we also make its definition local to ‘crate-version-dependencies’ or
would that defeat your caching goals?

I think it would, 'crate-version-dependencies' only deal with looking up the dependencies. If I made it local 'crate->guix-package' it aslo wouldn't work because we to cache across multiple calls to 'crate->guix-package'.

    (define version-number
      (or version
@@ -238,7 +241,7 @@ latest version of CRATE-NAME."
       containing pairs of (name version)"
      (sort (map (lambda (dep)
                   (let* ((name (crate-dependency-id dep))
-                        (crate (lookup-crate name))
+                        (crate (mem-lookup-crate name))
                          (req (crate-dependency-requirement dep))
                          (ver (find-version crate req)))
                     (list name
@@ -265,9 +268,11 @@ latest version of CRATE-NAME."
+(define mem-crate->guix-package (memoize crate->guix-package))
  (define* (crate-recursive-import crate-name #:key version)
    (recursive-import crate-name
-                    #:repo->guix-package crate->guix-package
+                    #:repo->guix-package mem-crate->guix-package

Please make ‘mem-crate->guix-package’ local to ‘crate-recursive-import’
and call it ‘crate->guix-package*’ for instance.

(Memoization should always be used as a last resort: it’s a neat hack,
but it’s a hack.  :-)  In particular, it has the problem that its cache
cannot be easily invalidated.  That said, I realize that other importers
do this already, so that’s OK.)

Understood. If its any trouble it is easy to drop this commit. Though on slow networks it can help quite a bit.

Let me know if anything else needs changed!


Attachment: v13-0006-guix-self-Adds-guile-semver-as-a-depenedency.patch
Description: Text Data

Attachment: v13-0005-import-crate-Parametrized-importing-of-dev-depen.patch
Description: Text Data

Attachment: v13-0004-import-utils-Trim-patch-version-from-names.patch
Description: Text Data

Attachment: v13-0003-import-crate-Memorize-crate-guix-package.patch
Description: Text Data

Attachment: v13-0002-import-crate-Use-guile-semver-to-resolve-module-.patch
Description: Text Data

Attachment: v13-0001-import-utils-recursive-import-accepts-an-optiona.patch
Description: Text Data

reply via email to

[Prev in Thread] Current Thread [Next in Thread]