Re: [PATCH] Add to the 2.1.x branch GUILE_SITE_CCACHE_DIR and GUILE_EXTENSION_DIR Autoconf macros along with needed siteccachdir entry in pkgconfig file

[Freja Nordsiek]

I improved the commit log and the comments to make them better
formatted and more terse. Does the commit log look better?

I also made it so that pkgconfig is tried first for finding
site-ccache and only if that fails will the interpreter be used (the
messages say specifically which method they are using now). It is
definitely better to have both methods in there.


Freja Nordsiek

On Tue, Mar 14, 2017 at 3:53 PM, Andy Wingo <address@hidden> wrote:
> Hi :)
>
> Great patch, some comments.
>
> On Tue 14 Mar 2017 15:08, Freja Nordsiek <address@hidden> writes:
>
>> From 90daf796c829f8e422a281d501f711138f21a334 Mon Sep 17 00:00:00 2001
>> From: Freja Nordsiek <address@hidden>
>> Date: Tue, 14 Mar 2017 15:04:38 +0100
>> Subject: [PATCH] Made GUILE_SITE_DIR Autoconf macro look for directories for
>>  compiled .go and C extensions in addition to the site directory for scheme
>>  files.
>
> Please adapt the commit log.  (Just look at any other commit to see what
> the standard is.)  You might also be interested in "info standards".
>
>> -## GUILE_SITE_DIR -- find path to Guile "site" directory
>> +## GUILE_SITE_DIR -- find path to Guile "site" directories for scheme, 
>> compiles GO files, and compiled C extensions
>
> Line too long.  Probably just s/directories.*/directories./.
>
>> -# GUILE_SITE_DIR -- find path to Guile "site" directory
>> +# GUILE_SITE_DIR -- find path to Guile site directories
>
>> -# This looks for Guile's "site" directory, usually something like
>> -# PREFIX/share/guile/site, and sets var @var{GUILE_SITE} to the path.
>> -# Note that the var name is different from the macro name.
>> +# This looks for Guile's "site" directory for Scheme files (usually 
>> something like
>> +# PREFIX/share/guile/site), "site-ccache" directory for compiled @code{.go} 
>> files
>> +# (usually something like 
>> PREFIX/lib/guile/@var{GUILE_EFFECTIVE_VERSION}/site-ccache),
>> +# and "extensions" directory for compiled C extensions (usually something 
>> like
>> +# PREFIX/lib/guile/@var{GUILE_EFFECTIVE_VERSION}/extensions). The variables
>> +# @var{GUILE_SITE}, @var{GUILE_SITE_CCACHE}, and @var{GUILE_EXTENSION} are 
>> set to these
>> +# paths respectively. The latter two are set to blank if they are not 
>> found. Note that
>> +# this macro will run the macros @code{GUILE_PKG} and @code{GUILE_PROGS} if 
>> they have
>> +# not already been run.
>
> Can we make the text more terse?  E.g. 'This looks for Guile's "site"
> directories.  The variable @var{GUILE_SITE} will be set to Guile's
> "site" directory for Scheme source files (usually [...]).
> @var{GUILE_SITE_CCACHE} will be set to the directory for compiled Scheme
> files (usually [...])' and so on.  Two spaces before periods please in
> comments in Guile code.
>
>> -# The variable is marked for substitution, as by @code{AC_SUBST}.
>> +# The variables are marked for substitution, as by @code{AC_SUBST}.
>>  #
>>  AC_DEFUN([GUILE_SITE_DIR],
>>   [AC_REQUIRE([GUILE_PKG])
>> +  AC_REQUIRE([GUILE_PROGS])
>
> I guess this is OK given that anyone installing Scheme files should
> install .go files, and you need GUILE_PROGS to build .go files.
>
>> +  AC_MSG_CHECKING([for Guile site-ccache directory])
>> +  GUILE_SITE_CCACHE=`$GUILE -c "(display (if (defined? '%site-ccache-dir) 
>> (%site-ccache-dir) \"\"))"`
>
> You prefer this rather than first trying pkg-config?  I would try
> pkg-config first; but it doesn't really matter I guess :)
>
> Otherwise looking fine.  Thanks!
>
> Andy

0001-GUILE_SITE_DIR-Find-site-directories-for-compiled-fi.patch
Description: Text Data