From mboxrd@z Thu Jan 1 00:00:00 1970 From: taylanbayirli@gmail.com (Taylan Ulrich =?utf-8?Q?Bay=C4=B1rl=C4=B1?= =?utf-8?Q?=2FKammer?=) Subject: Re: [PATCH] build: pull: Compile .scm files in one process. Date: Fri, 27 Nov 2015 16:16:29 +0100 Message-ID: <87bnaflbg2.fsf@T420.taylan> References: <87si4kxtge.fsf@T420.taylan> <87611gdul8.fsf@gnu.org> <87h9kzy09b.fsf@T420.taylan> <87bnb6c0nh.fsf@gnu.org> <874mgyxhgy.fsf@T420.taylan> <877flpohu6.fsf@gnu.org> <87mvuku444.fsf@T420.taylan> <87pozgfyzt.fsf@gnu.org> <87io57tt2s.fsf@T420.taylan> <876117mnef.fsf@igalia.com> <87egfvtnbw.fsf@T420.taylan> <87y4e3l7hm.fsf@igalia.com> <87a8qjtje8.fsf@T420.taylan> <876117t0ax.fsf@gnu.org> <877flmrn2m.fsf@T420.taylan> <87a8q0ies5.fsf@gnu.org> <87fuzrlt6f.fsf@T420.taylan> <87bnafbvrs.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:38731) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a2KlH-0006Fl-Cj for guix-devel@gnu.org; Fri, 27 Nov 2015 10:16:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a2KlF-0007EQ-AZ for guix-devel@gnu.org; Fri, 27 Nov 2015 10:16:35 -0500 In-Reply-To: <87bnafbvrs.fsf@gnu.org> ("Ludovic \=\?utf-8\?Q\?Court\=C3\=A8s\=22'\?\= \=\?utf-8\?Q\?s\?\= message of "Fri, 27 Nov 2015 11:07:35 +0100") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org To: Ludovic =?utf-8?Q?Court=C3=A8s?= Cc: guix-devel@gnu.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable ludo@gnu.org (Ludovic Court=C3=A8s) writes: > taylanbayirli@gmail.com (Taylan Ulrich "Bay=C4=B1rl=C4=B1/Kammer") skribi= s: > >> ludo@gnu.org (Ludovic Court=C3=A8s) writes: > > [...] > >>> ?: 2 [primitive-load "/gnu/store/d51z2xkwp1vh0dh6gqadyyzv21m0b772-gu= ix-latest/guix/scripts/import/hackage.scm"] >>> In ice-9/eval.scm: >>> 453: 1 Exception thrown while printing backtrace: >>> ERROR: In procedure package-location: Wrong type argument: Error while = printing exception. >>> >>> ice-9/eval.scm:387:11: In procedure eval: >>> ice-9/eval.scm:387:11: In procedure package-version: Wrong type argumen= t: Error while printing exception. >>> builder for `/gnu/store/pc1i5s6vx9yx97prhskx178gj5swxw4k-guix-latest.dr= v' failed with exit code 1 >>> guix pull: error: build failed: build of `/gnu/store/pc1i5s6vx9yx97prhs= kx178gj5swxw4k-guix-latest.drv' failed >>> >>> Any idea? >>> >>> To me it sounds like there are two record type descriptors in >>> the wild, which is why =E2=80=98package-location=E2=80=99 in the packag= e record printer >>> bails out. >> >> That's one of the errors that result from a "bad" order of compiling the >> files and when the 'load' hack isn't used to work around it, which isn't >> the case in that patch... Indeed I can't seem to reproduce the issue. >> >> The attached patch below also builds on the quoted patch, removes the >> thread-safe-port procedure, and just sets the warning port to a void >> port. Applied on top of the current master, it works for me. > > On top of current master, it fails for me in the same way as above. > > To be clear, I applied the patch, ran =E2=80=98make dist=E2=80=99, and th= en: > > time ./pre-inst-env guix pull --url=3Dfile://$PWD/guix-0.9.0.tar.gz > > Are you doing the same? The =E2=80=9Cloading=E2=80=9D part is done seque= ntially, so it > should be deterministic. Whoops, I had not rerun the whole 'make dist' after rebasing on master, only copied the new guix/build/pull.scm into an existing tarball (I had gotten used to doing that because it saves time while working on a single file), so changes in other files were missing. After some tinkering around I realized that the problem is that our workaround of loading files explicitly causes the package record type to be redefined after some packages have already been defined. More generally, we force the top-level of many files to be re-executed after they've already been executed as a result of a module import... It would be great if the whole circular import problem could somehow be solved by Guile (no idea how feasible it is). On the meanwhile, we'll have to work around problems introduced by workarounds. :-) Moving the loading of guix/package.scm to the very front seems to solve the issue. Other record types could still cause the same issue, but their relative rarity of use hopefully makes this a non-issue. I also moved the loading of guix/ files before gnu/ files again, which might also help with that. (For package.scm it wasn't sufficient, probably because some modules under guix/ import some gnu package modules, before package.scm is loaded explicitly.) One can imagine a wholly more robust version of the workaround, which avoids the re-execution of top-levels. A variant of load[-primitive] that doesn't load a file again if it was already loaded would do. That's basically what importing a module does, so scanning for module definitions in files and importing them might work, but seems somewhat hacky... For now, here's the patch that just loads package.scm first. --=-=-= Content-Type: text/x-diff; charset=utf-8 Content-Disposition: inline; filename=0001-build-pull-Compile-.scm-files-in-one-process.patch Content-Transfer-Encoding: quoted-printable >From dcb563f611c4fbd6e3e22106c60626f32c04f9e5 Mon Sep 17 00:00:00 2001 From: =3D?UTF-8?q?Taylan=3D20Ulrich=3D20Bay=3DC4=3DB1rl=3DC4=3DB1/Kammer?=3D Date: Fri, 27 Nov 2015 09:27:55 +0100 Subject: [PATCH] build: pull: Compile .scm files in one process. * guix/build/pull.scm (call-with-process, report-build-progress) (p-for-each): Remove. (build-guix): Load and compile files in one process. --- guix/build/pull.scm | 149 +++++++++++++++++++-----------------------------= ---- 1 file changed, 55 insertions(+), 94 deletions(-) diff --git a/guix/build/pull.scm b/guix/build/pull.scm index 281be23..3025442 100644 --- a/guix/build/pull.scm +++ b/guix/build/pull.scm @@ -1,5 +1,6 @@ ;;; GNU Guix --- Functional package management for GNU ;;; Copyright =C2=A9 2013, 2014 Ludovic Court=C3=A8s +;;; Copyright =C2=A9 2015 Taylan Ulrich Bay=C4=B1rl=C4=B1/Kammer ;;; ;;; This file is part of GNU Guix. ;;; @@ -22,6 +23,7 @@ #:use-module (ice-9 ftw) #:use-module (ice-9 match) #:use-module (ice-9 format) + #:use-module (ice-9 threads) #:use-module (srfi srfi-1) #:use-module (srfi srfi-11) #:use-module (srfi srfi-26) @@ -33,75 +35,10 @@ ;;; ;;; Code: =20 -(define (call-with-process thunk) - "Run THUNK in a separate process that will return 0 if THUNK terminates -normally, and 1 if an exception is raised." - (match (primitive-fork) - (0 - (catch #t - (lambda () - (thunk) - (primitive-exit 0)) - (lambda (key . args) - (print-exception (current-error-port) #f key args) - (primitive-exit 1)))) - (pid - #t))) - -(define* (report-build-progress total completed cont - #:optional (log-port (current-error-port))) - "Report that COMPLETED out of TOTAL files have been completed, and call -CONT." - (display #\cr log-port) - (format log-port "compiling...\t~5,1f% of ~d files" ;FIXME: i18n - (* 100. (/ completed total)) total) - (force-output log-port) - (cont)) - -(define* (p-for-each proc lst - #:optional (max-processes (current-processor-count)) - #:key (progress report-build-progress)) - "Invoke PROC for each element of LST in a separate process, using up to -MAX-PROCESSES processes in parallel. Call PROGRESS at each step, passing = it -the continuation. Raise an error if one of the processes exit with non-ze= ro." - (define total - (length lst)) - - (define (wait-for-one-process) - (match (waitpid WAIT_ANY) - ((_ . status) - (unless (zero? (status:exit-val status)) - (error "process failed" proc status))))) - - (let loop ((lst lst) - (running 0) - (completed 0)) - (match lst - (() - (or (zero? running) - (let ((running (- running 1)) - (completed (+ completed 1))) - (wait-for-one-process) - (progress total completed - (lambda () - (loop lst running completed)))))) - ((head . tail) - (if (< running max-processes) - (let ((running (+ 1 running))) - (call-with-process (cut proc head)) - (progress total completed - (lambda () - (loop tail running completed)))) - (let ((running (- running 1)) - (completed (+ completed 1))) - (wait-for-one-process) - (progress total completed - (lambda () - (loop lst running completed))))))))) - (define* (build-guix out source #:key gcrypt - (debug-port (%make-void-port "w"))) + (debug-port (%make-void-port "w")) + (log-port (current-error-port))) "Build and install Guix in directory OUT using SOURCE, a directory containing the source code. Write any debugging output to DEBUG-PORT." (setvbuf (current-output-port) _IOLBF) @@ -130,33 +67,57 @@ containing the source code. Write any debugging outpu= t to DEBUG-PORT." (set! %load-path (cons out %load-path)) (set! %load-compiled-path (cons out %load-compiled-path)) =20 - ;; Compile the .scm files. Do that in independent processes, =C3=A0 la - ;; 'make -j', to work around (FIXME). - ;; This ensures correctness, but is overly conservative and slow. - ;; The solution initially implemented (and described in the bug - ;; above) was slightly faster but consumed memory proportional to the - ;; number of modules, which quickly became unacceptable. - (p-for-each (lambda (file) - (let ((go (string-append (string-drop-right file 4) - ".go"))) - (format debug-port "~%compiling '~a'...~%" file) - (parameterize ((current-warning-port debug-port)) - (compile-file file - #:output-file go - #:opts - %auto-compilation-options)))) - - (filter (cut string-suffix? ".scm" <>) - - ;; Build guix/*.scm before gnu/*.scm to speed - ;; things up. - (sort (find-files out "\\.scm") - (let ((guix (string-append out "/guix")) - (gnu (string-append out "/gnu"))) - (lambda (a b) - (or (and (string-prefix? guix a) - (string-prefix? gnu b)) - (string (FIXME). + (let* ((files + ;; The above mentioned workaround means the top-level of many + ;; files will be executed twice. Load guix/packages.scm first, + ;; because it's crucial that the package data type doesn't get + ;; redefined halfway through. Also load guix/ modules before = gnu/ + ;; modules to get steadier progress reporting. + (sort (filter (cut string-suffix? ".scm" <>) + (find-files out "\\.scm")) + (let ((guix (string-append out "/guix")) + (gnu (string-append out "/gnu"))) + (lambda (a b) + (or (string-suffix? "/guix/packages.scm" a) + (and (string-prefix? guix a) + (string-prefix? gnu b)) + (string