Re: Accept GUILE 2 without extra configure options (issue 569390043 by address@hidden)

[hanwenn]

https://codereview.appspot.com/569390043/diff/571710044/aclocal.m4
File aclocal.m4 (right):

https://codereview.appspot.com/569390043/diff/571710044/aclocal.m4#newcode693
aclocal.m4:693: if [[ -n "$GUILE_FLAVOR" ]] ; then
On 2020/02/21 17:42:00, hahnjo wrote:
> I first thought this was bash syntax and not portable, but m4
substitution comes
> into play here and removes one set of [ ]. Could we write this as
> test -n "$GUILE_FLAVOR"
> instead?

oh, of course

"[[" is recommended in the google bash style guide, for reasons that
have to do with (IIRC) wildcard expansion.

https://codereview.appspot.com/569390043/diff/571710044/aclocal.m4#newcode694
aclocal.m4:694: PKG_CHECK_MODULES(GUILE, $GUILE_FLAVOR, true, true)
On 2020/02/21 17:42:00, hahnjo wrote:
> Please escape all arguments with [ ... ], here and in other locations.
> 
> Additionally we should probably reset GUILE_FLAVOR in the false case
to trigger
> the error below.

done. Note that we don't use [ ] in most pkg_check_modules() calls
elsewhere.

https://codereview.appspot.com/569390043/diff/571710044/aclocal.m4#newcode697
aclocal.m4:697: if [[ -z "$GUILE_FLAVOR" ]] ; then
On 2020/02/21 17:48:54, hahnjo wrote:
> You can actually make this a little nicer by putting the next check in
the false
> branch.

Done.

https://codereview.appspot.com/569390043/diff/571710044/aclocal.m4#newcode697
aclocal.m4:697: if [[ -z "$GUILE_FLAVOR" ]] ; then
On 2020/02/21 17:48:54, hahnjo wrote:
> You can actually make this a little nicer by putting the next check in
the false
> branch.

Done.

https://codereview.appspot.com/569390043/diff/571710044/config.make.in
File config.make.in (right):

https://codereview.appspot.com/569390043/diff/571710044/config.make.in#newcode28
config.make.in:28: GUILE_LIBS = @GUILE_LIBS@ @GUILE_CFLAGS@
On 2020/02/21 17:42:00, hahnjo wrote:
> Can be $(GUILE_CFLAGS), already substituted above

Done.

https://codereview.appspot.com/569390043/