Don't lock in a specific version of Lucene in the help feature
Closed this issue · 24 comments
The help feature includes Lucene bundles.
eclipse.platform.releng/features/org.eclipse.help-feature/feature.xml
Lines 59 to 79 in acb9634
This locks in a very specific version. The only bundle that actually requires Lucene allows a range of versions:
Does anything speak against removing the includes?
Go for it for next M1
The problem is that we've been on the "everyone use the latest" path and now people want to to use 9.5.0, e.g., Mylyn wants that. Of course people also want the Platform in their target platform and they want to be able to launch a self-hosted IDE, which means they have multiple versions of Lucene in their target platform, which is a complete nightmare mess...
Anyway, let's first see if this breaks anything...
I might be wrong but last time I suggested such changes it was argued that:
- source bundles might not be included
- bundle might not be included in update-sites
but if this is no longer an issue I think actually any third party dependency can be removed from platform features making them much more flexible (less version bumps and so on...)
Includes have been removed all over the place. That's been the general trend. It's even been suggested that all 3rd party dependencies be removed from includes in general. In other projects I've created a "dependencies"feature and put 3rd party bundles there to include them easily in the update site without including them in any user-installed feature...
FYI, in this issue we are discussing the details around using an m2e target location to create wrappers for Lucene 9.5.0...
Will the Platform be able/willing to upgrade to such a new version of Lucene?
Lucene upstream jars are not osgi so still Orbit wrappers are used and I've been the one doing the last few. So if there are already created wrappes that would be more than welcome.
Is orbit on github already? If not won't it be better to move there, enable dependabot, getting dependency updates notification, and so on? Or is it now the goal to replace orbit with some "oomph maven wrapped bundles" service?
I'm waiting for these reviews to complete, though the Platform doesn't use these specifically:
https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/8488
https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/8486
But yes, I already have drop-in replacement wrappers for Lucene 9.5.0 that I've been testing with Mylyn and with DataTools.
I'm testing with a self hosted launch with these new Lucene wrappers, but there are other problems that seem unrelated:
The Help view appears to work, but I don't know if that tells me anything...
Orbit is here:
https://github.com/eclipse/orbit
But they want to use this organization:
https://github.com/eclipse-orbit
Let's discuss goals for Orbit elsewhere. Here are their issues:
I don't think this is an orbit issue in general, you asked if platform can use the "whatever named new service" and platform already using orbit, so why not update the orbit versions so it seems not really we need a decision then?
It sounds like you want to discuss eclipse-orbit/orbit#8 here where few will see it so I suggest you discuss Orbit's plans on that Orbit issue where many will see the discussion.
I also think we should remove the lucense bundles from the feature.
Lucene upstream jars are not osgi so still Orbit wrappers are used and I've been the one doing the last few. So if there are already created wrappes that would be more than welcome.
The issue to request that is now on GH: apache/lucene#8573
It looks like lucene removed the split-packages with version 9:
https://github.com/apache/lucene/blob/main/lucene/MIGRATE.md#migration-from-lucene-8x-to-lucene-90
This would make upstream metadata much simpler.
From my preliminary analysis the only difficulty left is the use of ServiceLoader, but this can be tackeled with the usual ServierLoader Medaitor approach. Or one could use fragments, but I have not yet checked if that fits well into the existing bundle structure.
The biggest problem I have, is that they use Gradle and I don't know Gradle. Does anybody of you know it and can help to configure the bnd-gradle-plugin?
This locking in of a specific gson version is also annoying:
This is made worse by the fact that the metadata of the direct-from-maven bundle is different from that of the orbit wrapped bundle:
And lsp4j appears to be relying on the internal packages. But that's another problem...
This locking in of a specific gson version is also annoying:
Also I'm quite sure you are aware of it I just wanted to note for others that no one is really "locking" a version in a feature but this is an intrinsic feature of how feature.xml works, what is that included plugins/features are resolved (and replaced) by the version of the resolved build time artifact if not already specify a hard version requirement.
Last time I'd attempted to change that it resulted in a lot of complaints and was claimed that no one needs this and it behaves like every one wants it, so its a bit strange to complain now about this, the only solution would be to strip the gson feature completely...
Yes indeed, features are doing exactly what they are intended to do. I don't think we should change that because that's how an include of a bundle in a features is expected to work and how it's supposed to work.
In this issue I'm not complaining about the behavior---I'm not even complaining---I'm suggesting to remove the include, and asking what speaks against that.
It's been suggested in general to remove includes of 3rd party bundle includes from features:
eclipse-orbit/orbit#8 (comment)
As you've noted previously, now that Tycho supports many more useful things such as automatically including source bundles for otherwise included non-source bundles there seems to be less that speaks against this approach. (I suppose there might remain a concern about whether specific sources are installed in the Eclipse SDK, but no one has mentioned that.) Often other projects want specific 3rd party dependencies in their update site, but that can be accomplished by specifying the bundles directly in the category.xml or by creating a dependencies feature (one that the user does not install), including the bundles there, and including that in the category.xml. I have taken these approaches with several other projects, and specifically recently with Mylyn...
But I fully understand @akurtakov concern that it's rather late in the cycle to alter this without risk.
I'm suggesting to remove the include, and asking what speaks against that.
I support this approach. Inclusion in features now seems like a pre-p2 utility when features where distributed as zips to unpack in the eclipse installation; but with p2 fetching transitive deps and Platform building its repo with includeAllDependencies to include all these transitive deps, feature inclusions are for Platform most often a source of problems and extra-work than a help.
Removing the problem is 10x more valuable than trying to workaround it.
For the record, my main concern is not the general one about removal of Lucene from feature.xml but rather the fact about opening Help system to get new Lucene versions updated - Lucene is very strict with index versions (every minor version gets new version), Help system supports single version only and if new Lucene version is pushed by another plugin that would render would all pre-built help indexes useless causing user visible warnings about that, projects having to rebuild their documentation bundles and etc.
With all that said - Lucene is pretty fast at indexing already so these pre-build indexes served their point so a good solution might be to stop shipping pre-build indexes and let them be indexed on first usage.
I'm suggesting to remove the include, and asking what speaks against that.
Nothing special, but if you remove the gson bundle from the gson feature whats left there? :-)
Indeed, I've grown tried of doing qualifier bundles when updating the *.target!
There is no gson feature there is a tips feature:
Yes, I wondered about the indexing, because I've seen issues about such warnings. Consider this though:
Just because the feature requires to install a specific version does not mean that the bundle itself will be wired to that version. It's happy with either version. So yes, I think it would be better to stop pre-indexing even if we didn't remove the include...
Yes, you're correct about the resolution of lucene to newer version if available, still I find it a bit risky to do now as the Help system is not fun to debug and fix at all if smth breaks.
My vote goes for stop pre-indexing, deprecate the extension point and remove Lucene from the feature.xmls for 2023-09.
Alright I was confused because it uses an icon very similar to the feature icon here:
No problem! There's so much detail!!
I agree. We've lived with Lucene duplicates for a long time, so it's safest to wait for the next release rather to change things during RC1.
FYI,
I will meet with @jonahgraham next week to discuss Orbit. Then we can hopefully build new versions of Lucene 9.6 which we can contribute to the target platform...
I will meet with @jonahgraham next week to discuss Orbit. Then we can hopefully build new versions of Lucene 9.6 which we can contribute to the target platform...
Sounds good, I assume this also includes your new Maven-SimRel-Orbit and the general handling of Maven-targets?
Looking forward to the results. I already thought about, if it would be helpful to provide a little presentation/demo on how to set up/use Maven target properly in an Eclipse project that contributes to the SimRel.
Maybe this would convince more projects to use Maven-Targets directly.