Hi :)
Great patch, some comments.
On Tue 14 Mar 2017 15:08, Freja Nordsiek <fnord...@gmail.com> writes:
> From 90daf796c829f8e422a281d501f711138f21a334 Mon Sep 17 00:00:00 2001
> From: Freja Nordsiek <fnord...@gmail.com>
> 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
