Patch: Compile with Microsoft Visual C++

[aspell-helper]

Stephen J. Muir yadancer\@sf created a patch on 2011-05-17 13:43:09 UTC (Orig. from https://sourceforge.net/p/aspell/patches/168)

This patch allows you to compile version 0.60.6 using Microsoft Visual C++. I tested it using Microsoft Visual C++ 2008.

It is based on the 0.60.5 patch at: http://www.ndl.kiev.ua/content/patch-aspell-0605-win32-compilation

The patch completely replaces the contents of the win32 directory. It does not add or remove any other files, but it does modify a few.

After applying this patch, you can build on Windows by doing the following:

1. Download CMake from www.cmake.org . 2. Run CMake, point it at the win32 directory for the source and the win32/build directory (which it will create) for the destination. If you are using Microsoft Visual C++ 2008, you would select "Visual Studio 9 2008" as the generator.

When I tried running this with Apache HTTP Server/PHP/Pspell, replacing the Windows Aspell version 0.50.3 aspell-15.dll with this version (renaming aspell_shared.dll to aspell-15.dll), I got the following error when starting the Apache HTTP Server:

The ordinal 1546 could not be located in the dynamic link library aspell-15.dll .

This is because Pspell is referencing the methods by ordinal rather than by name.

To circumvent this, save the attached aspell_shared.def somewhere and, within Visual C++, navigate to Project/Properties/Configuration Properties/Linker/Input and set the Module Definition File to the full path to it.

[aspell-helper]

Kevin Atkinson kevina\@sf commented on 2011-05-18 12:37:33 UTC

Thanks for the patch.

Unfortunately, I can not accept it as is as it modifies automatically generated files and it doesn't look like they were recreated using the modified "mk-src.pl". For example the modifications in the beginning of "aspell.h" is not in the modified script.

It also adds a lot of noise and makes it difficult for me to see what other changes where made other than adding ASPELL_API.

In a few weeks I might be able to help you with the modified script if you are unable to do so. In the meantime it would be helpful if you could factor out the adding of ASPELL_API into a separate patch so I can see what other changes where made.

Also as a minor point, it looks like you got the direction wrong for the patch in the win32 directory as the files being removed (or blanked) rather than added. For the win32 directory it is acceptable to just give me a zip file since they are all new files.

[aspell-helper]

Stephen J. Muir yadancer\@sf commented on 2011-05-18 13:05:11 UTC

The direction is correct, because it's removing the entire original contents of the win32 so that they can be replaced. I don't know why the new contents are not in the patch, so I'm attaching the new contents of the win32 directory.

This patch doesn't involve running the configure script. I tried using the configure/make route, but couldn't get it to work. I couldn't get CygWin to work because it produces lots of errors. I tried using MinGW/MSYS but it needed perl and I couldn't get a version of that for Windows that worked correctly, so I gave up.

[aspell-helper]

Stephen J. Muir yadancer\@sf commented on 2011-05-18 13:07:23 UTC

win32.zip

[aspell-helper]

Stephen J. Muir yadancer\@sf commented on 2011-05-18 13:07:57 UTC

Added new version of win32 directory.

[aspell-helper]

Stephen J. Muir yadancer\@sf commented on 2011-05-18 13:10:12 UTC

aspell_shared.def

[aspell-helper]

Stephen J. Muir yadancer\@sf commented on 2011-05-18 13:12:21 UTC

Separated the zip file into its contents (2 files).

Even if this patch isn't accepted, it's the only way I have of building on Windows, and others may need to use it.

[aspell-helper]

Kevin Atkinson kevina\@sf commented on 2011-05-18 13:17:47 UTC

Like I said before in a few weeks I will likely be able help you with the modified script.

In the meantime if you could separate out any modifications to automatically generated files into a separate patch it would help we see what other changes where made. Otherwise, any additional comments will have to wait until I have time to look at it more closely in a few weeks.

In all likelihood I will accept some form of this patch.

[aspell-helper]

Stephen J. Muir yadancer\@sf commented on 2011-05-18 13:44:30 UTC

My employer asked me to produce a version of Aspell for Windows that doesn't crash, which I have now done. I have submitted all the changes that I made for this to you in the hope that the next release of Aspell for Windows can be made equally resilient.

Unfortunately, helping to incorporate these into the next official version of the product is beyond my remit and I doubt that my employer would be willing for me to spend significant time over and above what I've already done.

[aspell-helper]

Stephen J. Muir yadancer\@sf commented on 2011-05-18 14:42:50 UTC

As requested, I've separated aspell0.60.0_windows_visual_studio.patch into aspell0.60.0_windows_visual_studio_manual_edit.patch and aspell0.60.0_windows_visual_studio_auto_gen.patch depending on which files said they were auto generated.

[aspell-helper]

Stephen J. Muir yadancer\@sf commented on 2011-05-18 19:15:30 UTC

aspell0.60.6_windows_visual_studio_auto_gen.patch

[aspell-helper]

Stephen J. Muir yadancer\@sf commented on 2011-05-18 19:15:50 UTC

aspell0.60.6_windows_visual_studio_manual_edit.patch

[aspell-helper]

Kevin Atkinson kevina\@sf commented on 2011-05-19 01:35:03 UTC

Thanks for talking the time to submit the patch,

Any additional feedback I need from you would be in the form try this. That is I will give you a modified patch to try, If you do not have the time to do this, I understand. Especially, since it will likely be in a few weeks when you might have moved on to other projects.

[aspell-helper]

Stephen J. Muir yadancer\@sf commented on 2011-05-19 09:01:24 UTC

Do you have a Windows machine at your disposal? If so, all you need is CMake and Microsoft Visual C++ 2008 Express, which are both free.

P.S. I've already moved on to another project. ;)

[aspell-helper]

Stephen J. Muir yadancer\@sf commented on 2011-05-19 10:41:14 UTC

I've been authorized to spend some time on this whenever you're ready on condition that the amount of time is small.

It's important to us that an up-to-date official Windows version is available.

[aspell-helper]

Kevin Atkinson kevina\@sf commented on 2011-06-26 10:48:23 UTC

I plan on applying some form of this patch within a week.

One question. What exactly is ASPELL_API doing and why is an explicit annotation needed. Is there some way to mark a group of functions with __declspec similar to extern "C". Basically I would like to some how fold the extern "C" part into ASPELL_API as anything marked with ASPELL_API will also need to be extern "C".

[aspell-helper]

Stephen J. Muir yadancer\@sf commented on 2011-07-04 10:43:53 UTC

ASPELL_API is sometimes set to __declspec(dllexport) in win32\include\settings.h

If you do a web search for __declspec, you will find details of what it does.

I tried the following syntax:

extern "C" ASPELL_API { def1; def2; }

which gives syntax errors. I tried:

extern "C" { ASPELL_API def1; ASPELL_API def2; }

which compiles ok. If you were to fold the extern "C" into ASPELL_API, you would be able to have:

ASPELL_API def1; ASPELL_API def2;

However, in this case, you will need to ensure that all other platforms define ASPELL_API as extern "C". In my opinion, this would be a greater evil than repeating the extern "C".

[aspell-helper]

Kevin Atkinson kevina\@sf commented on 2011-07-04 10:50:48 UTC

I did that and figured out what it does. But why is it needed. What about just exporting all externally visible functions which is how it is done in Unix. Or in other words, is it possible to declare declspec to all externally visible functions when compiling a library.

I will likely accept you changes, I am just trying to justify sprinkling ASPELL_API everywhere.

[aspell-helper]

Kevin Atkinson kevina\@sf commented on 2011-07-04 11:04:51 UTC

Also, if at all possible please try to get Aspell to compile without NDEBUG defined. I just released Aspell 0.60.6.1 in which I output a warning during the build if NDEBUG is defined. Also in Aspell 0.60.7 I will include NDEBUG in the version string if it's defined. See http://aspell.net/ndebug.html for my reasons.

You might want to look at Patch #​1971805: Initial cmake support for aspell (https://sourceforge.net/tracker/?func=detail&aid=1971805&group_id=245&atid=300245 ), and base your work on that. However, its not a requirement --- I do not plan to officially support CMAKE for non-Windows platforms.

If you have the time please also look at 2989423: Fix --enable-win32-relocatable and maybe incorporate into your patch. Again, not a requirement.

[Note: #​1971805 = https://sourceforge.net/p/aspell/patches/158/ = #438] [Note: #​2989423 = https://sourceforge.net/p/aspell/patches/164/ = #472]

[aspell-helper]

Kevin Atkinson kevina\@sf commented on 2011-07-04 11:08:50 UTC

I hereby reject you changes to common/file_util.cpp, unless you can better justify them.

[aspell-helper]

Stephen J. Muir yadancer\@sf commented on 2011-07-04 12:17:31 UTC

Unfortunately, I'm not familiar with the Windows platform, so can't answer why __declspec is needed. As stated in the original details, this patch was based on the patch supplied at http://www.ndl.kiev.ua/content/patch-aspell-0605-win32-compilation so perhaps they could shed some light?

It may be something to do with the way php_pspell.dll links to it.

[aspell-helper]

Kevin Atkinson kevina\@sf commented on 2011-07-04 12:31:28 UTC

Ok, now I see. This will make accepting your patch a bit more difficult. I _might_ have to defer until I can work without a more experienced C++ developer for the Windows platform.