GNUAspell / aspell

http://aspell.net
GNU Lesser General Public License v2.1
243 stars 53 forks source link

Patch: Initial cmake support for aspell #438

Open aspell-helper opened 16 years ago

aspell-helper commented 16 years ago

Christian Ehrlicher chehrlic\@sf created a patch on 2008-05-25 10:34:34 UTC (Orig. from https://sourceforge.net/p/aspell/patches/158)

With this you can close #​1507425

It currently only works on Linux (I get compile errors on windows with msvc, will fix it when there's enough interest).

What it does: - check for various headers and functions (HAVE_foo_H) - generates settings.h and dirs.h - builds filters (static or shared, see option COMPILE_IN_FILTERS - build static or shared lib - sets SOVERSION - installs nearly all like ./configure does

What's missing: - windows support due to compile problems - check for some specific settings (REL_OPS_POLLUTION, USE_FILE_LOCKS, ...) - only in-source builds work because the perl-scripts in gen/ don't allow out-of-source builds :( - see TODO in CMakeLists.txt

The intention is *not* to replace automake but to have a buildsystem on windows that works out-of-the-box without the need for .vcproj + friends + additonal Makefiles for every compiler on windows.

[Note: #​1507425 = https://sourceforge.net/p/aspell/patches/135/ = #373]

aspell-helper commented 16 years ago

Kevin Atkinson kevina\@sf commented on 2008-05-25 10:47:19 UTC

Logged In: YES user_id=6591 Originator: NO

Just to double check, this is for Aspell 0.60.6 right?

aspell-helper commented 16 years ago

Kevin Atkinson kevina\@sf updated the issue on 2008-05-25 10:47:19 UTC

aspell-helper commented 16 years ago

Christian Ehrlicher chehrlic\@sf commented on 2008-05-25 11:21:32 UTC

Logged In: YES user_id=798735 Originator: YES

Yes, but I can also work against svnHead (or whatever you use) if you want.

aspell-helper commented 16 years ago

Kevin Atkinson kevina\@sf commented on 2008-05-25 11:39:07 UTC

Logged In: YES user_id=6591 Originator: NO

Aspell 0.60.6 is fine for now. Eventually I will also want one for Aspell 0.61 (CVS head).

Is CMake case sensitive, if not I would prefer that the CMake commands be lowercase. I find the nearly all uppercase make file somewhat difficult to read.

aspell-helper commented 16 years ago

Peter Kuemmel syntheticpp\@sf commented on 2008-05-25 12:11:59 UTC

Logged In: YES user_id=1159765 Originator: NO

The commands are case insensitive, but variables are.

When you introduce new variable you can choose, but not for the CMAKE_* variables.

aspell-helper commented 16 years ago

Christian Ehrlicher chehrlic\@sf commented on 2008-05-25 13:23:24 UTC

Logged In: YES user_id=798735 Originator: YES

ok, new version - fix FOREACH statements (2.6.0 is lazy and doesn't complain) - compiles mingw out-of-the-box (only asc_ctype.hpp include in file_util.cpp is needed) when filters are compiled into the shared lib - lowercase cmake commands - add check for ENABLE_NLS (only available when libintl.h is found) File Added: aspell_cmake.zip

aspell-helper commented 16 years ago

Kevin Atkinson kevina\@sf commented on 2008-05-25 21:24:50 UTC

Logged In: YES user_id=6591 Originator: NO

You have, CMAKE_MINIMUM_REQUIRED(VERSION 2.4.5)

Do you use features that require 2.4.5. I have a slightly dated system with cmake 2.2.3. Would it be possible for it to be able to work on that. If not, I will install cmake from source.

aspell-helper commented 16 years ago

Christian Ehrlicher chehrlic\@sf commented on 2008-05-25 21:31:54 UTC

Logged In: YES user_id=798735 Originator: YES

I don't know if 2.2 works - I would recommend to upgrade to 2.4.x or even 2.6.0. You also don't have to build cmake by your own - afaik the binaries run on a lot (all?) of linux-systems out-of-the-box. I choosed 2.4.5 as minimum because thats the curennt min allowed version for kde4. All below had some bugs which prevented kde4 from configuring.

aspell-helper commented 16 years ago

Christian Ehrlicher chehrlic\@sf commented on 2008-05-26 07:53:58 UTC

Logged In: YES user_id=798735 Originator: YES

And once more a new version - added a lot of configure checks (taken from ./configure, plz. check if they're correct) - moved out configure checks into own ConfigureChecks.cmake

the configure checks are nearly complete now - just some special ncurses checks are missing afaics

File Added: aspell_cmake.tgz

aspell-helper commented 16 years ago

Kevin Atkinson kevina\@sf commented on 2008-05-27 07:56:44 UTC

Logged In: YES user_id=6591 Originator: NO

Hi.

I will try to have a look at it latter this week.

In all likely hood I will accept it as is, all I am likely to do it try it myself using CMake.

If you could attach some instructions on how to build with cmake it will be helpful.

aspell-helper commented 16 years ago

Christian Ehrlicher chehrlic\@sf commented on 2008-05-27 08:12:09 UTC

Logged In: YES user_id=798735 Originator: YES

Ok, I'll try - make sure to use a recent cmake version (at least 2.4.5). You can get a binary from http://www.cmake.org/HTML/Download.html - unpack aspell_cmake.tgz into aspell source dir - create a new directory aspell-build at the same level like aspell sources and change into this dir - execute 'cmake ../aspell-src' to start configuring - if you want to see the settings you can use ccmake (cmake 2.4) or cmake-gui (cmake 2.6) or take a look into CMakeCache.txt - start compiling with 'make'

All files are generated into the builddir except the output from gen/mk-static-filter.pl - I don't know how to fix this script to write the files into a different directory. If I could pass a optional output dir to this script it would be fine. Also plz take a look if all the checks are correct on your system. I just copied them from ./configure

aspell-helper commented 16 years ago

Kevin Atkinson kevina\@sf commented on 2008-06-01 04:02:07 UTC

Logged In: YES user_id=6591 Originator: NO

A few initial comments.

1) How do I get cmake to echo the commands being executed like make normally does 2) I _do not_ support builds with -DNDEBUG. Most asserts perform important safety checks and disabling them could lead to data corruption and other nasty things. Also, they should have a negligible effect on performance.

aspell-helper commented 16 years ago

Christian Ehrlicher chehrlic\@sf commented on 2008-06-01 07:37:55 UTC

Logged In: YES user_id=798735 Originator: YES

set VERBOSE to 1 , see also http://www.cmake.org/Wiki/CMake_FAQ

The NDBEUG flag - should be no big problem to remove this flag (although you're the first one who doesn't lime this flag *g*)

aspell-helper commented 16 years ago

Kevin Atkinson kevina\@sf commented on 2008-06-01 10:06:24 UTC

Logged In: YES user_id=6591 Originator: NO

1) Thanks for the pointer, I should of checked that first

2) Where does the NDEBUG flag come from, does it come from the default in CMake?

aspell-helper commented 16 years ago

Christian Ehrlicher chehrlic\@sf commented on 2008-06-01 10:14:27 UTC

Logged In: YES user_id=798735 Originator: YES

NDEBUG is in the default cmake flags for build type - Release, MinSizeRel and RelWithDbInfo NDEBUG is not defined for debug build type. You can set the build type either with ccmake / cmake-gui (cmakesetup on windows) or with command line: cmake ../aspell-src -DCMAKE_BUILD_TYPE=Debug It's then stored in CMakeCache.txt so you don't need to specify it again when you need to rerun cmake (normally you don't need to rerun cmake)

aspell-helper commented 16 years ago

Kevin Atkinson kevina\@sf commented on 2008-06-01 10:39:04 UTC

Logged In: YES user_id=6591 Originator: NO

I will not accept this patch until NDEBUG is not defined in any compile mode by default.

Not sure how much control you have over CMake development but NDEBUG _should not_ be enabled in any mode by default unless requested in CMakeLists.txt.

As far as I know, most Unix programs are not compiled with NDEBUG defined. Automake for sure does not define it.

aspell-helper commented 16 years ago

Christian Ehrlicher chehrlic\@sf commented on 2008-06-01 10:59:39 UTC

Logged In: YES user_id=798735 Originator: YES

Hey come on and don't be so agressive just because one little thing isn't the way you want to have it. I never said it's not possible to remove this flag - I just said it's enabled by default. I wonder if it's worth working with someone who reacts in such a way just because of such a little detail... :(

aspell-helper commented 16 years ago

Kevin Atkinson kevina\@sf commented on 2008-06-01 11:13:10 UTC

Logged In: YES user_id=6591 Originator: NO

I do not see this as a little detail.

Also I did not mean to imply that you said that "it's not possible to remove this flag - I just said it's enabled by default". Sorry if I gave you this impression.

As far as being aggressive sorry, but I fell rather strongly on this.

aspell-helper commented 16 years ago

Kevin Atkinson kevina\@sf commented on 2008-06-01 11:42:22 UTC

Logged In: YES user_id=6591 Originator: NO

Please also see the discussion on "Does Anyone Compile Aspell with -DNDEBUG" in the aspell-devel archive. It may take a while before the messages appear there.

aspell-helper commented 16 years ago

Kevin Atkinson kevina\@sf commented on 2008-06-12 23:02:38 UTC

Logged In: YES user_id=6591 Originator: NO

Any news on this. I hope I didn't scare you away :(

aspell-helper commented 16 years ago

Christian Ehrlicher chehrlic\@sf commented on 2008-06-13 07:30:02 UTC

Logged In: YES user_id=798735 Originator: YES

I was lazy the last days and did not much programming at my free time. I've updated the script - removed NDEBUG from release and minsizerel build types - fixed LIB_SUFFIX - now we install into lib64 on 64 bit systems (can be modified with an option) - used Check_C_Source_Compiles instead my own implementation - don't know why I wrote an own one in the first place - stupidity :) File Added: aspell_cmake.tgz

aspell-helper commented 16 years ago

Christian Ehrlicher chehrlic\@sf commented on 2008-06-13 07:30:02 UTC

Version 4 aspell_cmake.tgz

aspell-helper commented 16 years ago

Kevin Atkinson kevina\@sf commented on 2008-06-14 21:39:11 UTC

Logged In: YES user_id=6591 Originator: NO

Where 64 bit libraries are installed is very system specific. Thus I don't think it is a good idea to default to anything other than the standard location (ie lib) unless you have some way to detect the proper location.

Thanks for removing NDEBUG, but what you did basically looks like a hack. For now I am satisfied. However, if there really is no other way to do it I think I am going to file a Bug Report on this asking for an option to avoid including NDEBUG as a default explaining the reasons in the aspell-devel thread http://lists.gnu.org/archive/html/aspell-devel/2008-06/msg00000.html.