You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Gabriela Gibson <ga...@gmail.com> on 2013/01/22 12:15:30 UTC

[PATCH] OPW 2013: Build System Gtest Addition

Part of my 2013 OPW Project for Subversion is to add the Googletest
test suite to be compiled by the build system.

Googletests's pages can be found here:
    http://code.google.com/p/googletest/

There are 3 attachments:  the patch, the log and a screen capture of the 
build system output.

Known problems:

1. Currently there is no option provided to make compilation of Gtest
optional.  The code in /trunk/build/ac-macros/gtest.m4 suggests that 
there should be provision for this, but I could not get it to work.

2.  Some headers are not found during the execution of autogen.sh -- 
this may be a Gtest internal issue:

WARNING: "gtest/internal/gtest-port.h" header not found, file 
gtest/src/gtest-internal-inl.h
WARNING: "gtest/gtest.h" header not found, file 
gtest/src/gtest-internal-inl.h
WARNING: "gtest/gtest-spi.h" header not found, file 
gtest/src/gtest-internal-inl.h

Other issues:

1. I have not tested whether Gtest works correctly.

2. I have not tested whether svn works correctly.











Re: [PATCH] Re: [PATCH] OPW 2013: Build System Gtest Addition

Posted by Branko Čibej <br...@wandisco.com>.
On 05.02.2013 14:28, Gabriela Gibson wrote:
> On 02/02/13 10:31, Branko Čibej wrote:
>> You could try adding ${abs_srcdir}/gtest/include to the include path. :)
>> Also, you'll get rid of those warnings by adding appropriate paths to
>> the private-includes list in build.conf.
>>
>> -- Brane
>>
> Thanks Brane,
>
> I think it is now working, please see attached patch and log.

I see this:

Index: get-deps.sh
=======================================

--- get-deps.sh	(revision 1442436)

+++ get-deps.sh	(working copy)

@@ -115,8 +115,12 @@

get_gtest() {
unzip -q $TEMPDIR/$GTEST.zip
- mv $GTEST gtest
+ mv $GTEST gtestlib 


And then this:

Index: Makefile.in
=======================================

--- Makefile.in	(revision 1442436)

+++ Makefile.in	(working copy)

@@ -135,7 +135,9 @@

APACHE_INCLUDES = @APACHE_INCLUDES@
APACHE_LIBEXECDIR = $(DESTDIR)@APACHE_LIBEXECDIR@
APACHE_LDFLAGS = @APACHE_LDFLAGS@
+GTEST_INCLUDES = -Igtest -Igtest/include


and scratch my head in confusion ... is it in gtestlib, or gtest? :)

-- Brane

-- 
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com


Re: [PATCH] Re: [PATCH] OPW 2013: Build System Gtest Addition

Posted by Branko Čibej <br...@wandisco.com>.
On 12.02.2013 18:36, Stefan Sperling wrote:
> On Wed, Feb 06, 2013 at 08:37:04AM +0000, Gabriela Gibson wrote:
>> On 05/02/13 17:00, Branko Čibej wrote:
>>> On 05.02.2013 14:28, Gabriela Gibson wrote:
>>> I'm almost sure you meant, "path = gtestlib".
>>>
>>> -- Brane
>>>
>> Nope, meant libgtest, ended up with both and the mistake compiled %-)
>>
>> I think it's working now, see attached patch.
> Branko, any news?

Fighting viruses (the biological kind), so I haven't tested the patch yet.

> This patch only seems to support a build with an in-tree libgtest
> obtained with get-deps.sh. Will there be support for using a system-wide
> gtest installation, too? Or does that not make sense?

It does not make sense, no. libgtest has to be built from source with
the same compiler and flags as the rest of the C++ code being tested.
While Ubuntu for example has a package that installs the sources in
/usr/src, I think it's OK for now to not worry about alternate source
locations.

> Gabriela, feel free to commit this patch to your branch, and then
> commit further follow-up fixes there, too, if any.

