[PATCH 0/2] Improving composer import.

  • Done
  • quality assurance status badge
Details
3 participants
  • jgart
  • Ludovic Courtès
  • Nicolas Graves
Owner
unassigned
Submitted by
Nicolas Graves
Severity
normal
N
N
Nicolas Graves wrote on 6 Apr 04:04 +0200
(address . guix-patches@gnu.org)(address . ngraves@ngraves.fr)
20240406020415.9360-1-ngraves@ngraves.fr
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
N
N
Nicolas Graves wrote on 6 Apr 04:08 +0200
[PATCH 1/2] guix: import: composer: Handle parsing failures.
(address . 70227@debbugs.gnu.org)(address . ngraves@ngraves.fr)
20240406020852.15996-1-ngraves@ngraves.fr
* 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(-)

Toggle diff (34 lines)
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
N
N
Nicolas Graves wrote on 6 Apr 04:08 +0200
[PATCH 2/2] guix: import: composer: Implement recursive package refresh.
(address . 70227@debbugs.gnu.org)(address . ngraves@ngraves.fr)
20240406020852.15996-2-ngraves@ngraves.fr
* 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(-)

Toggle diff (64 lines)
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
L
L
Ludovic Courtès wrote on 29 Apr 23:29 +0200
(name . Nicolas Graves)(address . ngraves@ngraves.fr)(address . 70227@debbugs.gnu.org)
87wmofzwl3.fsf@gnu.org
Hi!

Nicolas Graves <ngraves@ngraves.fr> skribis:

Toggle quote (7 lines)
> * 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. :-)


[...]

Toggle quote (25 lines)
> +(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’.
J
J
jgart wrote on 13 Jul 15:41 +0200
Re: [PATCH 0/2] Improving composer import.
(address . 70227@debbugs.gnu.org)
87le25e7f5.fsf@dismail.de
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
L
L
Ludovic Courtès wrote on 18 Jul 11:21 +0200
(name . jgart)(address . jgart@dismail.de)
87v813vywl.fsf@gnu.org
Hello,

jgart <jgart@dismail.de> skribis:

Toggle quote (2 lines)
> 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’.
N
N
Nicolas Graves wrote on 7 Oct 01:55 +0200
[PATCH v2 1/2] guix: import: composer: Handle parsing failures.
(address . 70227@debbugs.gnu.org)(name . Nicolas Graves)(address . ngraves@ngraves.fr)
20241006235605.24248-1-ngraves@ngraves.fr
* 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(-)

Toggle diff (34 lines)
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
N
N
Nicolas Graves wrote on 7 Oct 01:56 +0200
[PATCH v2 2/2] guix: import: composer: Implement recursive package refresh.
(address . 70227@debbugs.gnu.org)(name . Nicolas Graves)(address . ngraves@ngraves.fr)
20241006235605.24248-2-ngraves@ngraves.fr
* 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(-)

Toggle diff (87 lines)
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
L
L
Ludovic Courtès wrote on 12 Oct 19:46 +0200
Re: [bug#70227] [PATCH v2 1/2] guix: import: composer: Handle parsing failures.
(name . Nicolas Graves)(address . ngraves@ngraves.fr)(address . 70227-done@debbugs.gnu.org)
87cyk5rz7a.fsf@gnu.org
Hi,

Nicolas Graves <ngraves@ngraves.fr> skribis:

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

[...]

Toggle quote (8 lines)
> * 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’.
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 70227@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 70227
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch