Message ID | 1aa101af088dceee34ad9373f153b5f7dd7603ba.1633533541.git.h.goebel@crazy-compilers.com |
---|---|
State | Accepted |
Headers | show |
Series | [bug#42180,v2,01/23] guix: Add extracting-download. | expand |
Context | Check | Description |
---|---|---|
cbaines/comparison | success | View comparision |
cbaines/git branch | success | View Git branch |
cbaines/comparison | success | View comparision |
cbaines/git branch | success | View Git branch |
cbaines/applying patch | success | View Laminar job |
cbaines/issue | success | View issue |
cbaines/applying patch | success | View Laminar job |
cbaines/issue | success | View issue |
cbaines/comparison | success | View comparision |
cbaines/git branch | success | View Git branch |
cbaines/comparison | success | View comparision |
cbaines/git branch | success | View Git branch |
cbaines/applying patch | success | View Laminar job |
cbaines/issue | success | View issue |
cbaines/applying patch | success | View Laminar job |
cbaines/issue | success | View issue |
cbaines/comparison | success | View comparision |
cbaines/git branch | success | View Git branch |
cbaines/comparison | success | View comparision |
cbaines/git branch | success | View Git branch |
cbaines/applying patch | success | View Laminar job |
cbaines/issue | success | View issue |
cbaines/comparison | success | View comparision |
cbaines/git branch | success | View Git branch |
cbaines/applying patch | success | View Laminar job |
cbaines/issue | success | View issue |
cbaines/comparison | success | View comparision |
cbaines/git branch | success | View Git branch |
cbaines/applying patch | success | View Laminar job |
cbaines/issue | success | View issue |
cbaines/applying patch | success | View Laminar job |
cbaines/issue | success | View issue |
cbaines/comparison | success | View comparision |
cbaines/git branch | success | View Git branch |
cbaines/comparison | success | View comparision |
cbaines/git branch | success | View Git branch |
cbaines/comparison | success | View comparision |
cbaines/git branch | success | View Git branch |
cbaines/applying patch | success | View Laminar job |
cbaines/issue | success | View issue |
cbaines/applying patch | success | View Laminar job |
cbaines/issue | success | View issue |
cbaines/applying patch | success | View Laminar job |
cbaines/issue | success | View issue |
cbaines/applying patch | success | View Laminar job |
cbaines/issue | success | View issue |
Hartmut Goebel schreef op wo 06-10-2021 om 17:20 [+0200]: > +(define* (lower name > + #:key source inputs native-inputs outputs system target > + (rebar (default-rebar3)) > + (erlang (default-erlang)) > + #:allow-other-keys > + #:rest arguments) > + "Return a bag for NAME." > + (define private-keywords > + '(#:source #:target #:rebar #:inputs #:native-inputs)) > + > + (and (not target) ;XXX: no cross-compilation > + (bag > + (name name) > + (system system) > + (host-inputs `(,@(if source > + `(("source" ,source)) > + '()) > + ,@inputs > + ;; Keep the standard inputs of 'gnu-build-system'. > + ,@(standard-packages))) (standard-packages) contains packages like gcc, coreutils, bash, ..., which should be in build-inputs. Not that it matters much here, as this procedure doesn't support cross-compilation. > + (build-inputs `(("rebar" ,rebar) > + ("erlang" ,erlang) ;; for escriptize > + ,@native-inputs)) > + (outputs outputs) > + (build rebar3-build) > + (arguments (strip-keyword-arguments private-keywords arguments))))) Greetings, Maxime.
Am 06.10.21 um 20:56 schrieb Maxime Devos: >> + (host-inputs `(,@(if source >> + `(("source" ,source)) >> + '()) >> + ,@inputs >> + ;; Keep the standard inputs of 'gnu-build-system'. >> + ,@(standard-packages))) > (standard-packages) contains packages like gcc, coreutils, bash, ..., > which should be in build-inputs. Not that it matters much here, > as this procedure doesn't support cross-compilation. Not sure what you want to tell me. Anything you recommend me to change?
Hartmut Goebel schreef op wo 06-10-2021 om 22:27 [+0200]: > Am 06.10.21 um 20:56 schrieb Maxime Devos: > > > + (host-inputs `(,@(if source > > > + `(("source" ,source)) > > > + '()) > > > + ,@inputs > > > + ;; Keep the standard inputs of 'gnu-build-system'. > > > + ,@(standard-packages))) > > > > (standard-packages) contains packages like gcc, coreutils, bash, ..., > > which should be in build-inputs. Not that it matters much here, > > as this procedure doesn't support cross-compilation. ‘this procedure’ was a bit ambigious here, I meant the procedure 'lower' that is being defined. > > Not sure what you want to tell me. Anything you recommend me to change? I recommend moving ,@(standard-packages) from host-inputs to build-inputs. Greetings, Maxime
Am 06.10.21 um 23:25 schrieb Maxime Devos: > >> Not sure what you want to tell me. Anything you recommend me to change? > I recommend moving ,@(standard-packages) from host-inputs to build-inputs. Okay, I'll change this. Anyhow, I'm curious: Other modules (e.g. guix/build-system/python.scm) have ",@(standard-packages)" in host-inputs.
Hartmut Goebel schreef op wo 06-10-2021 om 23:36 [+0200]: > Am 06.10.21 um 23:25 schrieb Maxime Devos: > > > Not sure what you want to tell me. Anything you recommend me to change? > > I recommend moving ,@(standard-packages) from host-inputs to build-inputs. > > Okay, I'll change this. > > Anyhow, I'm curious: Other modules (e.g. guix/build-system/python.scm) > have ",@(standard-packages)" in host-inputs. These build systems are most likely buggy, and these build systems have a ‘(not target) ; XXX: no cross-compilation’ comment, so I wouldn't recommend looking at them to determine how to split packages between host-inputs (= inputs & propagated-inputs but for bags) and build-inputs (= native-inputs but for bags). Instead, I recommend looking at 'gnu-build-system'. Greetings, Maxime.
Hi Maxime, thanks for the review. I applied the changes as discussed, fixed some last-minute bug in the importer :-) and pushed as f86f7e24b39928247729020df0134e2e1c4cde62.
Hartmut Goebel <h.goebel@crazy-compilers.com> skribis: > * guix/build-system/rebar3.scm, guix/build/rebar3-build-system.scm: New files. > * Makefile.am (MODULES): Add them. This looks OK, except it’s undocumented: https://guix.gnu.org/manual/en/html_node/Build-Systems.html I’d be in favor of reverting until we can see the doc. Also, I would have suggested not adding new build systems on ‘master’: they’ll have to be adjusted on ‘core-updates-frozen’, and I’d rather distribute the workload on this and avoid merge issues. > +;;; GNU Guix --- Functional package management for GNU > +;;; Copyright © 2016 Ricardo Wurmus <rekado@elephly.net> Is Ricardo involved? > + (for-each > + (lambda (pkg) > + (for-each > + (lambda (dirname) > + (let ((src-dir (string-append build-dir "/" pkg "/" dirname)) > + (dst-dir (string-append pkg-dir "/" dirname))) > + (when (file-exists? src-dir) > + (copy-recursively src-dir dst-dir #:follow-symlinks? #t)) > + (false-if-exception > + (delete-file (string-append dst-dir "/.gitignore"))))) > + '("ebin" "include" "priv"))) Regarding the style, please avoid abbreviations: ‘source’, not ‘src-dir’, etc. https://guix.gnu.org/manual/en/html_node/Formatting-Code.html The root of a package directory should follow the usual layout: bin, sbin, share, lib, libexec. I think we should not have “ebin” and “priv”, unless there’s a very good justification. Thanks, Ludo’.
Hi, Hartmut Goebel <h.goebel@crazy-compilers.com> skribis: > thanks for the review. I applied the changes as discussed, fixed some > last-minute bug in the importer :-) and pushed as > f86f7e24b39928247729020df0134e2e1c4cde62. That’s really not how we work, nor how we should work IMO. There were ~24h between the time you sent this series and the time you pushed it, only quick comments on 2 patches out of 23, and zero “LGTMs”. I would very much like us to stick to our standards, in particular when proposing changes or additions to core APIs under (guix …). These things just cannot go in without proper review; they cannot go in without adequate testing and documentation, as has always been done in these areas. Perhaps views differ on what “proper review” is, and we can discuss it if in doubt. For one thing, and I thought you were familiar with it since you’ve been around for some time, when someone is done reviewing a specific bit, they explicitly say so with “LGTM” or similar. Now, how do we move forward? I would prefer a single patch reverting all 23 patches now rather than a myriad of tiny fixups hastily reviewed. WDYT? I should say that while I’m unhappy with the way this has been done, I’m glad you’re made this much progress on the Erlang front, and I think it’s a nice addition. Thanks, Ludo’.
Hi Ludo, > That’s really not how we work, nor how we should work IMO. There were > ~24h between the time you sent this series and the time you pushed it, > only quick comments on 2 patches out of 23, and zero “LGTMs”. I apologize for pushing the patch series that fast. I can't tell what exactly did make be rush on this - maybe I just wanted to get this from my table. Not good, anyhow. > Now, how do we move forward? I would prefer a single patch reverting > all 23 patches now rather than a myriad of tiny fixups hastily reviewed. > WDYT? Fine for me. (I'd appreciate if someone else would do this, as I'm not sure about a meaningful commit message. An I'll be offline during the day today.) I'll then provide a new set of three patches - split by topic: 1) extracting downloader, 2) hexpm-importer, 3) rebar-build-system and required packages. WDYT?
Am 08.10.21 um 00:20 schrieb Ludovic Courtès: > Now, how do we move forward? I would prefer a single patch reverting > all 23 patches now rather than a myriad of tiny fixups hastily reviewed. > WDYT? How should the commit message's first line look like? (I did not find a suitable example.) "Revert hasty commits for extracting-downloader, hexpm importer and rebar3"? I would go ahead and revert my faulty commit series. Since I'll be offline tomorrow (Saturday), I would not mind if somebody else is quicker. If it has time until Saturday, I'd bee happy to apply the commit then. Sorry to bother you with this late at Friday evening. And sorry again for not following the process.
Hi Hartmut, Hartmut Goebel <h.goebel@crazy-compilers.com> skribis: >> That’s really not how we work, nor how we should work IMO. There were >> ~24h between the time you sent this series and the time you pushed it, >> only quick comments on 2 patches out of 23, and zero “LGTMs”. > > I apologize for pushing the patch series that fast. I can't tell what > exactly did make be rush on this - maybe I just wanted to get this > from my table. Not good, anyhow. OK. For the record, the process is documented here: https://guix.gnu.org/manual/devel/en/html_node/Commit-Access.html If you think something needs to be clarified, please let us know. > Fine for me. (I'd appreciate if someone else would do this, as I'm not > sure about a meaningful commit message. An I'll be offline during the > day today.) I see Tobias reverted them (thanks!). > I'll then provide a new set of three patches - split by topic: 1) > extracting downloader, 2) hexpm-importer, 3) rebar-build-system and > required packages. Yes. Please, do take into account the partial review I’ve already done. In particular, be sure to follow our standards in terms of testing and documentation (it’s fine to delay documentation for a v2/v3 of the patch series until we’ve agreed on the interfaces; the commits that will be pushed will have to include code, documentation, and tests.) Thanks for your message, Ludo’.
diff --git a/Makefile.am b/Makefile.am index ce79d4bc04..bb0b5989d2 100644 --- a/Makefile.am +++ b/Makefile.am @@ -163,6 +163,7 @@ MODULES = \ guix/build-system/waf.scm \ guix/build-system/r.scm \ guix/build-system/rakudo.scm \ + guix/build-system/rebar3.scm \ guix/build-system/ruby.scm \ guix/build-system/scons.scm \ guix/build-system/texlive.scm \ @@ -216,6 +217,7 @@ MODULES = \ guix/build/r-build-system.scm \ guix/build/renpy-build-system.scm \ guix/build/rakudo-build-system.scm \ + guix/build/rebar3-build-system.scm \ guix/build/ruby-build-system.scm \ guix/build/scons-build-system.scm \ guix/build/texlive-build-system.scm \ diff --git a/guix/build-system/rebar3.scm b/guix/build-system/rebar3.scm new file mode 100644 index 0000000000..04601c930e --- /dev/null +++ b/guix/build-system/rebar3.scm @@ -0,0 +1,143 @@ +;;; GNU Guix --- Functional package management for GNU +;;; Copyright © 2016 Ricardo Wurmus <rekado@elephly.net> +;;; Copyright © 2020 Hartmut Goebel <h.goebel@crazy-compilers.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/>. + +(define-module (guix build-system rebar3) + #:use-module (guix store) + #:use-module (guix utils) + #:use-module (guix packages) + #:use-module (guix derivations) + #:use-module (guix search-paths) + #:use-module (guix build-system) + #:use-module (guix build-system gnu) + #:use-module (ice-9 match) + #:use-module (srfi srfi-26) + #:export (%rebar3-build-system-modules + rebar3-build + rebar3-build-system)) + +;; +;; Standard build procedure for Erlang packages using Rebar3. +;; + +(define %rebar3-build-system-modules + ;; Build-side modules imported by default. + `((guix build rebar3-build-system) + ,@%gnu-build-system-modules)) + +(define (default-rebar3) + "Return the default Rebar3 package." + ;; Lazily resolve the binding to avoid a circular dependency. + (let ((erlang-mod (resolve-interface '(gnu packages erlang)))) + (module-ref erlang-mod 'rebar3))) + +(define (default-erlang) + "Return the default Erlang package." + ;; Lazily resolve the binding to avoid a circular dependency. + (let ((erlang-mod (resolve-interface '(gnu packages erlang)))) + (module-ref erlang-mod 'erlang))) + +(define* (lower name + #:key source inputs native-inputs outputs system target + (rebar (default-rebar3)) + (erlang (default-erlang)) + #:allow-other-keys + #:rest arguments) + "Return a bag for NAME." + (define private-keywords + '(#:source #:target #:rebar #:inputs #:native-inputs)) + + (and (not target) ;XXX: no cross-compilation + (bag + (name name) + (system system) + (host-inputs `(,@(if source + `(("source" ,source)) + '()) + ,@inputs + ;; Keep the standard inputs of 'gnu-build-system'. + ,@(standard-packages))) + (build-inputs `(("rebar" ,rebar) + ("erlang" ,erlang) ;; for escriptize + ,@native-inputs)) + (outputs outputs) + (build rebar3-build) + (arguments (strip-keyword-arguments private-keywords arguments))))) + +(define* (rebar3-build store name inputs + #:key + (tests? #t) + (test-target "eunit") + (configure-flags ''()) + (make-flags ''("skip_deps=true" "-vv")) + (build-target "compile") + ;; TODO: pkg-name + (phases '(@ (guix build rebar3-build-system) + %standard-phases)) + (outputs '("out")) + (search-paths '()) + (system (%current-system)) + (guile #f) + (imported-modules %rebar3-build-system-modules) + (modules '((guix build rebar3-build-system) + (guix build utils)))) + "Build SOURCE with INPUTS." + (define builder + `(begin + (use-modules ,@modules) + (rebar3-build #:name ,name + #:source ,(match (assoc-ref inputs "source") + (((? derivation? source)) + (derivation->output-path source)) + ((source) + source) + (source + source)) + #:make-flags ,make-flags + #:configure-flags ,configure-flags + #:system ,system + #:tests? ,tests? + #:test-target ,test-target + #:build-target ,build-target + #:phases ,phases + #:outputs %outputs + #:search-paths ',(map search-path-specification->sexp + search-paths) + #:inputs %build-inputs))) + + (define guile-for-build + (match guile + ((? package?) + (package-derivation store guile system #:graft? #f)) + (#f ; the default + (let* ((distro (resolve-interface '(gnu packages commencement))) + (guile (module-ref distro 'guile-final))) + (package-derivation store guile system #:graft? #f))))) + + (build-expression->derivation store name builder + #:inputs inputs + #:system system + #:modules imported-modules + #:outputs outputs + #:guile-for-build guile-for-build)) + +(define rebar3-build-system + (build-system + (name 'rebar3) + (description "The standard Rebar3 build system") + (lower lower))) diff --git a/guix/build/rebar3-build-system.scm b/guix/build/rebar3-build-system.scm new file mode 100644 index 0000000000..d503fc9944 --- /dev/null +++ b/guix/build/rebar3-build-system.scm @@ -0,0 +1,150 @@ +;;; GNU Guix --- Functional package management for GNU +;;; Copyright © 2016, 2018 Ricardo Wurmus <rekado@elephly.net> +;;; Copyright © 2019 Björn Höfling <bjoern.hoefling@bjoernhoefling.de> +;;; Copyright © 2020 Hartmut Goebel <h.goebel@crazy-compilers.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/>. + +(define-module (guix build rebar3-build-system) + #:use-module ((guix build gnu-build-system) #:prefix gnu:) + #:use-module ((guix build utils) #:hide (delete)) + #:use-module (ice-9 match) + #:use-module (ice-9 ftw) + #:use-module (srfi srfi-1) + #:use-module (srfi srfi-26) + #:export (%standard-phases + rebar3-build)) + +;; +;; Builder-side code of the standard build procedure for Erlang packages using +;; rebar3. +;; +;; TODO: Think about whether bindir ("ebin"), libdir ("priv") and includedir +;; "(include") need to be configurable + +(define %erlang-libdir "/lib/erlang/lib") + +(define* (erlang-depends #:key inputs #:allow-other-keys) + (define input-directories + (match inputs + (((_ . dir) ...) + dir))) + (mkdir-p "_checkouts") + + (for-each + (lambda (input-dir) + (let ((elibdir (string-append input-dir %erlang-libdir))) + (when (directory-exists? elibdir) + (for-each + (lambda (dirname) + (symlink (string-append elibdir "/" dirname) + (string-append "_checkouts/" dirname))) + (list-directories elibdir))))) + input-directories) + #t) + +(define* (unpack #:key source #: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." + ;; archives from hexpm typicalls do not contain a directory level + ;; TODO: Check if archive contains a directory level + (mkdir "source") + (chdir "source") + (if (file-is-directory? source) + (begin + ;; Preserve timestamps (set to the Epoch) on the copied tree so that + ;; things work deterministically. + (copy-recursively source "." + #:keep-mtime? #t)) + (begin + (if (string-suffix? ".zip" source) + (invoke "unzip" source) + (invoke "tar" "xvf" source)))) + #t) + +(define* (build #:key (make-flags '()) (build-target "compile") + #:allow-other-keys) + (apply invoke `("rebar3" ,build-target ,@make-flags))) + +(define* (check #:key target (make-flags '()) (tests? (not target)) + (test-target "eunit") + #:allow-other-keys) + (if tests? + (apply invoke `("rebar3" ,test-target ,@make-flags)) + (format #t "test suite not run~%")) + #t) + +(define (erlang-package? name) + "Check if NAME correspond to the name of an Erlang package." + (string-prefix? "erlang-" name)) + +(define (package-name-version->erlang-name name+ver) + "Convert the Guix package NAME-VER to the corresponding Erlang 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 (erlang-package? name-) ; checks for "erlang-" prefix + (string-drop name- (string-length "erlang-")) + name-) + #\-) + "_"))) + +(define (list-directories directory) + "Return file names of the sub-directory of DIRECTORY." + (scandir directory + (lambda (file) + (and (not (member file '("." ".."))) + (file-is-directory? (string-append directory "/" file)))))) + +(define* (install #:key name outputs + (pkg-name (package-name-version->erlang-name name)) + #:allow-other-keys) + (let* ((out (assoc-ref outputs "out")) + (build-dir "_build/default/lib") + (pkg-dir (string-append out %erlang-libdir "/" pkg-name))) + (for-each + (lambda (pkg) + (for-each + (lambda (dirname) + (let ((src-dir (string-append build-dir "/" pkg "/" dirname)) + (dst-dir (string-append pkg-dir "/" dirname))) + (when (file-exists? src-dir) + (copy-recursively src-dir dst-dir #:follow-symlinks? #t)) + (false-if-exception + (delete-file (string-append dst-dir "/.gitignore"))))) + '("ebin" "include" "priv"))) + (list-directories build-dir)) + (false-if-exception + (delete-file (string-append pkg-dir "/priv/Run-eunit-loop.expect"))) + #t)) + +(define %standard-phases + (modify-phases gnu:%standard-phases + (replace 'unpack unpack) + (delete 'bootstrap) + (delete 'configure) + (add-before 'build 'erlang-depends erlang-depends) + (replace 'build build) + (replace 'check check) + (replace 'install install))) + +(define* (rebar3-build #:key inputs (phases %standard-phases) + #:allow-other-keys #:rest args) + "Build the given Erlang package, applying all of PHASES in order." + (apply gnu:gnu-build #:inputs inputs #:phases phases args))