+1


-- Brane

-- 
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com


Re: [PATCH] Re: [PATCH] OPW 2013: Build System Gtest Addition

Posted by Stefan Sperling <st...@elego.de>.
On Wed, Feb 06, 2013 at 08:37:04AM +0000, Gabriela Gibson wrote:
> On 05/02/13 17:00, Branko Čibej wrote:
> >On 05.02.2013 14:28, Gabriela Gibson wrote:
> >I'm almost sure you meant, "path = gtestlib".
> >
> >-- Brane
> >
> Nope, meant libgtest, ended up with both and the mistake compiled %-)
> 
> I think it's working now, see attached patch.

Branko, any news?

This patch only seems to support a build with an in-tree libgtest
obtained with get-deps.sh. Will there be support for using a system-wide
gtest installation, too? Or does that not make sense?

Gabriela, feel free to commit this patch to your branch, and then
commit further follow-up fixes there, too, if any.
The quickest way of doing this from a working copy of Subversion's
trunk code which contains the local modifications corresponding
to your patch is:
  svn copy ^/subversion/trunk ^/subversion/branches/gtest
  svn switch ^/subversion/branches/gtest
  svn commit
This makes the working copy point to the gtest branch. To commit to
trunk you'd have to switch it back to ^/trunk first.

Re: [PATCH] Re: [PATCH] OPW 2013: Build System Gtest Addition

Posted by Gabriela Gibson <ga...@gmail.com>.
On 05/02/13 17:00, Branko Čibej wrote:
> On 05.02.2013 14:28, Gabriela Gibson wrote:
> I'm almost sure you meant, "path = gtestlib".
>
> -- Brane
>
Nope, meant libgtest, ended up with both and the mistake compiled %-)

I think it's working now, see attached patch.



Re: [PATCH] Re: [PATCH] OPW 2013: Build System Gtest Addition

Posted by Branko Čibej <br...@wandisco.com>.
On 05.02.2013 14:28, Gabriela Gibson wrote:
> On 02/02/13 10:31, Branko Čibej wrote:
>> You could try adding ${abs_srcdir}/gtest/include to the include path. :)
>> Also, you'll get rid of those warnings by adding appropriate paths to
>> the private-includes list in build.conf.
>>
>> -- Brane
>>
> Thanks Brane,
>
> I think it is now working, please see attached patch and log.
>


You ask:

> ## I don't understand why, but I need to provide "-o ... -c" in order
> to get the object file in the correct directory

And my best guess is this:

+# Gtest targets
+#
+
+# renamed from gtest to libgtest because libtool couldn't output
+# a library that didn't have the prefix 'lib'
+[libgtest]
+description = Gtest Test Suite
+type = lib
+path = libgtest 


I'm almost sure you meant, "path = gtestlib".

-- Brane

-- 
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com


[PATCH] Re: [PATCH] OPW 2013: Build System Gtest Addition

Posted by Gabriela Gibson <ga...@gmail.com>.
On 02/02/13 10:31, Branko Čibej wrote:
> You could try adding ${abs_srcdir}/gtest/include to the include path. :)
> Also, you'll get rid of those warnings by adding appropriate paths to
> the private-includes list in build.conf.
>
> -- Brane
>
Thanks Brane,

I think it is now working, please see attached patch and log.


Re: [PATCH] OPW 2013: Build System Gtest Addition

