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

ASpell is still enabled by default although the ChangeLog says it's disabled by default #1493

Closed
ryandesign opened this issue Apr 20, 2024 · 4 comments

Comments

@ryandesign
Copy link
Contributor

ASpell is disabled by default now as of #1376 but ./configure --help still says it's enabled by default:

Optional Features:
  --disable-aspell        Build without ASpell support (default is enabled)

link-grammar/configure.ac

Lines 532 to 536 in 3a26127

do_aspell=yes
# ASpell Support is handled here
AC_ARG_ENABLE([aspell], [AS_HELP_STRING([--disable-aspell],
[Build without ASpell support (default is enabled)])],
[do_aspell="${enableval}"])

@ryandesign
Copy link
Contributor Author

Actually it looks like ASpell is still enabled by default, just as configure help says it is.

ASpell is disabled if HunSpell is found. But if HunSpell is not installed and ASpell is, then link-grammar does enable ASpell, and at the end of the configure output it prints:

   ----------
   Caution: Aspell version 0.60.8 and possibly others leak memory!
   Do not enable spell-guessing in production servers!
   ----------

This is unexpected given that the last ChangeLog entry that mentions ASpell says:

* Disable aspell; it leaks memory. #1373

@ryandesign ryandesign changed the title ASpell is disabled by default but configure help says it is enabled by default ASpell is still enabled by default although the ChangeLog says it's disabled by default Apr 20, 2024
@ampli
Copy link
Member

ampli commented Apr 20, 2024

@linas,
I propose to change configure as follows:

  • If hunspell is not found, issue a warning.
  • Set the default for aspell to "disabled".

@linas
Copy link
Member

linas commented Apr 25, 2024

I did open a bug report w/aspell around the time I first spotted that mem leak (more than a year ago, when #1373 was opened) and the developer did eventually fix it (about 3 months after opening the bug report) So now we wait until all the distros pick up the newest version of aspell. Current Debian stable is still aspell 0.60.8

Solution is to leave aspell enabled by default. It only leaks maybe 32 bytes per call, so even after a million calls, its still only 32MBytes and so more-or-less unnoticable. I hit this with opencog, where I was doing billions of calls and I special-case worked around it, there.

The ChangeLog and the bug summary #1376 was wrong; the actual commit just prints a warning. I think that's good enough.

@linas
Copy link
Member

linas commented Apr 25, 2024

Closing; I fixed the ChangeLog in #1515 If there's something I've missed, open a new bug report.

@linas linas closed this as completed Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants