Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch: Initial cmake support for aspell #438

Open
aspell-helper opened this issue May 25, 2008 · 23 comments
Open

Patch: Initial cmake support for aspell #438

aspell-helper opened this issue May 25, 2008 · 23 comments

Comments

@aspell-helper
Copy link
Collaborator

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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

  • milestone: --> 0.60
  • priority: 5 --> 6
  • assigned_to: nobody --> kevina

@aspell-helper
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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

Version 4
aspell_cmake.tgz

@aspell-helper
Copy link
Collaborator Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant