diff mbox

Building Guix with Guile 2.1

Message ID 87fuotl367.fsf@gnu.org
State New
Headers show

Commit Message

Ludovic Courtès Sept. 21, 2016, 8:23 a.m. UTC
Hello!

Nalaginrut reported that Guix fails to build with Guile 2.2, which was a
bit of a shame, hence commits e465d9e19087ab150f7e31f21c09e4a147b93b36
and 9d126aa2b504bb9fad536eac186805ff623e96be.

Now, the hack in build-aux/compile-all.scm doesn’t quite work with 2.2,
and it was already quite fragile.

So I think we need a different approach.  A more robust approach that
comes to mind would be to look at our module dependency graph (with help
from (guix modules)), run N Guile processes (instead of threads), and
have each of them build a subset of the graph.  It would essentially
require us to reimplement a subset of Make (or guix-daemon).

I’ve tried generating makefile rules from module dependencies.  That’s
pretty cool, but compilation of the package modules remains damn slow
(2nd patch attached).

What do people think?  Anyone has a better idea?

The attached patch allows us to build the modules sequentially but in
topologically-sorted order, which means we can avoid the ugly
load-modules-first workaround.  Still, it’s a bit too slow.

Ludo’.

Comments

Andy Wingo Sept. 21, 2016, 5:03 p.m. UTC | #1
On Wed 21 Sep 2016 10:23, ludo@gnu.org (Ludovic Courtès) writes:

> Hello!
>
> Nalaginrut reported that Guix fails to build with Guile 2.2, which was a
> bit of a shame, hence commits e465d9e19087ab150f7e31f21c09e4a147b93b36
> and 9d126aa2b504bb9fad536eac186805ff623e96be.
>
> Now, the hack in build-aux/compile-all.scm doesn’t quite work with 2.2,
> and it was already quite fragile.

Which hack?  You mean loading modules before compiling?  I guess you
should load the compiler modules too FWIW, perhaps that's an issue.

If there is a nice change we can do to make module-loading thread-safe,
let's think about that :)  It's probably the biggest thread-safety
problem we have in Guile and now would be the right time to fix it.

Andy
Taylan Ulrich Bayırlı =?utf-8?Q?=2FKammer?= Sept. 21, 2016, 7:40 p.m. UTC | #2
Andy Wingo <wingo@igalia.com> writes:

> On Wed 21 Sep 2016 10:23, ludo@gnu.org (Ludovic Courtès) writes:
>
>> Hello!
>>
>> Nalaginrut reported that Guix fails to build with Guile 2.2, which was a
>> bit of a shame, hence commits e465d9e19087ab150f7e31f21c09e4a147b93b36
>> and 9d126aa2b504bb9fad536eac186805ff623e96be.
>>
>> Now, the hack in build-aux/compile-all.scm doesn’t quite work with 2.2,
>> and it was already quite fragile.
>
> Which hack?  You mean loading modules before compiling?  I guess you
> should load the compiler modules too FWIW, perhaps that's an issue.
>
> If there is a nice change we can do to make module-loading thread-safe,
> let's think about that :)  It's probably the biggest thread-safety
> problem we have in Guile and now would be the right time to fix it.

It would be neat to make module loading thread-safe indeed!  Not sure if
this issue is related to that though...  Currently the .go compilation
phase of 'make' errors like:

> [... snip ...]
>   LOAD     (guix build-system haskell)
>   LOAD     (guix build-system perl)
>   LOAD     (guix build-system python)
>   LOAD     (guix build-system waf)
> Backtrace:
>           18 (primitive-load "/home/taylan/src/guix/./build-aux/comp?")
> In ice-9/eval.scm:
>     608:8 17 (_ #(#(#(#(#(#<directory (guile-user) 26?> ?) ?) ?) ?) ?))
> In ice-9/boot-9.scm:
>    262:13 16 (for-each1 ("guix/build-system/waf.scm" "guix/build-?" ?))
>   2788:17 15 (resolve-interface (guix build-system waf) #:select _ # ?)
>   2713:10 14 (_ (guix build-system waf) _ _ #:ensure _)
>   2989:16 13 (try-module-autoload _ _)
>    2325:4 12 (save-module-excursion #<procedure 2fb5390 at ice-9/boo?>)
>   3009:22 11 (_)
> In unknown file:
>           10 (primitive-load-path "guix/build-system/waf" #<procedur?>)
> In ice-9/eval.scm:
>    710:20  9 (primitive-eval (define-module (guix build-system #) # ?))
> In ice-9/psyntax.scm:
>   1209:36  8 (expand-top-sequence ((define-module (guix # waf) # ?)) ?)
>   1156:24  7 (parse _ (("placeholder" placeholder)) ((top) #(# # ?)) ?)
>    279:10  6 (parse _ (("placeholder" placeholder)) (()) _ c&e (eval) ?)
> In ice-9/eval.scm:
>    293:34  5 (_ #<module (#{ g141635}#) 4809000>)
> In ice-9/boot-9.scm:
>   2849:10  4 (define-module* _ #:filename _ #:pure _ #:version _ # _ ?)
>   2801:10  3 (resolve-interface (guix build-system python) #:select _ ?)
>    262:13  2 (for-each1 (default-python default-python2))
>   2806:38  1 (_ _)
> In unknown file:
>            0 (scm-error misc-error #f "~A" ("no binding `default-p?") ?)
>
> ERROR: In procedure scm-error:
> ERROR: no binding `default-python' in module (guix build-system python)

That's in the phase where the modules are all loaded by calling
'resolve-interface' on their names, which is *not* done in parallel.
To quote compile-all.scm:

> [... snip ...]
>
> ;;; To work around <http://bugs.gnu.org/15602> (FIXME), we want to load all
> ;;; files to be compiled first.  We do this via resolve-interface so that the
> ;;; top-level of each file (module) is only executed once.
> (define (load-module-file file)
>   (let ((module (file->module file)))
>     (format #t "  LOAD     ~a~%" module)
>     (resolve-interface module)))
>
> [... snip ...]
>
> (match (command-line)
>   ((_ . files)
>    (let ((files (filter file-needs-compilation? files)))
>      (for-each load-module-file files)
>      (let ((mutex (make-mutex)))
>        (par-for-each (lambda (file)
>                        (compile-file* file mutex))
>                      files)))))

I ran guile via ./pre-inst-env and executed (resolve-interface '(guix
build-system waf)) manually and actually got a similar / the same error:

> [... snip ...]
> ;;; WARNING: compilation of /home/taylan/src/guix/guix/build-system/waf.scm failed:
> ;;; ERROR: no binding `default-python' in module (guix build-system python)
> ERROR: In procedure scm-error:
> ERROR: no binding `default-python' in module (guix build-system python)

Looking into (guix build-system waf), it contains:

> ...
>   #:use-module ((guix build-system python)
>                 #:select (default-python default-python2))
> ...

but (guix build-system python) does not export default-python.

Apparently Guile 2.0 allows this (to select private bindings), but 2.2
does not.

Reading the documentation for #:select I can't really tell which
behavior is intended.

Taylan
Taylan Ulrich Bayırlı =?utf-8?Q?=2FKammer?= Sept. 21, 2016, 7:48 p.m. UTC | #3
By the way, for testing guix with guile 2.2 easily:

> $ cat ~/tmp/guix-with-guile-next.scm
> (use-modules
>  (guix packages)
>  (guix build utils)
>  (gnu packages guile)
>  (gnu packages package-management))
>
> (package
>   (inherit guix)
>   (inputs (alist-replace "guile"
>                          (list guile-next)
>                          (package-inputs guix))))
>
> $ guix environment --pure -l ~/tmp/guix-with-guile-next.scm
> $ autoreconf ...

Taylan
Ludovic Courtès Sept. 24, 2016, 3:31 a.m. UTC | #4
Hi!

Andy Wingo <wingo@igalia.com> skribis:

> On Wed 21 Sep 2016 10:23, ludo@gnu.org (Ludovic Courtès) writes:
>
>> Hello!
>>
>> Nalaginrut reported that Guix fails to build with Guile 2.2, which was a
>> bit of a shame, hence commits e465d9e19087ab150f7e31f21c09e4a147b93b36
>> and 9d126aa2b504bb9fad536eac186805ff623e96be.
>>
>> Now, the hack in build-aux/compile-all.scm doesn’t quite work with 2.2,
>> and it was already quite fragile.
>
> Which hack?  You mean loading modules before compiling?  I guess you
> should load the compiler modules too FWIW, perhaps that's an issue.
>
> If there is a nice change we can do to make module-loading thread-safe,
> let's think about that :)  It's probably the biggest thread-safety
> problem we have in Guile and now would be the right time to fix it.

I agree this is the right thing to do.

My guess is that this would have to be approached in C, to avoid the fat
mutex overhead.  But hmm, I dunno exactly what this entails!

Ludo’.
diff mbox

Patch

From 9621aa00b4378036e906f58dc40e431748ba9790 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Wed, 21 Sep 2016 23:45:07 +0900
Subject: [PATCH] build: Generate makefile rules from .scm files.

* guix/modules.scm (source-module-dependencies): Export.
* build-aux/generate-make-rules.scm: New file.
* Makefile.am (EXTRA_DIST): Add it.
(AM_V_GUILEC, AM_V_GUILEC_, AM_V_GUILEC_0)
(GUILD_COMPILE_FLAGS): New variables.
(.scm.go): New rule.
(%.go, make-go): Remove.
(build-aux/module-rules.mk): New rule.  Add "-include" statement.
(CLEANFILES): Add this file.
---
 Makefile.am                       | 45 +++++++++++++++++++------
 build-aux/generate-make-rules.scm | 71 +++++++++++++++++++++++++++++++++++++++
 guix/modules.scm                  |  3 +-
 3 files changed, 108 insertions(+), 11 deletions(-)
 create mode 100644 build-aux/generate-make-rules.scm

diff --git a/Makefile.am b/Makefile.am
index f9fe141..6720773 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -355,6 +355,7 @@  EXTRA_DIST =						\
   .dir-locals.el					\
   build-aux/build-self.scm				\
   build-aux/compile-all.scm				\
+  build-aux/generate-make-rules.scm			\
   build-aux/hydra/evaluate.scm				\
   build-aux/hydra/gnu-system.scm			\
   build-aux/hydra/demo-os.scm				\
@@ -390,26 +391,50 @@  CLEANFILES =					\
   $(GOBJECTS)					\
   $(SCM_TESTS:tests/%.scm=%.log)
 
+AM_V_GUILEC = $(AM_V_GUILEC_$(V))
+AM_V_GUILEC_ = $(AM_V_GUILEC_$(AM_DEFAULT_VERBOSITY))
+AM_V_GUILEC_0 = @echo "  GUILEC" $@;
+
+# Flags passed to 'guild compile'.
+GUILD_COMPILE_FLAGS =				\
+  -Wformat -Wunbound-variable -Warity-mismatch
+
 # Unset 'GUILE_LOAD_COMPILED_PATH' altogether while compiling.  Otherwise, if
 # $GUILE_LOAD_COMPILED_PATH contains $(moduledir), we may find .go files in
 # there that are newer than the local .scm files (for instance because the
 # user ran 'make install' recently).  When that happens, we end up loading
 # those previously-installed .go files, which may be stale, thereby breaking
-# the whole thing.  Likewise, set 'XDG_CACHE_HOME' to avoid loading possibly
-# stale files from ~/.cache/guile/ccache.
-%.go: make-go ; @:
-make-go: $(MODULES) guix/config.scm guix/tests.scm
-	$(AM_V_at)echo "Compiling Scheme modules..." ;			\
+# the whole thing.
+#
+# XXX: Use the C locale for when Guile lacks
+# <http://git.sv.gnu.org/cgit/guile.git/commit/?h=stable-2.0&id=e2c6bf3866d1186c60bacfbd4fe5037087ee5e3f>.
+.scm.go:
+	$(AM_V_GUILEC)$(MKDIR_P) `dirname "$@"` ;			\
+	$(AM_V_P) && out=1 || out=- ;					\
 	unset GUILE_LOAD_COMPILED_PATH ;				\
-	XDG_CACHE_HOME=/nowhere						\
-	host=$(host) srcdir="$(top_srcdir)"				\
+	LC_ALL=C							\
 	$(top_builddir)/pre-inst-env					\
-	$(GUILE) -L "$(top_builddir)" -L "$(top_srcdir)"		\
-	  --no-auto-compile 						\
-	  -s "$(top_srcdir)"/build-aux/compile-all.scm $^
+	$(GUILD) compile -L "$(top_builddir)" -L "$(top_srcdir)"	\
+	  $(GUILD_COMPILE_FLAGS) --target="$(host)"			\
+	  -o "$@" "$<" >&$$out
+
 
 SUFFIXES = .go
 
+# Dependency among Scheme modules.
+-include build-aux/module-rules.mk
+
+build-aux/module-rules.mk: $(MODULES)
+	$(AM_V_GEN)$(MKDIR_P) `dirname "$@"` ;			\
+	unset GUILE_LOAD_COMPILED_PATH ;			\
+	LC_ALL=C srcdir="$(srcdir)"				\
+	$(top_builddir)/pre-inst-env				\
+	  $(GUILE) --no-auto-compile				\
+	  build-aux/generate-make-rules.scm $^ > "$@.tmp"
+	mv "$@.tmp" "$@"
+
+CLEANFILES += build-aux/module-rules.mk
+
 # Make sure source files are installed first, so that the mtime of
 # installed compiled files is greater than that of installed source
 # files.  See
diff --git a/build-aux/generate-make-rules.scm b/build-aux/generate-make-rules.scm
new file mode 100644
index 0000000..c38b364
--- /dev/null
+++ b/build-aux/generate-make-rules.scm
@@ -0,0 +1,71 @@ 
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2016 Ludovic Courtès <ludo@gnu.org>
+;;;
+;;; 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/>.
+
+(use-modules (guix modules)
+             (guix utils)
+             (srfi srfi-26)
+             (ice-9 match))
+
+(define (relevant-module? module)
+  (match module
+    (('guix _ ...) #t)
+    (('gnu 'packages _ ...) #f)
+    (('gnu _ ...) #t)
+    (_ #f)))
+
+(define srcdir (or (getenv "srcdir")"."))
+
+(define (relative-file file)
+  (if (string-prefix? (string-append srcdir "/") file)
+      (string-drop file (+ 1 (string-length srcdir)))
+      file))
+
+(define (file->module file)
+  (let* ((relative (relative-file file))
+         (module-path (string-drop-right relative 4)))
+    (map string->symbol
+         (string-split module-path #\/))))
+
+(define (module->file module)
+  (string-append srcdir "/"
+                 (string-join (map symbol->string module) "/")))
+
+(define (display-module-rule module dependencies port)
+  (format port "~a.go: ~a.scm~{ ~a~}~%"
+          (module->file module)
+          (module->file module)
+          (map (compose (cut string-append <> ".go")
+                        module->file) dependencies)))
+
+(match (command-line)
+  ((command . files)
+   (format #t "# Automatically generated.           -*- read-only -*-\n")
+   (for-each (lambda (file)
+               (match (file->module file)
+                 (('gnu 'packages _ ...)
+                  ;; We don't want to rebuild package modules too often,
+                  ;; especially since it's rarely useful.  Thus, generate a
+                  ;; simple static rule.
+                  (format #t "~a.go: ~a guix/packages.go~%"
+                          (file-sans-extension file) file))
+                 (module
+                  (display-module-rule module
+                                       (filter relevant-module?
+                                               (source-module-dependencies module))
+                                       (current-output-port)))))
+             files)))
diff --git a/guix/modules.scm b/guix/modules.scm
index 24f613f..d78faf2 100644
--- a/guix/modules.scm
+++ b/guix/modules.scm
@@ -21,7 +21,8 @@ 
   #:use-module (guix sets)
   #:use-module (srfi srfi-26)
   #:use-module (ice-9 match)
-  #:export (source-module-closure
+  #:export (source-module-dependencies
+            source-module-closure
             live-module-closure
             guix-module-name?))
 
-- 
2.10.0