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

Upgrade morfologik from 1.10.0 to 2.0.1 #342

Closed
wants to merge 1 commit into from

Conversation

aldenquimby
Copy link
Contributor

This also upgrades carrotsearch hppc, which will make languagetool 3.2 compatible with other projects that depend on the newest version of hppc.

TODO

  • Regenerate dictionaries to fix the following issues, which appear to be causing all test failures:
    • Use an explicit fsa.dict.encoder=SUFFIX metadata key
    • Deprecated encoder keys in metadata. Use fsa.dict.encoder=SUFFIX

@danielnaber
Copy link
Member

Thanks! I wonder why you also added commons-cli?

@aldenquimby
Copy link
Contributor Author

I had to add commons-cli because standalone + tools use it but morfologik no longer does, so it's not pulled in transitively. See below for the relevant output from mvn dependency:tree -Dverbose -Dincludes=commons-cli before and after my commit.

Would you like me to move the version into parent properties? It looks like you have this for some dependencies but not all, so I wasn't sure which you'd prefer.

Also do you have documentation on dictionary generation? It looks like the explicit SUFFIX error will make this upgrade more painful than I originally thought. I'm not sure how to proceed with re-generating dictionaries. RuleTest#testJavaRules is an example of a failing test.

before morfo upgrade


[INFO] org.languagetool:languagetool-standalone:jar:3.2-SNAPSHOT
[INFO] \- org.carrot2:morfologik-tools:jar:1.10.0:compile
[INFO]    \- commons-cli:commons-cli:jar:1.2:compile

...

[INFO] org.languagetool:languagetool-wikipedia:jar:3.2-SNAPSHOT
[INFO] \- commons-cli:commons-cli:jar:1.2:compile

...

[INFO] org.languagetool:languagetool-tools:jar:3.2-SNAPSHOT
[INFO] \- org.carrot2:morfologik-tools:jar:1.10.0:compile
[INFO]    \- commons-cli:commons-cli:jar:1.2:compile

...

[INFO] org.languagetool:languagetool-dev:jar:3.2-SNAPSHOT
[INFO] +- org.languagetool:languagetool-wikipedia:jar:3.2-SNAPSHOT:compile
[INFO] |  \- commons-cli:commons-cli:jar:1.2:compile
[INFO] +- org.apache.hadoop:hadoop-mapreduce-client-core:jar:2.4.0:compile
[INFO] |  \- org.apache.hadoop:hadoop-yarn-common:jar:2.4.0:compile
[INFO] |     \- (commons-cli:commons-cli:jar:1.2:compile - omitted for duplicate)
[INFO] \- org.apache.hadoop:hadoop-common:jar:2.4.0:compile
[INFO]    \- (commons-cli:commons-cli:jar:1.2:compile - omitted for duplicate)

after morfo upgrade

[INFO] org.languagetool:languagetool-standalone:jar:3.2-SNAPSHOT
[INFO] \- commons-cli:commons-cli:jar:1.2:compile

...

[INFO] org.languagetool:languagetool-wikipedia:jar:3.2-SNAPSHOT
[INFO] \- commons-cli:commons-cli:jar:1.2:compile

...

[INFO] org.languagetool:languagetool-tools:jar:3.2-SNAPSHOT
[INFO] \- commons-cli:commons-cli:jar:1.2:compile

...

[INFO] org.languagetool:languagetool-dev:jar:3.2-SNAPSHOT
[INFO] +- org.languagetool:languagetool-wikipedia:jar:3.2-SNAPSHOT:compile
[INFO] |  \- commons-cli:commons-cli:jar:1.2:compile
[INFO] +- org.apache.hadoop:hadoop-mapreduce-client-core:jar:2.4.0:compile
[INFO] |  \- org.apache.hadoop:hadoop-yarn-common:jar:2.4.0:compile
[INFO] |     \- (commons-cli:commons-cli:jar:1.2:compile - omitted for duplicate)
[INFO] \- org.apache.hadoop:hadoop-common:jar:2.4.0:compile
[INFO]    \- (commons-cli:commons-cli:jar:1.2:compile - omitted for duplicate)

@danielnaber
Copy link
Member

SpellDictionaryBuilder should be able to re-create the dicts. Some documentation, probably only partially useful for this case, is at http://wiki.languagetool.org/hunspell-support#toc4. About the version properties: as it's used from two pom.xmls it makes sense to introduce a property I think.

@danielnaber
Copy link
Member

Jaume has made the update, so I will close this request. Please let us know if you think there are still problems.

@danielnaber danielnaber closed this Apr 2, 2016
@aldenquimby
Copy link
Contributor Author

Thanks @danielnaber, upgrading from 3.1 to 3.3 fixes the version conflict I had for com.carrotsearch:hppc.

However, the upgrade also produces the following dependency convergence error with maven-enforcer-plugin:

Dependency convergence error for org.carrot2:morfologik-stemming:2.1.0 paths to dependency are:
  +-org.languagetool:language-en:3.3
    +-org.languagetool:languagetool-core:3.3
      +-org.carrot2:morfologik-speller:2.1.0
        +-org.carrot2:morfologik-stemming:2.1.0
and
  +-org.languagetool:language-en:3.3
    +-org.languagetool:languagetool-core:3.3
      +-org.carrot2:morfologik-speller:2.1.0
        +-org.carrot2:morfologik-polish:2.0.0
          +-org.carrot2:morfologik-stemming:2.0.0
and
  +-org.languagetool:language-en:3.3
    +-org.languagetool:languagetool-core:3.3
      +-org.carrot2:morfologik-stemming:2.1.0

I think this issue was introduced with morfologik/morfologik-stemming@3ada8d8 - the morfologik-polish dependency uses a hard 2.0.0 instead of ${project.version}.

Any thoughts on this? I can reach out to dweiss if you think that makes sense

@danielnaber
Copy link
Member

Yes, sounds like a problem on the morfologik side if I get it correctly. Would be nice if you could contact them.

@aldenquimby
Copy link
Contributor Author

Sounds good, I will keep you posted on the status there - if a 2.1.1 patch gets pushed out, it would be great to see a new languagetool release that uses it.

Also, it would be nice if languagetool enforced dependency convergence, so that any user enforcing it in their project could count on convergence within the single languagetool dependency. It should speed up development time too if you ever have clashing dependency versions - instead of finding out at runtime that the wrong version of a library is on the classpath, you'll get an error in the mvn validate phase before compile.

@danielnaber
Copy link
Member

Sounds good, a pull request to the pom.xml is welcome.

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

Successfully merging this pull request may close these issues.

2 participants