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

Explicit lucene version #389 #390

Closed
wants to merge 1 commit into from
Closed

Explicit lucene version #389 #390

wants to merge 1 commit into from

Conversation

gnl42
Copy link
Contributor

@gnl42 gnl42 commented Jan 31, 2024

Strange missing class error after doing a Clean Up of Tasks

Tasks is using an explicit reference to 9.5.0, p2 -> 9.5.1.
Changing the version to 0.0 .0 clears the error

Task-Url: #389

@gnl42 gnl42 requested review from merks and ruspl-afed January 31, 2024 07:38
Copy link
Contributor

@ruspl-afed ruspl-afed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have strong objections.
WDYT @merks ?

@merks
Copy link
Contributor

merks commented Jan 31, 2024

Give me a few minutes. I'd like to understand how this is possible.

@gnl42
Copy link
Contributor Author

gnl42 commented Jan 31, 2024

It's possible you may have clean up some bad conversions

@merks
Copy link
Contributor

merks commented Jan 31, 2024

I don't know which project had the error being fixed:

#389 (comment)

I don't have a big objection to doing what's necessary to fix the errors, but it all seems very strange to me.

@akurtakov
Copy link
Contributor

Not having lower boundary opens the door for a case where the package is there but the API is not what we expect. I understand this is quite unlikely as Platform will bring new enough Lucene in most products but still something to be considered.

org.apache.lucene.search;version="9.5.0",
org.apache.lucene.store;version="9.5.0",
org.apache.lucene.util;version="9.5.0"
Import-Package: org.apache.lucene.analysis;version="0.0.0",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should not use a version of 0.0.0 here, thats the default and only highly confusing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately the discussion is split better the PR and the issue. I think the appropriate fix is what I show here:

#389 (comment)

@laeubi
Copy link

laeubi commented Jan 31, 2024

I don't understand what this should fix either... 9.5.0 is a open range and includes every higher version (including 9.5.1), this fix looks suspicious and more likely is a sign of wrong dependency chains.

Copy link
Contributor

@ruspl-afed ruspl-afed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gnl42 what if try to import Lucene package(s) for bundles that started to require it after clean-up?

@gnl42 gnl42 closed this Jan 31, 2024
@gnl42
Copy link
Contributor Author

gnl42 commented Jan 31, 2024

Adding explicit import to the test project seems to fix the issue. Somthing to watch out for

@laeubi
Copy link

laeubi commented Jan 31, 2024

Adding explicit import to the test project seems to fix the issue. Somthing to watch out for

Nooo please see my comment please check if you need to update the BREE instead!

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.

5 participants