GNU bug report logs - #42338
[PATCH] Add composer build system (PHP)

Previous Next

Full log

View this message in rfc822 format

From: Ludovic Courtès <ludo <at> gnu.org>
To: Nicolas Graves <ngraves <at> ngraves.fr>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, Tobias Geerinckx-Rice <me <at> tobias.gr>, Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>, 42338 <at> debbugs.gnu.org, Ricardo Wurmus <rekado <at> elephly.net>, Christopher Baines <guix <at> cbaines.net>
Subject: [bug#42338] [PATCH v3 1/7] guix: import: Add composer importer.
Date: Sat, 14 Oct 2023 17:48:18 +0200

Hi Nicolas,

Nicolas Graves <ngraves <at> ngraves.fr> skribis:

> * guix/import/composer.scm: New file.
> * guix/scripts/import/composer.scm: New file.
> * guix/tests/composer.scm: New file.
> * Makefile.am: Add them.
> * guix/scripts/import.scm: Add composer importer.
> * doc/guix.texi (Invoking guix import): Mention it.

I’m a bit at loss with this patch series because there are two v3
threads, one v4 thread that contains a single patch, and the original
versions contained many more patches.

I think it’s OK to separate out the “gnu: Add php-*” patches, it’s
probably clearer.  However, could you come up with a v5 that includes
all the ‘guix import composer’ changes that we would need to apply?
How does that sound?

I have a couple of comments:

> +@item composer
> +@cindex Composer
> +@cindex PHP
> +Import metadat from the @uref{https://getcomposer.org/, Composer} package
                ^
Typo.

> +++ b/guix/import/composer.scm
> @@ -0,0 +1,270 @@
> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright © 2019 Julien Lepiller <julien <at> lepiller.eu>

Maybe add yourself too?

> +;; XXX adapted from (guix scripts hash)
> +(define (file-hash file select? recursive?)
> +  ;; Compute the hash of FILE.
> +  (if recursive?
> +      (let-values (((port get-hash) (open-sha256-port)))
> +        (write-file file port #:select? select?)
> +        (force-output port)
> +        (get-hash))
> +      (call-with-input-file file port-sha256)))
> +
> +;; XXX taken from (guix scripts hash)
> +(define (vcs-file? file stat)

These two procedures are now exported from (guix hash), so you can
remove them and #:use-module (guix hash) instead.

> +(define* (composer-fetch name #:optional version)
> +  "Return an alist representation of the Composer metadata for the package NAME,
> +or #f on failure."
> +  (let ((package (json-fetch
> +                   (string-append (%composer-base-url) "/p/" name ".json"))))
> +    (if package
> +        (let* ((packages (assoc-ref package "packages"))
> +               (package (or (assoc-ref packages name) package))
> +               (versions (filter
> +                           (lambda (version)
> +                             (and (not (string-contains version "dev"))
> +                                  (not (string-contains version "beta"))))
> +                           (map car package)))
> +               (version (or (if (null? version) #f version)
> +                            (latest-version versions))))
> +          (assoc-ref package version))

Instead of returning an alist, directly return a <composer-package>
record.

> +(define (php-package? package)
> +  "Return true if PACKAGE is a PHP package from Packagist."
> +  (and
> +    (eq? (build-system-name (package-build-system package)) 'composer)

Rather: (eq? (package-build-system package) composer-build-system).

(The ‘name’ field is for debugging purposes only.)

> +(define (latest-release package)
> +  "Return an <upstream-source> for the latest release of PACKAGE."
> +  (let* ((php-name (guix-package->composer-name package))
> +         (metadata (composer-fetch php-name))
> +         (package (json->composer-package metadata))
> +         (version (composer-package-version package))
> +         (url (composer-source-url (composer-package-source package))))
> +    (upstream-source
> +     (package (package-name package))
> +     (version version)
> +     (urls (list url)))))

Maybe we can do that later, but note that <upstream-source> has an
‘inputs’ field nowadays; if you feel it in, ‘guix refresh -u’ is able to
update dependencies in addition to version/hash.

(If you leave it for later, please add a TODO.)

> +;; Avoid collisions with other tests.
> +(%http-server-port 10450)

This is now unnecessary: by default a random unused port is chosen and
everything’s fine.

> +(test-begin "composer")
> +
> +(test-assert "composer->guix-package"
> +  ;; Replace network resources with sample data.
> +  (with-http-server `((200 ,test-json)
> +                      (200 ,test-source))
> +    (parameterize ((%composer-base-url (%local-url))
> +                   (current-http-proxy (%local-url)))
> +      (match (composer->guix-package "foo/bar")
> +        (('package
> +           ('name "php-foo-bar")
> +           ('version "0.1")

For clarity, you can write:

  (match …
    (`(package
        (name "php-foo-bar")
        (version "0.1")
        …) …))

See commit 654fcf9971bb01389d577be07c6ec0f68940c743.

> +           ('native-inputs
> +            ('quasiquote
> +             (("php-phpunit-phpunit" ('unquote 'php-phpunit-phpunit)))))

Please change the importer so that it emits inputs without labels:

  (native-inputs (list php-phpunit-phpunit))

One last thing: consider adding an ‘etc/news.scm’ entry so people can
learn about the new importer.

Thanks in advance!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.