[Gc] Dependency tracking for configuration macros

Hi!

Post by Petter Urkedal
The attached patch adds AC_CONFIG_HEADER to configure.ac, so that the
command-line "-D"-options are replace by the conditional inclusion of a
generated config.h header. The major part of the patch is the addition
of documentation to configuration macros.

Opinion:
1. instead of inclusion of "gc_config_macros.h" from misc .c files (like backgraph.c one), I'd rather include "gc.h" - see my patch in http://permalink.gmane.org/gmane.comp.programming.garbage-collection.boehmgc/2575 (mostly taken from GCC/boehm-gc).

2. Inclusion of a "private/..." file from a public header ("gc_config_macros.h") is not a good idea (even if it is ifdef'ed), I think.

Post by Petter Urkedal
I tested the patch on various configurations on Linux x86_64 and i386 by
diff-ing the disassembly of the object files before and after applying
the patch.
-----%<-----
Dependency tracking for configuration macros.
* Invoke AC_CONFIG_HEADER.
* Added documentation of configuration macros.
* Commented out the unused macros STACKBASE and DATASTART_IS_ETEXT.

STACKBASE should be also removed in misc.c file.

Post by Petter Urkedal
* Moved include paths to Makefile.am.
* Added include path for build directory.
* Conditionally include "private/config.h".
include/gc.h, include/private/gc_pmark.h,
* Minor adjustments to make sure that the config macros are
defined before use.

Bye.

Petter Urkedal

2009-05-20 20:29:58 UTC

1. instead of inclusion of "gc_config_macros.h" from misc .c files (like backgraph.c one), I'd rather include "gc.h" - see my patch in http://permalink.gmane.org/gmane.comp.programming.garbage-collection.boehmgc/2575 (mostly taken from GCC/boehm-gc).

Yes, I don't mind that. If that patch is to be committed, I could wait
and rebase my changes. (Hans?)

Post by Ivan Maidanski
2. Inclusion of a "private/..." file from a public header ("gc_config_macros.h") is not a good idea (even if it is ifdef'ed), I think.

I principal I agree, but the problem is that "gc.h" includes
"gc_version.h", and the latter file must be included after the Autoconf
macros, if they are to be included. This is the reason I felt it
necessary to add GC_PRIVATE_CONFIG_H as a supplement to the standard
CONFIG_H definition. I consider the ifdef-guard safe since
GC_PRIVATE_CONFIG_H is in the "namespace" of the collector, but granted,
it looks a bit ugly to an end-user who reads the include file.

On a related note, it might also be of interest to make configuration
macros available from public headers. In my own code I usually
transform the config.h by adding a project-specific prefix to all macros
which do not already start with said prefix.

STACKBASE should be also removed in misc.c file.

Yes, I was puzzled by that lone #undef...

Ivan Maidanski

2009-05-21 11:25:51 UTC

Hi!

Yes, I don't mind that. If that patch is to be committed, I could wait
and rebase my changes. (Hans?)

Post by Ivan Maidanski
2. Inclusion of a "private/..." file from a public header ("gc_config_macros.h") is not a good idea (even if it is ifdef'ed), I think.

Another way:
1. include "private/gc_priv.h" at the beginning of backgraph.c, checksums.c, gcj_mlc.c, real_malloc.c and gc_pmark.h;
2. include "gc_version.h" and configuration file (ifdef'ed) at the beginning of "gc_priv.h" (before including "gc.h").
3. I'd like also to see #define GC_BUILD (ifndef'ed) in gc_priv.h after config file included.

What do You think?

Post by Petter Urkedal
On a related note, it might also be of interest to make configuration
macros available from public headers. In my own code I usually
transform the config.h by adding a project-specific prefix to all macros
which do not already start with said prefix.

May be. But how many macros could be there? GC_THREADS, GC_DLL (Win32 only), GC_USE_LD_WRAP (Unix only), the rest are of rare use.

The other Q: are You going to make this 'public' config auto-gen'ed by GC too? Otherwise, it's not clear how this would be looking in the client code:

#include "my_app_config.h"
#define I_HAVE_CUSTOM_GC_API_CONFIG /* looks a bit ugly here */
#include "gc.h"

If we move "define I_HAVE_..." to my_app_config then the Q: isn't that easy to just have "include my_gc_config.h" in it ("my_app_config.h") instead (or directly have "define GC_THREADS"... unless you have a standalone cfg script for the gc API in your app)?

STACKBASE should be also removed in misc.c file.

Yes, I was puzzled by that lone #undef...

Bye.

Petter Urkedal

2009-05-21 14:04:42 UTC

Post by Ivan Maidanski
2. Inclusion of a "private/..." file from a public header ("gc_config_macros.h") is not a good idea (even if it is ifdef'ed), I think.

1. include "private/gc_priv.h" at the beginning of backgraph.c, checksums.c, gcj_mlc.c, real_malloc.c and gc_pmark.h;
2. include "gc_version.h" and configuration file (ifdef'ed) at the beginning of "gc_priv.h" (before including "gc.h").

I don't have a strong opinion here. We avoid littering gc.h with the
conditional macro inclusion, but at the price of making the order of
inclusion significant.

Post by Ivan Maidanski
3. I'd like also to see #define GC_BUILD (ifndef'ed) in gc_priv.h after config file included.
What do You think?

Even though gc_priv.h and gc_pmark.h are private, there may be cases
where client code includes them, esp to customize the marking process
for specific runtime environments. So, I'd leave this one for the build
to #define.

(A small refinement is to set "libgc_la_CFLAGS = -DGC_BUILD" in
Makefile.am, since the option applies exactly when building libgc.la.
The other builds must be adjusted analogously before we can get rid of
the #undef GC_BUILD in test.c.)

May be. But how many macros could be there? GC_THREADS, GC_DLL (Win32 only), GC_USE_LD_WRAP (Unix only), the rest are of rare use.

I probably shouldn't have brought this up. The config.h macros can be
useful when inlined code or type definitions depend on system-specific
features or feature/tuning parameters decided by ./configure. However,
the libgc public headers use system/compiler-specific predefined
constants rather than feature-tests. Since also the main GC API has
little inlining and few datastructures, config.h-dependencies seems to
be avoidable.

Post by Ivan Maidanski
#include "my_app_config.h"
#define I_HAVE_CUSTOM_GC_API_CONFIG /* looks a bit ugly here */
#include "gc.h"
If we move "define I_HAVE_..." to my_app_config then the Q: isn't that easy to just have "include my_gc_config.h" in it ("my_app_config.h") instead (or directly have "define GC_THREADS"... unless you have a standalone cfg script for the gc API in your app)?

I'm not sure whether I understood the question, but config.h should only
contain definitions which are decided once and for all by ./configure.
If there are feature-macros which the client can #define to tweak the
API, then they don't belong in config.h.

Petter

Ivan Maidanski

2009-05-21 16:03:26 UTC

Hi!

...
I probably shouldn't have brought this up. The config.h macros can be useful when inlined code or type definitions depend on system-specific features or feature/tuning parameters decided by ./configure. However,

the libgc public headers use system/compiler-specific predefined
constants rather than feature-tests. Since also the main GC API has
little inlining and few datastructures, config.h-dependencies seems to
be avoidable.

Miscommunicating a bit...
The above code is for gc.h (or gc_config_macros.h) containing (as I understood what You suggest):

#if defined(I_HAVE_CUSTOM_GC_API_CONFIG)
/* define it (before including gc.h or friends) if your app has this config file */
#include "gc_api_config.h"
#endif

As You wrote above "config.h-dependencies seems to be avoidable", the question is closed (again, if I understood that right).

Petter

Bye.

Petter Urkedal

2009-05-21 16:23:39 UTC

Miscommunicating a bit...
#if defined(I_HAVE_CUSTOM_GC_API_CONFIG)
/* define it (before including gc.h or friends) if your app has this config file */
#include "gc_api_config.h"
#endif

If API depended on it, we'd probably want to manifest the configuration
as an installed header, independent of how the collector was built.

Post by Ivan Maidanski
As You wrote above "config.h-dependencies seems to be avoidable", the question is closed (again, if I understood that right).

Yes, case closed.

Petter

Ivan Maidanski

2009-09-10 22:36:35 UTC

Hi!

....

Petter, could you repost all your ideas and pending suggested patches regarding build scripts (including for acinclude.m4) having today's CVS shapshot? (It's now possible to have config.h at GC build.) It might be good to freshen some scripts now.

Bye.

Petter Urkedal

2009-09-11 21:54:43 UTC

Post by Ivan Maidanski
Petter, could you repost all your ideas and pending suggested patches regarding build scripts (including for acinclude.m4) having today's CVS shapshot? (It's now possible to have config.h at GC build.) It might be good to freshen some scripts now.

I can see you already added the "#include"s, so the new patch mostly
affects the build files. I put config.h under private since that's how
they're used.

2009-09-12 Petter Urkedal <***@nbi.dk>
Generate and use config.h instead of command line -D options.

* configure.ac: Move include flag from ${INCLUDE} ...
* Makefile.am: ... to AM_CPPFLAGS and also add the build directory.

* configure.ac: Call AM_CONFIG_HEADER([include/private/config.h]).
* configure.ac: Add documentation to all AC_DEFINE either directly
or using AH_TEMPLATE.
* include/private/gc_priv.h, include/private/gc_pmark.h, gc_cpp.cc,
tests/test.c: Change include of config.h to private/config.h.
* tests/test_cpp.cc: Include private/config.h.

Ivan Maidanski

2009-09-12 08:31:07 UTC

Hi!

I can see you already added the "#include"s, so the new patch mostly
affects the build files. I put config.h under private since that's how
they're used.

My considerations below are for .c/h files only (the script changes are for review by Hans).

1. I don't think "config.h" -> "private/config.h" change is needed for gc_priv.h and gc_pmark.h (unless you are having a broken compiler like old BCC32).

2. I'll post a standalone patch dealing with "config.h" -> "private/config.h" change in gc_cpp.cc, tests/test.c and include "private/config.h" addition in test_cpp.cc (I also add #undef GC_BUILD to it like in test.c).

Post by Petter Urkedal
Generate and use config.h instead of command line -D options.
* configure.ac: Move include flag from ${INCLUDE} ...
* Makefile.am: ... to AM_CPPFLAGS and also add the build directory.
* configure.ac: Call AM_CONFIG_HEADER([include/private/config.h]).
* configure.ac: Add documentation to all AC_DEFINE either directly
or using AH_TEMPLATE.
* include/private/gc_priv.h, include/private/gc_pmark.h, gc_cpp.cc,
tests/test.c: Change include of config.h to private/config.h.
* tests/test_cpp.cc: Include private/config.h.
...

Bye.

Petter Urkedal

2009-09-12 08:55:45 UTC

I can see you already added the "#include"s, so the new patch mostly
affects the build files. I put config.h under private since that's how
they're used.

It is needed when not building from source directory.

Post by Ivan Maidanski
2. I'll post a standalone patch dealing with "config.h" -> "private/config.h" change in gc_cpp.cc, tests/test.c and include "private/config.h" addition in test_cpp.cc (I also add #undef GC_BUILD to it like in test.c).

Okay, I split up the changes at
http://git.eideticdew.org/cgit/~urkedal/bdwgc/log/?h=t-config_h which
contains (attached)

Patch 1

Fix/add inclusion of private/config.h.
* include/private/gc_priv.h, include/private/gc_pmark.h, gc_cpp.cc,
tests/test.c: Change include of config.h to private/config.h.
* tests/test_cpp.cc: Include private/config.h.

Patch 2

Generate and use config.h instead of -D options.
* configure.ac: Move include flag from ${INCLUDE} ...
* Makefile.am: ... to AM_CPPFLAGS and also add the build directory.

* configure.ac: Call AM_CONFIG_HEADER([include/private/config.h]).
* configure.ac: Add documentation to all AC_DEFINE either directly
or using AH_TEMPLATE.

Ivan Maidanski

2009-09-12 09:38:06 UTC

Hi!

I can see you already added the "#include"s, so the new patch mostly
affects the build files. I put config.h under private since that's how
they're used.

It is needed when not building from source directory.

Ok (but I think "#undef GC_BUILD" should be added after including config.h in test_cpp.cc (as in test.c)). I'll commit patch 1 in several days if Hans don't mind.

Bye.

Petter Urkedal

2009-09-12 12:42:46 UTC

Post by Petter Urkedal
Patch 1
Fix/add inclusion of private/config.h.
* include/private/gc_priv.h, include/private/gc_pmark.h, gc_cpp.cc,
tests/test.c: Change include of config.h to private/config.h.
* tests/test_cpp.cc: Include private/config.h.
...

Ok (but I think "#undef GC_BUILD" should be added after including
config.h in test_cpp.cc (as in test.c)). I'll commit patch 1 in
several days if Hans don't mind.

Thanks. I don't think #undef GC_BUILD is necessary on behalf of
config.h, since GC_BUILD is not a configuration macro. As far as I
understand the use of GC_BUILD, it indicates that the libgc library is
being built, and this is only known by the build system, not by
configure.

Ivan Maidanski

2009-09-12 13:51:34 UTC

Hi!

Ok (but I think "#undef GC_BUILD" should be added after including
config.h in test_cpp.cc (as in test.c)). I'll commit patch 1 in
several days if Hans don't mind.

Yes GC_BUILD is not a configuration macro but: just to look/behave the same as test.c (GC_BUILD could be specified in the cmd-line or GC makefile (for backward compatibility) so it should be undefined in the test modules otherwise GC_API wouldn't turn into __dllimport for GC_DLL case).

Bye.

Ivan Maidanski

2009-09-16 12:13:03 UTC

Hi!

Post by Petter Urkedal
...

I've checked in this small one (with a minor correction: use "" instead of <> for private/config.h, and undef GC_BUILD in test_cpp for safety).

Bye.

Ivan Maidanski

2009-09-24 19:29:38 UTC

Hi!

Hans -

What do you think of this patch for Makefile.am by Petter Urkedal?

The description: http://permalink.gmane.org/gmane.comp.programming.garbage-collection.boehmgc/2662

The patch (only N2): http://permalink.gmane.org/gmane.comp.programming.garbage-collection.boehmgc/3147

Petter, could you repost all your ideas and pending suggested patches regarding build scripts (including for acinclude.m4) having today's CVS snapshot? (It's now possible to have config.h at GC build.) It might be good to freshen some scripts now.

...
Patch 2
Generate and use config.h instead of -D options.
* configure.ac: Move include flag from ${INCLUDE} ...
* Makefile.am: ... to AM_CPPFLAGS and also add the build directory.
* configure.ac: Call AM_CONFIG_HEADER([include/private/config.h]).
* configure.ac: Add documentation to all AC_DEFINE either directly
or using AH_TEMPLATE.
...
diff --git a/Makefile.am b/Makefile.am
index fb658e2..6fb4a47 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -32,6 +32,8 @@
# endif
SUBDIRS =
+AM_CPPFLAGS = -I$(top_builddir)/include -I$(top_srcdir)/include
+
# Initialize variables so that we can declare files locally.
EXTRA_DIST =
lib_LTLIBRARIES =
diff --git a/acinclude.m4 b/acinclude.m4
index 72602a0..070f85d 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -37,10 +37,13 @@ AC_DEFUN(GC_SET_VERSION, [
AC_MSG_ERROR([nonconforming PACKAGE_VERSION='$PACKAGE_VERSION'])
fi
- AC_DEFINE_UNQUOTED(GC_VERSION_MAJOR, $GC_VERSION_MAJOR)
- AC_DEFINE_UNQUOTED(GC_VERSION_MINOR, $GC_VERSION_MINOR)
+ AC_DEFINE_UNQUOTED([GC_VERSION_MAJOR], $GC_VERSION_MAJOR,
+ [The major version number of this GC release.])
+ AC_DEFINE_UNQUOTED([GC_VERSION_MINOR], $GC_VERSION_MINOR,
+ [The minor version number of this GC release.])
if test :$GC_ALPHA_VERSION: != :: ; then
- AC_DEFINE_UNQUOTED(GC_ALPHA_VERSION, $GC_ALPHA_VERSION)
+ AC_DEFINE_UNQUOTED([GC_ALPHA_VERSION], $GC_ALPHA_VERSION,
+ [The alpha version number, if applicable.])
fi
...

Bye.

Hans Boehm

2009-09-25 04:51:58 UTC

This looks fine to me. I suspect Petter knows appreciably more about
autotools than I do.

I did not test it. I just looked at the patch.

Hans

Post by Ivan Maidanski
Hi!
Hans -
What do you think of this patch for Makefile.am by Petter Urkedal?
The description: http://permalink.gmane.org/gmane.comp.programming.garbage-collection.boehmgc/2662
The patch (only N2): http://permalink.gmane.org/gmane.comp.programming.garbage-collection.boehmgc/3147

Bye.
_______________________________________________
Gc mailing list
http://www.hpl.hp.com/hosted/linux/mail-archives/gc/

Ivan Maidanski

2009-09-25 05:21:29 UTC

Hi!

Post by Hans Boehm
This looks fine to me. I suspect Petter knows appreciably more about
autotools than I do.
I did not test it. I just looked at the patch.
Hans

I checked in this one. Needs testing (since configure.ac has been changed recently).

...
Patch 2
...

Bye.

Petter Urkedal

2009-09-25 07:14:08 UTC

Post by Hans Boehm
This looks fine to me. I suspect Petter knows appreciably more about
autotools than I do.
I did not test it. I just looked at the patch.
Hans

I checked in this one. Needs testing (since configure.ac has been changed recently).

Thanks! Some minor updates:

* configure.ac: Add documentation for GC_THREADS and
EMPTY_GETENV_RESULTS.
* libatomic_ops-1.2/configure.ac: Request AM_PROG_CC_C_O due to compiler
flags on test case.

I'm omitting generated files, so this works after "autoreconf -vif".

Ivan Maidanski

2009-09-25 08:05:33 UTC

Hi!

Post by Hans Boehm
This looks fine to me. I suspect Petter knows appreciably more about
autotools than I do.
I did not test it. I just looked at the patch.
Hans

I checked in this one. Needs testing (since configure.ac has been changed recently).

* configure.ac: Add documentation for GC_THREADS and
EMPTY_GETENV_RESULTS.
* libatomic_ops-1.2/configure.ac: Request AM_PROG_CC_C_O due to compiler
flags on test case.

Checked in only the first part. Confused with AM_PROG_CC_C_O: in the other libatomic_ops files we are using "AM_PROG_CC". Why do we delete it in configure.ac?

Other questions:
1. Why are we using DARWIN_THREADS (and friends) in Makefile.am w/o GC_ (while having them prefixed in configure.ac)?
2. Is this realy needed: "EXTRA_DIST += libtool.m4"?

Post by Petter Urkedal
I'm omitting generated files, so this works after "autoreconf -vif".

Announce the full list (of the auto-generated files), please.

Hans -
I guess these ones should be regenerated soon. But what's the policy for presence of the auto-generated files in:
1. CVS?
2. in a tar-ball?
And, if Makefile should be present (in CVS/tarball), for which target it should be generated?

Bye.

Petter Urkedal

2009-09-25 17:12:10 UTC

Post by Ivan Maidanski
Checked in only the first part. Confused with AM_PROG_CC_C_O: in the
other libatomic_ops files we are using "AM_PROG_CC". Why do we delete
it in configure.ac?

AM_PROG_CC_C_O subsumes AM_PROG_CC. The former macro also checks that
the compiler supports -c -o OUTFILE options or equivalent, which is
required here because test_atomic.c is compiled into to different
objects (test_atomic.o and test_atomic_pthreads.o) using different
compiler flags. (I'm not sure if the macro has a workaround when -o is
unsupported.)

Post by Ivan Maidanski
1. Why are we using DARWIN_THREADS (and friends) in Makefile.am w/o GC_ (while having them prefixed in configure.ac)?

I guess you are referring to the "if" statements? These are different
things, and they are defined unprefixed in AM_CONDITIONAL. They are not
exported to the C code like AC_DEFINE or directly substituted elsewhere
like AC_SUBST. Therefore they don't escape the project build in any
other way than by the effect they have on the makefiles.

Post by Ivan Maidanski
2. Is this realy needed: "EXTRA_DIST += libtool.m4"?

I was going to suggest that we remove it along with the file itself.

Post by Petter Urkedal
I'm omitting generated files, so this works after "autoreconf -vif".

Announce the full list (of the auto-generated files), please.

Ivan Maidanski

2009-09-25 17:51:44 UTC

Hi!

Post by Ivan Maidanski
Checked in only the first part. Confused with AM_PROG_CC_C_O: in the
other libatomic_ops files we are using "AM_PROG_CC". Why do we delete
it in configure.ac?

Ok. Checked it in.

Post by Ivan Maidanski
1. Why are we using DARWIN_THREADS (and friends) in Makefile.am w/o GC_ (while having them prefixed in configure.ac)?

Post by Ivan Maidanski
2. Is this realy needed: "EXTRA_DIST += libtool.m4"?

I was going to suggest that we remove it along with the file itself.

Ok. Prepare the patch for it.

Post by Petter Urkedal
I'm omitting generated files, so this works after "autoreconf -vif".

Announce the full list (of the auto-generated files), please.

Boehm, Hans

2009-09-25 18:45:06 UTC

From: Ivan Maidanski
Sent: Friday, September 25, 2009 10:52 AM

Post by Ivan Maidanski
Hans -
I guess these ones should be regenerated soon. But what's

the policy

Post by Ivan Maidanski
1. CVS?
2. in a tar-ball?
And, if Makefile should be present (in CVS/tarball), for which
target it should be generated?

The files generated by autoreconf (thus libtool, autoconf,

automake)

are target independent. Target dependencies are detected

by ./configure.
End users

who build from the repo need the dev tools.)
About the Makefile in the toplevel directory: Looking at the diff,
this seems to be an outdated version of Makefile.direct.

In that case

I suggest removal. In any case it is overwritten by ./configure.

Let this be info for Hans...

I think the usual policy is to check in enough files that both:

1) Developers can regenerate everything that's not manually written, provided they install the appropriate autottools packages.

2) Users can type configure; make and have the collector build. In the case cp Makefile.direct Makefile; make should also work.

I think we want the latter to work from a direct cvs check-out as well, since we want users to be able to build the CVS version without installing autotools.

I was under the mistaken impression that Makefile was already gone. I'm fine with removing it. It used to be a copy of Makefile.direct, so a simple "make" would also work on many platforms. That could probably still work too, but then it should be a real copy of Makefile.direct, not an obsolete one. It has the down side that it's somewhat at odds with GNU conventions. (The fact that it's overwritten by configure seems OK, since it's easy to get back.)

Hans

Petter Urkedal

2009-09-25 19:52:57 UTC

Post by Boehm, Hans
1) Developers can regenerate everything that's not manually written, provided they install the appropriate autottools packages.
2) Users can type configure; make and have the collector build. In the case cp Makefile.direct Makefile; make should also work.

Or type "make -f Makefile.direct". I attach a documentation patch, but
you may want to rewrite it yourself.

Post by Boehm, Hans
I think we want the latter to work from a direct cvs check-out as well, since we want users to be able to build the CVS version without installing autotools.

Okay. In that case I think we need to keep most of the files. The
generated *.m4 files (aclocal.m4 and libtools.m4) could be removed
according to that policy, but I'm not sure what the common practice is.

Petter Urkedal

2009-09-25 19:37:54 UTC

Post by Ivan Maidanski
2. Is this realy needed: "EXTRA_DIST += libtool.m4"?

I was going to suggest that we remove it along with the file itself.

Ok. Prepare the patch for it.

Simple enough. Let's see what Hans says about keeping or removing the
file itself.

Boehm, Hans

2009-09-25 18:36:25 UTC

We should regenerate and check in configure whenever configure.ac is updated. I can do that later today, if it's difficult for you to do so. I quickly tried to generate a configure, but didn't get a working version.

One problem with the current tree appears to be a missing config.h.in in include/private. Autogen.sh currently doesn't run autoheader. Should it? We probably also need to check in the resulting config.h.in, so that a plain configure will work again. I tried that , but ended up with libtool version issues on my machine that will take a bit more time to deal with.

Hans

-----Original Message-----
Sent: Thursday, September 24, 2009 10:21 PM
Cc: Ludovic CourtХs; Petter Urkedal
Subject: Re[3]: [Gc]: Dependency tracking for configuration macros
Hi!

Post by Hans Boehm
This looks fine to me. I suspect Petter knows appreciably

more about

Post by Hans Boehm
autotools than I do.
I did not test it. I just looked at the patch.
Hans

I checked in this one. Needs testing (since configure.ac has
been changed recently).

Post by Ivan Maidanski
Hi!
Hans -
What do you think of this patch for Makefile.am by Petter Urkedal?

http://permalink.gmane.org/gmane.comp.programming.garbage-collection

Post by Ivan Maidanski
.boehmgc/2662

http://permalink.gmane.org/gmane.comp.programming.garbage-collection

Post by Ivan Maidanski
.boehmgc/3147

Post by Ivan Maidanski
Petter, could you repost all your ideas and pending

suggested patches regarding build scripts (including for
acinclude.m4) having today's CVS snapshot? (It's now possible
to have config.h at GC build.) It might be good to freshen
some scripts now.

Post by Petter Urkedal
...
Patch 2
...

Bye.
_______________________________________________
Gc mailing list
http://www.hpl.hp.com/hosted/linux/mail-archives/gc/

Petter Urkedal

2009-09-25 19:33:23 UTC

Post by Boehm, Hans
We should regenerate and check in configure whenever configure.ac is updated. I can do that later today, if it's difficult for you to do so. I quickly tried to generate a configure, but didn't get a working version.
One problem with the current tree appears to be a missing config.h.in in include/private. Autogen.sh currently doesn't run autoheader. Should it? We probably also need to check in the resulting config.h.in, so that a plain configure will work again. I tried that , but ended up with libtool version issues on my machine that will take a bit more time to deal with.

