You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ramkumar Ramachandra <ar...@gmail.com> on 2010/11/30 08:39:10 UTC
[PATCH] Fix Makefile.svn to build APR with threads
Hi,
Here's a quick patch to fix Makefile.svn. I need a +1 to commmit this.
Thanks.
[[[
Makefile.svn: Fix build
* tools/dev/unix-build/Makefile.svn: Configure APR to build with
threads enabled to prevent breaking the Subversion build.
]]]
Index: tools/dev/unix-build/Makefile.svn
===================================================================
--- tools/dev/unix-build/Makefile.svn (revision 1040428)
+++ tools/dev/unix-build/Makefile.svn (working copy)
@@ -238,8 +238,7 @@
cd $(APR_OBJDIR) \
&& env CFLAGS="-O0 -g" $(APR_SRCDIR)/configure \
--prefix=$(PREFIX)/apr \
- --enable-maintainer-mode \
- --disable-threads
+ --enable-maintainer-mode
touch $@
# compile apr
Re: [PATCH] Fix Makefile.svn to build APR with threads
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Dec 01, 2010 at 10:48:37AM +0530, Ramkumar Ramachandra wrote:
> Hi Stefan,
>
> Thanks for the suggestions. How about this?
+1
A single 'Approved by: stsp' line in the log message is sufficient,
you don't necessarily need the 'suggested' and 'review' lines.
Thanks!
>
> [[[
> Makefile.svn: Optionally allow building with threading support
>
> * tools/dev/unix-build/Makefile.svn: Add a new THREADING variable to
> control whether APR and sqlite should be built with threading
> support.
>
> Suggested by: stsp
> Review by: stsp
> ]]]
>
> Index: tools/dev/unix-build/Makefile.svn
> ===================================================================
> --- tools/dev/unix-build/Makefile.svn (revision 1040690)
> +++ tools/dev/unix-build/Makefile.svn (working copy)
> @@ -233,6 +233,12 @@
> fi
> touch $@
>
> +ifdef THREADING
> +THREADS_FLAG=--enable-threads
> +else
> +THREADS_FLAG=--disable-threads
> +endif
> +
> # configure apr
> $(APR_OBJDIR)/.configured: $(APR_OBJDIR)/.retrieved
> cp $(APR_SRCDIR)/build/apr_hints.m4 \
> @@ -246,7 +252,7 @@
> $(APR_SRCDIR)/configure \
> --prefix=$(PREFIX)/apr \
> --enable-maintainer-mode \
> - --disable-threads
> + $(THREADS_FLAG)
> touch $@
>
> # compile apr
> @@ -704,6 +710,12 @@
> tar -C $(SRCDIR) -zxf $(DISTDIR)/$(SQLITE_DIST)
> touch $@
>
> +ifdef THREADING
> +THREADSAFE_FLAG=--enable-threadsafe
> +else
> +THREADSAFE_FLAG=--disable-threadsafe
> +endif
> +
> # configure sqlite
> $(SQLITE_OBJDIR)/.configured: $(SQLITE_OBJDIR)/.retrieved
> cd $(SQLITE_OBJDIR) \
> @@ -711,7 +723,7 @@
> $(SQLITE_SRCDIR)/configure \
> --prefix=$(PREFIX)/sqlite \
> --disable-tcl \
> - --disable-threadsafe
> + $(THREADSAFE_FLAG)
> touch $@
>
> # compile sqlite
Re: [PATCH] Fix Makefile.svn to build APR with threads
Posted by Ramkumar Ramachandra <ar...@gmail.com>.
Hi Stefan,
Thanks for the suggestions. How about this?
[[[
Makefile.svn: Optionally allow building with threading support
* tools/dev/unix-build/Makefile.svn: Add a new THREADING variable to
control whether APR and sqlite should be built with threading
support.
Suggested by: stsp
Review by: stsp
]]]
Index: tools/dev/unix-build/Makefile.svn
===================================================================
--- tools/dev/unix-build/Makefile.svn (revision 1040690)
+++ tools/dev/unix-build/Makefile.svn (working copy)
@@ -233,6 +233,12 @@
fi
touch $@
+ifdef THREADING
+THREADS_FLAG=--enable-threads
+else
+THREADS_FLAG=--disable-threads
+endif
+
# configure apr
$(APR_OBJDIR)/.configured: $(APR_OBJDIR)/.retrieved
cp $(APR_SRCDIR)/build/apr_hints.m4 \
@@ -246,7 +252,7 @@
$(APR_SRCDIR)/configure \
--prefix=$(PREFIX)/apr \
--enable-maintainer-mode \
- --disable-threads
+ $(THREADS_FLAG)
touch $@
# compile apr
@@ -704,6 +710,12 @@
tar -C $(SRCDIR) -zxf $(DISTDIR)/$(SQLITE_DIST)
touch $@
+ifdef THREADING
+THREADSAFE_FLAG=--enable-threadsafe
+else
+THREADSAFE_FLAG=--disable-threadsafe
+endif
+
# configure sqlite
$(SQLITE_OBJDIR)/.configured: $(SQLITE_OBJDIR)/.retrieved
cd $(SQLITE_OBJDIR) \
@@ -711,7 +723,7 @@
$(SQLITE_SRCDIR)/configure \
--prefix=$(PREFIX)/sqlite \
--disable-tcl \
- --disable-threadsafe
+ $(THREADSAFE_FLAG)
touch $@
# compile sqlite
Re: [PATCH] Fix Makefile.svn to build APR with threads
Posted by Stefan Sperling <st...@elego.de>.
On Tue, Nov 30, 2010 at 07:52:36PM +0100, Stefan Sperling wrote:
> ifdef THREADING
> THREADING_FLAG=--enable-threads
> else
> THREADING_FLAG=--enable-threads
> endif
Oops... s/enable/disable/ on the second line obviously :)
Re: [PATCH] Fix Makefile.svn to build APR with threads
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Dec 01, 2010 at 12:18:08AM +0530, Ramkumar Ramachandra wrote:
> > Your patch is too short. It's missing at least one hunk. Because if threading
> > is enabled for APR, you should also enable it for sqlite and any other
> > dependencies affected.
>
> Ok, got it. Here's my second try.
>
> [[[
> Makefile.svn: Optionally allow building with threading support
>
> * tools/dev/unix-build/Makefile.svn: Add a new ENABLE_THREADING
> variable to control whether APR and sqlite should be built with
> threading support.
> ]]]
>
>
> Index: tools/dev/unix-build/Makefile.svn
> ===================================================================
> --- tools/dev/unix-build/Makefile.svn (revision 1040658)
> +++ tools/dev/unix-build/Makefile.svn (working copy)
> @@ -3,6 +3,7 @@
> # WARNING: This may or may not work on your system. This Makefile is
> # an example, rather than a ready-made universal solution.
>
> +ENABLE_THREADING ?= no # OpenBSD doesn't have kernel threads for example
You can call this variable THREADING so it's shorter to type on the
command line.
> ENABLE_PYTHON_BINDINGS ?= yes
> ENABLE_RUBY_BINDINGS ?= yes
> ENABLE_PERL_BINDINGS ?= yes
> @@ -241,12 +242,21 @@
> | sed -e '/^.*APR_ADDTO(CPPFLAGS, \[-D_POSIX_THREADS\]).*$$/d' \
> > $(APR_SRCDIR)/build/apr_hints.m4
> cd $(APR_SRCDIR) && ./buildconf
> - cd $(APR_OBJDIR) \
> - && env CFLAGS="-O0 -g $(PROFILE_CFLAGS)" \
> - $(APR_SRCDIR)/configure \
> - --prefix=$(PREFIX)/apr \
> - --enable-maintainer-mode \
> - --disable-threads
> + if [ $(ENABLE_THREADING) = yes ]; then \
> + cd $(APR_OBJDIR) \
> + && env CFLAGS="-O0 -g $(PROFILE_CFLAGS)" \
> + $(APR_SRCDIR)/configure \
> + --prefix=$(PREFIX)/apr \
> + --enable-maintainer-mode \
> + --enable-threads; \
> + else \
> + cd $(APR_OBJDIR) \
> + && env CFLAGS="-O0 -g $(PROFILE_CFLAGS)" \
> + $(APR_SRCDIR)/configure \
> + --prefix=$(PREFIX)/apr \
> + --enable-maintainer-mode \
> + --disable-threads; \
> + fi;
> touch $@
>
> # compile apr
I would define a make variable that contains threading or non-threading
flags depending on THREADING=1, like this:
ifdef THREADING
THREADING_FLAG=--enable-threads
else
THREADING_FLAG=--enable-threads
endif
Then you can do this:
cd $(APR_OBJDIR) \
&& env CFLAGS="-O0 -g $(PROFILE_CFLAGS)" \
$(APR_SRCDIR)/configure \
--prefix=$(PREFIX)/apr \
--enable-maintainer-mode \
$(THREADING_FLAG); \
> @@ -706,12 +716,21 @@
>
> # configure sqlite
> $(SQLITE_OBJDIR)/.configured: $(SQLITE_OBJDIR)/.retrieved
> - cd $(SQLITE_OBJDIR) \
> - && env CFLAGS="-g $(PROFILE_CFLAGS)" \
> - $(SQLITE_SRCDIR)/configure \
> - --prefix=$(PREFIX)/sqlite \
> - --disable-tcl \
> - --disable-threadsafe
> + if [ $(ENABLE_THREADING) = yes ]; then \
> + cd $(SQLITE_OBJDIR) \
> + && env CFLAGS="-g $(PROFILE_CFLAGS)" \
> + $(SQLITE_SRCDIR)/configure \
> + --prefix=$(PREFIX)/sqlite \
> + --disable-tcl \
> + --enable-threadsafe; \
> + else \
> + cd $(SQLITE_OBJDIR) \
> + && env CFLAGS="-g $(PROFILE_CFLAGS)" \
> + $(SQLITE_SRCDIR)/configure \
> + --prefix=$(PREFIX)/sqlite \
> + --disable-tcl \
> + --disable-threadsafe; \
> + fi;
> touch $@
>
> # compile sqlite
Same here.
Stefan
Re: [PATCH] Fix Makefile.svn to build APR with threads
Posted by Ramkumar Ramachandra <ar...@gmail.com>.
Hi,
Stefan Sperling writes:
> The first is that I use OpenBSD and OpenBSD's threading implementation
> is a pure userspace implementation. It has to set stdin and stdout to
> non-blocking -- otherwise, whenever a thread blocks for i/o in the kernel the
> userspace thread-scheduler would also be blocked and couldn't switch to
> another thread. However, non-blocking stdio makes it very inconvenient
> to do things like "svn diff | less" because less expects blocking reads.
> The lines get garbled when you start scrolling in less.
> (Kernel thread support is being worked on by OpenBSD developers.)
>
> The second reason is that virtually nobody else is compiling and running
> without thread support in APR. This sometimes causes thread-less cases
> to be overlooked. Most recently I found a bug in the performance branch
> due to this. So it's good to have test coverage for the thread-less case.
> Because of this I will likely continue compiling APR thread-less even when
> OpenBSD finally gets kernel thread support.
Thanks for the explanation! Unfortunately, I'm not able to get the
build to break with threading disabled anymore :| Have to run a few
more experiments before I can conclusively say what went wrong back
then.
> Your patch is too short. It's missing at least one hunk. Because if threading
> is enabled for APR, you should also enable it for sqlite and any other
> dependencies affected.
Ok, got it. Here's my second try.
[[[
Makefile.svn: Optionally allow building with threading support
* tools/dev/unix-build/Makefile.svn: Add a new ENABLE_THREADING
variable to control whether APR and sqlite should be built with
threading support.
]]]
Index: tools/dev/unix-build/Makefile.svn
===================================================================
--- tools/dev/unix-build/Makefile.svn (revision 1040658)
+++ tools/dev/unix-build/Makefile.svn (working copy)
@@ -3,6 +3,7 @@
# WARNING: This may or may not work on your system. This Makefile is
# an example, rather than a ready-made universal solution.
+ENABLE_THREADING ?= no # OpenBSD doesn't have kernel threads for example
ENABLE_PYTHON_BINDINGS ?= yes
ENABLE_RUBY_BINDINGS ?= yes
ENABLE_PERL_BINDINGS ?= yes
@@ -241,12 +242,21 @@
| sed -e '/^.*APR_ADDTO(CPPFLAGS, \[-D_POSIX_THREADS\]).*$$/d' \
> $(APR_SRCDIR)/build/apr_hints.m4
cd $(APR_SRCDIR) && ./buildconf
- cd $(APR_OBJDIR) \
- && env CFLAGS="-O0 -g $(PROFILE_CFLAGS)" \
- $(APR_SRCDIR)/configure \
- --prefix=$(PREFIX)/apr \
- --enable-maintainer-mode \
- --disable-threads
+ if [ $(ENABLE_THREADING) = yes ]; then \
+ cd $(APR_OBJDIR) \
+ && env CFLAGS="-O0 -g $(PROFILE_CFLAGS)" \
+ $(APR_SRCDIR)/configure \
+ --prefix=$(PREFIX)/apr \
+ --enable-maintainer-mode \
+ --enable-threads; \
+ else \
+ cd $(APR_OBJDIR) \
+ && env CFLAGS="-O0 -g $(PROFILE_CFLAGS)" \
+ $(APR_SRCDIR)/configure \
+ --prefix=$(PREFIX)/apr \
+ --enable-maintainer-mode \
+ --disable-threads; \
+ fi;
touch $@
# compile apr
@@ -706,12 +716,21 @@
# configure sqlite
$(SQLITE_OBJDIR)/.configured: $(SQLITE_OBJDIR)/.retrieved
- cd $(SQLITE_OBJDIR) \
- && env CFLAGS="-g $(PROFILE_CFLAGS)" \
- $(SQLITE_SRCDIR)/configure \
- --prefix=$(PREFIX)/sqlite \
- --disable-tcl \
- --disable-threadsafe
+ if [ $(ENABLE_THREADING) = yes ]; then \
+ cd $(SQLITE_OBJDIR) \
+ && env CFLAGS="-g $(PROFILE_CFLAGS)" \
+ $(SQLITE_SRCDIR)/configure \
+ --prefix=$(PREFIX)/sqlite \
+ --disable-tcl \
+ --enable-threadsafe; \
+ else \
+ cd $(SQLITE_OBJDIR) \
+ && env CFLAGS="-g $(PROFILE_CFLAGS)" \
+ $(SQLITE_SRCDIR)/configure \
+ --prefix=$(PREFIX)/sqlite \
+ --disable-tcl \
+ --disable-threadsafe; \
+ fi;
touch $@
# compile sqlite
Re: [PATCH] Fix Makefile.svn to build APR with threads
Posted by Stefan Sperling <st...@elego.de>.
On Tue, Nov 30, 2010 at 02:09:10PM +0530, Ramkumar Ramachandra wrote:
> Hi,
>
> Here's a quick patch to fix Makefile.svn. I need a +1 to commmit this.
If the build breaks because of threadless APR, that's a bug in Subversion,
not in Makefile.svn.
If you want to build with thread support, please make it conditional
on a global Makefile flag, as in "make THREADED=1".
I'd like to keep the default non-threaded for two reasons.
The first is that I use OpenBSD and OpenBSD's threading implementation
is a pure userspace implementation. It has to set stdin and stdout to
non-blocking -- otherwise, whenever a thread blocks for i/o in the kernel the
userspace thread-scheduler would also be blocked and couldn't switch to
another thread. However, non-blocking stdio makes it very inconvenient
to do things like "svn diff | less" because less expects blocking reads.
The lines get garbled when you start scrolling in less.
(Kernel thread support is being worked on by OpenBSD developers.)
The second reason is that virtually nobody else is compiling and running
without thread support in APR. This sometimes causes thread-less cases
to be overlooked. Most recently I found a bug in the performance branch
due to this. So it's good to have test coverage for the thread-less case.
Because of this I will likely continue compiling APR thread-less even when
OpenBSD finally gets kernel thread support.
Your patch is too short. It's missing at least one hunk. Because if threading
is enabled for APR, you should also enable it for sqlite and any other
dependencies affected.
Stefan
>
> Thanks.
>
> [[[
> Makefile.svn: Fix build
>
> * tools/dev/unix-build/Makefile.svn: Configure APR to build with
> threads enabled to prevent breaking the Subversion build.
> ]]]
>
> Index: tools/dev/unix-build/Makefile.svn
> ===================================================================
> --- tools/dev/unix-build/Makefile.svn (revision 1040428)
> +++ tools/dev/unix-build/Makefile.svn (working copy)
> @@ -238,8 +238,7 @@
> cd $(APR_OBJDIR) \
> && env CFLAGS="-O0 -g" $(APR_SRCDIR)/configure \
> --prefix=$(PREFIX)/apr \
> - --enable-maintainer-mode \
> - --disable-threads
> + --enable-maintainer-mode
> touch $@
>
> # compile apr