[bug#70227] [PATCH 0/2] Improving composer import.

[bug#70227] [PATCH 0/2] Improving composer import.

[Nicolas Graves via Guix-patches via]

This patch series superseeds former patches 67895, 67897 and 67906.
It fixes bugs, handles parsing failures and implements recursive package refresh.

Nicolas Graves (2):
  guix: import: composer: Handle parsing failures.
  guix: import: composer: Implement recursive package refresh.

 guix/import/composer.scm | 41 +++++++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 11 deletions(-)

-- 
2.41.0

[bug#70227] [PATCH 1/2] guix: import: composer: Handle parsing failures.

[Nicolas Graves via Guix-patches via]

* guix/import/composer (latest-release): Handle parsing
failures. Rename package to composer-package for clarity.

Change-Id: I57f6fba7b05122b031177681e76cf0b5c9547736
---
 guix/import/composer.scm | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/guix/import/composer.scm b/guix/import/composer.scm
index 1ad608964b..75419ca63e 100644
--- a/guix/import/composer.scm
+++ b/guix/import/composer.scm
@@ -243,16 +243,19 @@ (define (php-package? package)
    (eq? (package-build-system package) composer-build-system)
    (string-prefix? "php-" (package-name package))))
 
-(define (latest-release package)
+(define* (latest-release package #:key (version #f))
   "Return an <upstream-source> for the latest release of PACKAGE."
   (let* ((php-name (guix-package->composer-name package))
-         (package (composer-fetch php-name))
-         (version (composer-package-version package))
-         (url (composer-source-url (composer-package-source package))))
-    (upstream-source
-     (package (package-name package))
-     (version version)
-     (urls (list url)))))
+         (composer-package (composer-fetch php-name #:version version)))
+    (if composer-package
+        (upstream-source
+         (package (composer-package-name composer-package))
+         (version (composer-package-version composer-package))
+         (urls (list (composer-source-url
+                      (composer-package-source composer-package)))))
+        (begin
+          (warning (G_ "failed to parse ~a~%") php-name)
+          #f))))
 
 (define %composer-updater
   (upstream-updater
-- 
2.41.0

[bug#70227] [PATCH 2/2] guix: import: composer: Implement recursive package refresh.

[Nicolas Graves via Guix-patches via]

* guix/import/composer.scm
 (composer-fetch): Replace reduce by fold to correct version selection
 logic.
 (latest-release): Implement recursive package refresh. Rename to
 import-release.
 (import-release): New function, formerly known as latest-release.

Change-Id: I8f629b4d1da866f5986d39b4e159f2b44af9ee49
---
 guix/import/composer.scm | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/guix/import/composer.scm b/guix/import/composer.scm
index 75419ca63e..e2a60969aa 100644
--- a/guix/import/composer.scm
+++ b/guix/import/composer.scm
@@ -113,7 +113,7 @@ (define* (composer-fetch name #:key (version #f))
          (if version
              (assoc-ref packages version)
              (cdr
-              (reduce
+              (fold
                (lambda (new cur-max)
                  (match new
                    (((? valid-version? version) . tail)
@@ -243,16 +243,32 @@ (define (php-package? package)
    (eq? (package-build-system package) composer-build-system)
    (string-prefix? "php-" (package-name package))))
 
-(define* (latest-release package #:key (version #f))
-  "Return an <upstream-source> for the latest release of PACKAGE."
+(define* (import-release package #:key (version #f))
+  "Return an <upstream-source> for VERSION or the latest release of PACKAGE."
   (let* ((php-name (guix-package->composer-name package))
          (composer-package (composer-fetch php-name #:version version)))
     (if composer-package
-        (upstream-source
-         (package (composer-package-name composer-package))
-         (version (composer-package-version composer-package))
-         (urls (list (composer-source-url
-                      (composer-package-source composer-package)))))
+        (let* ((guix-name (composer-package-name composer-package))
+               (inputs
+                (append
+                 (map (lambda (dep)
+                        (upstream-input
+                         (name php-name)
+                         (downstream-name guix-name)
+                         (type 'regular)))
+                      (composer-package-require composer-package))
+                 (map (lambda (dep)
+                        (upstream-input
+                         (name php-name)
+                         (downstream-name guix-name)
+                         (type 'native)))
+                      (composer-package-dev-require composer-package)))))
+          (upstream-source
+           (package guix-name)
+           (version (composer-package-version composer-package))
+           (urls (list (composer-source-url
+                        (composer-package-source composer-package))))
+           (inputs inputs)))
         (begin
           (warning (G_ "failed to parse ~a~%") php-name)
           #f))))
@@ -262,7 +278,7 @@ (define %composer-updater
    (name 'composer)
    (description "Updater for Composer packages")
    (pred php-package?)
-   (import latest-release)))
+   (import import-release)))
 
 (define* (composer-recursive-import package-name #:optional version)
   (recursive-import package-name
-- 
2.41.0

[bug#70227] [PATCH 2/2] guix: import: composer: Implement recursive package refresh.

[Ludovic Courtès]

Hi!

Nicolas Graves <ngraves@ngraves.fr> skribis:

> * guix/import/composer.scm
>  (composer-fetch): Replace reduce by fold to correct version selection
>  logic.
>  (latest-release): Implement recursive package refresh. Rename to
>  import-release.
>  (import-release): New function, formerly known as latest-release.

Nice, glad to see this feature put to good use.  :-)


[...]

> +(define* (import-release package #:key (version #f))
> +  "Return an <upstream-source> for VERSION or the latest release of PACKAGE."
>    (let* ((php-name (guix-package->composer-name package))
>           (composer-package (composer-fetch php-name #:version version)))
>      (if composer-package
> -        (upstream-source
> -         (package (composer-package-name composer-package))
> -         (version (composer-package-version composer-package))
> -         (urls (list (composer-source-url
> -                      (composer-package-source composer-package)))))
> +        (let* ((guix-name (composer-package-name composer-package))
> +               (inputs
> +                (append
> +                 (map (lambda (dep)
> +                        (upstream-input
> +                         (name php-name)
> +                         (downstream-name guix-name)
> +                         (type 'regular)))
> +                      (composer-package-require composer-package))
> +                 (map (lambda (dep)
> +                        (upstream-input
> +                         (name php-name)
> +                         (downstream-name guix-name)
> +                         (type 'native)))

Shouldn’t it be:

  (upstream-input
    (name dep)
    (downstream-name (php-package-name dep))
    (type …))

?

As a slight improvement, since the ‘inputs’ field of <upstream-source>
is delayed (wrapped in a promise, so that its computation only happens
when it’s needed), you could arrange to replace the ‘inputs’ variable
above like so:

  (define (dependency->input dependency type)
    (upstream-input …))

  (upstream-source
    ;; …
    (inputs (append (map (cut dependency->input <> 'regular) …)
                    (map (cut dependency->input <> 'native) …))))

I hope this makes sense.

Thanks!

Ludo’.

[bug#70227] [PATCH 0/2] Improving composer import.

[jgart via Guix-patches via]

Hi Nicolas,

Are you still working on this ticket?

Feel free to send a v2 if you find the time with ludo's suggestions.
-- 
all the best,
jgart

[bug#70227] [PATCH 0/2] Improving composer import.

[Ludovic Courtès]

Hello,

jgart <jgart@dismail.de> skribis:

> Feel free to send a v2 if you find the time with ludo's suggestions.

Oh yes, we were close to the finish line!

(Thanks jgart for following up on these forgotten issues; much
appreciated.)

Ludo’.

[bug#70227] [PATCH v2 1/2] guix: import: composer: Handle parsing failures.

[Nicolas Graves via Guix-patches via]

* guix/import/composer (latest-release): Handle parsing
failures. Rename package to composer-package for clarity.

Change-Id: I57f6fba7b05122b031177681e76cf0b5c9547736
---
 guix/import/composer.scm | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/guix/import/composer.scm b/guix/import/composer.scm
index 1ad608964b..75419ca63e 100644
--- a/guix/import/composer.scm
+++ b/guix/import/composer.scm
@@ -243,16 +243,19 @@ (define (php-package? package)
    (eq? (package-build-system package) composer-build-system)
    (string-prefix? "php-" (package-name package))))
 
-(define (latest-release package)
+(define* (latest-release package #:key (version #f))
   "Return an <upstream-source> for the latest release of PACKAGE."
   (let* ((php-name (guix-package->composer-name package))
-         (package (composer-fetch php-name))
-         (version (composer-package-version package))
-         (url (composer-source-url (composer-package-source package))))
-    (upstream-source
-     (package (package-name package))
-     (version version)
-     (urls (list url)))))
+         (composer-package (composer-fetch php-name #:version version)))
+    (if composer-package
+        (upstream-source
+         (package (composer-package-name composer-package))
+         (version (composer-package-version composer-package))
+         (urls (list (composer-source-url
+                      (composer-package-source composer-package)))))
+        (begin
+          (warning (G_ "failed to parse ~a~%") php-name)
+          #f))))
 
 (define %composer-updater
   (upstream-updater
-- 
2.46.0

[bug#70227] [PATCH v2 2/2] guix: import: composer: Implement recursive package refresh.

[Nicolas Graves via Guix-patches via]

* guix/import/composer.scm
(guix-package->composer-name): Simplify.
 (composer-fetch): Replace reduce by fold to correct version selection logic.
 (latest-release): Implement recursive package refresh. Rename to import-release.
 (import-release): New function, formerly known as latest-release.

Change-Id: I8f629b4d1da866f5986d39b4e159f2b44af9ee49
---
 guix/import/composer.scm | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/guix/import/composer.scm b/guix/import/composer.scm
index 75419ca63e..abc9023be4 100644
--- a/guix/import/composer.scm
+++ b/guix/import/composer.scm
@@ -19,12 +19,14 @@
 (define-module (guix import composer)
   #:use-module (ice-9 match)
   #:use-module (json)
-  #:use-module (guix hash)
   #:use-module (guix base32)
   #:use-module (guix build git)
   #:use-module (guix build utils)
   #:use-module (guix build-system)
   #:use-module (guix build-system composer)
+  #:use-module ((guix diagnostics) #:select (warning))
+  #:use-module (guix hash)
+  #:use-module (guix i18n)
   #:use-module (guix import json)
   #:use-module (guix import utils)
   #:use-module ((guix licenses) #:prefix license:)
@@ -113,7 +115,7 @@ (define* (composer-fetch name #:key (version #f))
          (if version
              (assoc-ref packages version)
              (cdr
-              (reduce
+              (fold
                (lambda (new cur-max)
                  (match new
                    (((? valid-version? version) . tail)
@@ -217,13 +219,8 @@ (define (guix-name->composer-name name)
 (define (guix-package->composer-name package)
   "Given a Composer PACKAGE built from Packagist, return the name of the
 package in Packagist."
-  (let ((upstream-name (assoc-ref
-                         (package-properties package)
-                         'upstream-name))
-        (name (package-name package)))
-    (if upstream-name
-      upstream-name
-      (guix-name->composer-name name))))
+  (or (assoc-ref (package-properties package) 'upstream-name)
+      (guix-name->composer-name (package-name package))))
 
 (define (string->license str)
   "Convert the string STR into a license object."
@@ -243,8 +240,14 @@ (define (php-package? package)
    (eq? (package-build-system package) composer-build-system)
    (string-prefix? "php-" (package-name package))))
 
-(define* (latest-release package #:key (version #f))
-  "Return an <upstream-source> for the latest release of PACKAGE."
+(define (dependency->input dependency type)
+  (upstream-input
+   (name dependency)
+   (downstream-name (php-package-name dependency))
+   (type type)))
+
+(define* (import-release package #:key (version #f))
+  "Return an <upstream-source> for VERSION or the latest release of PACKAGE."
   (let* ((php-name (guix-package->composer-name package))
          (composer-package (composer-fetch php-name #:version version)))
     (if composer-package
@@ -252,7 +255,12 @@ (define* (latest-release package #:key (version #f))
          (package (composer-package-name composer-package))
          (version (composer-package-version composer-package))
          (urls (list (composer-source-url
-                      (composer-package-source composer-package)))))
+                      (composer-package-source composer-package))))
+         (inputs (append
+                  (map (cut dependency->input <> 'regular)
+                       (composer-package-require composer-package))
+                  (map (cut dependency->input <> 'native)
+                       (composer-package-dev-require composer-package)))))
         (begin
           (warning (G_ "failed to parse ~a~%") php-name)
           #f))))
@@ -262,7 +270,7 @@ (define %composer-updater
    (name 'composer)
    (description "Updater for Composer packages")
    (pred php-package?)
-   (import latest-release)))
+   (import import-release)))
 
 (define* (composer-recursive-import package-name #:optional version)
   (recursive-import package-name
-- 
2.46.0

bug#70227: [PATCH v2 1/2] guix: import: composer: Handle parsing failures.

[Ludovic Courtès]

Hi,

Nicolas Graves <ngraves@ngraves.fr> skribis:

> * guix/import/composer (latest-release): Handle parsing
> failures. Rename package to composer-package for clarity.
>
> Change-Id: I57f6fba7b05122b031177681e76cf0b5c9547736

[...]

> * guix/import/composer.scm
> (guix-package->composer-name): Simplify.
>  (composer-fetch): Replace reduce by fold to correct version selection logic.
>  (latest-release): Implement recursive package refresh. Rename to import-release.
>  (import-release): New function, formerly known as latest-release.
>
> Change-Id: I8f629b4d1da866f5986d39b4e159f2b44af9ee49

Applied, thanks!

There are no new tests (we could improve on that…) but at least those
already in ‘tests/composer.scm’ still pass.

Ludo’.