[bug#66801,'PATCH,v2',01/14] build-system: Add mix-build-system.

* guix/build-system/mix.scm,
* guix/build/mix-build-system.scm: New modules.

Change-Id: I2cbf6c963a530e73420da0eb17ffaf92827451bf
---
 guix/build-system/mix.scm       | 237 +++++++++++++++++++++++++
 guix/build/mix-build-system.scm | 303 ++++++++++++++++++++++++++++++++
 2 files changed, 540 insertions(+)
 create mode 100644 guix/build-system/mix.scm
 create mode 100644 guix/build/mix-build-system.scm


base-commit: a0d337e79c87d7c38c79d0291974f490cb137a52

Am Montag, dem 13.11.2023 um 21:26 +0100 schrieb Pierre-Henry Fröhring:
> * guix/build-system/mix.scm,
> * guix/build/mix-build-system.scm: New modules.
> 
> Change-Id: I2cbf6c963a530e73420da0eb17ffaf92827451bf
> ---
>  guix/build-system/mix.scm       | 237 +++++++++++++++++++++++++
>  guix/build/mix-build-system.scm | 303
> ++++++++++++++++++++++++++++++++
>  2 files changed, 540 insertions(+)
>  create mode 100644 guix/build-system/mix.scm
>  create mode 100644 guix/build/mix-build-system.scm
> 
> diff --git a/guix/build-system/mix.scm b/guix/build-system/mix.scm
> new file mode 100644
> index 00000000..ae4407c9
> --- /dev/null
> +++ b/guix/build-system/mix.scm
> @@ -0,0 +1,237 @@
> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright © 2023 Pierre-Henry Fröhring
> <phfrohring@deeplinks.com>
> +;;;
> +;;; This file is part of GNU Guix.
> +;;;
> +;;; GNU Guix is free software; you can redistribute it and/or modify
> it
> +;;; under the terms of the GNU General Public License as published
> by
> +;;; the Free Software Foundation; either version 3 of the License,
> or (at
> +;;; your option) any later version.
> +;;;
> +;;; GNU Guix is distributed in the hope that it will be useful, but
> +;;; WITHOUT ANY WARRANTY; without even the implied warranty of
> +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +;;; GNU General Public License for more details.
> +;;;
> +;;; You should have received a copy of the GNU General Public
> License
> +;;; along with GNU Guix.  If not, see
> <http://www.gnu.org/licenses/>.
> +
> +;; Commentary:
> +;;
> +;; Standard build procedure for Elixir packages using 'mix'.  This
> is
> +;; implemented as an extension of 'gnu-build-system'.
> +;;
> +;; Code:
> +
> +(define-module (guix build-system mix)
> +  #:use-module (gnu packages base)
> +  #:use-module (gnu packages elixir)
> +  #:use-module (gnu packages elixir-xyz)
> +  #:use-module (gnu packages erlang)
You probably want to resolve those imports rather than use-modules
them.
> +  #:use-module (guix build mix-build-system)
> +  #:use-module (guix build-system gnu)
> +  #:use-module (guix build-system)
> +  #:use-module (guix gexp)
> +  #:use-module (guix monads)
> +  #:use-module (guix packages)
> +  #:use-module (guix search-paths)
> +  #:use-module (guix store)
> +  #:use-module (guix utils)
> +  #:use-module (ice-9 match)
> +  #:use-module (srfi srfi-1)
> +  #:use-module (srfi srfi-26)
> +  #:export (mix-build-system hexpm-uri))
> +
> +(define (hexpm-uri name version)
> +  "Return the URI where to fetch the sources of a Hex package NAME
> at VERSION.
> +See: https://github.com/hexpm/specifications/blob/main/endpoints.md"
> +  (string-append "https://repo.hex.pm/tarballs/"
> +                 (string-replace-substring (strip-elixir-prefix
> name) "-" "_")
> +                 "-" version ".tar"))
> +
> +(define glibc-utf8-locales
> +  (make-glibc-utf8-locales glibc
> +                           #:locales (list "en_US")
> +                           #:name "glibc-utf8-locales"))
> +
> +(define (default-elixir)
> +  "Return the default Elixir package."
> +  ;; Lazily resolve the binding to avoid a circular dependency.
> +  (let ((elixir (resolve-interface '(gnu packages elixir))))
> +    (module-ref elixir 'elixir)))
> +
> +(define imported-modules
> +  `((guix build mix-build-system)
> +    ,@%gnu-build-system-modules))
> +
> +(define modules
> +  '((guix build mix-build-system)
> +    (guix build utils)))
> +
> +;; The prefix of Elixir packages.
> +(define %elixir-prefix "elixir-")
> +
> +;; The prefix of Erlang packages.
> +(define %erlang-prefix "erlang-")
> +
> +(define (erlang-package? package)
> +  "Tell whether PACKAGE is an Erlang package."
> +  (string-prefix? %erlang-prefix (package-name package)))
> +
> +(define (elixir-package? package)
> +  "Tell whether PACKAGE is an Elixir package."
> +  (string-prefix? %elixir-prefix (package-name package)))
> +
> +(define (erlang-or-elixir-pkg? package)
> +  "Tell whether PACKAGE is an Elixir or an Erlang package."
> +  (or (erlang-package? package)
> +      (elixir-package? package)))
You can simplify these five variables into one to three (three being
quite many already depending on whether you'll actually use the smaller
ones).

> +(define (erlang-or-elixir-input? input)
> +  "Tell whether INPUT is an Elixir or an Erlang input."
> +  (match input
> +    ((_ package)
> +     (erlang-or-elixir-pkg? package))))
Skip.

> +(define (input=? input1 input2)
> +  "Tell whether inputs INPUT1 and INPUT2 are equal."
> +  (define pkg1 (match input1 ((_ pkg) pkg)))
> +  (define pkg2 (match input2 ((_ pkg) pkg)))
> +  (string=? (package-name pkg1) (package-name pkg2)))
Again, checking for label equivalence is a bad idea.

> +;; A number of environment variables specific to the Mix build
> system are reflected here.
> +;; They are documented here:
> https://hexdocs.pm/mix/1.15.7/Mix.html#module-environment-variables.
> +;; Other parameters located in mix.exs are defined here:
> +;;
> https://hexdocs.pm/mix/1.15.7/Mix.Project.html#module-configuration
> +(define* (mix-build name
> +                    inputs
> +                    #:key
> +                    source
> +                    elixir-X.Y ;The major and minor of Elixir.
> +                    (tests? #t)
> +                    (mix-path #f) ;See MIX_PATH.
> +                    (mix-exs "mix.exs") ;See MIX_EXS.
> +                    (build-per-environment #t) ;See
> :build_per_environment.
> +                    (phases '%standard-phases)
> +                    (outputs '("out"))
> +                    (search-paths '())
> +                    (system (%current-system))
> +                    (guile #f)
> +                    (imported-modules imported-modules)
> +                    (modules modules))
> +  "Build SOURCE using Elixir, and with INPUTS."
> +
> +  ;; Check the documentation of :build_per_environment here:
> +  ;;
> https://hexdocs.pm/mix/1.15.7/Mix.Project.html#module-configuration A
> nd
> +  ;; "Environments" here:
> +  ;; https://hexdocs.pm/mix/1.15.7/Mix.html#module-environments
> +  (define mix-environments
> +    (if build-per-environment
> +        `("prod" ,@(if tests? '("test") '()))
> +        '("shared")))
> +
> +  (define builder
> +    (with-imported-modules imported-modules
> +      #~(begin
> +
> +          (use-modules #$@(sexp->gexp modules))
> +
> +          #$(with-build-variables inputs outputs
> +              #~(mix-build #:name #$name
> +                           #:source #+source
> +                           #:system #$system
> +                           #:tests? #$tests?
> +                           #:mix-path #$mix-path
> +                           #:mix-exs #$mix-exs
> +                           #:elixir-X.Y #$elixir-X.Y
> +                           #:mix-environments '#$mix-environments
> +                           #:build-per-environment #$build-per-
> environment
> +                           #:phases #$(if (pair? phases)
> +                                          (sexp->gexp phases)
> +                                          phases)
> +                           #:outputs %outputs
> +                           #:search-paths '#$(sexp->gexp
> +                                              (map
> +                                               search-path-
> specification->sexp
> +                                               search-paths))
> +                           #:inputs
> +                           %build-inputs)))))
> +
> +  (mlet %store-monad ((guile (package->derivation (or guile
> (default-guile))
> +                                                  system
> +                                                  #:graft? #f)))
> +    (gexp->derivation name
> +                      builder
> +                      #:system system
> +                      #:graft? #f       ;consistent with 'gnu-build'
> +                      #:target #f
> +                      #:guile-for-build guile)))
> +
> +(define* (lower name
> +                #:key
> +                source
> +                (inputs '())
> +                (tests? #t)
> +                (native-inputs '())
> +                (propagated-inputs '())
> +                outputs
> +                system
> +                target
> +                (elixir (default-elixir))
> +                #:allow-other-keys #:rest arguments)
> +  "Return a bag for NAME."
> +  (define private-keywords
> +    '(#:inputs #:native-inputs #:outputs #:system #:target
> #:elixir))
> +
> +  ;; Libraries are compiled using a given version of Elixir. This
> fact is
> +  ;; encoded by the name of a sub-directory like lib/elixir/X.Y. We
> compute
> +  ;; the value of X.Y here which is valid for the whole build.
> +  (define elixir-X.Y (version-major+minor (package-version elixir)))
> +
> +  ;; Elixir depends on a specific version of Erlang, this one.
> +  (define erlang (lookup-package-input elixir "erlang"))
> +
> +  ;; For mix to compile and test a package, it needs to find all
> inputs,
> +  ;; native-inputs and propagated-inputs (including the transitive
> ones) under
> +  ;; _build directories like _build/prod/lib.
> +  ;;
> +  ;; Given inputs and native-inputs of the package, we need to
> compute the
> +  ;; transitive closure of all Erlang and Elixir propagated inputs
> and add
> +  ;; them to the build inputs.
> +  (define all-propagated-inputs
> +    ((compose
> +      (cut delete-duplicates <> input=?)
> +      (cut filter erlang-or-elixir-input? <>)
> +      (cut append-map package-transitive-propagated-inputs <>)
> +      (cut map cadr <>))
> +     (append inputs native-inputs)))
> +
> +  (define build-inputs
> +    `(,@(standard-packages)
> +      ("glibc-utf8-locales" ,glibc-utf8-locales)
> +      ("erlang" ,erlang)
> +      ("rebar3" ,rebar3)
> +      ("elixir" ,elixir)
> +      ("elixir-hex" ,elixir-hex)
> +      ,@all-propagated-inputs
> +      ,@inputs
> +      ,@native-inputs))
> +
> +  ;; Some inputs, such as C programs, may be architecture dependent.
> +  (define host-inputs (if target inputs '()))
> +
> +  (bag (name name)
> +       (system system)
> +       (build-inputs build-inputs)
> +       (host-inputs host-inputs)
> +       (outputs outputs)
> +       (build mix-build)
> +       (arguments (append `(#:elixir-X.Y ,elixir-X.Y)
> +                          (strip-keyword-arguments private-keywords
> arguments)))))
> +
> +(define mix-build-system
> +  (build-system (name 'mix)
> +                (description "The standard Mix build system")
> +                (lower lower)))
> +
> +;;; mix.scm ends here
> diff --git a/guix/build/mix-build-system.scm b/guix/build/mix-build-
> system.scm
> new file mode 100644
> index 00000000..86a5db51
> --- /dev/null
> +++ b/guix/build/mix-build-system.scm
> @@ -0,0 +1,303 @@
> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright © 2023 Pierre-Henry Fröhring
> <phfrohring@deeplinks.com>
> +;;;
> +;;; This file is part of GNU Guix.
> +;;;
> +;;; GNU Guix is free software; you can redistribute it and/or modify
> it
> +;;; under the terms of the GNU General Public License as published
> by
> +;;; the Free Software Foundation; either version 3 of the License,
> or (at
> +;;; your option) any later version.
> +;;;
> +;;; GNU Guix is distributed in the hope that it will be useful, but
> +;;; WITHOUT ANY WARRANTY; without even the implied warranty of
> +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +;;; GNU General Public License for more details.
> +;;;
> +;;; You should have received a copy of the GNU General Public
> License
> +;;; along with GNU Guix.  If not, see
> <http://www.gnu.org/licenses/>.
> +
> +;; Commentary:
> +;;
> +;; Code:
> +
> +(define-module (guix build mix-build-system)
> +  #:use-module ((guix build gnu-build-system) #:prefix gnu:)
> +  #:use-module (guix build utils)
> +  #:use-module (ice-9 ftw)
> +  #:use-module (ice-9 match)
> +  #:use-module (ice-9 regex)
> +  #:use-module (ice-9 string-fun)
> +  #:use-module (srfi srfi-1)
> +  #:use-module (srfi srfi-26)
> +  #:export (mix-build
> +            strip-elixir-prefix
> +            %standard-phases))
> +
> +(define (f-join . paths)
> +  "Return a path (a string) formed from joining each component in
> PATHS (list of strings).
> +Example: \"a\" \"b\" \"c\" → \"a/b/c\""
> +  (string-join paths file-name-separator-string))
It is wiser to write these out as regular string-appends.

> +(define (strip-prefix prefix name)
> +  "Return NAME without the prefix PREFIX."
> +  (if (string-prefix? prefix name)
> +      (string-drop name (string-length prefix))
> +      name))
I'm pretty sure we have a function looking like this somewhere, but I
can't recall its name atm.  

> +;; All Elixir package names start with this prefix. If a package
> name starts
> +;; with this prefix, then it is an Elixir package.
> +(define %elixir-prefix "elixir-")
Useless magic constant.
> +
> +(define (path->elixir-lib path X.Y)
> +  "Return the path to the directory within PATH where libraries of
> an Elixir
> +package are installed. Here, X.Y represents the major and minor
> version
> +numbers of Elixir used for compilation."
> +  (f-join path "lib" "elixir" X.Y))
Is path itself ever useful here or does it simply point to the store?
This can likely be simplified to 
(define (elixir-lib version)
  (string-append "/lib/elixir/" version))

> +(define (elixir-name? name)
> +  "Determines if NAME is the name or the label associated to an
> Elixir
> +package."
> +  (string-prefix? %elixir-prefix name))
Useless use of magic constant.
> +
> +(define (elixir-input? X.Y input)
> +  "Determines if the given INPUT is an Elixir input."
> +  (match input
> +    ((label . path)
> +     ;; XXX: The second condition may be enough.
> +     (and (elixir-name? label)
> +          (directory-exists? (path->elixir-lib path X.Y))))))
Ahem, search-path-as-list.
Also, leaking the version is kinda bad, API-wise.

> +(define (strip-elixir-prefix name)
> +  "Strip %elixir-prefix from NAME."
> +  (strip-prefix %elixir-prefix name))
> 
> +;; All Erlang package names start with this prefix. If a package
> name starts
> +;; with this prefix, then it is an Erlang package.
> +(define %erlang-prefix "erlang-")
> +
> +(define (path->erlang-lib path)
> +  "Return the path of the directory where libraries of an Erlang
> package are
> +installed in the store."
> +  (f-join path "lib" "erlang" "lib"))
> +
> +(define (erlang-name? name)
> +  "Determines if NAME is the name or the label associated to an
> Erlang
> +package."
> +  (string-prefix? %erlang-prefix name))
> +
> +(define (erlang-input? input)
> +  "Determines if the given INPUT is an Erlang input."
> +  (match input
> +    ((label . path)
> +     ;; XXX: one condition may be enough. Without the first one, the
> erlang
> +     ;; input is considered an input when we just want Erlang
> packages.
> +     (and (erlang-name? label)
> +          (directory-exists? (path->erlang-lib path))))))
> +
> +(define (strip-erlang-prefix name)
> +  "Strip %erlang-prefix from NAME."
> +  (strip-prefix %erlang-prefix name))
Duplicated boilerplate for neither fun nor profit.

> +(define (erlang-or-elixir-input? X.Y input)
> +  "Determines if the given INPUT is an Erlang or Elixir input."
> +  (or (erlang-input? input)
> +      (elixir-input? X.Y input)))
Leak, leak, leak.
> +
> +(define (snakecase-name name)
> +  "Return a snakecase version of NAME."
> +  (string-replace-substring (string-downcase name) "-" "_"))
> +
> +(define (label->library-name label)
> +  "Return the library name associated to an input label LABEL."
> +  (define stripped-label
> +    (cond
> +     ((erlang-name? label)
> +      (strip-erlang-prefix label))
> +     ((elixir-name? label)
> +      (strip-elixir-prefix label))
> +     (#t (error "Invalid label: expected an Erlang or Elixir label."
> 'label label))))
> +  (snakecase-name stripped-label))
Don't rely on labels.

> +(define (pkg-name->library-name name)
> +  "Return the name of the library deduced from the name of the Guix
> package.
> +Example: elixir-a-pkg-1.0.2 → a_pkg
> +See: https://www.erlang.org/doc/man/code#code-path"
> +  ((compose
> +    label->library-name
> +    (cut string-join <> "-")
> +    (cut drop-right <> 1)
> +    (cut string-split <> #\-))
> +   name))
> +
> +;; We fix as many variables as possible.
> +;; See:
> https://hexdocs.pm/mix/1.15.7/Mix.html#module-environment-variables
> +(define MIX_HOME "MIX_HOME")
> +
> +(define MIX_ARCHIVES "MIX_ARCHIVES")
> +(define (%mix-archives mix-home) (f-join mix-home "archives"))
> +(define MIX_BUILD_ROOT "MIX_BUILD_ROOT")
> +(define %mix-build-root "_build")
> +(define MIX_DEPS_PATH "MIX_DEPS_PATH")
> +(define %mix-deps-path "deps")
> +(define MIX_PATH "MIX_PATH")
> +(define MIX_REBAR3 "MIX_REBAR3")
> +(define MIX_EXS "MIX_EXS")
> +(define %mix-exs "mix.exs")
> +;; XXX: if different architecture are needed, then use this
> variable.
> +(define MIX_TARGET "MIX_TARGET")
> +(define MIX_ENV "MIX_ENV")
> +(define %mix-env-prod "prod")
> +(define %mix-env-test "test")
> +(define %mix-env-shared "shared")
Gratuitous definitions are gratuitous.

> +;; The name of the directory where compiled libraries by mix are
> stored.
> +(define %mix-lib "lib")
> +
> +;; Useful because Elixir expects a UTF-8 locale.
> +(define LC_ALL "LC_ALL")
> +
> +(define (mix-build-dir mix-env)
> +  "Return the directory where build artifacts are to be installed
> according to
> +en environment MIX-ENV in the current directory."
> +  (f-join %mix-build-root mix-env %mix-lib))
> +
> +(define* (unpack #:key source mix-path #:allow-other-keys)
> +  "Unpack SOURCE in the working directory, and change directory
> within the
> +source.  When SOURCE is a directory, copy it in a sub-directory of
> the current
> +working directory."
> +  (let ((gnu-unpack (assoc-ref gnu:%standard-phases 'unpack)))
> +    (gnu-unpack #:source source)
> +    (let ((contents "contents.tar.gz"))
> +      (when (file-exists? contents)
> +        (invoke "tar" "xvf" contents)))))
> +
> +(define* (configure #:key inputs mix-path mix-exs #:allow-other-
> keys)
> +  "Set environment variables."
> +  (setenv LC_ALL "en_US.UTF-8")
> +  (setenv MIX_HOME (getcwd))
> +  (setenv MIX_ARCHIVES (%mix-archives (getenv MIX_HOME)))
> +  (setenv MIX_BUILD_ROOT %mix-build-root)
> +  (setenv MIX_DEPS_PATH %mix-deps-path)
> +  (setenv MIX_PATH (or mix-path ""))
> +  (setenv MIX_REBAR3 (f-join (assoc-ref inputs "rebar3") "bin"
> "rebar3"))
> +  (setenv MIX_EXS mix-exs))
> +
> +(define* (install-hex #:key inputs name elixir-X.Y #:allow-other-
> keys)
> +  "Install Hex."
> +  (define hex-name "hex")
> +  (define hex-path (assoc-ref inputs "elixir-hex"))
> +  (define hex-lib (f-join (path->elixir-lib hex-path elixir-X.Y)
> hex-name))
> +  (define hex-archive-path (f-join (getenv MIX_ARCHIVES) hex-name))
> +  (mkdir-p hex-archive-path)
> +  (symlink hex-lib (f-join hex-archive-path hex-name)))
> +
> +(define* (install-dependencies #:key
> +                               name
> +                               tests?
> +                               build-per-environment
> +                               (inputs '())
> +                               (native-inputs '())
> +                               elixir-X.Y
> +                               mix-environments
> +                               #:allow-other-keys
> +                               #:rest rest)
> +  "Install dependencies.
> +Given an environment mix-env, we define all-inputs(mix-env) as the
> set of all
> +necessary Erlang and Elixir inputs and associated propagated inputs
> (and
> +transitive propagated inputs).
> +
> +For example, all-inputs(prod) represents all the Erlang and Elixir
> inputs and
> +propagated inputs necessary to compile the Mix project for the prod
> +environment.
> +
> +If dep belongs to all-inputs(mix-env) and its library name is dep-
> name, then
> +it is installed under (f-join (mix-build-dir mix-env) dep-name) as a
> symbolic
> +link."
The docstring divulges implementation details without really explaining
what the function itself does.  
> +  (define (all-inputs mix-env)
> +    (define env-inputs
> +      (cond
> +       ((string=? mix-env %mix-env-prod)
> +        inputs)
> +       ((member mix-env (list %mix-env-test %mix-env-shared))
> +        (append inputs native-inputs))
> +       (#t (error "Unexpected Mix environment." 'mix-env mix-env))))
You probably want and ice-9 match here, which has an implicit error on
failed matches.
> +    (filter (cut erlang-or-elixir-input? elixir-X.Y <>) env-inputs))
> +
> +  (define (install-input mix-env input)
> +    (let ((dir (mix-build-dir mix-env)))
> +      (mkdir-p dir)
> +      (match input
> +        ((label . path)
> +         (let ((lib-name (label->library-name label)))
> +           (symlink
> +            (f-join (path->elixir-lib path elixir-X.Y) lib-name)
> +            (f-join dir lib-name)))))))
> +
> +  (define (install-inputs mix-env)
> +    (for-each (cut install-input mix-env <>)
> +              (all-inputs mix-env)))
> +
> +  (for-each install-inputs mix-environments))
> +
> +(define* (build #:key mix-environments #:allow-other-keys)
> +  "Builds the Mix project."
> +  (define (compile mix-env)
> +    (setenv MIX_ENV mix-env)
> +    (invoke "mix" "compile" "--no-deps-check"))
> +  (for-each compile mix-environments))
> +
> +(define* (check #:key (tests? #t) #:allow-other-keys)
> +  "Test the Mix project."
> +  (if tests?
> +      (invoke "mix" "test" "--no-deps-check")
> +      (format #t "tests? = ~a~%" tests?)))
> +
> +(define* (remove-mix-dirs . _)
> +  "Remove all .mix/ directories.
> +We do not want to copy them to the installation directory."
> +  (define mix-dirs
> +    (find-files "."
> +                (file-name-predicate "\\.mix$")
> +                #:directories? #t))
> +  (for-each delete-file-recursively mix-dirs))
> +
> +(define* (install #:key
> +                  inputs
> +                  outputs
> +                  name
> +                  build-per-environment
> +                  elixir-X.Y
> +                  #:allow-other-keys)
> +  "Install build artifacts in the store."
> +  (define lib-name (pkg-name->library-name name))
> +
> +  (define dir-build
> +    (f-join (mix-build-dir (if build-per-environment %mix-env-prod
> %mix-env-shared))
> +            lib-name))
> +
> +  (define dir-install
> +    (f-join (path->elixir-lib (assoc-ref outputs "out") elixir-X.Y)
> +            lib-name))
> +  (mkdir-p dir-install)
> +
> +  (copy-recursively dir-build dir-install
> +                    #:follow-symlinks? #t))
> +
> +(define %standard-phases
> +  (modify-phases gnu:%standard-phases
> +    (delete 'bootstrap)
> +    (replace 'configure configure)
> +    (replace 'unpack unpack)
> +    (add-after 'patch-generated-file-shebangs 'install-hex install-
> hex)
> +    (add-after 'install-hex 'install-dependencies install-
> dependencies)
> +    (replace 'build build)
> +    (replace 'check check)
> +    (add-before 'install 'remove-mix-dirs remove-mix-dirs)
> +    (replace 'install install)))
> +
> +(define* (mix-build #:key inputs (phases %standard-phases)
> +                    #:allow-other-keys #:rest args)
> +  "Build the given Mix package, applying all of PHASES in order."
> +  (apply gnu:gnu-build #:inputs inputs #:phases phases args))
> +
> +;;; mix-build-system.scm ends here
> 
> base-commit: a0d337e79c87d7c38c79d0291974f490cb137a52
I probably missed some, but I think you get the gist.  While this isn't
exactly code golf, try to express yourself in terms of the least
verbose primitives and don't add meaningless layers of indirection – we
already have enough actual meaningful ones.

Cheers

> > +(define-module (guix build-system mix)
> > +  #:use-module (gnu packages base)
> > +  #:use-module (gnu packages elixir)
> > +  #:use-module (gnu packages elixir-xyz)
> > +  #:use-module (gnu packages erlang)
> You probably want to resolve those imports rather than use-modules
> them.

You mean something like this?
#+begin_src scheme
(define-module (guix build-system mix)
  #:use-module ((gnu packages base) #:select (glibc
make-glibc-utf8-locales))
  #:use-module ((gnu packages elixir) #:select (elixir elixir-hex))
  #:use-module ((gnu packages erlang) #:select (rebar3))
#+end_src

I have moved `elixir-hex' to elixir.scm to avoid a circular dependency.


> > +(define (input=? input1 input2)
> > +  "Tell whether inputs INPUT1 and INPUT2 are equal."
> > +  (define pkg1 (match input1 ((_ pkg) pkg)))
> > +  (define pkg2 (match input2 ((_ pkg) pkg)))
> > +  (string=? (package-name pkg1) (package-name pkg2)))
> Again, checking for label equivalence is a bad idea.

OK, after reading a bit more about Guile, I understand that `equal?'
is what should be used here, right? The intent is to remove duplicated
inputs in the code below:

#+begin_src scheme
(define all-propagated-inputs
    ((compose
      (cut delete-duplicates <> equal?) ;<-- Here
      (cut filter erlang-or-elixir-input? <>)
      (cut append-map package-transitive-propagated-inputs <>)
      (cut map cadr <>))
     (append inputs native-inputs)))
#+end_src


> > +(define (elixir-input? X.Y input)
> > +  "Determines if the given INPUT is an Elixir input."
> > +  (match input
> > +    ((label . path)
> > +     ;; XXX: The second condition may be enough.
> > +     (and (elixir-name? label)
> > +          (directory-exists? (path->elixir-lib path X.Y))))))
> Ahem, search-path-as-list.
> Also, leaking the version is kinda bad, API-wise.

Does this mean that build artifacts should be installed under
`$out/lib/elixir/$libname` instead of `$out/lib/elixir/X.Y/$libname`?


Cheers

Am Dienstag, dem 14.11.2023 um 11:37 +0100 schrieb Pierre-Henry
Fröhring:
> > > +(define-module (guix build-system mix)
> > > +  #:use-module (gnu packages base)
> > > +  #:use-module (gnu packages elixir)
> > > +  #:use-module (gnu packages elixir-xyz)
> > > +  #:use-module (gnu packages erlang)
> > You probably want to resolve those imports rather than use-modules
> > them.
> 
> You mean something like this?
> #+begin_src scheme
> (define-module (guix build-system mix)
>   #:use-module ((gnu packages base) #:select (glibc make-glibc-utf8-
> locales))
>   #:use-module ((gnu packages elixir) #:select (elixir elixir-hex))
>   #:use-module ((gnu packages erlang) #:select (rebar3))
> #+end_src
> 
> I have moved `elixir-hex' to elixir.scm to avoid a circular
> dependency.
No.  Look at all the other build systems.  None of them use submodules
of (gnu packages), for a good reason.

> > > +(define (input=? input1 input2)
> > > +  "Tell whether inputs INPUT1 and INPUT2 are equal."
> > > +  (define pkg1 (match input1 ((_ pkg) pkg)))
> > > +  (define pkg2 (match input2 ((_ pkg) pkg)))
> > > +  (string=? (package-name pkg1) (package-name pkg2)))
> > Again, checking for label equivalence is a bad idea.
> 
> OK, after reading a bit more about Guile, I understand that `equal?'
> is what should be used here, right?  The intent is to remove
> duplicated inputs in the code below:
> 
> #+begin_src scheme
> (define all-propagated-inputs
>     ((compose
>       (cut delete-duplicates <> equal?) ;<-- Here
>       (cut filter erlang-or-elixir-input? <>)
>       (cut append-map package-transitive-propagated-inputs <>)
>       (cut map cadr <>))
>      (append inputs native-inputs)))
> #+end_src
Yep, that would work.  Note that delete-duplicates is O(n^2), though. 
We have a little bit of code where it's done in (I assume) O(n*log(n))
with vhashes.

> 
> > > +(define (elixir-input? X.Y input)
> > > +  "Determines if the given INPUT is an Elixir input."
> > > +  (match input
> > > +    ((label . path)
> > > +     ;; XXX: The second condition may be enough.
> > > +     (and (elixir-name? label)
> > > +          (directory-exists? (path->elixir-lib path X.Y))))))
> > Ahem, search-path-as-list.
> > Also, leaking the version is kinda bad, API-wise.
> 
> Does this mean that build artifacts should be installed under
> `$out/lib/elixir/$libname` instead of `$out/lib/elixir/X.Y/$libname`?
Not necessarily, but you want a different way of building
$out/lib/elixir/X.Y/ that doesn't leak through the function signature.

Btw. I think that you're resolving transitive inputs twice; once on the
build system code and once by fattening the outputs.  You probably only
need either of those, not both.

Cheers

> No.  Look at all the other build systems.  None of them use submodules of
(gnu
> packages), for a good reason.

Ok, I see. It would introduce circular dependencies. By "resolve," you
mean something like: ~(resolve-interface '(gnu packages yyy))~. So, in
our case it means:
#+begin_src scheme
(define (default-glibc-utf8-locales)
  (let* ((base (resolve-interface '(gnu packages base)))
         (glibc (module-ref base 'glibc))
         (make-glibc-utf8-locales (module-ref base
'make-glibc-utf8-locales)))
    (make-glibc-utf8-locales glibc #:locales (list "en_US"))))

(define (default-elixir-hex)
  (let ((elixir (resolve-interface '(gnu packages elixir))))
    (module-ref elixir 'elixir-hex)))

(define (default-rebar3)
  (let ((erlang (resolve-interface '(gnu packages erlang))))
    (module-ref erlang 'rebar3)))
#+end_src

Then:
#+begin_src scheme
(define* (lower name
                #:key
                (elixir (default-elixir))
                (elixir-hex (default-elixir-hex))
                (glibc-utf8-locales (default-glibc-utf8-locales))
                (rebar3 (default-rebar3))
                …
                #:allow-other-keys #:rest arguments)
  …)
#+end_src

Is this correct?


> Not necessarily, but you want a different way of building
$out/lib/elixir/X.Y/
> that doesn't leak through the function signature.

Following ~python-build-system.scm~, it means something like:
#+begin_src scheme
(define (elixir-version elixir)
  (let* ((version     (last (string-split elixir #\-)))
         (components  (string-split version #\.))
         (major+minor (take components 2)))
    (string-join major+minor ".")))

(define (install-dir inputs outputs)
  "Return the path of the current output's Elixir library."
  (let ((out (assoc-ref outputs "out"))
        (elixir (assoc-ref inputs "elixir")))
    (string-append out "/lib/elixir/" (elixir-version elixir)
"/site-packages")))
#+end_src

Is this correct?


> Btw. I think that you're resolving transitive inputs twice; once on the
build
> system code and once by fattening the outputs.  You probably only need
either
> of those, not both.

Ah. Propagated inputs are propagated. Who would have thought? So, this is
not necessary:
#+begin_src scheme
(define* (lower …)
  …
  (define all-propagated-inputs
    ((compose
      (cut delete-duplicates <> equal?)
      (cut filter erlang-or-elixir-input? <>)
      (cut append-map package-transitive-propagated-inputs <>)
      (cut map cadr <>))
     (append inputs native-inputs)))

  (define build-inputs
    `(…
      ,@all-propagated-inputs
      ,@inputs
      ,@native-inputs))

  (bag …
       (build-inputs build-inputs)
       …))
#+end_src
I've just removed ~all-propagated-inputs~ and all packages build just fine.

Is this what you meant?


> Yep, that would work.  Note that delete-duplicates is O(n^2), though.  We
have
> a little bit of code where it's done in (I assume) O(n*log(n)) with
vhashes.

If ~all-propagated-inputs~ is removed, then the discussion of this comment
is
closed.

Correction: (string-append out "/lib/elixir/" (elixir-version elixir)
"/site-packages") -> (string-append out "/lib/elixir/" (elixir-version
elixir))

> No.  Look at all the other build systems.  None of them use submodules of
(gnu
> packages), for a good reason.

Ok, I see. It would introduce circular dependencies. By "resolve," you
mean something like: ~(resolve-interface '(gnu packages yyy))~. So, in
our case it means:
#+begin_src scheme
(define (default-glibc-utf8-locales)
  (let* ((base (resolve-interface '(gnu packages base)))
         (glibc (module-ref base 'glibc))
         (make-glibc-utf8-locales (module-ref base
'make-glibc-utf8-locales)))
    (make-glibc-utf8-locales glibc #:locales (list "en_US"))))

(define (default-elixir-hex)
  (let ((elixir (resolve-interface '(gnu packages elixir))))
    (module-ref elixir 'elixir-hex)))

(define (default-rebar3)
  (let ((erlang (resolve-interface '(gnu packages erlang))))
    (module-ref erlang 'rebar3)))
#+end_src

Then:
#+begin_src scheme
(define* (lower name
                #:key
                (elixir (default-elixir))
                (elixir-hex (default-elixir-hex))
                (glibc-utf8-locales (default-glibc-utf8-locales))
                (rebar3 (default-rebar3))
                …
                #:allow-other-keys #:rest arguments)
  …)
#+end_src

Is this correct?


> Not necessarily, but you want a different way of building
$out/lib/elixir/X.Y/
> that doesn't leak through the function signature.

Following ~python-build-system.scm~, it means something like:
#+begin_src scheme
(define (elixir-version elixir)
  (let* ((version     (last (string-split elixir #\-)))
         (components  (string-split version #\.))
         (major+minor (take components 2)))
    (string-join major+minor ".")))

(define (install-dir inputs outputs)
  "Return the path of the current output's Elixir library."
  (let ((out (assoc-ref outputs "out"))
        (elixir (assoc-ref inputs "elixir")))
    (string-append out "/lib/elixir/" (elixir-version elixir))))
#+end_src

Is this correct?


> Btw. I think that you're resolving transitive inputs twice; once on the
build
> system code and once by fattening the outputs.  You probably only need
either
> of those, not both.

Ah. Propagated inputs are propagated. Who would have thought? So, this is
not necessary:
#+begin_src scheme
(define* (lower …)
  …
  (define all-propagated-inputs
    ((compose
      (cut delete-duplicates <> equal?)
      (cut filter erlang-or-elixir-input? <>)
      (cut append-map package-transitive-propagated-inputs <>)
      (cut map cadr <>))
     (append inputs native-inputs)))

  (define build-inputs
    `(…
      ,@all-propagated-inputs
      ,@inputs
      ,@native-inputs))

  (bag …
       (build-inputs build-inputs)
       …))
#+end_src
I've just removed ~all-propagated-inputs~ and all packages build just fine.

Is this what you meant?


> Yep, that would work.  Note that delete-duplicates is O(n^2), though.  We
have
> a little bit of code where it's done in (I assume) O(n*log(n)) with
vhashes.

If ~all-propagated-inputs~ is removed, then the discussion of this comment
is
closed.

Am Mittwoch, dem 15.11.2023 um 10:57 +0100 schrieb Pierre-Henry
Fröhring:
> > No.  Look at all the other build systems.  None of them use
> > submodules of (gnu
> > packages), for a good reason.
> 
> Ok, I see. It would introduce circular dependencies. By "resolve,"
> you
> mean something like: ~(resolve-interface '(gnu packages yyy))~. So,
> in
> our case it means:
> #+begin_src scheme
> (define (default-glibc-utf8-locales)
>   (let* ((base (resolve-interface '(gnu packages base)))
>          (glibc (module-ref base 'glibc))
>          (make-glibc-utf8-locales (module-ref base 'make-glibc-utf8-
> locales)))
>     (make-glibc-utf8-locales glibc #:locales (list "en_US"))))
> 
> (define (default-elixir-hex)
>   (let ((elixir (resolve-interface '(gnu packages elixir))))
>     (module-ref elixir 'elixir-hex)))
> 
> (define (default-rebar3)
>   (let ((erlang (resolve-interface '(gnu packages erlang))))
>     (module-ref erlang 'rebar3)))
> #+end_src
> 
> Then:
> #+begin_src scheme
> (define* (lower name
>                 #:key
>                 (elixir (default-elixir))
>                 (elixir-hex (default-elixir-hex))
>                 (glibc-utf8-locales (default-glibc-utf8-locales))
>                 (rebar3 (default-rebar3))
>                 …
>                 #:allow-other-keys #:rest arguments)
>   …)
> #+end_src
> 
> Is this correct?
You shouldn't need to provide glibc-utf8-locales this way (there's
already a variable for it IIRC), but yeah.

> > Not necessarily, but you want a different way of building
> > $out/lib/elixir/X.Y/
> > that doesn't leak through the function signature.
> 
> Following ~python-build-system.scm~, it means something like:
> #+begin_src scheme
> (define (elixir-version elixir)
>   (let* ((version     (last (string-split elixir #\-)))
>          (components  (string-split version #\.))
>          (major+minor (take components 2)))
>     (string-join major+minor ".")))
> 
> (define (install-dir inputs outputs)
>   "Return the path of the current output's Elixir library."
>   (let ((out (assoc-ref outputs "out"))
>         (elixir (assoc-ref inputs "elixir")))
>     (string-append out "/lib/elixir/" (elixir-version elixir) "/site-
> packages")))
> #+end_src
> 
> Is this correct?
LGTM.

> > Btw. I think that you're resolving transitive inputs twice; once on
> > the build system code and once by fattening the outputs.  You
> > probably only need either of those, not both.
> 
> Ah. Propagated inputs are propagated. Who would have thought? So,
> this is not necessary:
> #+begin_src scheme
> (define* (lower …)
>   …
>   (define all-propagated-inputs
>     ((compose
>       (cut delete-duplicates <> equal?)
>       (cut filter erlang-or-elixir-input? <>)
>       (cut append-map package-transitive-propagated-inputs <>)
>       (cut map cadr <>))
>      (append inputs native-inputs)))
> 
>   (define build-inputs
>     `(…
>       ,@all-propagated-inputs
>       ,@inputs
>       ,@native-inputs))
> 
>   (bag …
>        (build-inputs build-inputs)
>        …))
> #+end_src
> I've just removed ~all-propagated-inputs~ and all packages build just
> fine.
> 
> Is this what you meant?
Yep.  I assume this makes your packages way lighter.

Note that propagated inputs have become a (not great) default for
interpreted languages such as Python or Emacs Lisp.

> > Yep, that would work.  Note that delete-duplicates is O(n^2),
> > though.  We have a little bit of code where it's done in (I assume)
> > O(n*log(n)) with vhashes.
> 
> If ~all-propagated-inputs~ is removed, then the discussion of this
> comment is closed.
Sure.

So, the attached file `review.org' contains all the discussions so far on
the first patch 01/14.
The following email is the patch itself.
Thanks.
Cheers.