[PATCH] Declare `GC_dump' ourselves if <gc/gc.h> doesn't.

[PATCH] Declare `GC_dump' ourselves if <gc/gc.h> doesn't.

[Thien-Thi Nguyen]

This patch removes a harmless but annoying warning.

thi

___________________________________________________________________________
From 39fd870610ff2cc1177e0cd0aba845806f0213c6 Mon Sep 17 00:00:00 2001
From: Thien-Thi Nguyen <ttn@gnuvola.org>
Date: Sat, 9 Jan 2010 14:17:20 +0100
Subject: [PATCH] Declare `GC_dump' ourselves if <gc/gc.h> doesn't.

* configure.ac: Check for `GC_dump' declaration.
Define `HAVE_DECL_GC_DUMP' if <gc/gc.h> provides it.
* libguile/gc.c [!HAVE_DECL_GC_DUMP] (GC_dump): Declare it.

Signed-off-by: Thien-Thi Nguyen <ttn@gnuvola.org>
---
 configure.ac  |    8 ++++++++
 libguile/gc.c |    4 ++++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/configure.ac b/configure.ac
index 5143dcc..78c6418 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1227,6 +1227,14 @@ PKG_CHECK_MODULES([BDW_GC], [bdw-gc])
 CFLAGS="$BDW_GC_CFLAGS $CFLAGS"
 LIBS="$BDW_GC_LIBS $LIBS"
 
+# `GC_dump' is available in GC 6.8 but not declared.
+AC_CHECK_FUNCS([GC_dump])
+AC_CHECK_DECL([GC_dump],
+  [AC_DEFINE([HAVE_DECL_GC_DUMP], [1],
+    [Define this if the `GC_dump' function is declared])],
+  [],
+  [#include <gc/gc.h>])
+
 # `GC_do_blocking ()' is available in GC 7.1 but not declared.
 AC_CHECK_FUNCS([GC_do_blocking])
 AC_CHECK_DECL([GC_do_blocking],
diff --git a/libguile/gc.c b/libguile/gc.c
index 700f3a4..295b663 100644
--- a/libguile/gc.c
+++ b/libguile/gc.c
@@ -69,6 +69,10 @@ extern unsigned long * __libc_ia64_register_backing_store_base;
 #include <unistd.h>
 #endif
 
+#ifndef HAVE_DECL_GC_DUMP
+extern void GC_dump (void);
+#endif
+
 /* Lock this mutex before doing lazy sweeping.
  */
 scm_i_pthread_mutex_t scm_i_sweep_mutex = SCM_I_PTHREAD_MUTEX_INITIALIZER;
-- 
1.6.3.2

Re: [PATCH] Declare `GC_dump' ourselves if <gc/gc.h> doesn't.

[Andy Wingo]

On Sat 09 Jan 2010 14:28, Thien-Thi Nguyen <ttn@gnuvola.org> writes:

> +# `GC_dump' is available in GC 6.8 but not declared.

Neil, you also compile with pre-7.x, no? Do we need to support this?

Just wondering if it wouldn't be easier to simply require 7.x without
workarounds. It seems debian is the sticker here..

Andy
-- 
http://wingolog.org/

Re: [PATCH] Declare `GC_dump' ourselves if <gc/gc.h> doesn't.

[Ken Raeburn]

On Jan 9, 2010, at 17:30, Andy Wingo wrote:
> On Sat 09 Jan 2010 14:28, Thien-Thi Nguyen <ttn@gnuvola.org> writes:
>
>> +# `GC_dump' is available in GC 6.8 but not declared.
>
> Neil, you also compile with pre-7.x, no? Do we need to support this?
>
> Just wondering if it wouldn't be easier to simply require 7.x without
> workarounds. It seems debian is the sticker here..

Given how much everyone seems to keep saying that you should really  
use 7.x, I think it would be a good idea for configure and/or some  
file including <gc/gc.h> to check the version (if possible), and  
reject 6.x.  Unless guile *will* work reliably with 6.8, in which case  
people should stop saying such things. :-)

Ken

P.S.  I'd offer to write up a patch, but I'm in that annoying in- 
between state of having a new job (yay!), but not having disclaimer  
paperwork in to the FSF yet (boo!)...

Re: [PATCH] Declare `GC_dump' ourselves if <gc/gc.h> doesn't.

[Ludovic Courtès]

Hi,

Andy Wingo <wingo@pobox.com> writes:

> On Sat 09 Jan 2010 14:28, Thien-Thi Nguyen <ttn@gnuvola.org> writes:
>
>> +# `GC_dump' is available in GC 6.8 but not declared.
>
> Neil, you also compile with pre-7.x, no? Do we need to support this?

No.  Normally checking for bdw-gc.pc rules out 6.8, but apparently
Debian (?) is shipping the .pc file for 6.8 too.

Guile doesn’t requires 7.x (the ‘README’ file says so.)

Thanks,
Ludo’.

Re: [PATCH] Declare `GC_dump' ourselves if <gc/gc.h> doesn't.

[Ludovic Courtès]

ludo@gnu.org (Ludovic Courtès) writes:

> Guile doesn’t requires 7.x (the ‘README’ file says so.)

I meant “Guile requires 7.x”.

Ludo’.

Re: [PATCH] Declare `GC_dump' ourselves if <gc/gc.h> doesn't.

[Thien-Thi Nguyen]

() ludo@gnu.org (Ludovic Courtès)
() Mon, 11 Jan 2010 23:32:57 +0100

   ludo@gnu.org (Ludovic Courtès) writes:

   > Guile doesn’t requires 7.x (the ‘README’ file says so.)

   I meant “Guile requires 7.x”.

Hmm, requiring versions is not the Autoconf Way.  What feature /
interface elements of libgc 7.x could we use in the configure
script to check for it?  If someone would tell me this (or point
me towards a 6 vs 7 comparison relevant to Guile), i will prepare
a new patch.

thi

Re: [PATCH] Declare `GC_dump' ourselves if <gc/gc.h> doesn't.

[Thien-Thi Nguyen]

() ludo@gnu.org (Ludovic Courtès)
() Mon, 11 Jan 2010 23:09:15 +0100

   Andy Wingo <wingo@pobox.com> writes:

   > On Sat 09 Jan 2010 14:28, Thien-Thi Nguyen <ttn@gnuvola.org> writes:
   >
   >> +# `GC_dump' is available in GC 6.8 but not declared.
   >
   > Neil, you also compile with pre-7.x, no? Do we need to support this?

   No.  Normally checking for bdw-gc.pc rules out 6.8, but apparently
   Debian (?) is shipping the .pc file for 6.8 too.

Debian Etch doesn't ship with a .pc file (AFAICT).
I infer the version from the output of

  M-x apt-utils-show-package RET libgc-dev RET

reproduced below.  Am i missing something?

thi

___________________________________________________________
Package: libgc-dev (Installed)
Priority: optional
Section: libdevel
Installed-Size: 516
Maintainer: Ryan Murray <rmurray@debian.org>
Architecture: i386
Source: libgc
Version: 1:6.8-1
Depends: libgc1c2 (= 1:6.8-1), libc-dev
Filename: pool/main/libg/libgc/libgc-dev_6.8-1_i386.deb
Size: 160808
MD5sum: 8fd9c6b5451059499b79c4334e907c87
SHA1: bc60de45dbd3b5ffff06497ec6a697e959ea6858
SHA256: e7cbb58a14758a914168fa617e5f3b0a46f9f703e481a00d805047862c98663f
Description: conservative garbage collector for C (development)
 Boehm's GC is a garbage collecting storage allocator that is
 intended to be used as a plug-in replacement for C's malloc.
 This package is required to compile and link programs that use
 libgc1.
 .
 Since the collector does not require pointers to be tagged, it does
 not attempt to ensure that all inaccessible storage is reclaimed.
 However it has typically been more successful at reclaiming unused
 memory than most C programs using explicit deallocation.  Unlike
 manually introduced leaks, the amount of unreclaimed memory typically
 stays bounded.
 .
 This version of the collector is thread safe, has C++ support, and uses the
 defaults for everything else.  Particularly, it does not work as a malloc()
 replacement.

Re: [PATCH] Declare `GC_dump' ourselves if <gc/gc.h> doesn't.

[Ludovic Courtès]

Hi,

Thien-Thi Nguyen <ttn@gnuvola.org> writes:

> () ludo@gnu.org (Ludovic Courtès)
> () Mon, 11 Jan 2010 23:09:15 +0100

[...]

>    No.  Normally checking for bdw-gc.pc rules out 6.8, but apparently
>    Debian (?) is shipping the .pc file for 6.8 too.
>
> Debian Etch doesn't ship with a .pc file (AFAICT).

So how did ‘configure’ find your libgc?

My ‘config.log’ reads this:

--8<---------------cut here---------------start------------->8---
configure:30138: checking for BDW_GC
configure:30145: $PKG_CONFIG --exists --print-errors "bdw-gc"
configure:30148: $? = 0
configure:30161: $PKG_CONFIG --exists --print-errors "bdw-gc"
configure:30164: $? = 0
configure:30218: result: yes
--8<---------------cut here---------------end--------------->8---

Thanks,
Ludo’.

Re: [PATCH] Declare `GC_dump' ourselves if <gc/gc.h> doesn't.

[Thien-Thi Nguyen]

() ludo@gnu.org (Ludovic Courtès)
() Tue, 12 Jan 2010 15:17:06 +0100

   So how did ‘configure’ find your libgc?

I've modified configure.ac (see "Don't clobber..." patch) to look like:

| dnl See note for PKG_CHECK_MODULES in aclocal.m4.
| PKG_PROG_PKG_CONFIG
| if test "$BDW_GC_CFLAGS" || test "$BDW_GC_LIBS" ; then :
|   dnl We don't need to declare those env vars precious;
|   dnl PKG_CHECK_MODULES does that.
| else
|   PKG_CHECK_MODULES([BDW_GC], [bdw-gc])
| fi

Then, i call configure like so:

$ .../configure -C \
      --with-threads \
      --prefix /tmp/a/b/z \
      BDW_GC_LIBS='-lgc' \
      LDFLAGS='-L/home/ttn/local/lib'

(The `LDFLAGS' is incidentally for finding libltdl.)

thi

Re: [PATCH] Declare `GC_dump' ourselves if <gc/gc.h> doesn't.

[Ludovic Courtès]

Hi,

Thien-Thi Nguyen <ttn@gnuvola.org> writes:

> () ludo@gnu.org (Ludovic Courtès)
> () Tue, 12 Jan 2010 15:17:06 +0100
>
>    So how did ‘configure’ find your libgc?
>
> I've modified configure.ac (see "Don't clobber..." patch) to look like:
>
> | dnl See note for PKG_CHECK_MODULES in aclocal.m4.
> | PKG_PROG_PKG_CONFIG
> | if test "$BDW_GC_CFLAGS" || test "$BDW_GC_LIBS" ; then :
> |   dnl We don't need to declare those env vars precious;
> |   dnl PKG_CHECK_MODULES does that.
> | else
> |   PKG_CHECK_MODULES([BDW_GC], [bdw-gc])
> | fi

This patch shouldn’t be necessary since it duplicates what
‘PKG_CHECK_MODULES’ does.

> Then, i call configure like so:
>
> $ .../configure -C \
>       --with-threads \
>       --prefix /tmp/a/b/z \
>       BDW_GC_LIBS='-lgc' \

... without reading ‘README’, so you basically shoot yourself in the
foot and there’s not much we can do.  :-)

Thanks,
Ludo’.

Re: [PATCH] Declare `GC_dump' ourselves if <gc/gc.h> doesn't.

[Thien-Thi Nguyen]

() ludo@gnu.org (Ludovic Courtès)
() Tue, 12 Jan 2010 18:19:44 +0100

   > | dnl See note for PKG_CHECK_MODULES in aclocal.m4.
   > | PKG_PROG_PKG_CONFIG
   > | if test "$BDW_GC_CFLAGS" || test "$BDW_GC_LIBS" ; then :
   > |   dnl We don't need to declare those env vars precious;
   > |   dnl PKG_CHECK_MODULES does that.
   > | else
   > |   PKG_CHECK_MODULES([BDW_GC], [bdw-gc])
   > | fi

   This patch shouldn’t be necessary since it duplicates what
   ‘PKG_CHECK_MODULES’ does.

Not exactly.  If there is no bdw-gc.pc on the system (as is the
case here on a Debian Etch installation),  PKG_CHECK_MODULES dies,
and so does the configure script.  The original README recommends
in such case to munge three variables.  Two of them are valid:
BDW_GC_CFLAGS and BDW_GC_LIBS.  The other (PKG_CONFIG) is bogus
because munging it interferes with "make installcheck".

   ... without reading ‘README’, so you basically shoot yourself
   in the foot and there’s not much we can do.  :-)

I don't know where you got that idea.  Here's what actually
happened: I read the README, applied the bogus advice, noticed the
breakage, and then worked on the patch so that the now-updated
README gives good advice, supported by the now-updated configure
script.  Lastly, i posted the patch for review.

There IS something you can do: take a stand against pkg-config and
DTRT (the Autoconf Way).  I know there are other packages that are
heavily reliant on pkg-config, but Guile doesn't seem to be one of
them.  You can do this by helping identify what are the features
of libgc (whatever version) that Guile requires.  With that info,
it would not be hard to write a test for the configure script.
So, please help answer that question (if you can).

thi

Re: [PATCH] Declare `GC_dump' ourselves if <gc/gc.h> doesn't.

[Andy Wingo]

Hello,

On Tue 12 Jan 2010 19:51, Thien-Thi Nguyen <ttn@gnuvola.org> writes:

> There IS something you can do: take a stand against pkg-config and
> DTRT (the Autoconf Way).

FWIW...  I'm sympathetic to your desire to compile against 6.8, but once
7.x is more widely distributed, I think it's a win to just use
pkg-config. Less hassle in the configure.ac. I like pkg-config.

Andy
-- 
http://wingolog.org/

Re: [PATCH] Declare `GC_dump' ourselves if <gc/gc.h> doesn't.

[Andy Wingo]

Hi,

On Mon 11 Jan 2010 23:09, ludo@gnu.org (Ludovic Courtès) writes:

> Andy Wingo <wingo@pobox.com> writes:
>
>> On Sat 09 Jan 2010 14:28, Thien-Thi Nguyen <ttn@gnuvola.org> writes:
>>
>>> +# `GC_dump' is available in GC 6.8 but not declared.
>>
>> Neil, you also compile with pre-7.x, no? Do we need to support this?
>
> No.  Normally checking for bdw-gc.pc rules out 6.8, but apparently
> Debian (?) is shipping the .pc file for 6.8 too.

It seems many people have been compiling against 6.8, manually setting
CFLAGS &c, apparently without bad results.
>
> Guile [..] requires 7.x (the ‘README’ file says so.)

It also says this:

    Guile's ./configure script uses pkg-config to discover the correct
    compile and link options for libgc.  If you don't have pkg-config
    installed, or you have a version of libgc that doesn't provide a
    .pc file, you can work around this by setting some environment
    variables before running ./configure:

    - PKG_CONFIG=true

    - BDW_GC_CFLAGS=<compile flags for picking up libgc headers>

    - BDW_GC_LIBS=<linker flags for picking up the libgc library>

Ludovic, is there a reason to *require* 7.x, given that 6.8 seems to
work fine for people?

Andy
-- 
http://wingolog.org/

Re: [PATCH] Declare `GC_dump' ourselves if <gc/gc.h> doesn't.

[Ludovic Courtès]

Hi,

Andy Wingo <wingo@pobox.com> writes:

> Ludovic, is there a reason to *require* 7.x, given that 6.8 seems to
> work fine for people?

Is there a reason for wanting 6.8 *in addition* to 7.x given that 7.x
works fine for people?  :-)

See libguile/bdw-gc.h: indeed, there are bits and pieces to help
compatibility between 6 and 7.  It might work for 6.8, but it’s
basically untested.  And it’s a mess, because the libgc API changed in
subtle ways (e.g., see ‘scm_storage_prehistory’).  Also, 6.8 was slower,
the compile-time defaults were typically inappropriate, which would
degrade performance, and so on.

GC 7.0 was released in July 2007, and it’s an improvement.  I personally
don’t want to maintain backward compatibility at the cost of horrid
#ifdefs, deal with bug reports with the old libgc, etc. etc. just
because Debian has been lagging behind.

Thanks,
Ludo’.

Re: [PATCH] Declare `GC_dump' ourselves if <gc/gc.h> doesn't.

[Neil Jerram]

Andy Wingo <wingo@pobox.com> writes:

> On Sat 09 Jan 2010 14:28, Thien-Thi Nguyen <ttn@gnuvola.org> writes:
>
>> +# `GC_dump' is available in GC 6.8 but not declared.
>
> Neil, you also compile with pre-7.x, no? Do we need to support this?

No, I use 7.1.  I thought I recalled Ludo saying that we need at least
that, but I see from other messages in this thread that that
recollection is wrong.

     Neil