Compile and Link Flags

[Randall Leeds <ra...@gmail.com>]

Hey devs,

After mucking around in the build system the past few weeks my
compulsions got the best of me and I did things.
I think it's an improvement. I apologize for digging this up. It's not
glamorous.

I put a branch up on github based on current trunk[1].

I started by trying to make the build system find libmozjs and jsapi.h
from a xulrunner development installation's pkg-config files.
I also wanted to make libcurl a soft dependency. Paul committed this,
but it's got an error fixed on this branch[2].

After staring at configure.ac this long, I got twitchy.
In particular, the following sorts of things really irked me:

1) "# manually linking libm is requred for FreeBSD 7.0"

I think this was needed by the icu driver, which is why icu-config
--ldflags includes -lm on my system.
But we weren't using icu-config --ldflags, we were just using the
search paths. Why? ICU wants other stuff for me.
In particular, AC_CHECK_ICU sets ICU_CFLAGS and ICU_LIBS for us but we
were ignoring them. That's craziness.

2) CFLAGS="-O2"
But only on the Windows build?
There's one place where we differentiate between windows/nix in
configure.ac but each case contained mostly the same stuff!
Only a -D CFLAG for SpiderMonkey needs to be different.
This CFLAG was also dropped into the generic FLAGS variable, along
with ERLANG_FLAGS, which gets duplicated in CPPFLAGS and LDFLAGS!
LTFLAGS was set to CFLAGS, but only on windows. I believe this is
default for libtool anyway.

Anyway, once I broke all this stuff down more explicitly and passed
only what was needed everywhere and let ICU tell us how it should be
linked I found that my ICU install says it should be compiled with
-ansi, so I fixed the comments in the icu driver to be C style (/*
*/).

Everything passes here and builds nicely.

Combining it all we get:
A configure.ac that is easier on the eyes.
Less platform specific stuff strew about the build system.
No need for --with-js-* on systems with a system-wide xulrunner.
A (correct this time) soft dependency on libcurl.

So, is this stuff that should go in trunk? Or was this all for naught?
Again, sorry for the non-functional changes. I promise I'll submit
patches that actually improve functionality soon.

[1] https://github.com/tilgovi/couchdb/tree/flagmadness
[2] https://github.com/tilgovi/couchdb/commit/0185faa2150f072a8c71b03325424c20e24c3eab

Re: Compile and Link Flags

[Klaus Trainer <kl...@web.de>]

Looks awesome! As soon as Noah or so tells us that he hasn't found any
issue in it during his review, I wanna have that!


On Tue, 2011-02-01 at 20:36 -0800, Randall Leeds wrote:
> Hey devs,
> 
> After mucking around in the build system the past few weeks my
> compulsions got the best of me and I did things.
> I think it's an improvement. I apologize for digging this up. It's not
> glamorous.
> 
> I put a branch up on github based on current trunk[1].
> 
> I started by trying to make the build system find libmozjs and jsapi.h
> from a xulrunner development installation's pkg-config files.
> I also wanted to make libcurl a soft dependency. Paul committed this,
> but it's got an error fixed on this branch[2].
> 
> After staring at configure.ac this long, I got twitchy.
> In particular, the following sorts of things really irked me:
> 
> 1) "# manually linking libm is requred for FreeBSD 7.0"
> 
> I think this was needed by the icu driver, which is why icu-config
> --ldflags includes -lm on my system.
> But we weren't using icu-config --ldflags, we were just using the
> search paths. Why? ICU wants other stuff for me.
> In particular, AC_CHECK_ICU sets ICU_CFLAGS and ICU_LIBS for us but we
> were ignoring them. That's craziness.
> 
> 2) CFLAGS="-O2"
> But only on the Windows build?
> There's one place where we differentiate between windows/nix in
> configure.ac but each case contained mostly the same stuff!
> Only a -D CFLAG for SpiderMonkey needs to be different.
> This CFLAG was also dropped into the generic FLAGS variable, along
> with ERLANG_FLAGS, which gets duplicated in CPPFLAGS and LDFLAGS!
> LTFLAGS was set to CFLAGS, but only on windows. I believe this is
> default for libtool anyway.
> 
> Anyway, once I broke all this stuff down more explicitly and passed
> only what was needed everywhere and let ICU tell us how it should be
> linked I found that my ICU install says it should be compiled with
> -ansi, so I fixed the comments in the icu driver to be C style (/*
> */).
> 
> Everything passes here and builds nicely.
> 
> Combining it all we get:
> A configure.ac that is easier on the eyes.
> Less platform specific stuff strew about the build system.
> No need for --with-js-* on systems with a system-wide xulrunner.
> A (correct this time) soft dependency on libcurl.
> 
> So, is this stuff that should go in trunk? Or was this all for naught?
> Again, sorry for the non-functional changes. I promise I'll submit
> patches that actually improve functionality soon.
> 
> [1] https://github.com/tilgovi/couchdb/tree/flagmadness
> [2] https://github.com/tilgovi/couchdb/commit/0185faa2150f072a8c71b03325424c20e24c3eab

Re: Compile and Link Flags

[Benoit Chesneau <bc...@gmail.com>]

On Wed, Feb 2, 2011 at 5:36 AM, Randall Leeds <ra...@gmail.com> wrote:
> Hey devs,
>
> After mucking around in the build system the past few weeks my
> compulsions got the best of me and I did things.
> I think it's an improvement. I apologize for digging this up. It's not
> glamorous.
>
> I put a branch up on github based on current trunk[1].
>
> I started by trying to make the build system find libmozjs and jsapi.h
> from a xulrunner development installation's pkg-config files.
> I also wanted to make libcurl a soft dependency. Paul committed this,
> but it's got an error fixed on this branch[2].
>
> After staring at configure.ac this long, I got twitchy.
> In particular, the following sorts of things really irked me:
>
> 1) "# manually linking libm is requred for FreeBSD 7.0"
>
> I think this was needed by the icu driver, which is why icu-config
> --ldflags includes -lm on my system.
> But we weren't using icu-config --ldflags, we were just using the
> search paths. Why? ICU wants other stuff for me.
> In particular, AC_CHECK_ICU sets ICU_CFLAGS and ICU_LIBS for us but we
> were ignoring them. That's craziness.
>
> 2) CFLAGS="-O2"
> But only on the Windows build?
> There's one place where we differentiate between windows/nix in
> configure.ac but each case contained mostly the same stuff!
> Only a -D CFLAG for SpiderMonkey needs to be different.
> This CFLAG was also dropped into the generic FLAGS variable, along
> with ERLANG_FLAGS, which gets duplicated in CPPFLAGS and LDFLAGS!
> LTFLAGS was set to CFLAGS, but only on windows. I believe this is
> default for libtool anyway.
>
> Anyway, once I broke all this stuff down more explicitly and passed
> only what was needed everywhere and let ICU tell us how it should be
> linked I found that my ICU install says it should be compiled with
> -ansi, so I fixed the comments in the icu driver to be C style (/*
> */).
>
> Everything passes here and builds nicely.
>
> Combining it all we get:
> A configure.ac that is easier on the eyes.
> Less platform specific stuff strew about the build system.
> No need for --with-js-* on systems with a system-wide xulrunner.
> A (correct this time) soft dependency on libcurl.
>
> So, is this stuff that should go in trunk? Or was this all for naught?
> Again, sorry for the non-functional changes. I promise I'll submit
> patches that actually improve functionality soon.
>
> [1] https://github.com/tilgovi/couchdb/tree/flagmadness
> [2] https://github.com/tilgovi/couchdb/commit/0185faa2150f072a8c71b03325424c20e24c3eab
>

Looks good for me. Will try this branch in afternoon on my bsd systems.

- benoit

Re: Compile and Link Flags

[Noah Slater <ns...@apache.org>]

On 2 Feb 2011, at 04:36, Randall Leeds wrote:

> After mucking around in the build system the past few weeks my
> compulsions got the best of me and I did things.
> I think it's an improvement. I apologize for digging this up. It's not
> glamorous.

I've been hoping someone with a better understanding of C would come in at some point and get all this stuff sorted out. So thanks, it's been long overdue. Your changes sound like they make sense — though of course, I don't have the skills to test them all properly myself. So if you think it's all working like it's supposed to, then I'd say go ahead and merge this back to CouchDB proper.

Re: Compile and Link Flags

[Jan Lehnardt <ja...@apache.org>]

On 2 Feb 2011, at 05:36, Randall Leeds wrote:

> So, is this stuff that should go in trunk? Or was this all for naught?
> Again, sorry for the non-functional changes. I promise I'll submit
> patches that actually improve functionality soon.

Even if you think these aren't glamorous, I think they are important to get
right, so I'm glad you spending time on this :)

Cheers
Jan
-- 

Re: Compile and Link Flags

[Paul Davis <pa...@gmail.com>]

On Tue, Feb 1, 2011 at 11:36 PM, Randall Leeds <ra...@gmail.com> wrote:
> Hey devs,
>
> After mucking around in the build system the past few weeks my
> compulsions got the best of me and I did things.
> I think it's an improvement. I apologize for digging this up. It's not
> glamorous.
>
> I put a branch up on github based on current trunk[1].
>
> I started by trying to make the build system find libmozjs and jsapi.h
> from a xulrunner development installation's pkg-config files.
> I also wanted to make libcurl a soft dependency. Paul committed this,
> but it's got an error fixed on this branch[2].
>
> After staring at configure.ac this long, I got twitchy.
> In particular, the following sorts of things really irked me:
>
> 1) "# manually linking libm is requred for FreeBSD 7.0"
>
> I think this was needed by the icu driver, which is why icu-config
> --ldflags includes -lm on my system.
> But we weren't using icu-config --ldflags, we were just using the
> search paths. Why? ICU wants other stuff for me.
> In particular, AC_CHECK_ICU sets ICU_CFLAGS and ICU_LIBS for us but we
> were ignoring them. That's craziness.
>
> 2) CFLAGS="-O2"
> But only on the Windows build?
> There's one place where we differentiate between windows/nix in
> configure.ac but each case contained mostly the same stuff!
> Only a -D CFLAG for SpiderMonkey needs to be different.
> This CFLAG was also dropped into the generic FLAGS variable, along
> with ERLANG_FLAGS, which gets duplicated in CPPFLAGS and LDFLAGS!
> LTFLAGS was set to CFLAGS, but only on windows. I believe this is
> default for libtool anyway.
>
> Anyway, once I broke all this stuff down more explicitly and passed
> only what was needed everywhere and let ICU tell us how it should be
> linked I found that my ICU install says it should be compiled with
> -ansi, so I fixed the comments in the icu driver to be C style (/*
> */).
>
> Everything passes here and builds nicely.
>
> Combining it all we get:
> A configure.ac that is easier on the eyes.
> Less platform specific stuff strew about the build system.
> No need for --with-js-* on systems with a system-wide xulrunner.
> A (correct this time) soft dependency on libcurl.
>
> So, is this stuff that should go in trunk? Or was this all for naught?
> Again, sorry for the non-functional changes. I promise I'll submit
> patches that actually improve functionality soon.
>
> [1] https://github.com/tilgovi/couchdb/tree/flagmadness
> [2] https://github.com/tilgovi/couchdb/commit/0185faa2150f072a8c71b03325424c20e24c3eab
>

Neato.

Looks pretty good to me. Though I'm gonna let Noah eyeball it before
committing it.