[sword-devel] -O3 -g

[Jonathan Marsden]

Troy A. Griffitts wrote:

> Ok, I still don't think we've gotten to the bottom of this. ...

I'll test on the crosswire.org server itself, and if I can get the
errors to appear there, with my .bashrc renamed out of the way,
hopefully that will that be sufficient :)

> g++ -DHAVE_CONFIG_H -I. -Iinclude -Iinclude -I/usr/include
> -DUSE_AUTOTOOLS -DUNIX -Dunix -D__unix__ -DSWICU_DATA=
> \"/usr/lib/sword/1.6.0_icu_4.0.1\" -D_FTPLIB_NO_COMPAT -D_ICU_ -g3 -O0
> -Wall -Werror -D_ICU_ -ftemplate-depth-25 -DCURLAVAILABLE -I/usr/include
> -I/usr/lib64 -DUSELUCENE -g -O2 -g -Wall -DUSBINARY -c
> src/mgr/filemgr.cpp  -fPIC -DPIC

Sure:

jonathan at black:~/sword$ g++ -DHAVE_CONFIG_H -I. -Iinclude -Iinclude
-I/usr/include -DUSE_AUTOTOOLS -DUNIX -Dunix -D__unix__
-DSWICU_DATA=\"/usr/lib/sword/1.6.0_icu_4.0.1\" -D_FTPLIB_NO_COMPAT
-D_ICU_ -g3 -O0 -Wall -Werror -D_ICU_ -ftemplate-depth-25
-DCURLAVAILABLE -I/usr/include -I/usr/lib64 -DUSELUCENE -g -O2 -g -Wall
-DUSBINARY -c src/mgr/filemgr.cpp  -fPIC -DPIC
cc1plus: warnings being treated as errors
src/mgr/filemgr.cpp: In member function ‘signed char
sword::FileMgr::trunc(sword::FileDesc*)’:
src/mgr/filemgr.cpp:285: error: ignoring return value of ‘ssize_t
write(int, const void*, size_t)’, declared with attribute warn_unused_result
src/mgr/filemgr.cpp: In static member function ‘static int
sword::FileMgr::copyFile(const char*, const char*)’:
src/mgr/filemgr.cpp:418: error: ignoring return value of ‘ssize_t
write(int, const void*, size_t)’, declared with attribute warn_unused_result
jonathan at black:~/sword$

> I do not get an error when building on various systems.

See above :)

After much playing around, I have found a way to generate these same
warnings/errors on the crosswire.org server itself:

 # Obtain the SWORD source tree as ~/swordtest
 cd ~ && rm -rf swordtest # Delete any previous ~/swordtest setup
 svn -q co http://crosswire.org/svn/sword/trunk swordtest
 # Check for all warnings (and treat them as errors) in non-debug builds
 sed -i -e 's/AM_CXXFLAGS="-O3"/AM_CXXFLAGS="-O3 -Wall -Werror
-D_FORTIFY_SOURCE=2"/' swordtest/configure.ac
 cd swordtest && ./autogen.sh && ./configure && make

The sed command (which should be all on one line) changes the set of
CXXFLAGS for non-debug builds to

  -O3 -Wall -Werror -D_FORTIFY_SOURCE=2

and then builds SWORD using a straight ./configure && make

Your server, your chosen versions of gcc and friends.  I realize
-D_FORTIFY_SOURCE=2 is not a default in the Fedora 10 environment, but
it *is* what gets done when building packages for Debian and Ubuntu
(and, I suspect, also Fedora packages these days).

> Gotcha.  The configure default is obviously what the standard configure
> default should be.

Agreed :)

> I think providing any '<insert_your_preferred_configure_name_here>.sh'
> is above what most projects provide.  I agree maybe further changing the
> name to devist.sh or something might be a little more clear to people
> who try it, but most people don't read the README and thus will just 
> ./configure && make

Which is great, if the configure.ac defaults are what normal people need
to build SWORD from the sources :)

Either set things up so ./configure has good defaults, or provide a
script with the defaults in it; just don't provide a script that the
docs say is "normal", but which upon inspection is really not normal :)

Maybe I'm really weird... I started out by downloading the tarball,
reading the README and INSTALL, and doing what they said.  I used that
as the basis for what came later.  I think that's entirely conventional
for a developer working with a sourcecode tarball, but ... maybe I'm
really weird :)

Those docs (still!) say /usr/share/sword is the standard place for your
SWORD modules, of course, and still build a configuration file that uses
that location -- so reading that got me into all kinds of hot water with
those who disagree with that approach... :)

> Honestly, I don't think it is a big deal.  The things set
> in ./usrinst.sh currently are very minimal.  Everything is standard to
> what users would need to set with any package they ./configure EXCEPT:
> 
> --without-conf : doesn't install /etc/sword.conf on make install
> --with-icu : enable SWORD icu usage

And the lib64 thing, and enable-debug, and the lack of shared libs, and
remembering to --enable-utilities, and perhaps enable-tests should be
the default too :)

I just think that you have effectively turned what was originally
intended to be a set of "normal" configuration options for the project
into what is really "Troy's preferred settings for his particular needs
and his particular runtime environment".  And since the configure.ac
defaults aren't really all that sane either, there is no "normal"!

> I supposed we could just change configure.ac to make these settings the
> default?

Sure.  I think the configure.ac defaults really should be something that
works well for a majority of power-user-building-from-source situations
-- no forcing of lib64, no disabling of shared libs, etc. etc.  Just
build the thing, all of it, by default.  Once you do that, *then* you
can add *.sh files for special situations all day long, including one
for your own convenience -- just make sure the INSTALL and README
instructions tell people to use ./configure and to ignore the *.sh files
for "normal" builds.

Maybe I'm the only person in the world who will ever download the SWORD
sources as a tarball and try to build them by following the INSTALL and
README documentation...?  But if not, then we should fix this so it
works well when people (who are not members of the development team) try
building SWORD this way :)

> I know other projects like Bibletime and Xiphos use their own
> Unicode frameworks (Qt and gtk+ respectively) instead of ICU.  Thoughts?
> Are you enabling icu in SWORD now when you build packages?

Yes.  That's what was in usrinst.sh, and there was discussion about it
on sword-devel early on, and IIRC you/Chris/someone seemed pretty
adamant that ICU really really should be used, I think.  So I used it,
and so the SWORD packages depend on it.

If the goal is that the configure.ac defaults work for people just
compiling SWORD itself, as I have suggested above, then IMO --with-icu
is probably the right default.  If someone chooses to replace it, fine,
they should know enough about the codebase at that point to use
--without-icu.  That's how I'd see it, at least.  Rationale: if instead
you have a --without-icu default, then the resulting build behaves
strangely for some SWORD modules (right?), and it is unclear to a
newcomer why.  So that would be a poor default.  A default build should
"just work".

Jonathan