search

rrthomas / enchant

enchant spellchecking library
http://rrthomas.github.io/enchant/
GNU Lesser General Public License v2.1
347 stars 60 forks source link

Replace Getopt with GLib option parsing #378

Open danyeaw opened 6 months ago

danyeaw commented 6 months ago

Gvsbuild has a very old version of Enchant from an old XChat fork. Since Getopt isn't very cross-platform, would it be possible to replace it with GLib's command parsing functionality?

rrthomas commented 6 months ago

Did you have an example in mind? I've never had a report of a system on which Enchant won't build owing to getopt. GitHub CI builds it on macOS, Windows and Ubuntu. getopt(3) is a POSIX standard since at least 1996. I can always use the gnulib polyfill if necessary.

Having said that, I would happily accept a patch to the vala-commands branch to use GLib (i.e. written in Vala). Since I am in the process of rewriting the commands in Vala, I'll only be making essential bug fixes to the current C versions from now on.

danyeaw commented 6 months ago

Hi @rrthomas, yup, Windows with MSVC doesn't have POSIX GNU tools and libraries, here is the traceback:

(tar) Exporting enchant
Building project enchant (2.7.3)
        mkdir .\..\bin
        mkdir .\..\bin\release
        mkdir .\..\bin\release
A subdirectory or file .\..\bin\release already exists.
        mkdir .\..\bin\release\obj
        mkdir .\..\bin\release\pdb
        cl -I.\..\src  -I.\..  -Ic:\usr\include  -IC:\gtk-build\gtk\x64\release\include\glib-2.0  -IC:\gtk-build\gtk\x64\release\include\glib-2.0\glib  -IC:\g
tk-build\gtk\x64\release\include\glib-2.0\gmodule  -IC:\gtk-build\gtk\x64\release\include\glib-2.0\..\..\lib\glib-2.0\include -FI ".\..\src\config.h"  -MD  -W
1  -EHsc  -DNDEBUG -D_NDEBUG -Ox -MP2  -DWINDOWS  -D_WINDOWS  -DWIN32  -D_WIN32  -DUNICODE  -D_UNICODE  -Fo".\..\bin\release\obj\\" /favor:AMD64 /D_WIN64 -c .\..\src\enchant.c -D_ENCHANT_BUILD=1  -DWINDLL
Microsoft (R) C/C++ Optimizing Compiler Version 19.39.33523 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

enchant.c
.\..\src\enchant.c(38): fatal error C1083: Cannot open include file: 'getopt.h': No such file or directory
NMAKE : fatal error U1077: 'cl -I.\..\src  -I.\..  -Ic:\usr\include  -IC:\gtk-build\gtk\x64\release\include\glib-2.0  -IC:\gtk-build\gtk\x64\release\include\g
lib-2.0\glib  -IC:\gtk-build\gtk\x64\release\include\glib-2.0\gmodule  -IC:\gtk-build\gtk\x64\release\include\glib-2.0\..\..\lib\glib-2.0\include -FI ".\..\sr
c\config.h"  -MD  -W1  -EHsc  -DNDEBUG -D_NDEBUG -Ox -MP2  -DWINDOWS  -D_WINDOWS  -DWIN32  -D_WIN32  -DUNICODE  -D_UNICODE  -Fo".\..\bin\release\obj\\" /favor:AMD64 /D_WIN64 -c .\..\src\enchant.c -D_ENCHANT_BUILD=1  -DWINDLL' : return code '0x2'
Stop.

Making this part of the updates as you move the commands to Vala sounds like a great plan.

rrthomas commented 6 months ago

Windows with MSVC doesn't have POSIX GNU tools and libraries

I'd be happy to add support for proprietary toolchains as paid work.

Otherwise, please try rrthomas/master where I have added the gnulib getopt-posix module, which should fix this problem. Please let me know either way.

rrthomas commented 6 months ago

I would also accept a patch to the GitHub Actions to build Enchant on Windows with MSVC, once it's confirmed working.

danyeaw commented 6 months ago

Hi @rrthomas,

Here is what I get:

