Re: [PATCH] Add native ruby binding to Subversion distribution

On Mon, Oct 29, 2001 at 11:41:17PM +0900, Yoshiki Hayashi wrote:
>...
> to ask. This patch adds -lpthread to LDFLAG if APR is
> compiled with pthread. The problem is, if you load Svn.so
> linked without -lpthread option, Ruby interpreter cannot
> find libpthread and stop execution saying missing symbol or
> something.

I'd like to know more about why the Ruby interpreter breaks this. If you
compile and link using -pthread (what APR discovers), then everything should
be just fine. What is going on with Ruby? Why does it mess up?

We shouldn't monkey our build system because of a problem with Ruby's
loading of DSOs. If there is a problem in there, then we should fix it in
APR, but we can't know more until we have an accurate description of the
"missing symbol" and the related problems.

> Greg said -lpthread should be picked up by APR and I thought
> so but it is not. The problem is the way APR checks CFLAGS
> to gcc and LDFLAGS. First, APR tries to add one of
> following CFLAGS will be necessary to build pthread support.
> -kthread -pthread -pthreads -mthreads -Kthread -threads -mt
>
> Second, it tries to add -lpthread -lpthreads -lc_r to
> LDFLAGS if first step failed.
>
> Here's how it works on my system (Debian unstable):
>
> 1. APR picks up -pthread as CFLAGS. It allows you to
> compile and link pthread programs.
>
> 2. If I enable second check regardless of the result of
> first check, -lpthread will not be added to LDFLAGS
> because -pthread CFLAGS is all you need.
>
> 3. If I reverse the order of CFLAGS and LDFLAGS checking,
> APR correctly picks up -lpthread and Ruby binding are
> happy without LDFLAGS kludge.
>
> It looks like I should post patch to dev@apr to do #3. I
> don't know if it will bring any portability problem but APR
> people can probably figure it out. Anyway, I must go now.
> The time of the last train is approaching. ;-)

The order that APR checks them is correct, as far as I know. Patching it to
do #3 is simply futzing around with the order to get it to the state you
want. That doesn't mean that the "desired state" is correct.

>...
> +++ ./Makefile.in Mon Oct 29 21:08:56 2001
> @@ -53,18 +53,23 @@
> APACHE_INCLUDES = @APACHE_INCLUDES@
> APACHE_TARGET = @APACHE_TARGET@
>
> +RUBY_INCLUDES = @RUBY_INCLUDES@
> +ruby_moddir = @ruby_moddir@
> +
> MKDIR = @MKDIR@
>
> CFLAGS = @CFLAGS@
> CPPFLAGS = @CPPFLAGS@
> -LDFLAGS =
> +LDFLAGS = @LDFLAGS@
>
> COMPILE = $(CC) $(CPPFLAGS) $(CFLAGS) $(INCLUDES)
> LT_COMPILE = $(LIBTOOL) $(LTFLAGS) --mode=compile $(COMPILE) -o $@ -c $<
>
> COMPILE_APACHE_MOD = $(LIBTOOL) $(LTFLAGS) --mode=compile $(CC) $(CFLAGS) $(APACHE_INCLUDES) $(INCLUDES) -o $@ -c $<
> +COMPILE_RUBY_MOD = $(LIBTOOL) $(LTFLAGS) --mode=compile $(CC) $(CFLAGS) $(RUBY_INCLUDES) $(INCLUDES) -o $@ -c $<
>
> LINK = $(LIBTOOL) $(LTFLAGS) --mode=link $(LT_LDFLAGS) $(COMPILE) $(LDFLAGS) -rpath $(libdir)
> +MODULE = $(LIBTOOL) $(LTFLAGS) $(LT_LDFLAGS) $(COMPILE) -module -avoid-version $(LDFLAGS) -rpath $(libdir)

Let's call this LINK_DSO rather than MODULE. The latter doesn't describe the
operation that is occurring.

We might be able to use this for the Apache module, too. I need to learn
more about the -module switch to see what that does to the output, compared
to what we do now.

>...
> @@ -303,6 +316,10 @@
> install = 'lib'
> elif type == 'doc':
> pass
> + elif type == 'dso':
> + ignore, tname = string.split(name, '_')
> + tfile = tname + '.la'
> + self.objext = '.lo'

It would be good to have a comment in here on what is happening.
Specifically, that you're converting "ruby_svn" into "svn". Also, I think
you should use string.index() rather than split(). That would allow for
modules like "mod_dav_svn" which have more than underscore.

>...
> +++ ./build.conf Mon Oct 29 21:10:56 2001
> @@ -145,6 +145,14 @@
> # there are some .c files included by others, so *.c isn't appropriate
> sources = hashtable.c xmlparse.c xmlrole.c xmltok.c
>
> +[Ruby_Svn]

Let's keep this all lower-case, like the other targets.

>...
> dnl Check for libtool -- we'll definitely need it for all our shared libs!
> echo "configuring libtool now"
> +AC_LIBTOOL_DLOPEN

Do we really need this? And the -module flag? It is interesting to note that
mod_dav_svn has been working quite fine for a long while, without the need
for this. Apache dlopen's it all the time.

>...
> +AC_SUBST(LDFLAGS)

We need to watch out here, as I mentioned before.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org