From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Tobin Harding Newsgroups: gmane.lisp.guile.devel Subject: Re: add command line option to quiet compiler messages Date: Wed, 20 Jul 2016 16:41:24 +1000 Message-ID: <20160720064124.GA1213@eros.local> References: <20160713040806.GA13513@eros.local> <87k2go1qu1.fsf@pobox.com> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1468996926 29788 80.91.229.3 (20 Jul 2016 06:42:06 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Wed, 20 Jul 2016 06:42:06 +0000 (UTC) Cc: guile-devel@gnu.org To: Andy Wingo Original-X-From: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Wed Jul 20 08:41:55 2016 Return-path: Envelope-to: guile-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1bPlCc-00005H-MM for guile-devel@m.gmane.org; Wed, 20 Jul 2016 08:41:54 +0200 Original-Received: from localhost ([::1]:60930 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPlCc-0003ac-2m for guile-devel@m.gmane.org; Wed, 20 Jul 2016 02:41:54 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:56651) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPlCW-0003aW-55 for guile-devel@gnu.org; Wed, 20 Jul 2016 02:41:49 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bPlCQ-00066q-Er for guile-devel@gnu.org; Wed, 20 Jul 2016 02:41:47 -0400 Original-Received: from out5-smtp.messagingengine.com ([66.111.4.29]:51385) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPlCN-00065v-Vy for guile-devel@gnu.org; Wed, 20 Jul 2016 02:41:42 -0400 Original-Received: from compute2.internal (compute2.nyi.internal [10.202.2.42]) by mailout.nyi.internal (Postfix) with ESMTP id 42A102045C; Wed, 20 Jul 2016 02:41:30 -0400 (EDT) Original-Received: from frontend2 ([10.202.2.161]) by compute2.internal (MEProxy); Wed, 20 Jul 2016 02:41:30 -0400 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=tobin.cc; h=cc :content-type:date:from:in-reply-to:message-id:mime-version :references:subject:to:x-sasl-enc:x-sasl-enc; s=mesmtp; bh=jyaJ2 crGvx+yIrf27PFlCl1CuS8=; b=j8ylIqlozf9tGZrKwSvK6xtzS0eogMFxifOCe Uv0Xp1/z1mmPSORHQPFpmFOr8FAwgPxot3h6h90PwMYJQYBddmTpJFSmyKW44As3 +2nlcuHCWAUZWbKjgaacAKUc5Q3v1FTHpjQmemifdhlTdGPEPNbMPlARuprKfkB0 PFbMzw= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-sasl-enc :x-sasl-enc; s=smtpout; bh=jyaJ2crGvx+yIrf27PFlCl1CuS8=; b=KAJcH xZO/AakHbe5rWFJ6aTe2ZNhBj5a1M7O1ydwgnqRSx1tzpVBAw3Q4VjeBM9lgaBal m7i4T85AkPTGW5wkCUren95laq+tABdKJd8ZMRN8CglzT5UcLTfyDuP0O9gridZT WRGIgZ5d90fHFRhxaW7MjEOGka0zJfJRg8shFE= X-Sasl-enc: 0v+w/z6vDKw4mvkMdffhptYwBzZIVUNrolbcuhxFwlip 1468996887 Original-Received: from eros.local (ppp121-44-12-69.lns20.syd4.internode.on.net [121.44.12.69]) by mail.messagingengine.com (Postfix) with ESMTPA id 55449CCDB7; Wed, 20 Jul 2016 02:41:27 -0400 (EDT) Content-Disposition: inline In-Reply-To: <87k2go1qu1.fsf@pobox.com> X-Mailer: Mutt 1.6.1 (2016-04-27) User-Agent: Mutt/1.6.1 (2016-04-27) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 66.111.4.29 X-BeenThere: guile-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Developers list for Guile, the GNU extensibility library" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Original-Sender: "guile-devel" Xref: news.gmane.org gmane.lisp.guile.devel:18590 Archived-At: On Thu, Jul 14, 2016 at 12:01:10PM +0200, Andy Wingo wrote: > On Wed 13 Jul 2016 06:08, Tobin Harding writes: > > > While working on this I discovered that compile messages are output from two > > separate places (load.c and boot-9.scm). Each file contains identical strings > > for the messages. This goes against the rule of SPOT. > > Sadly it has to be like this for now. I do not have suggestions to fix > it; it's gnarly. Ok, Will forget about this. I would love to know why though, solely out of interest. > > > - scm_puts (";;; note: source file ", scm_current_warning_port ()); > > - scm_display (full_filename, scm_current_warning_port ()); > > - scm_puts ("\n;;; newer than compiled ", scm_current_warning_port ()); > > - scm_display (compiled_filename, scm_current_warning_port ()); > > - scm_puts ("\n", scm_current_warning_port ()); > > + if (scm_is_false (*scm_loc_quiet)) { > > + scm_puts (";;; note: source file ", scm_current_warning_port ()); > > + scm_display (full_filename, scm_current_warning_port ()); > > + scm_puts ("\n;;; newer than compiled ", scm_current_warning_port ()); > > + scm_display (compiled_filename, scm_current_warning_port ()); > > + scm_puts ("\n", scm_current_warning_port ()); > > + } > > } > > Please resend without tabs, please. Thanks :) I'm not sure about this one. I manually put spaces in front of each line as directed but it does not show up as a change in 'git diff'. Is the rest of the patch correct in regards to Guile project tab/space guidelines. (I couldn't find any such guideline, is there some specific Emacs setup I should be using for Guile development?) > > > > > > > +;;; Don't know where to put these: > > +(define (cond_warn_compiling file) > > + (unless %quiet > > + (format (current-warning-port) ";;; compiling ~a\n" file))) > > + > > +(define (cond_warn_compiled file) > > + (unless %quiet > > + (format (current-warning-port) ";;; compiled ~a\n" file))) > > + > > +(define (cond_warn_newer new old) > > + (unless %quiet > > + (format (current-warning-port) > > + ";;; note: source file ~a\n;;; newer than compiled ~a\n" > > + name go-file-name))) > > These definitions are inappropriate for being at the top-level of > boot-9.scm -- they would become automatically a part of all > environments. I would leave out this change. Fixed. I have put these definitions at the top of the procedure that calls them, does that fit in with the rest of the code ok? > Note also that the Guile convention is hyphen-case not snake_case. My bad, amateur mistake. > > > diff --git a/module/ice-9/command-line.scm b/module/ice-9/command-line.scm > > index 98d3855..9a3f7e1 100644 > > --- a/module/ice-9/command-line.scm > > +++ b/module/ice-9/command-line.scm > > @@ -136,6 +136,7 @@ If FILE begins with `-' the -s switch is mandatory. > > --listen[=P] listen on a local port or a path for REPL clients; > > if P is not given, the default is local port 37146 > > -q inhibit loading of user init file > > + --quiet inhibit compile and load messages > > --use-srfi=LS load SRFI modules for the SRFIs in LS, > > which is a list of numbers like \"2,13,14\" > > -h, --help display this help and exit > > What do people think about the name of this command line option? > > To me "inhibit compile and load messages" does not quite capture what it > does. But maybe it is good enough. I've changed it to; --quiet inhibit informational compiler output, e.g name of files being compiled Open to suggestions still. Unlike most programmers I find documenting my work harder than writing the code :) > Incidentally this change will need a documentation update as well. I > often find that adding documentation makes it more clear how things > should be named -- that the process of explaining things makes it > apparent what things should be changed :) > > > + ((string=? arg "--quiet") > > + (set! %quiet #t) > > + (parse args out)) > > + > > Probably %quiet should be a parameter, and unless we plan on using it > for other purposes it should have a more specific name > (%verbose-auto-compilation-messages or so; no idea). Could you clarify please, a parameter to what? > > Or, you could always just (current-warning-port (%make-void-port "w")); > then the patch is super simple, no conditions needed. That is what I > was originally thinking, but it has the disadvantage that you lose other > uses of the current warning port, but maybe that's an advantage too. Won't we loose compiler warnings then? I did not want to inhibit those. thanks, Tobin. reworked patch follows :) --- libguile/load.c | 34 ++++++++++++++++++++++------------ module/ice-9/boot-9.scm | 22 +++++++++++++++++----- module/ice-9/command-line.scm | 6 ++++++ 3 files changed, 45 insertions(+), 17 deletions(-) diff --git a/libguile/load.c b/libguile/load.c index 7ad9a75..e2d6c4a 100644 --- a/libguile/load.c +++ b/libguile/load.c @@ -226,6 +226,9 @@ static SCM *scm_loc_fresh_auto_compile; /* The fallback path for auto-compilation */ static SCM *scm_loc_compile_fallback_path; +/* Whether we should output compiler informational messages */ +static SCM *scm_loc_verbose_compiler_messages; + /* Ellipsis: "..." */ static SCM scm_ellipsis; @@ -561,11 +564,13 @@ compiled_is_fresh (SCM full_filename, SCM compiled_filename, else { compiled_is_newer = 0; - scm_puts (";;; note: source file ", scm_current_warning_port ()); - scm_display (full_filename, scm_current_warning_port ()); - scm_puts ("\n;;; newer than compiled ", scm_current_warning_port ()); - scm_display (compiled_filename, scm_current_warning_port ()); - scm_puts ("\n", scm_current_warning_port ()); + if (scm_is_true (*scm_loc_verbose_compiler_messages)) { + scm_puts (";;; note: source file ", scm_current_warning_port ()); + scm_display (full_filename, scm_current_warning_port ()); + scm_puts ("\n;;; newer than compiled ", scm_current_warning_port ()); + scm_display (compiled_filename, scm_current_warning_port ()); + scm_puts ("\n", scm_current_warning_port ()); + } } return compiled_is_newer; @@ -1006,10 +1011,11 @@ do_try_auto_compile (void *data) { SCM source = SCM_PACK_POINTER (data); SCM comp_mod, compile_file; - - scm_puts (";;; compiling ", scm_current_warning_port ()); - scm_display (source, scm_current_warning_port ()); - scm_newline (scm_current_warning_port ()); + if (scm_is_true (*scm_loc_verbose_compiler_messages)) { + scm_puts (";;; compiling ", scm_current_warning_port ()); + scm_display (source, scm_current_warning_port ()); + scm_newline (scm_current_warning_port ()); + } comp_mod = scm_c_resolve_module ("system base compile"); compile_file = scm_module_variable (comp_mod, sym_compile_file); @@ -1036,9 +1042,11 @@ do_try_auto_compile (void *data) /* Assume `*current-warning-prefix*' has an appropriate value. */ res = scm_call_n (scm_variable_ref (compile_file), args, 5); - scm_puts (";;; compiled ", scm_current_warning_port ()); - scm_display (res, scm_current_warning_port ()); - scm_newline (scm_current_warning_port ()); + if (scm_is_true (*scm_loc_verbose_compiler_messages)) { + scm_puts (";;; compiled ", scm_current_warning_port ()); + scm_display (res, scm_current_warning_port ()); + scm_newline (scm_current_warning_port ()); + } return res; } else @@ -1349,6 +1357,8 @@ scm_init_load () = SCM_VARIABLE_LOC (scm_c_define ("%load-should-auto-compile", SCM_BOOL_F)); scm_loc_fresh_auto_compile = SCM_VARIABLE_LOC (scm_c_define ("%fresh-auto-compile", SCM_BOOL_F)); + scm_loc_verbose_compiler_messages + = SCM_VARIABLE_LOC (scm_c_define ("%verbose-compiler-messages", SCM_BOOL_T)); scm_ellipsis = scm_from_latin1_string ("..."); diff --git a/module/ice-9/boot-9.scm b/module/ice-9/boot-9.scm index 99543e7..018ca6a 100644 --- a/module/ice-9/boot-9.scm +++ b/module/ice-9/boot-9.scm @@ -3762,6 +3762,20 @@ when none is available, reading FILE-NAME with READER." ;; Return GO-FILE-NAME after making sure that it contains a freshly ;; compiled version of source file NAME with stat SCMSTAT; return #f ;; on failure. + (define (cond-warn-compiling file) + (if %verbose-compiler-messages + (format (current-warning-port) ";;; compiling ~a\n" file))) + + (define (cond-warn-compiled file) + (if %verbose-compiler-messages + (format (current-warning-port) ";;; compiled ~a\n" file))) + + (define (cond-warn-newer new old) + (if %verbose-compiler-messages + (format (current-warning-port) + ";;; note: source file ~a\n;;; newer than compiled ~a\n" + name go-file-name))) + (false-if-exception (let ((gostat (and (not %fresh-auto-compile) (stat go-file-name #f)))) @@ -3769,15 +3783,13 @@ when none is available, reading FILE-NAME with READER." (load-thunk-from-file go-file-name) (begin (when gostat - (format (current-warning-port) - ";;; note: source file ~a\n;;; newer than compiled ~a\n" - name go-file-name)) + (cond-warn-newer name go-file-name) (cond (%load-should-auto-compile (%warn-auto-compilation-enabled) - (format (current-warning-port) ";;; compiling ~a\n" name) + (cond-warn-compiling name) (let ((cfn (compile name))) - (format (current-warning-port) ";;; compiled ~a\n" cfn) + (cond-warn-compiled cfn) (load-thunk-from-file cfn))) (else #f))))) #:warning "WARNING: compilation of ~a failed:\n" name)) diff --git a/module/ice-9/command-line.scm b/module/ice-9/command-line.scm index 98d3855..fe639bb 100644 --- a/module/ice-9/command-line.scm +++ b/module/ice-9/command-line.scm @@ -136,6 +136,8 @@ If FILE begins with `-' the -s switch is mandatory. --listen[=P] listen on a local port or a path for REPL clients; if P is not given, the default is local port 37146 -q inhibit loading of user init file + --quiet inhibit informational compiler output, e.g name of + files being compiled --use-srfi=LS load SRFI modules for the SRFIs in LS, which is a list of numbers like \"2,13,14\" -h, --help display this help and exit @@ -358,6 +360,10 @@ If FILE begins with `-' the -s switch is mandatory. (set! inhibit-user-init? #t) (parse args out)) + ((string=? arg "--quiet") + (set! %verbose-compiler-messages #f) + (parse args out)) + ((string-prefix? "--use-srfi=" arg) (let ((srfis (map (lambda (x) (let ((n (string->number x))) -- 2.8.3