Yes, autogen.sh should run autoheader. I don't think we really need
autogen.sh any longer since autoreconf is part of the Autoconf package,
but in case you want to keep it, I attach a patch which

* autogen.sh: Add autoheader and reorder the other tools to match that
of autoreconf.

Did you remove libtool.m4 before attempting to regenerate? The file
cause plenty of warnings with recent autotools. I think the file can be
safely removed from CVS because it's already included into configure for
the end user, and a developer who has the toolchain is likely also to
have a copy of libtool.m4 available.

Ivan Maidanski

2009-09-25 20:20:18 UTC

Hi!

Post by Petter Urkedal
...
Yes, autogen.sh should run autoheader. I don't think we really need
autogen.sh any longer since autoreconf is part of the Autoconf package,
but in case you want to keep it, I attach a patch which
* autogen.sh: Add autoheader and reorder the other tools to match that
of autoreconf.
Did you remove libtool.m4 before attempting to regenerate? The file
cause plenty of warnings with recent autotools. I think the file can be
safely removed from CVS because it's already included into configure for
the end user, and a developer who has the toolchain is likely also to
have a copy of libtool.m4 available.

I've deleted .m4 files and run autotools on Linux.

autoreconf works correctly but configure failed with error "can't make a symbolic link" (that's because I've placed "bdwgc" on NTFS drive) and make, then, failed too.
Everything works correctly on a native Linux FS.

Should we fix this?

Bye.

Petter Urkedal

2009-09-25 20:31:50 UTC

Post by Ivan Maidanski
autoreconf works correctly but configure failed with error "can't make a symbolic link" (that's because I've placed "bdwgc" on NTFS drive) and make, then, failed too.
Everything works correctly on a native Linux FS.

Do you think that may be the libatomic_ops-related "ln" commands on
lines 718--729? If that's the only ones, we could probably fix it after
the version-number is dropped from the directory. (That is a pending
change as far as I remember.) For a workaround you could rename/copy
the libatomic_ops directory and copy the two files atomic_ops.c and
atomic_ops_sysdeps.S into the top-level directory, so that the "ln"
commands are skipped.

Ivan Maidanski

2009-09-26 06:10:53 UTC

Hi!

Post by Ivan Maidanski
autoreconf works correctly but configure failed with error "can't make a symbolic link" (that's because I've placed "bdwgc" on NTFS drive) and make, then, failed too.

I'm sorry, this was on FAT (which I use for RAM disk), NTFS should support "ln".

Post by Ivan Maidanski
Everything works correctly on a native Linux FS.

Do you think that may be the libatomic_ops-related "ln" commands on
lines 718--729?

Yes.

Post by Petter Urkedal
If that's the only ones, we could probably fix it after
the version-number is dropped from the directory. (That is a pending
change as far as I remember.) For a workaround you could rename/copy
the libatomic_ops directory and copy the two files atomic_ops.c and
atomic_ops_sysdeps.S into the top-level directory, so that the "ln"
commands are skipped.

As a workaround, I copied this to LinuxFS (so, this would require additional "cp" on each regenerate).

Post by Petter Urkedal
I'm with Petter on this. If it's easy, great.
If it involves getting rid of the version number from the atomic_ops directory, even better, since that was on my list anyway. Libatomic_ops should either be handled as a separate package, or should just use GC version numbers. And I think the former option requires more work for which we're currently lacking volunteers.

If I understand right, the latter case is to cut "-1.2" for "libatomic_ops" (in CVS and tar-ball) and replace such things as /Ilibatomic_ops-$(AO_VERSION) to /Ilibatomic_ops (and remove AO_VERSION assignment). For me, this is the prefered way. But the problem is CVS - could we rename a folder in a repo (preserving the CVS log)?

If the final decesion would be to cut "-1.2", I'll prepare the patch for makefiles (for other building scripts, let this be on Petter).

Bye.

Boehm, Hans

2009-09-25 22:42:55 UTC

From: Ivan Maidanski
I've deleted .m4 files and run autotools on Linux.
autoreconf works correctly but configure failed with error
"can't make a symbolic link" (that's because I've placed
"bdwgc" on NTFS drive) and make, then, failed too.
Everything works correctly on a native Linux FS.

Same here. Except I didn't try the NTFS case.

Should we fix this?

I'm with Petter on this. If it's easy, great.

If it involves getting rid of the version number from the atomic_ops directory, even better, since that was on my list anyway. Libatomic_ops should either be handled as a separate package, or should just use GC version numbers. And I think the former option requires more work for which we're currently lacking volunteers.

On the other hand, if it's hard to fix, I think it's not unreasonable to require a *n*x file system for the *n*x build process.

I expect to check in my pending cancellation patches shortly. Please let me know if I should check in Petter's patches from today as well. Otherwise I'll assume that Ivan will do so and update configure.

Hans

Bye.
_______________________________________________
Gc mailing list
http://www.hpl.hp.com/hosted/linux/mail-archives/gc/

Petter Urkedal

2009-09-26 13:40:56 UTC

Post by Boehm, Hans

Same here. Except I didn't try the NTFS case.

Should we fix this?

I'm with Petter on this. If it's easy, great.

You can see my suggestion on the last two commits in

http://git.eideticdew.org/cgit/~urkedal/bdwgc/log/?h=t-mv-ao

In involves a rewrite the libatomic_ops-related part of configure.ac,
making it much simpler.

As Ivan points out, CVS does not support moves, so this isn't straight
forward to commit without loosing history. An alternative may be to
keep for version (as long as the project is under CVS), hard-code it
into the build, and call it an artifact.

Post by Boehm, Hans
If it involves getting rid of the version number from the atomic_ops directory, even better, since that was on my list anyway. Libatomic_ops should either be handled as a separate package, or should just use GC version numbers. And I think the former option requires more work for which we're currently lacking volunteers.

I think libatomic_ops is very useful on its own right. Nevertheless,
the choice of keeping it under the same SCM doesn't need to affect the
end-user, since a packager is free to build the two separately. (I made
sure in the cited topic branch to maintain that option.)

Ivan Maidanski

2009-09-29 14:36:42 UTC

Hi!

Post by Boehm, Hans

Same here. Except I didn't try the NTFS case.

Should we fix this?

I'm with Petter on this. If it's easy, great.

Could you re-post the patch in this ML (if you think it should be applied to bdwgc)?

Post by Petter Urkedal
As Ivan points out, CVS does not support moves, so this isn't straight
forward to commit without loosing history. An alternative may be to
keep for version (as long as the project is under CVS), hard-code it
into the build, and call it an artifact.

Nonetheless, I've committed the rename. I don't think having such an artifact is good - the hard-coded version, e.g., may make someone think bdwgc can work only with a particular libatomic_ops version. The cvs version numbers start from 1.1 but the old numbers and log history is in the old place - the only inconvenience is diff across rename (same as for libatomic_ops-1.1 to libatomic_ops-1.2 rename).

Post by Petter Urkedal
I think libatomic_ops is very useful on its own right. Nevertheless,
the choice of keeping it under the same SCM doesn't need to affect the
end-user, since a packager is free to build the two separately. (I made
sure in the cited topic branch to maintain that option.)

Bye.

Petter Urkedal

2009-09-29 19:02:22 UTC

Post by Ivan Maidanski
Could you re-post the patch in this ML (if you think it should be applied to bdwgc)?

Attached is the patch, excluding generated files. Log:

* configure.ac: Rewrite the tests for external or internal
libatomic_ops.
* configure.ac: In particular, drop the symbolic links. Add option
--with-libatomic-ops for forced selection.
* Makefile.am: Adjust the path of source files from libatomic_ops to
not use the links.
* Makefile.am (libgc_la_LIBADD): Add $(ATOMIC_OPS_LIBS). This will be
empty if we use the bundled AO sources.

Post by Ivan Maidanski
Nonetheless, I've committed the rename. I don't think having such an artifact is good - the hard-coded version, e.g., may make someone think bdwgc can work only with a particular libatomic_ops version. The cvs version numbers start from 1.1 but the old numbers and log history is in the old place - the only inconvenience is diff across rename (same as for libatomic_ops-1.1 to libatomic_ops-1.2 rename).

Good. I agree it wouldn't have been nice. (BTW, Git, which I use,
preserves the history across the rename. The reason is that it does not
rely on metadata to detect moves but deduce it from the content of the
files.)

Ivan Maidanski

2009-09-30 08:51:38 UTC

Hi!

Post by Ivan Maidanski
Could you re-post the patch in this ML (if you think it should be applied to bdwgc)?

* configure.ac: Rewrite the tests for external or internal
libatomic_ops.
* configure.ac: In particular, drop the symbolic links. Add option
--with-libatomic-ops for forced selection.
* Makefile.am: Adjust the path of source files from libatomic_ops to
not use the links.
* Makefile.am (libgc_la_LIBADD): Add $(ATOMIC_OPS_LIBS). This will be
empty if we use the bundled AO sources.

