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

Added net6.0 target to Lucene.Net.Analysis.OpenNLP and changed to using MavenReference #892

Conversation

NightOwl888
Copy link
Contributor

@NightOwl888 NightOwl888 commented Jan 13, 2024

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a change, please open an issue to discuss the change or find an existing issue.

Summary of the changes (Less than 80 chars)
Added net6.0 target to Lucene.Net.Analysis.OpenNLP and changed to using MavenReference

Closes #852, closes #460, related: #890

Description

This supersedes and closes #852. It drops the package dependency on OpenNLP.NET, and instead uses <MavenReference> to add a dependency on opennlp-tools.

<MavenReference> does not add a binary to our distribution (except for IKVM and its dependencies). Instead, it transitively gets added to any SDK-style project that depends on Lucene.Net.Analysis.OpenNLP. Each dependency from Maven is downloaded and converted from bytecode into IL on the user's build machine.

This allows the user to add additional <MavenReference> dependencies, which will use Maven to resolve any versioning conflicts between common dependencies. More importantly, this ensures that only one copy of each data type exists in the project, which wouldn't be the case with a dependency on OpenNLP.NET. It also allows adding other OpenNLP components as required by the project, rather than having one binary that incorporates all of them.

The documentation from Lucene 8.2.0 was also brought over and a MavenReference demo was created to demonstrate how it can be used to pull in libraries from Maven to build analyzers that are compatible with Lucene.NET.

This PR also includes a new feature - Lucene.Net.Analysis.Miscellaneous.TypeAsSynonymFilter which is helpful for working with the filters in Lucene.Net.Analysis.OpenNLP.

laimis and others added 19 commits January 20, 2024 16:05
…61 is selected and net7.0 when net5.0 is selected. In CI, we set IsTestProject=false and IsPublishable=false to skip these tests.
…and net6.0 for Lucene.Net.Tests.Analysis.OpenNLP tests.
…Services.Unsafe for netstandard2.0 and net462 to ensure the version will work with any combination of Lucene.Net components. This is a transitive dependency in a few 3rd party DLLs, but there may be version conflicts if this isn't done on .NET Framework.
…or netstandard2.0 and net462, since it is being used in Lucene.Net.Facet.
… to pin the version so it matches the reference of IKVM 8.5.0 (6.0.6).
…d to use <MavenReference> to build opennlp-tools instead of using the pre-built OpenNLP.NET NuGet package.
…ly compile with IKVM in the mix, both on .NET Framework and .NET Core
…to eliminate build warnings (at least 1 type is referenced in opennlp-tools)
@NightOwl888 NightOwl888 force-pushed the opennlp-dotnet-core-support-mavenreference branch from bfbe2e7 to 57df67c Compare January 20, 2024 09:06
@paulirwin
Copy link
Contributor

I'm not entirely sure why from reviewing the code, but this build is failing on macOS arm64, and the error is in the net472 target:

error MSB4018: The "IkvmCompiler" task failed unexpectedly. [{...}/src/Lucene.Net.Analysis.OpenNLP/Lucene.Net.Analysis.OpenNLP.csproj::TargetFramework=net472]

I don't see a corresponding change that would obviously cause it to try to run a .NET FX build on macOS, but something is amiss.

… on Windows due to lack of non-Windows build support in IKVM 8.7.3 (see: ikvmnet/ikvm-maven#49).
@paulirwin
Copy link
Contributor

Update from my prior comment: this now builds with skipping the netfx target on macOS. Approved.

…build issues with opennlp-uima on 1.9.4. This aligns with Lucene 8.2.0.
… from Lucene 8.2.0 because it is called out in the docs as part of the process of configuring Lucene.Net.Analysis.OpenNLP. Changed CannedTokenStream to set ITypeAttribute.Type because it is required by the tests for TypeAsSynonymFilter.
…ed comment with lucene version compatibility level (to indicate we ported it from Lucene 8.2.0)
…which filters are included in the package (there is no NER filter in the box)
@NightOwl888 NightOwl888 merged commit 41ad676 into apache:master Feb 1, 2024
200 checks passed
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.

NLP Support (OpenNLP)
3 participants