Package: guix-patches;
Reported by: Pierre-Henry Fröhring <phfrohring <at> deeplinks.com>
Date: Sat, 28 Oct 2023 20:21:02 UTC
Severity: normal
Tags: patch
Done: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Liliana Marie Prikler <liliana.prikler <at> gmail.com> To: Pierre-Henry Fröhring <phfrohring <at> deeplinks.com>, 66801 <at> debbugs.gnu.org Subject: [bug#66801] [PATCH v3 01/14] build-system: Add mix-build-system. Date: Thu, 16 Nov 2023 16:11:50 +0100
Am Donnerstag, dem 16.11.2023 um 14:01 +0100 schrieb Pierre-Henry
Fröhring:
> Do you mind if I paste the conversation using the following org
> format?
I do mind you pasting it in HTML format :P
> * 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]].
Note that you can use SRFI-71 let and let* to bind multiple values at
once. So you can write the first one without receive.
>
> * 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
I'd use %elixir-version and perhaps make it a fluent rather than a
parameter (not quite sure whether parameters get reset when a function
goes out of scope).
> * 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
Good point: you shouldn't be setting LC_ALL anyway, that's already done
by install-locale.
> * 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?
Is hex not an (implicit) native-input in your build system? Anything
that keeps it from functioning is a packaging bug imho.
> * 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)))))
Maybe move the mkdir-p into the when or at the start of install-lib.
> (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
Not an expert on elixir, but that sounds borked. You might get around
this with propagated-inputs maybe? That being said, native-inputs
shouldn't blow up a build if missing.
> * 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
Looks okay.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.