Checked in the patch (with regenerate). Linux on FAT now works.

Before running autoreconf -vif, I deleted all .m4 files. Am I doing the right thing?
If yes, this is a bit compilated, so should we remove these files from GC (if they are used only during regeneration)?
If not, should config.h.in really contain undef GC version macros?

Bye.

Petter Urkedal

2009-09-30 17:18:06 UTC

Post by Ivan Maidanski
Before running autoreconf -vif, I deleted all .m4 files. Am I doing the right thing?

I'm constantly deleting libtool.m4 first myself since it's outdated.
But don't delete acinclude.m4. That one is part of bdwgc.

Post by Ivan Maidanski
If yes, this is a bit compilated, so should we remove these files from GC (if they are used only during regeneration)?

It's a matter of taste (and maybe heated discussions...), but the m4
files are not used by configure, so by deleting them we still provide a
working ./configure for the casual end-user while minimising the number
of generated files in the repository.

Post by Ivan Maidanski
If not, should config.h.in really contain undef GC version macros?

The way we do it, config.h.in should contain whatever Autoheader says,
and as said, we keep acinclude.m4.

Ivan Maidanski

2009-09-30 17:41:54 UTC

Hi!

Post by Ivan Maidanski
Before running autoreconf -vif, I deleted all .m4 files. Am I doing the right thing?

I'm constantly deleting libtool.m4 first myself since it's outdated.
But don't delete acinclude.m4. That one is part of bdwgc.

Post by Ivan Maidanski
If yes, this is a bit compilated, so should we remove these files from GC (if they are used only during regeneration)?

Post by Ivan Maidanski
If not, should config.h.in really contain undef GC version macros?

The way we do it, config.h.in should contain whatever Autoheader says,
and as said, we keep acinclude.m4.

Peter -
Could you send me your autogenerated files that are different from that in the latest CVS?

Bye.

Petter Urkedal

2009-09-30 18:27:41 UTC

Post by Ivan Maidanski
Peter -
Could you send me your autogenerated files that are different from that in the latest CVS?

I'm not sure what you'll use them for, but if I run autoreconf -vif on my end
most files change:

Makefile.in | 30 +-
aclocal.m4 | 9091 +++++----
config.guess | 209 +-
config.sub | 218 +-
configure |24539 ++++++++++------------
include/private/config.h.in | 13 +
libatomic_ops/Makefile.in | 10 +-
libatomic_ops/config.guess | 209 +-
libatomic_ops/config.sub | 218 +-
libatomic_ops/configure | 5664 +++---
libatomic_ops/doc/Makefile.in | 10 +-
libatomic_ops/src/Makefile.in | 10 +-
libatomic_ops/src/atomic_ops/Makefile.in | 10 +-
libatomic_ops/src/atomic_ops/sysdeps/Makefile.in | 10 +-
libatomic_ops/tests/Makefile.in | 10 +-
libtool.m4 | 6397 ------
ltmain.sh | 8473 +++++---
17 files changed, 25462 insertions(+), 29659 deletions(-)

That's with AUtomake-1.9.6 and Autoconf-2.63. I wouldn't worry about it, as
long as configure works and you and Hans stick to agreed upon versions between
commits to reduce the noise. You can browse/download the diffs/files at

http://git.eideticdew.org/cgit/~urkedal/bdwgc/log/?h=t-autoreconf

until I delete it. Follow the first entry.

Ivan Maidanski

2009-09-30 19:38:21 UTC

Hi!

Post by Ivan Maidanski
Peter -
Could you send me your auto-generated files that are different from that in the latest CVS?

I'm not sure what you'll use them for, but if I run autoreconf -vif on my end
...
17 files changed, 25462 insertions(+), 29659 deletions(-)
That's with AUtomake-1.9.6 and Autoconf-2.63. I wouldn't worry about it, as
long as configure works and you and Hans stick to agreed upon versions between
commits to reduce the noise. You can browse/download the diffs/files at
http://git.eideticdew.org/cgit/~urkedal/bdwgc/log/?h=t-autoreconf
until I delete it. Follow the first entry.

Thanks. I've regenerated (deleting that file before) but the result is different from yours. I've checked in the regenerated files. If this doesn't satisfy someone, let him either point me that's wrong (I'm not an expert in it) or do regeneration on his side (and commit).

Bye.

Petter Urkedal

2009-10-02 06:58:07 UTC

Post by Ivan Maidanski
Could you re-post the patch in this ML (if you think it should be applied to bdwgc)?

I broke the distcheck target with this; good thing there were no need to
release yet. I was thinking about ways to fix it, like copying instead
of linking, but I think the cleanest solution is to disable the
compilation of object files into subdirectories:

* configure.ac: Drop the subdir-objects Automake option, since it's
incompatible with picking source files from libatomic_ops.

This option is nice, but not really important for libgc, since most of
the sources are in the same directory. Hopefully the last build fix for
now...

Ivan Maidanski

2009-10-02 08:33:45 UTC

Hi!

Post by Ivan Maidanski
Could you re-post the patch in this ML (if you think it should be applied to bdwgc)?

I broke the distcheck target with this; good thing there were no need to
release yet. I was thinking about ways to fix it, like copying instead
of linking, but I think the cleanest solution is to disable the
* configure.ac: Drop the subdir-objects Automake option, since it's
incompatible with picking source files from libatomic_ops.
This option is nice, but not really important for libgc, since most of
the sources are in the same directory. Hopefully the last build fix for
now...
diff --git a/configure.ac b/configure.ac
index f1dcd65..ff400f3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -24,7 +24,7 @@ AC_CANONICAL_TARGET
AC_PREREQ(2.53)
AC_REVISION($Revision: 1.51 $)
GC_SET_VERSION
-AM_INIT_AUTOMAKE([foreign dist-bzip2 subdir-objects nostdinc])
+AM_INIT_AUTOMAKE([foreign dist-bzip2 nostdinc])
AM_CONFIG_HEADER([include/private/config.h])
AM_MAINTAINER_MODE

I've checked it in (and regenerated).

Bye.

Andreas Tobler

2009-10-08 19:33:24 UTC

Post by Ivan Maidanski
Could you re-post the patch in this ML (if you think it should be applied to bdwgc)?

I broke the distcheck target with this; good thing there were no need to
release yet. I was thinking about ways to fix it, like copying instead
of linking, but I think the cleanest solution is to disable the
* configure.ac: Drop the subdir-objects Automake option, since it's
incompatible with picking source files from libatomic_ops.
This option is nice, but not really important for libgc, since most of
the sources are in the same directory. Hopefully the last build fix for
now...
diff --git a/configure.ac b/configure.ac
index f1dcd65..ff400f3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -24,7 +24,7 @@ AC_CANONICAL_TARGET
AC_PREREQ(2.53)
AC_REVISION($Revision: 1.51 $)
GC_SET_VERSION
-AM_INIT_AUTOMAKE([foreign dist-bzip2 subdir-objects nostdinc])
+AM_INIT_AUTOMAKE([foreign dist-bzip2 nostdinc])
AM_CONFIG_HEADER([include/private/config.h])
AM_MAINTAINER_MODE

I've checked it in (and regenerated).

This patch breaks builds outside the source directory. When you have the
build inside a separate objdir which is on the same level as the src
directory the build breaks with the following message:

make[1]: *** No rule to make target `atomic_ops.c', needed by
`atomic_ops.lo'. Stop.
make: *** [all-recursive] Error 1

Just a fyi.
Andreas

Petter Urkedal

2009-10-08 21:52:37 UTC

Post by Petter Urkedal
diff --git a/configure.ac b/configure.ac
index f1dcd65..ff400f3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -24,7 +24,7 @@ AC_CANONICAL_TARGET
AC_PREREQ(2.53)
AC_REVISION($Revision: 1.51 $)
GC_SET_VERSION
-AM_INIT_AUTOMAKE([foreign dist-bzip2 subdir-objects nostdinc])
+AM_INIT_AUTOMAKE([foreign dist-bzip2 nostdinc])
AM_CONFIG_HEADER([include/private/config.h])
AM_MAINTAINER_MODE

I've checked it in (and regenerated).

This patch breaks builds outside the source directory. When you have the
build inside a separate objdir which is on the same level as the src
make[1]: *** No rule to make target `atomic_ops.c', needed by
`atomic_ops.lo'. Stop.
make: *** [all-recursive] Error 1

I can't reproduce this, but I'm not sure if I understood you correctly.
I tried the following from the source directory

$ mkdir _build
$ cd _build
$ ../configure
$ make

as well as putting _build one level above. Usually I build from a
completely different directory, which also works. Can you list the
commands to reproduce the problem, starting from the original sources
and an empty build directory? What's your environment? Do you have any
idea yourself about the cause? I'm wondering why "make" looks for
atomic_ops.c when the file is listed as libatomic_ops/src/atomic_ops.c
in Makefile.am.

Andreas Tobler

2009-10-09 18:53:15 UTC

Hi!

I've checked it in (and regenerated).

I can't reproduce this, but I'm not sure if I understood you correctly.
I tried the following from the source directory
$ mkdir _build
$ cd _build
$ ../configure
$ make
as well as putting _build one level above. Usually I build from a
completely different directory, which also works. Can you list the
commands to reproduce the problem, starting from the original sources
and an empty build directory? What's your environment? Do you have any
idea yourself about the cause? I'm wondering why "make" looks for
atomic_ops.c when the file is listed as libatomic_ops/src/atomic_ops.c
in Makefile.am.

Well, it must be something darwin specific, or my environment. I can not
reproduce the failure neither on sparc-solaris nor under x86_64 linux.

The tree I build from is a fresh co, w/o modifications from my side.
Like you describe above.

The thing I found so far is, that .deps/atomic_ops.Plo does not contain
the right path names:

darwin .deps/atomic_ops.Plo:

atomic_ops.lo atomic_ops.o: atomic_ops.c \
....

sparc .deps/atomic_ops.Plo:

atomic_ops.lo atomic_ops.o: ../bdwgc/libatomic_ops/src/atomic_ops.c \
...

The sparc one works correctly since it has the leading
../bdwgc/libatomic_ops.

I could not find out so far what the cause is for that.

Andreas

Petter Urkedal

2009-10-09 19:38:52 UTC

Post by Andreas Tobler
This patch breaks builds outside the source directory. When you have the
build inside a separate objdir which is on the same level as the src
make[1]: *** No rule to make target `atomic_ops.c', needed by
`atomic_ops.lo'. Stop.
make: *** [all-recursive] Error 1

I can't reproduce this, but I'm not sure if I understood you correctly.
I tried the following from the source directory
$ mkdir _build
$ cd _build
$ ../configure
$ make
as well as putting _build one level above. Usually I build from a
completely different directory, which also works. Can you list the
commands to reproduce the problem, starting from the original sources
and an empty build directory? What's your environment? Do you have any
idea yourself about the cause? I'm wondering why "make" looks for
atomic_ops.c when the file is listed as libatomic_ops/src/atomic_ops.c
in Makefile.am.

Which compiler do you use? There is an analogue dependency hard-coded
in Makefile.in

atomic_ops.lo: libatomic_ops/src/atomic_ops.c

which contains the actual rule. (This one seems to rely on VPATH, since
it does not list the build directory.) This rule has three-cases
conditioned by substitution of comment characters. Your Makefile
probably has the first case enabled (@am__fastdepCC_TRUE@), so the
compiler generates the dependencies. I'm thinking maybe it strips off
paths from dependency rules. If that's the case, configure should
probably decide to not use the fastdepCC case.

Andreas Tobler

2009-10-09 20:18:26 UTC

I can't reproduce this, but I'm not sure if I understood you correctly.
I tried the following from the source directory
$ mkdir _build
$ cd _build
$ ../configure
$ make
as well as putting _build one level above. Usually I build from a
completely different directory, which also works. Can you list the
commands to reproduce the problem, starting from the original sources
and an empty build directory? What's your environment? Do you have any
idea yourself about the cause? I'm wondering why "make" looks for
atomic_ops.c when the file is listed as libatomic_ops/src/atomic_ops.c
in Makefile.am.

Which compiler do you use? There is an analogue dependency hard-coded
in Makefile.in

Currently the system compiler: gcc version 4.0.1 (Apple Inc. build 5493)
But I can try with a self built one tomorrow. (Need to build them first)

Post by Petter Urkedal
atomic_ops.lo: libatomic_ops/src/atomic_ops.c
which contains the actual rule. (This one seems to rely on VPATH, since
it does not list the build directory.) This rule has three-cases
conditioned by substitution of comment characters. Your Makefile
compiler generates the dependencies. I'm thinking maybe it strips off
paths from dependency rules. If that's the case, configure should
probably decide to not use the fastdepCC case.

Interesting! Was not aware of that.

Thanks for helping out here!

Andreas

Petter Urkedal

2009-10-09 23:22:34 UTC

I can't reproduce this, but I'm not sure if I understood you correctly.
I tried the following from the source directory
$ mkdir _build
$ cd _build
$ ../configure
$ make
as well as putting _build one level above. Usually I build from a
completely different directory, which also works. Can you list the
commands to reproduce the problem, starting from the original sources
and an empty build directory? What's your environment? Do you have any
idea yourself about the cause? I'm wondering why "make" looks for
atomic_ops.c when the file is listed as libatomic_ops/src/atomic_ops.c
in Makefile.am.

Which compiler do you use? There is an analogue dependency hard-coded
in Makefile.in

Currently the system compiler: gcc version 4.0.1 (Apple Inc. build 5493)
But I can try with a self built one tomorrow. (Need to build them first)

Given that it's GCC and not an ancient version, it seems like an
implausible cause. If you haven't done so, I'd try to compare the gcc
command lines with that of a working system first. If the command lines
are different you could also try to regenerate the makefiles with the
most recent autotoolchain, in case there are fixes. If the command
lines are essentially the same, the next step could be to try to run the
commands manually on both system with exactly the same options (incl.
-M*) and compare the .Plo outputs.

Andreas Tobler

2009-10-10 20:31:08 UTC

I can't reproduce this, but I'm not sure if I understood you correctly.
I tried the following from the source directory
$ mkdir _build
$ cd _build
$ ../configure
$ make
as well as putting _build one level above. Usually I build from a
completely different directory, which also works. Can you list the
commands to reproduce the problem, starting from the original sources
and an empty build directory? What's your environment? Do you have any
idea yourself about the cause? I'm wondering why "make" looks for
atomic_ops.c when the file is listed as libatomic_ops/src/atomic_ops.c
in Makefile.am.

Which compiler do you use? There is an analogue dependency hard-coded
in Makefile.in

Currently the system compiler: gcc version 4.0.1 (Apple Inc. build 5493)
But I can try with a self built one tomorrow. (Need to build them first)

Hm, confused.

Petter Urkedal

2009-10-11 10:41:24 UTC

Post by Andreas Tobler
Hm, confused.

Andreas Tobler

2009-10-11 20:27:47 UTC

Post by Andreas Tobler
Hm, confused.

Petter Urkedal

2009-10-12 17:00:58 UTC

Post by Andreas Tobler
Hm, confused.

Andreas Tobler

2009-10-12 17:17:09 UTC

Post by Andreas Tobler
Hm, confused.

Petter Urkedal

2009-10-12 19:16:41 UTC

Post by Andreas Tobler
Hm, confused.

Boehm, Hans

2009-05-23 00:31:40 UTC

I'd also generally like to see us move away from direct use of the GC_XXX_THREADS macros, and just define GC_THREADS, relying on gc_config_macros.h to expand that to the appropriate platform-specific macro.

But that's not pressing.

Hans

Ivan Maidanski

2009-05-23 06:01:26 UTC

Hi!

Post by Boehm, Hans
I'd also generally like to see us move away from direct use of the GC_XXX_THREADS macros, and just define GC_THREADS, relying on gc_config_macros.h to expand that to the appropriate platform-specific macro.

For my test platforms (Win32, Cygwin, Solaris, Linux, FreeBSD) I already use GC_THREADS instead of GC_XXX_THREADS (not sure for CVS version).

See gc_config_macros.h changes in my diff64 (Feb 28).
(This patch also finishes the movement of 'extra' files (setjmp_t.c, gcname.c and friends) in the base folder to "extra" folder done in patches diff41a/b (11.11.2008)).

Although, I've left building scripts untouched (out of my scope).

Every thing is guided in gc_config_macros.c (so this file should be included somehow from every .c file at the beginning).

Post by Boehm, Hans
But that's not pressing.

There should be some changes NETBSD and DGUX. I'll provide a patch shortly.

Post by Boehm, Hans
Hans

Bye.

Petter Urkedal

2009-05-21 17:02:29 UTC

Attached is a slightly modified patch due to some of Ivan's suggestions.
Hans, please let me know if you have an opinion on Ivan's other points
(esp his second email), or other suggestions.

Petter

-----%<-----
Dependency tracking for configuration macros:

* configure.ac, acinclude.m4: Invoke AC_CONFIG_HEADER. Added
documentation of configuration macros.
* configure.ac, Makefile.am: Moved include paths to Makefile.am. Added
include path for build directory.

* include/gc_config_macros.h, include/private/gcconfig.h: Conditionally
include "private/config.h".
* include/gc.h, include/private/gc_pmark.h, backgraph.c, checksums.c,
gcj_mlc.c, real_malloc.c, tests/test.c: Minor adjustments to make sure
that the config macros are defined before use.

* configure.ac: Removed the unused macros STACKBASE and
DATASTART_IS_ETEXT.
* misc.c: Removed unused #undef STACKBASE.
-----%<-----

Boehm, Hans

2009-05-22 23:43:07 UTC

How about the following:

- I've committed Ivan's diff61, minus the gc.h includes, so that it's off the table.

- I'm not enthusiastic about the complicated include chain through gc.h. Could we include private/config.h from only private/gcconfig.h, and then include that, or gc_priv.h, in the places in which gc.h is being newly included, and possibly in any others it might be needed? That way this stays out of client visible headers, which I think is the right thing?

I think that would only involve a minor rebuild of the patch.

Hans

-----Original Message-----
Sent: Thursday, May 21, 2009 10:02 AM
Subject: Re: [Gc] Dependency tracking for configuration macros
Attached is a slightly modified patch due to some of Ivan's
suggestions.
Hans, please let me know if you have an opinion on Ivan's
other points (esp his second email), or other suggestions.
Petter
-----%<-----
* configure.ac, acinclude.m4: Invoke AC_CONFIG_HEADER. Added
documentation of configuration macros.
* configure.ac, Makefile.am: Moved include paths to
Makefile.am. Added include path for build directory.
Conditionally include "private/config.h".
* include/gc.h, include/private/gc_pmark.h, backgraph.c,
checksums.c, gcj_mlc.c, real_malloc.c, tests/test.c: Minor
adjustments to make sure that the config macros are defined
before use.
* configure.ac: Removed the unused macros STACKBASE and
DATASTART_IS_ETEXT.
* misc.c: Removed unused #undef STACKBASE.
-----%<-----

Petter Urkedal

2009-05-23 08:44:10 UTC

Post by Boehm, Hans
- I've committed Ivan's diff61, minus the gc.h includes, so that it's off the table.
- I'm not enthusiastic about the complicated include chain through gc.h. Could we include private/config.h from only private/gcconfig.h, and then include that, or gc_priv.h, in the places in which gc.h is being newly included, and possibly in any others it might be needed? That way this stays out of client visible headers, which I think is the right thing?

I've arranged so that including "gcconfig.h" is sufficient. It must
still be included before public headers because of the version check.

I had to include "gc_config_macros.h" directly from "gcconfig.h". As I
interpreted your other comment about the GC_*_THREADS macros,
"gc_config_macros.h" is a mandatory supplement to the configured
macros, so I think this makes sense anyway.

-----%<-----
Dependency tracking for configuration macros:

* configure.ac, acinclude.m4: Invoke AC_CONFIG_HEADER. Added
documentation of configuration macros.
* configure.ac, Makefile.am: Moved include paths to Makefile.am. Added
include path for build directory.

* include/gc.h: Use include guards for gc_config_macros.h.
* include/private/gcconfig.h: Also include gc_config_macros.h here, and
include config.h if available.
* include/gc_config_macros.h: Define the include guard.

* include/private/gc_priv.h, include/private/gc_pmark.h, backgraph.c,
checksums.c, gcj_mlc.c, real_malloc.c, tests/test.c: Minor adjustments
to make sure that the config macros are defined before use.

* configure.ac: Removed the unused macros STACKBASE and
DATASTART_IS_ETEXT.
* misc.c: Removed unused #undef STACKBASE.
-----%<-----

Petter

Petter Urkedal

2009-05-23 11:36:15 UTC

Please hold this. I seem to have broken a few things in the last
version of the patch. The original patch is the only version which I
properly tested. I'll have a better look at it.

Petter

Petter Urkedal

2009-05-24 07:39:22 UTC