(git) Cloning https://github.com/rrthomas/enchant.git to C:\gtk-build\src\git-exp\enchant
Cloning into 'C:\gtk-build\src\git-exp\enchant'...
remote: Enumerating objects: 8983, done.
remote: Counting objects: 100% (1490/1490), done.
remote: Compressing objects: 100% (434/434), done.
remote: Total 8983 (delta 1044), reused 1249 (delta 955), pack-reused 7493
Receiving objects: 100% (8983/8983), 9.65 MiB | 1.43 MiB/s, done.
Resolving deltas: 100% (6124/6124), done.
Already on 'master'
Your branch is up to date with 'origin/master'.
(git) Exporting directory C:\gtk-build\build\x64\release\enchant
Building project enchant (2.7.3)
        mkdir .\..\bin
        mkdir .\..\bin\release
        mkdir .\..\bin\release
A subdirectory or file .\..\bin\release already exists.
        mkdir .\..\bin\release\obj
        mkdir .\..\bin\release\pdb
        cl -I.\..\src  -I.\..  -Ic:\usr\include  -IC:\gtk-build\gtk\x64\release\include\glib-2.0  -IC:\gtk-build\gtk\x64\release\include\glib-2.0\glib  -IC:\g
tk-build\gtk\x64\release\include\glib-2.0\gmodule  -IC:\gtk-build\gtk\x64\release\include\glib-2.0\..\..\lib\glib-2.0\include -FI ".\..\src\config.h"  -MD  -W
1  -EHsc  -DNDEBUG -D_NDEBUG -Ox -MP2  -DWINDOWS  -D_WINDOWS  -DWIN32  -D_WIN32  -DUNICODE  -D_UNICODE  -Fo".\..\bin\release\obj\\" /favor:AMD64 /D_WIN64 -c .\..\src\enchant.c -D_ENCHANT_BUILD=1  -DWINDLL
Microsoft (R) C/C++ Optimizing Compiler Version 19.39.33523 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

enchant.c
.\..\src\enchant.c(38): fatal error C1083: Cannot open include file: 'getopt.h': No such file or directory
NMAKE : fatal error U1077: 'cl -I.\..\src  -I.\..  -Ic:\usr\include  -IC:\gtk-build\gtk\x64\release\include\glib-2.0  -IC:\gtk-build\gtk\x64\release\include\g
lib-2.0\glib  -IC:\gtk-build\gtk\x64\release\include\glib-2.0\gmodule  -IC:\gtk-build\gtk\x64\release\include\glib-2.0\..\..\lib\glib-2.0\include -FI ".\..\sr
c\config.h"  -MD  -W1  -EHsc  -DNDEBUG -D_NDEBUG -Ox -MP2  -DWINDOWS  -D_WINDOWS  -DWIN32  -D_WIN32  -DUNICODE  -D_UNICODE  -Fo".\..\bin\release\obj\\" /favor:AMD64 /D_WIN64 -c .\..\src\enchant.c -D_ENCHANT_BUILD=1  -DWINDLL' : return code '0x2'
Stop.

Yup, I can contribute a GitHub Action.

rrthomas commented 6 months ago

I don't understand how the build is run here, maybe you can help. In particular, Makefile.am contains:

AM_CPPFLAGS = -I$(top_srcdir) $(ISYSTEM)$(top_builddir)/libgnu $(ISYSTEM)$(top_srcdir)/libgnu -I$(top_srcdir)/lib $(GLIB_CFLAGS) $(WARN_CFLAGS) -DG_LOG_DOMAIN='"libenchant"'

Note the two occurrences of libgnu. This is the directory that contains getopt.h after bootstrap is run.

But I don't see libgnu mentioned anywhere in the invocation of the C compiler above.

rrthomas commented 6 months ago

Yup, I can contribute a GitHub Action.

Great, a PR that adds a suitable stanza to .github/workflows/c-cpp.yml would be much appreciated.

danyeaw commented 6 months ago

I looked at this more closely on how the build is working on Windows. We are patching in a NMake makefile.mak which is posted here: https://github.com/wingtk/gvsbuild/blob/main/gvsbuild/patches/enchant/src/makefile.mak

nmake /nologo -f makefile.mak DLL=1 X64=1 MFLAGS=-MD GLIBDIR=%(gtk_dir)s\include\glib-2.0

So your changes on your fork aren't getting pulled in because we aren't using the bootstrap script (no bash) or GNU Make. This might be a larger project than just replacing Getopt.

rrthomas commented 6 months ago

OK, so this is not an Enchant issue. Thanks!

rrthomas commented 6 months ago

Reopening, as I will use GLib's command-line parser in the Vala commands.

rrthomas commented 6 months ago

Unfortunately, GOption doesn't support long options that start with a single dash, so I won't be able to use it for enchant-lsmod until version 3 (see #374), i.e. breaking backwards compatibility.