[bug#33437] [PATCH 01/10] build: Add dune-build-system.

[Ludovic Courtès]

Hello Julien!

Julien Lepiller <address@hidden> skribis:

> * guix/build/dune-build-system.scm,
>   guix/build-system/dune.scm: New files.
> * Makefile.am (MODULES): Add them.
> * doc/guix.texi (Build Systems): Document dune-build-system.
> * guix/build-system/ocaml.scm (lower, default-findlib, default-ocaml): Export
> them.
> (package-with-explicit-ocaml): Also transform packages built with
> dune-build-system.

Nice!

> address@hidden {Scheme Variable} dune-build-system
> +This variable is exported by @code{(guix build-system dune)}.  It
> +supports builds of packaes using Dune, a build tool for the
                           ^
Typo.

> address@hidden://ocaml.org, OCaml programming language}.  It is implemented

Maybe add a link for Dune rather than for OCaml?


> +as an extension of the @code{ocaml-build-system} which is describe below.
                                                                    ^
Typo.

> +It automically adds the @code{dune} package to the set of inputs.
          ^^
“automatically”

> +The @code{'configure} phase is removed as dune packages typically

Maybe: “There is no @code{configure} phase because Dune packages…”?

> +The @code{#:legacy?} parameter can be passed to use the @code{jbuild}
> +command instead of the more recent @code{dune} command while building
> +a package.  Its default value is @code{#f}.

Should it be called #:jbuild? instead?  Because eventually everything
becomes “legacy”.  :-)

> +;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
> +(define-module (guix build-system dune)

Nitpick: please insert a newline before ‘define-module’.   :-)

> +(define* (dune-build store name inputs
> +                     #:key (guile #f)
> +                     (outputs '("out"))
> +                     (search-paths '())
> +                     (build-flags ''())
> +                     (out-of-source? #t)
> +                     (legacy? #f)
> +                     (tests? #t)
> +                     (test-flags ''())
> +                     (test-target "test")
> +                     (install-target "install")
> +                     (validate-runpath? #t)
> +                     (patch-shebangs? #t)
> +                     (strip-binaries? #t)
> +                     (strip-flags ''("--strip-debug"))
> +                     (strip-directories ''("lib" "lib64" "libexec"
> +                                           "bin" "sbin"))
> +                     (phases '(@ (guix build dune-build-system)
> +                                 %standard-phases))
> +                     (system (%current-system))
> +                     (imported-modules %dune-build-system-modules)
> +                     (modules '((guix build dune-build-system)
> +                                (guix build utils))))

Would it make sense to add (guix build ocaml-build-system) as well to
the default #:modules?

> +++ b/guix/build-system/ocaml.scm
> @@ -31,6 +31,9 @@
>              package-with-ocaml4.02
>              strip-ocaml4.01-variant
>              strip-ocaml4.02-variant
> +            default-findlib
> +            default-ocaml
> +            lower
>              ocaml-build
>              ocaml-build-system))
>  
> @@ -76,6 +79,13 @@
>    (let ((module (resolve-interface '(gnu packages ocaml))))
>      (module-ref module 'ocaml-findlib)))
>  
> +(define (default-dune-build-system)
> +  "Return the dune-build-system."
> +
> +  ;; Do not use `@' to avoid introducing circular dependencies.
> +  (let ((module (resolve-interface '(guix build-system dune))))
> +    (module-ref module 'dune-build-system)))
> +
>  (define (default-ocaml4.01)
>    (let ((ocaml (resolve-interface '(gnu packages ocaml))))
>      (module-ref ocaml 'ocaml-4.01)))
> @@ -119,7 +129,8 @@ pre-defined variants."
>        => force)
>  
>       ;; Otherwise build the new package object graph.
> -     ((eq? (package-build-system p) ocaml-build-system)
> +     ((or (eq? (package-build-system p) ocaml-build-system)
> +          (eq? (package-build-system p) (default-dune-build-system)))

I don’t have a better solution to offer here, but this whole
‘package-with-explicit-XYZ’ is clearly showing its limits here.  :-/

Otherwise LGTM, thank you!

Ludo’.