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

[Andy Wingo]

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