Subject: Re: [PATCH v3 01/14] build-system: Add mix-build-system.

From: Pierre-Henry Fröhring <[email-masked]>
Date: Thu, 16 Nov 2023 14:01:36 +0100

Do you mind if I paste the conversation using the following org format?

* Comment
** lilyp
> +(define (elixir-version elixir)
> +  "Return an X.Y string where X and Y are respectively the major and
> minor version number of ELIXIR.
> +Example: /gnu/store/…-elixir-1.14.0 → 1.14"
> +  ((compose
> +    (cut string-join <> ".")
> +    (cut take <> 2)
> +    (cut string-split <> #\.)
> +    last)
> +   (string-split elixir #\-)))

I don't think we need to be overly cute here.  The let-binding version
from python-build-system is less surprising to the uninitiated reader.

See also strip-store-file-name and package-name->name+version.

** phf
Maybe:
#+begin_src scheme
(define (elixir-version elixir)
  "Return an X.Y string where X and Y are respectively the major and minor
version number of ELIXIR.
Example: /gnu/store/…-elixir-1.14.0 → 1.14"
  (receive (_ version) (package-name->name+version (strip-store-file-name
elixir))
    (let* ((components  (string-split version #\.))
           (major+minor (take components 2)))
      (string-join major+minor "."))))
#+end_src

or:
#+begin_src scheme
(define (elixir-version elixir)
  "Return an X.Y string where X and Y are respectively the major and minor
version number of ELIXIR.
Example: /gnu/store/…-elixir-1.14.0 → 1.14"
  (let* ((version     (last (string-split elixir #\-)))
         (components  (string-split version #\.))
         (major+minor (take components 2)))
    (string-join major+minor ".")))
#+end_src

or: just inline the code as it is used just once. See
[[id:76abe0e4-a0e2-4176-bdc0-9ff241e8ba42][next comment]].

* Comment
:PROPERTIES:
:ID:       76abe0e4-a0e2-4176-bdc0-9ff241e8ba42
:END:

** lilyp
> +(define (elixir-libdir elixir path)
> +  "Return the path where all libraries for a specified ELIXIR
> version are installed."
> +  (string-append path "/lib/elixir/" (elixir-version elixir)))

You probably want to swap path and elixir and perhaps also find a way
to implicitly parameterize the latter so as to make it optional.

** phf
Is this what you mean?
#+begin_src scheme
;; The Elixir version is constant as soon as it is computable from the
current
;; execution. It is a X.Y string where X and Y are respectively the major
and
;; minor version number of the Elixir used in the build.
(define elixir-version (make-parameter "X.Y"))

(define* (elixir-libdir path #:optional (version (elixir-version)))
  "Return the path where all libraries for a specified ELIXIR version are
installed."
  (string-append path "/lib/elixir/" version))

(define* (configure #:key inputs mix-path mix-exs #:allow-other-keys)
  …
  (elixir-version
   (receive (_ version) (package-name->name+version (strip-store-file-name
(assoc-ref inputs "elixir")))
     (let* ((components  (string-split version #\.))
            (major+minor (take components 2)))
       (string-join major+minor ".")))))
#+end_src

* Comment
** lilyp
> +(define* (configure #:key inputs mix-path mix-exs #:allow-other-
> keys)
> +  "Set environment variables.
> +See:
> https://hexdocs.pm/mix/1.15.7/Mix.html#module-environment-variables"
> +  (setenv "LC_ALL" "en_US.UTF-8")
> +  (setenv "MIX_HOME" (getcwd))
> +  (setenv "MIX_ARCHIVES" "archives")
> +  (setenv "MIX_BUILD_ROOT" "_build")
> +  (setenv "MIX_DEPS_PATH" "deps")
> +  (setenv "MIX_PATH" (or mix-path ""))
> +  (setenv "MIX_REBAR3" (string-append (assoc-ref inputs "rebar3")
> "/bin/rebar3"))
> +  (setenv "MIX_EXS" mix-exs))

This does not appear to be a configure phase in the traditional sense
of the wording.  Instead, it should be a post 'set-paths' 'set-mix-env'
imho.

** phf
After ~install-locale~ since ~(setenv "LC_ALL" "en_US.UTF-8")~ is called in
this
phase.
#+begin_src scheme
(define %standard-phases
  (modify-phases gnu:%standard-phases
    …
    (delete 'configure)
    (add-after 'install-locale 'set-mix-env set-mix-env)
    (replace 'unpack unpack)
    …))
#+end_src

* Comment
** lilyp
> +(define* (install-hex #:key name inputs outputs #:allow-other-keys)
> +  "Install Hex."
> +  (let ((hex-archive-path (string-append (getenv "MIX_ARCHIVES")
> "/hex")))
> +    (mkdir-p hex-archive-path)
> +    (symlink (car (list-directories (elixir-libdir (assoc-ref inputs
> "elixir")
> +                                                   (assoc-ref inputs
> "elixir-hex"))))
> +             (string-append hex-archive-path "/hex"))))

Why do we need this?  It looks like we'll be pasting the same (native?)
input all over the store, which imho would be bad.

** phf
Without ~Hex~, you get this error:
#+begin_example
starting phase `build'
Could not find Hex, which is needed to build dependency :ex_doc
Shall I install Hex? (if running non-interactively, use "mix local.hex
--force") [Yn]
#+end_example
This message is called from ~handle_rebar_not_found~ in
~lib/mix/lib/mix/tasks/deps.compile.ex~ ; which is called from ~rebar_cmd~
because
a ~manager~ (~Hex~) could not be found. Hex must be present => install-hex
must be
executed.

I thought that this should not be a problem since Hex is needed at build
time,
and just symlinked. Maybe should it be copied instead?

* Comment
** lilyp
> +  (define (install-input mix-env input)
> +    (let ((dir (mix-build-dir mix-env)))
> +      (mkdir-p dir)
> +      (match input
> +        ((_ . path)
> +         ((compose
> +           (cut for-each (cut install-lib <> dir) <>)
> +           (cut append-map list-directories <>)
> +           (cut filter directory-exists? <>))
> +          (list (elixir-libdir (assoc-ref inputs "elixir") path)
> +                (erlang-libdir path)))))))

I think you're at the wrong layer of abstraction here.

  (match input
    ((_ . prefix)
     (begin
      (install-subdirectories (elixir-libdir path))
      (install-subdirectories (erlang-libdir path)))))

where (install-subdirectories PATH) is basically
  (when (directory-exists? PATH)
    (for-each (cute install-lib <> (mix-build-dir mix-env))
              (list-directories PATH)))

** phf
Does this work?
#+begin_src scheme
(define (install-lib lib dir)
    (let ((lib-name (last (string-split lib #\/))))
      (symlink lib (string-append dir "/" lib-name))))

  (define (install-subdirectories mix-env path)
    (let ((build-dir (mix-build-dir mix-env)))
      (mkdir-p build-dir)
      (when (directory-exists? path)
        (for-each (cute install-lib <> build-dir)
                  (list-directories path)))))

  (define (install-input mix-env input)
    (match input
      ((_ . path)
       (begin
         (install-subdirectories mix-env (elixir-libdir path))
         (install-subdirectories mix-env (erlang-libdir path))))))
#+end_src

* Comment
** lilyp
> +  (define (install-inputs mix-env)
> +    (for-each (cut install-input mix-env <>)
> +              (append inputs native-inputs)))

Installing native inputs: probably a bad idea (inhibits cross-
compilation).

** phf
A slight variant that does not link things when it should not:
#+begin_src scheme
(define (install-inputs mix-env)
    (for-each (cut install-input mix-env <>)
              (cond
                ((string=? mix-env "prod") inputs)
                ((member mix-env '("shared" "test")) (append inputs
native-inputs))
                (else (error (format #f "Unexpected Mix env: ~a~%"
mix-env))))))
#+end_src

I did not consider cross-compilation yet. The following might be wrong be
here
we go. I far as I understand, Erlang applications are largely platform
independent. Once the code is compiled to BEAM bytecode, it can run on any
platform that has the Erlang VM installed. If an Erlang library uses native
extensions, then cross-compilation might be required. For a build to succeed
in a given environment (one of "prod", "test", "shared"), the BEAM files of
all dependencies should be present on the build machine. So, all
dependencies
must be installed

* Comment
** lilyp
> +(define (library-name pkg-name)
> +  "Return the library name deduced from PKG-NAME.
> +A package should be named: elixir-lib-name-X.Y.Z from which the
> library name
> +lib_name is deduced."
> +  ((compose
> +    (cut string-join <> "_")
> +    (cut drop-right <> 1)
> +    (cut string-split <> #\-))
> +   (strip-elixir-prefix pkg-name)))

Consider defining (package-name-version->elixir-name) analogous to
(package-name-version->erlang-name) in rebar-build-system.  Also allow
overriding it through a build system argument.  The thing you currently
have will blow up with git-version.

** phf
Faily close to the ~rebar-build-system~ version.
#+begin_src scheme
(define (package-name-version->elixir-name name+ver)
  "Convert the Guix package NAME-VER to the corresponding Elixir
name-version
format.  Essentially drop the prefix used in Guix and replace dashes by
underscores."
  (let* ((name- (package-name->name+version name+ver)))
    (string-join
     (string-split
      (if (string-prefix? "elixir-" name-)
          (string-drop name- (string-length "elixir-"))
          name-)
      #\-)
     "_")))
#+end_src