Posted by Branko Čibej <br...@wandisco.com>.
On 01.02.2013 19:34, Gabriela Gibson wrote:
> On 22/01/13 12:03, Stefan Sperling wrote:
>> On Tue, Jan 22, 2013 at 11:15:30AM +0000, Gabriela Gibson wrote:
>>> Part of my 2013 OPW Project for Subversion is to add the Googletest
> First of all thanks to Ben for rescuing my messy post =)
>
> Also thanks to everyone who gave me hints!
>
> ---
>
> At this point I have ./configure --enable-gtest working but the show
> stops here:
>
> [[[
> g++ -std=c++98  -D_REENTRANT -D_GNU_SOURCE -D_LARGEFILE64_SOURCE   -g
> -O2  -I./subversion/include -I./subversion -I/home/g/trunk/\
> apr/include   -I/home/g/trunk/apr-util/include
> -I/home/g/trunk/sqlite-amalgamation  gtest/src/gtest-all.cc
> gtest/src/gtest-all.cc:39:25: fatal error: gtest/gtest.h: No such file
> or directory
> (...snip...)
> gtest/src/gtest-all.cc:39:25: fatal error: gtest/gtest.h: No such file
> or directory
> compilation terminated.
> make: *** [gtest/src/gtest-all.lo] Error 1
> ]]]
>
> Looking at the file in question I find:
>
> [[[
> // This line ensures that gtest.h can be compiled on its own, even
> // when it's fused.
> #include "gtest/gtest.h"
> ]]]
>
> The file in question is located here: /trunk/gtest/include/gtest/gtest.h
>
> also a similar problem exists when using autogen.sh:
>
> [[[
> Creating build-outputs.mk...
> WARNING: "gtest/internal/gtest-port.h" header not found, file
> gtest/src/gtest-internal-inl.h
> WARNING: "gtest/gtest.h" header not found, file gtest/src/gtest-internal-inl.h
> WARNING: "gtest/gtest-spi.h" header not found, file
> gtest/src/gtest-internal-inl.h
> Creating svn_private_config.h.in...
> ]]]
>
> again, the files exist, but not where gtest thinks they should be.
>
> What can I do about that, if anything?
You could try adding ${abs_srcdir}/gtest/include to the include path. :)
Also, you'll get rid of those warnings by adding appropriate paths to
the private-includes list in build.conf.

-- Brane

-- 
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com


Re: [PATCH] OPW 2013: Build System Gtest Addition

Posted by Gabriela Gibson <ga...@gmail.com>.
On 22/01/13 12:03, Stefan Sperling wrote:
>
> On Tue, Jan 22, 2013 at 11:15:30AM +0000, Gabriela Gibson wrote:
>>
>> Part of my 2013 OPW Project for Subversion is to add the Googletest

First of all thanks to Ben for rescuing my messy post =)

Also thanks to everyone who gave me hints!

---

At this point I have ./configure --enable-gtest working but the show
stops here:

[[[
g++ -std=c++98  -D_REENTRANT -D_GNU_SOURCE -D_LARGEFILE64_SOURCE   -g
-O2  -I./subversion/include -I./subversion -I/home/g/trunk/\
apr/include   -I/home/g/trunk/apr-util/include
-I/home/g/trunk/sqlite-amalgamation  gtest/src/gtest-all.cc
gtest/src/gtest-all.cc:39:25: fatal error: gtest/gtest.h: No such file
or directory
(...snip...)
gtest/src/gtest-all.cc:39:25: fatal error: gtest/gtest.h: No such file
or directory
compilation terminated.
make: *** [gtest/src/gtest-all.lo] Error 1
]]]

Looking at the file in question I find:

[[[
// This line ensures that gtest.h can be compiled on its own, even
// when it's fused.
#include "gtest/gtest.h"
]]]

The file in question is located here: /trunk/gtest/include/gtest/gtest.h

also a similar problem exists when using autogen.sh:

[[[
Creating build-outputs.mk...
WARNING: "gtest/internal/gtest-port.h" header not found, file
gtest/src/gtest-internal-inl.h
WARNING: "gtest/gtest.h" header not found, file gtest/src/gtest-internal-inl.h
WARNING: "gtest/gtest-spi.h" header not found, file
gtest/src/gtest-internal-inl.h
Creating svn_private_config.h.in...
]]]

again, the files exist, but not where gtest thinks they should be.

What can I do about that, if anything?

thanks,

Gabriela

Ps.: it turns out that configure is issuing a misleading warning:
configure: WARNING: unrecognized options: --enable-gtest
but it also does it for --without-gpg-agent and --with-kwallet

Re: [PATCH] OPW 2013: Build System Gtest Addition

Posted by Stefan Sperling <st...@elego.de>.
On Tue, Jan 22, 2013 at 11:15:30AM +0000, Gabriela Gibson wrote:
> Part of my 2013 OPW Project for Subversion is to add the Googletest
> test suite to be compiled by the build system.
> 
> Googletests's pages can be found here:
>    http://code.google.com/p/googletest/
> 
> There are 3 attachments:  the patch, the log and a screen capture of
> the build system output.

This is looking very promising!

> Known problems:
> 
> 1. Currently there is no option provided to make compilation of Gtest
> optional.  The code in /trunk/build/ac-macros/gtest.m4 suggests that
> there should be provision for this, but I could not get it to work.

I think gtest should be an optional build dependency on trunk.
Else, it would disrupt other developer's environments and the buildbots.
So committing this to trunk as it is would not be a very good idea.

I think this is a good opportunity to offer you partial commit access
to Subversion so you can work on this on a branch. Then you can commit
the patch there and keep making improvements. Once it is ready for trunk
we can merge the branch back.
(See http://subversion.apache.org/docs/community-guide/roles.html#partial-commit-access)

I'll send you private email about this shortly!

Re: [PATCH] OPW 2013: Build System Gtest Addition

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Branko Čibej wrote on Wed, Jan 30, 2013 at 11:34:13 +0100:
> Whether we want GTest itself to use threads is another question -- I
> think the answer is a definite no for now; I can't think of a good
> reason to write multithreaded tests at this point.

AFAIK, the only uses of threading in subversion/tests/** are in the
regression test for #4129, and in the Python tests harness.

Not sure whether the binding tests use threads, but I would guess they
don't.

Re: [PATCH] OPW 2013: Build System Gtest Addition

Posted by Branko Čibej <br...@wandisco.com>.
On 30.01.2013 10:41, Stefan Sperling wrote:
>> # find and set these flags initially, do we need to add a --gtest-CXXFLAGS=?
>> # or does the bogstandard CXXUSERFLAGS suffice?
> I think just CXXUSERFLAGS is fine.

Yes. It is in fact a bad idea to add specific flags just for GTest.

>>> # not sure how to do this: need user set variable?  Set a default that
>>> # can be overridden with (say) --gtest-config=/trunk/gtest/scripts?
>>> AC_ARG_VAR([GTEST_CONFIG],
>>>            [The exact path of Google Test's 'gtest-config' script.])
>>> GTEST-CONFIG = "scripts/gtest-config"
>>> # but note the perms being set from the gtest configure.ac:
>>> AC_CONFIG_FILES([scripts/gtest-config], [chmod +x scripts/gtest-config])

Do not rely on GTest's configure. Just refer to the GTest sources
directly in our build.conf, and let our makefile build it. In fact, this
is the only sane way to eventually make it work on Windows.

>>> # TODO(chandlerc@google.com): Currently we aren't running the Python tests
>>> # against the interpreter detected by AM_PATH_PYTHON, and so we condition
>>> # HAVE_PYTHON by requiring "python" to be in the PATH, and that interpreter's
> This is bad. The python binary might not even be called 'python'
> on a given system (e.g. it might be called 'python2.7'). You don't
> need to address this right away, but eventually we should fix this.

We should simply ignore it since we will not be running GTest tests,
we'll be using GTest for running our tests. Hence, as I said above, we
can just ignore the GTest configury and simply compile their sources
into two libraries (libgtest_main and libgtest). Refer to GTest build to
find which sources belong where, but other than that, ignore it.

The GTest sources are completely standalone and do not require any
external dependencies in order to be used as a test framework.

>>> # version to be >= 2.3. This will allow the scripts to use a "/usr/bin/env"
>>> # hashbang.
>>> PYTHON=  # We *do not* allow the user to specify a python interpreter
>>> AC_PATH_PROG([PYTHON],[python],[:])
>>> AS_IF([test "$PYTHON" != ":"],
>>>       [AM_PYTHON_CHECK_VERSION([$PYTHON],[2.3],[:],[PYTHON=":"])])
>>> AM_CONDITIONAL([HAVE_PYTHON],[test "$PYTHON" != ":"])
>>>
>>> ========================================================================
>>> # Configure pthreads.  There is ~/trunk/gtest/m4/acx_pthread.m4 and it
>>> # looks like we need it.  does this need translating? Ie. the AM_ /
>>> # AS_IF macros rewritten?
> This should probably check whether APR was compiled with thread support,
> and obtain the appropriate pthread linking flags from APR somehow, if
> possible. It will likely work as-is for now, but we should keep this in
> mind as well.

We already have magic in place to see if we need to add -lpthread to the
link lines. And in fact it's irrelevant for building the GTest libraries
-- it only matters when building the test binaries, IIRC.

Whether we want GTest itself to use threads is another question -- I
think the answer is a definite no for now; I can't think of a good
reason to write multithreaded tests at this point.

-- Brane

-- 
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com


Re: [PATCH] OPW 2013: Build System Gtest Addition

Posted by Stefan Sperling <st...@elego.de>.
On Tue, Jan 29, 2013 at 10:39:03AM -0800, Ben Reser wrote:
> On Tue, Jan 29, 2013 at 9:37 AM, Gabriela Gibson
> <ga...@gmail.com> wrote:
> > Please see the attachment for a long long list of questions :)
> 
> Would have been easier for me to look at this if you'd just have
> included it inline in the email or at least as a text/plain attachment
> (your mailer seems to have decided it was application/x-shellscript,
> so I had to save the file in order to read it).

I agree that reading text inline rather than in attachments
is more convenient. Old-fashioned text-email interface (mutt)
user here :)
 
> One idiom around the list is people including blocks of code, output,
> etc wrapped like so:
> 
> [[[
> some code here
> ]]]
> 
> Which would have worked pretty well for this email.
> 
> To that end I've included your file quoted below with such changes:

Thanks Ben!

I'm replying to some of Gabriela's questions below:

> > The longer I look at the build system, the more confused I get, so I
> > think now is a good time to take stock and ask some questions.
> >
> > First of all, a list of the 'storyline' of the build system I think is
> > happening:
> >
> > http://wiki.apache.org/subversion/Build%20System%20Map2
> >
> > this is not complete, currently it just lists what autogen.sh,
> > build.conf, configure.ac, gen-make.py contain.  There are a few
> > dustbunnies hiding there :)
> >
> > Now my current guess as to what I need to do:
> >
> > [[[
> > # code for build.conf I think I should add
> >
> > dnl Search for gtest --------------
> > $svn_lib_gtest = "no"
> > dnl use build/ac-local/gtest.m4 to set things up.
> > GTEST_LIB_CHECK(1.6.0)
> > if test "$svn_lib_gtest" = "yes"; then
> >   AC_DEFINE([SVN_HAVE_GTEST], 1,
> >             [Defined if gtest is enabled])
> > fi
> > dnl
> > ]]]
> >
> >  and then what?  I cannot see how to tell the build system to use this info.

In configure.ac, extend BUILD_RULES, INSTALL_RULES, and
STATIC_INSTALL_RULES as appropriate. It may help to look at how
this is done for other optional dependencies, such as kwallet (which
is probably a good example to refer to because it also relies on C++).

This controls the rules that gen-make.py will consider when generating
Makefiles based on information from build.conf.

The configure script also produces the file subversion/svn_private_config.h
which maps configure script results to C preprocessor constants
(#define FOO bar) which we can use in the source code to detect how
the build was configured.

In your case the define would say
  #define SVN_HAVE_GTEST 1
if gtest was found. If it wasn't found, SVN_HAVE_GTEST won't be defined.
So in the source code this maps to:

#ifdef SVN_HAVE_GTEST

and:

#ifndef SVN_HAVE_GTEST

respectively.

> > [[[
> > # ----------------------------------------------------------------------------
> > #
> > # Gtest targets
> > #
> >
> > [gtest]
> > description = Gtest Test Suite
> >
> > type = lib
> > path = gtest
> > sources = src/*.cc
> > install = gtest-install
> >
> > # there are headers in 2 places:
> > # /home/g/trunk/gtest/include/gtest/internal/gtest-port.h
> > # /home/g/trunk/gtest/include/gtest/
> > # this causes autogen.sh to complain -- do we have a headers2 I can use?
> > headers = include/gtest
> >
> > # find and set these flags initially, do we need to add a --gtest-CXXFLAGS=?
> > # or does the bogstandard CXXUSERFLAGS suffice?

I think just CXXUSERFLAGS is fine.

> > compile-cmd = $(GTEST_CXXFLAGS)
> >
> > # find and set these flags initially, do we need to add a --gtest-LDFLAGS=?
> > link-cmd = $(GTEST_LDFLAGS)
> >
> > ========================================================================
> > # Code from/for gtest.m4 I think I should add / or otherwise use
> >
> > AC_ARG_WITH([gtest],
> > [AS_HELP_STRING([--with-gtest],
> >                   [Enable tests using the Google C++ Testing Framework.
> >                   ])],
> >   [],
> >   [with_gtest=no])
> >
> >
> > # not sure how to do this: need user set variable?  Set a default that
> > # can be overridden with (say) --gtest-config=/trunk/gtest/scripts?
> > AC_ARG_VAR([GTEST_CONFIG],
> >            [The exact path of Google Test's 'gtest-config' script.])
> > GTEST-CONFIG = "scripts/gtest-config"
> > # but note the perms being set from the gtest configure.ac:
> > AC_CONFIG_FILES([scripts/gtest-config], [chmod +x scripts/gtest-config])
> >
> >
> > # What to do with this?  --gtest-CPPFLAGS option for configure?
> > AC_ARG_VAR([GTEST_CPPFLAGS],
> >            [C-like preprocessor flags for Google Test.])
> > GTEST_CPPFLAGS = CXXCPP
> >
> > # What to do with this? --gtest-LIBS option for configure?
> > AC_ARG_VAR([GTEST_LIBS],
> >            [Library linking flags for Google Test.])
> >
> > # de we need the versioning check?  Gtest hasn't been updated in ages,
> > # seems to be mature
> > AC_ARG_VAR([GTEST_VERSION],
> >            [The version of Google Test available.])
> > ]]]
> >
> > it looks like
> > https://svn.apache.org/repos/asf/subversion/trunk/build/ac-macros/serf.m4
> > has a few useful bits I could rewrite.
> >
> > [[[
> > # path & version check (note that AM_ macros are not working at all,
> > # nor is AS_IF) do we need this check?  We already ensure python
> > # >=2.5?
> >
> > # TODO(chandlerc@google.com): Currently we aren't running the Python tests
> > # against the interpreter detected by AM_PATH_PYTHON, and so we condition
> > # HAVE_PYTHON by requiring "python" to be in the PATH, and that interpreter's

This is bad. The python binary might not even be called 'python'
on a given system (e.g. it might be called 'python2.7'). You don't
need to address this right away, but eventually we should fix this.

> > # version to be >= 2.3. This will allow the scripts to use a "/usr/bin/env"
> > # hashbang.
> > PYTHON=  # We *do not* allow the user to specify a python interpreter
> > AC_PATH_PROG([PYTHON],[python],[:])
> > AS_IF([test "$PYTHON" != ":"],
> >       [AM_PYTHON_CHECK_VERSION([$PYTHON],[2.3],[:],[PYTHON=":"])])
> > AM_CONDITIONAL([HAVE_PYTHON],[test "$PYTHON" != ":"])
> >
> > ========================================================================
> > # Configure pthreads.  There is ~/trunk/gtest/m4/acx_pthread.m4 and it
> > # looks like we need it.  does this need translating? Ie. the AM_ /
> > # AS_IF macros rewritten?

This should probably check whether APR was compiled with thread support,
and obtain the appropriate pthread linking flags from APR somehow, if
possible. It will likely work as-is for now, but we should keep this in
mind as well.

> > AC_ARG_WITH([pthreads],
> >             [AS_HELP_STRING([--with-pthreads],
> >                [use pthreads (default is yes)])],
> >             [with_pthreads=$withval],
> >             [with_pthreads=check])
> >
> > have_pthreads=no
> > AS_IF([test "x$with_pthreads" != "xno"],
> >       [ACX_PTHREAD(
> >         [],
> >         [AS_IF([test "x$with_pthreads" != "xcheck"],
> >                [AC_MSG_FAILURE(
> >                  [--with-pthreads was specified, but unable to be used])])])
> >        have_pthreads="$acx_pthread_ok"])
> > AM_CONDITIONAL([HAVE_PTHREADS],[test "x$have_pthreads" == "xyes"])
> > AC_SUBST(PTHREAD_CFLAGS)
> > AC_SUBST(PTHREAD_LIBS)
> > ]]]
> >
> > thanks for any advice you might have!

Re: [PATCH] OPW 2013: Build System Gtest Addition

Posted by Ben Reser <be...@reser.org>.
On Tue, Jan 29, 2013 at 9:37 AM, Gabriela Gibson
<ga...@gmail.com> wrote:
> Please see the attachment for a long long list of questions :)

Would have been easier for me to look at this if you'd just have
included it inline in the email or at least as a text/plain attachment
(your mailer seems to have decided it was application/x-shellscript,
so I had to save the file in order to read it).

One idiom around the list is people including blocks of code, output,
etc wrapped like so:

[[[
some code here
]]]

Which would have worked pretty well for this email.

To that end I've included your file quoted below with such changes:

> Hi,
>
> The longer I look at the build system, the more confused I get, so I
> think now is a good time to take stock and ask some questions.
>
> First of all, a list of the 'storyline' of the build system I think is
> happening:
>
> http://wiki.apache.org/subversion/Build%20System%20Map2
>
> this is not complete, currently it just lists what autogen.sh,
> build.conf, configure.ac, gen-make.py contain.  There are a few
> dustbunnies hiding there :)
>
> Now my current guess as to what I need to do:
>
> [[[
> # code for build.conf I think I should add
>
> dnl Search for gtest --------------
> $svn_lib_gtest = "no"
> dnl use build/ac-local/gtest.m4 to set things up.
> GTEST_LIB_CHECK(1.6.0)
> if test "$svn_lib_gtest" = "yes"; then
>   AC_DEFINE([SVN_HAVE_GTEST], 1,
>             [Defined if gtest is enabled])
> fi
> dnl
> ]]]
>
>  and then what?  I cannot see how to tell the build system to use this info.
>
> [[[
> # ----------------------------------------------------------------------------
> #
> # Gtest targets
> #
>
> [gtest]
> description = Gtest Test Suite
>
> type = lib
> path = gtest
> sources = src/*.cc
> install = gtest-install
>
> # there are headers in 2 places:
> # /home/g/trunk/gtest/include/gtest/internal/gtest-port.h
> # /home/g/trunk/gtest/include/gtest/
> # this causes autogen.sh to complain -- do we have a headers2 I can use?
> headers = include/gtest
>
> # find and set these flags initially, do we need to add a --gtest-CXXFLAGS=?
> # or does the bogstandard CXXUSERFLAGS suffice?
> compile-cmd = $(GTEST_CXXFLAGS)
>
> # find and set these flags initially, do we need to add a --gtest-LDFLAGS=?
> link-cmd = $(GTEST_LDFLAGS)
>
> ========================================================================
> # Code from/for gtest.m4 I think I should add / or otherwise use
>
> AC_ARG_WITH([gtest],
> [AS_HELP_STRING([--with-gtest],
>                   [Enable tests using the Google C++ Testing Framework.
>                   ])],
>   [],
>   [with_gtest=no])
>
>
> # not sure how to do this: need user set variable?  Set a default that
> # can be overridden with (say) --gtest-config=/trunk/gtest/scripts?
> AC_ARG_VAR([GTEST_CONFIG],
>            [The exact path of Google Test's 'gtest-config' script.])
> GTEST-CONFIG = "scripts/gtest-config"
> # but note the perms being set from the gtest configure.ac:
> AC_CONFIG_FILES([scripts/gtest-config], [chmod +x scripts/gtest-config])
>
>
> # What to do with this?  --gtest-CPPFLAGS option for configure?
> AC_ARG_VAR([GTEST_CPPFLAGS],
>            [C-like preprocessor flags for Google Test.])
> GTEST_CPPFLAGS = CXXCPP
>
> # What to do with this? --gtest-LIBS option for configure?
> AC_ARG_VAR([GTEST_LIBS],
>            [Library linking flags for Google Test.])
>
> # de we need the versioning check?  Gtest hasn't been updated in ages,
> # seems to be mature
> AC_ARG_VAR([GTEST_VERSION],
>            [The version of Google Test available.])
> ]]]
>
> it looks like
> https://svn.apache.org/repos/asf/subversion/trunk/build/ac-macros/serf.m4
> has a few useful bits I could rewrite.
>
> [[[
> # path & version check (note that AM_ macros are not working at all,
> # nor is AS_IF) do we need this check?  We already ensure python
> # >=2.5?
>
> # TODO(chandlerc@google.com): Currently we aren't running the Python tests
> # against the interpreter detected by AM_PATH_PYTHON, and so we condition
> # HAVE_PYTHON by requiring "python" to be in the PATH, and that interpreter's
> # version to be >= 2.3. This will allow the scripts to use a "/usr/bin/env"
> # hashbang.
> PYTHON=  # We *do not* allow the user to specify a python interpreter
> AC_PATH_PROG([PYTHON],[python],[:])
> AS_IF([test "$PYTHON" != ":"],
>       [AM_PYTHON_CHECK_VERSION([$PYTHON],[2.3],[:],[PYTHON=":"])])
> AM_CONDITIONAL([HAVE_PYTHON],[test "$PYTHON" != ":"])
>
> ========================================================================
> # Configure pthreads.  There is ~/trunk/gtest/m4/acx_pthread.m4 and it
> # looks like we need it.  does this need translating? Ie. the AM_ /
> # AS_IF macros rewritten?
> AC_ARG_WITH([pthreads],
>             [AS_HELP_STRING([--with-pthreads],
>                [use pthreads (default is yes)])],
>             [with_pthreads=$withval],
>             [with_pthreads=check])
>
> have_pthreads=no
> AS_IF([test "x$with_pthreads" != "xno"],
>       [ACX_PTHREAD(
>         [],
>         [AS_IF([test "x$with_pthreads" != "xcheck"],
>                [AC_MSG_FAILURE(
>                  [--with-pthreads was specified, but unable to be used])])])
>        have_pthreads="$acx_pthread_ok"])
> AM_CONDITIONAL([HAVE_PTHREADS],[test "x$have_pthreads" == "xyes"])
> AC_SUBST(PTHREAD_CFLAGS)
> AC_SUBST(PTHREAD_LIBS)
> ]]]
>
> thanks for any advice you might have!

Re: [PATCH] OPW 2013: Build System Gtest Addition

Posted by Gabriela Gibson <ga...@gmail.com>.
On 22/01/13 11:15, Gabriela Gibson wrote:
> Part of my 2013 OPW Project for Subversion is to add the Googletest
> test suite to be compiled by the build system.

Please see the attachment for a long long list of questions :)