Community
Participate
Working Groups
In bug 518351 it's planned to remove update.configurator but pde.core still uses it to consult old installations (pre 3.4). It's useless feature which should be gone now to ease maintenance.
New Gerrit change created: https://git.eclipse.org/r/111763
Gerrit change https://git.eclipse.org/r/111763 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=f6a86aef93fd0774831a4f7e02e3d97c25ff8d2e
New Gerrit change created: https://git.eclipse.org/r/112285
Gerrit change https://git.eclipse.org/r/112285 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=5f91451096c94318a509f4a6d8d43aa2d10652fa
New Gerrit change created: https://git.eclipse.org/r/112286
(In reply to Eclipse Genie from comment #4) > Gerrit change https://git.eclipse.org/r/112285 was merged to [master]. > Commit: > http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=5f91451096c94318a509f4a6d8d43aa2d10652fa > This reverted the previous change. Can you give a short explanation why it was reverted?
ITargetPlatformService.newFeatureLocation can no longer properly read the contents as it tries to read it as p2 repo but for the tests old update style is used. Maybe we should mandate feature location to be p2 site from now on?
Dani, Vikas, any concerns with proposal in the previous comment?
1) Will clients be broken ? ( those that use old update style similar to tests). 2) Not sure if making API that existed for a long time should be made no-op and @deprecated in the same go. 3) I think such changes late in the iteration should be avoided ( provided we have convincing answers for 1) and 2) ). Dani, your views ?
(In reply to Vikas Chandra from comment #9) > 1) Will clients be broken ? ( those that use old update style similar to > tests). One can use http://andrei.gmxhome.de/eclipse/ update site which is "p2-free" and totally old-school. If the updates from there aren't working anymore, this would be a no-go from my side :-)
(In reply to Vikas Chandra from comment #9) > 2) Not sure if making API that existed for a long time should be made no-op > and @deprecated in the same go. Combine those steps is definitely a no go unless it was announced before in the removal guide. Note that we now have an API Tools warning for PDE Core; http://download.eclipse.org/eclipse/downloads/drops4/I20171126-2000/apitools/analysis/html/org.eclipse.pde.core/report.html The minor version should be the same for version 3.12.0, since no new APIs have been added since version 3.11.100 ==> An API filter is needed.
(In reply to Vikas Chandra from comment #9) > 1) Will clients be broken ? ( those that use old update style similar to > tests). The question is for how long we plan to support that. IMHO it's time to get rid of it following the deprecation/removal procedure. And IMHO requiring p2 sites nowadays is quite normal. What do you think of this as a goal? (not about procedures, we will exec things according to the book). > 2) Not sure if making API that existed for a long time should be made no-op > and @deprecated in the same go. Yes we should not do it in one go. > 3) I think such changes late in the iteration should be avoided ( provided > we have convincing answers for 1) and 2) ). Actually, it's not that late in the iteration as soon enough we will be doing 3 month long iteration and we still have more than 6 in this one. > > Dani, your views ? (In reply to Dani Megert from comment #11) > (In reply to Vikas Chandra from comment #9) > > 2) Not sure if making API that existed for a long time should be made no-op > > and @deprecated in the same go. > > Combine those steps is definitely a no go unless it was announced before in > the removal guide. So announcing the removal is the next step to do. > > > Note that we now have an API Tools warning for PDE Core; > > http://download.eclipse.org/eclipse/downloads/drops4/I20171126-2000/apitools/ > analysis/html/org.eclipse.pde.core/report.html > > The minor version should be the same for version 3.12.0, since no new APIs > have been > added since version 3.11.100 > > > ==> An API filter is needed. Will be fixed ASAP.
New Gerrit change created: https://git.eclipse.org/r/112547
Created attachment 271700 [details] errors in log on startup If I start latest SDK build 4.8.0.I20171128-2000 I see tons of errors in the error log reported, similar to this: eclipse.buildId=4.8.0.I20171128-2000 java.version=1.8.0_131 java.vendor=Oracle Corporation BootLoader constants: OS=linux, ARCH=x86_64, WS=gtk, NL=en_US Command-line arguments: -os linux -ws gtk -arch x86_64 org.eclipse.update.configurator Error Wed Nov 29 16:10:38 CET 2017 Could not install bundle plugins/org.eclipse.ui.views_3.9.100.v20171030-2117.jar A bundle is already installed with the name "org.eclipse.ui.views" and version "3.9.100.v20171030-2117" Also I see few other errors like: org.osgi.framework.BundleException: Could not resolve module: org.eclipse.emf.ecore.xmi [2849] Another singleton bundle selected: osgi.identity; type="osgi.bundle"; version:Version="2.13.0.v20170609-0707"; osgi.identity="org.eclipse.emf.ecore.xmi"; singleton:="true" at org.eclipse.osgi.container.Module.start(Module.java:444) at org.eclipse.osgi.container.ModuleContainer$ContainerStartLevel.dispatchEvent(ModuleContainer.java:1550) at org.eclipse.osgi.container.ModuleContainer$ContainerStartLevel.dispatchEvent(ModuleContainer.java:1) at org.eclipse.osgi.framework.eventmgr.EventManager.dispatchEvent(EventManager.java:230) at org.eclipse.osgi.framework.eventmgr.EventManager$EventThread.run(EventManager.java:340) Is this known? If not, it must be fixed for M4, otherwise we will be flooded by the bug reports.
This is a known issue tracked in Bug 527783. Should be fixed with today's build.
(In reply to Alexander Kurtakov from comment #12) > > ==> An API filter is needed. > > Will be fixed ASAP. This is still not fixed. See http://download.eclipse.org/eclipse/downloads/drops4/I20180123-2000/apitools/analysis/html/org.eclipse.pde.core/report.html
New Gerrit change created: https://git.eclipse.org/r/115956
Gerrit change https://git.eclipse.org/r/115956 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=f2629918ebe0138ed741c5ad30885d7d01badd8b
(In reply to Eclipse Genie from comment #18) > Gerrit change https://git.eclipse.org/r/115956 was merged to [master]. > Commit: > http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=f2629918ebe0138ed741c5ad30885d7d01badd8b > Thanks!
Can this be marked as fixed?
(In reply to Lars Vogel from comment #20) > Can this be marked as fixed? No, this is still pending task which I haven't found time to look at.
*** Bug 559480 has been marked as a duplicate of this bug. ***
I'm happy to take this cleanup on.
(In reply to Alex Blewitt from comment #23) > I'm happy to take this cleanup on. Thanks
New Gerrit change created: https://git.eclipse.org/r/164019
New Gerrit change created: https://git.eclipse.org/r/164020
New Gerrit change created: https://git.eclipse.org/r/164023
New Gerrit change created: https://git.eclipse.org/r/164034
New Gerrit change created: https://git.eclipse.org/r/164035
New Gerrit change created: https://git.eclipse.org/r/164036
New Gerrit change created: https://git.eclipse.org/r/164037
New Gerrit change created: https://git.eclipse.org/r/164038
New Gerrit change created: https://git.eclipse.org/r/164039
I've worked through the programmatic changes needed to remove this and broken them apart into small reviews (hence the multitude of changes on here). I am having difficulty with the testing, because org.eclipse.performance.tests is missing from the workspace and the README doesn't have any information about what's needed. Unfortunately the gerrit bot hangs so it's not possible ot use that to verify whether or not the tests work as expected. I have done some smoke testing on the changes, but I'm prepared that there may be other scenarios which are worth testing and I'm happy to respond to feedback and reorder where necessary. Finally, some parts of the code appear to depend on a 'pedbuild.jar' and I'm not sure where that's coming from. That has some of the old API in it which may result in problems when being used elsewhere. One possibility is that we could leave the old PluginPathFinder and UpdateManagerHelper into a fragment which attaches to the host plugin, and to extend the fragment to depend on the updateconfigurator, but I think that will prolong the problem. Obviously these changes should be targeted for 4.17 - they are not intended for merging in the 4.16 timeframe.
Can someone summarize what this removal effectively entails? I've never (consciously) used update configurator. Do we just lose support for targets <= 3.3? (In reply to Alex Blewitt from comment #34) > I've worked through the programmatic changes needed to remove this and > broken them apart into small reviews (hence the multitude of changes on > here). I am having difficulty with the testing, because > org.eclipse.performance.tests is missing from the workspace and the README > doesn't have any information about what's needed. Try using the oomph setup: https://wiki.eclipse.org/Eclipse_Platform_SDK_Provisioning > Finally, some parts of the code appear to depend on a 'pedbuild.jar' and I'm > not sure where that's coming from. That has some of the old API in it which > may result in problems when being used elsewhere. That's from the org.eclipse.pde.build bundle. You can clone it via the oomph setup above.
(In reply to Julian Honnen from comment #35) > Can someone summarize what this removal effectively entails? I've never > (consciously) used update configurator. > > Do we just lose support for targets <= 3.3? Before P2, Eclipse used Update Manager to install new plugins and features. Update Manager used update.configurator under the covers to do the plumbing work, and so PDE supported both “Platforms with P2” and “Platforms with Update Manager”. Update manager went away a while ago, which means that the UI is no longer present and users of Eclipse can’t use it any more. Most of the dependencies of update.configurator went away. The only one still present is PDE, because PDE had support for old/new world during P2’s introduction, and those code paths were never removed. As a result, PDE today still has a dependency on update.configurator and is the reason why it’s still in IDEs to this day. Unfortunately, update.configurator runs early on in the boot process and takes (on my machine, at least) ~350ms to start up, for something that’s never going to be used. The goal of this bug is to remove the code paths that lead to update.configurator (I doubt anyone using Eclipse 2020-* is using it to launch a pre-Eclipse 3.3 instance) such that the update.configurator no longer needs to be installed in IDE packages, which will allow for a (slightly) faster startup. > (In reply to Alex Blewitt from comment #34) > > I've worked through the programmatic changes needed to remove this and > > broken them apart into small reviews (hence the multitude of changes on > > here). I am having difficulty with the testing, because > > org.eclipse.performance.tests is missing from the workspace and the README > > doesn't have any information about what's needed. > Try using the oomph setup: > https://wiki.eclipse.org/Eclipse_Platform_SDK_Provisioning OK it might be good if that were mentioned in the README :) I’ll propose another bug/patch for that. > > Finally, some parts of the code appear to depend on a 'pedbuild.jar' and I'm > > not sure where that's coming from. That has some of the old API in it which > > may result in problems when being used elsewhere. > That's from the org.eclipse.pde.build bundle. You can clone it via the oomph > setup above. Sounds good, thanks. I wonder if the same changes will be needed there as well.
Alex, one scenario to test is the support of the dropins folder. (I think Andrey already said this somewhere in one of the reviews) IIRC this used some old code and was one of the reasons why we did not manage to remove update manager even though it was announced to be deleted years ago. You may have covered that already in your Gerrits but just wanted to highlight that again.
(In reply to Lars Vogel from comment #37) > Alex, one scenario to test is the support of the dropins folder. (I think > Andrey already said this somewhere in one of the reviews) > > IIRC this used some old code and was one of the reasons why we did not > manage to remove update manager even though it was announced to be deleted > years ago. You may have covered that already in your Gerrits but just wanted > to highlight that again. That runtime support would be part of Platform/Equinox not PDE, right?
(In reply to Julian Honnen from comment #38) > (In reply to Lars Vogel from comment #37) > > Alex, one scenario to test is the support of the dropins folder. (I think > > Andrey already said this somewhere in one of the reviews) > > > > IIRC this used some old code and was one of the reasons why we did not > > manage to remove update manager even though it was announced to be deleted > > years ago. You may have covered that already in your Gerrits but just wanted > > to highlight that again. > That runtime support would be part of Platform/Equinox not PDE, right? I think so too.
Gerrit change https://git.eclipse.org/r/164020 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=019fbb31a420e07e340e8ddbf81c5433ce7b58f0
Lars noted in https://git.eclipse.org/r/#/c/164023/ that the dialog box was seen when running an RCP application, when it's expected this would not require update manager. I wasn't able to reproduce that, but I was able to reproduce it by creating a dummy OSGi application with the following bundles: 0 ACTIVE org.eclipse.osgi_3.15.300.qualifier 1 ACTIVE org.apache.felix.gogo.runtime_1.1.0.v20180713-1646 2 ACTIVE org.eclipse.equinox.console_1.4.100.v20200525-1407 3 ACTIVE org.apache.felix.gogo.shell_1.1.0.v20180713-1646 Run without this change, ti works as expected. With the change in 164023, it throws the exception when launched as an Eclipse app. When launched as an OSGi app, it works, but in both cases is listing all the installed bundles. Some more work to do here, I think.
Example I used: https://www.vogella.com/tutorials/EclipseRCP/article.html#generatetemplatewithcontent
I've fixed the issue that Lars saw, but I am still working on the Equinox launcher, which appears to be loading many bundles more than it should.
Think the above might be a different issue; I've raised bug 564251 to cover it.
Gerrit change https://git.eclipse.org/r/164023 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=68f3f3f4d52d2039b723ffb3f771281ab4de4c2c
This bug has become a bit of a mess. To summarize: 1) In 2017 it was decided to mark support for non-p2 update sites for removal and leave the functionality in place (comment 12). This affects the following API (please comment if there's more): org.eclipse.pde.core.plugin.TargetPlatform::createPlatformConfiguration AFAICS that removal notice was never sent though. If there are no vetos to keep that ancient functionality, I'll do that now. 2) The recently merged changes removed support for target platforms based on eclipse <= 3.3 3) Any further cleanup is blocked until the API removal process is complete.
> 3) Any further cleanup is blocked until the API removal process is complete. The API was marked for removal 2 years ago, in Jan 2018. https://github.com/eclipse/eclipse.pde.ui/commit/f2629918ebe0138ed741c5ad30885d7d01badd8b We can keep the API in place but change the behaviour to throw an exception instead. That way the API remains in place but has no effect.
Julian, I'll take a look at what we can do to preserve that part of the API and have it behave the same. I don't think it's necessary to stop work on this, because it's only one method from UpdateHelper and we could e.g. in-line the implementation into this method and keep the same behaviour. If you could send out the removal notice and get that running that would be good. I suggest we have a separate bug for that rather than this one though :)
> AFAICS that removal notice was never sent though. If there are no vetos to keep that ancient functionality, I'll do that no The PMC decided that individual announcement is not necessary, that is why we send one note linking to the api removal document per release. So no need to send this individual message.
(In reply to Lars Vogel from comment #49) > The PMC decided that individual announcement is not necessary, that is why > we send one note linking to the api removal document per release. So no need > to send this individual message. I can't find it in PMC archives or removal guide either. https://git.eclipse.org/c/platform/eclipse.platform.common.git/tree/bundles/org.eclipse.platform.doc.isv/porting/removals.html
(In reply to Julian Honnen from comment #50) > (In reply to Lars Vogel from comment #49) > > The PMC decided that individual announcement is not necessary, that is why > > we send one note linking to the api removal document per release. So no need > > to send this individual message. > I can't find it in PMC archives or removal guide either. > https://git.eclipse.org/c/platform/eclipse.platform.common.git/tree/bundles/ > org.eclipse.platform.doc.isv/porting/removals.html I assumed that https://bugs.eclipse.org/bugs/show_bug.cgi?id=311590 would also cover any dependent API (which I think we have here). If others think we need a separate removal for this (IMHO dependent) PDE API, then +1 to mark it for removal.
(In reply to Lars Vogel from comment #51) > I assumed that https://bugs.eclipse.org/bugs/show_bug.cgi?id=311590 would > also cover any dependent API (which I think we have here). If that's the case, great but the removal document lists only 1. Update Manager Bundle org.eclipse.update.core from the old update manager API was removed. This API was marked for deletion in the 4.2. release via bug 311590. and not the org.eclipse.update.configurator bundle PDE dependent on. If we're not covered already by that notice we should mark update.configurator for removal now as well.
(In reply to Julian Honnen from comment #52) > (In reply to Lars Vogel from comment #51) > > I assumed that https://bugs.eclipse.org/bugs/show_bug.cgi?id=311590 would > > also cover any dependent API (which I think we have here). > If that's the case, great but the removal document lists only > > 1. Update Manager > Bundle org.eclipse.update.core from the old update manager API was removed. > This API was marked for deletion in the 4.2. release via bug 311590. > > > and not the org.eclipse.update.configurator bundle PDE dependent on. If > we're not covered already by that notice we should mark update.configurator > for removal now as well. From https://bugs.eclipse.org/bugs/show_bug.cgi?id=311590 ----------- The bundles to be removed are: - org.eclipse.update.configurator - org.eclipse.update.core - org.eclipse.update.scheduler - org.eclipse.update.ui -------------- Maybe the API removal process was handled different in 2010. https://bugs.eclipse.org/bugs/show_bug.cgi?id=311590#c8 says the only reason why the bundles were not deleted is that they are used and that is was not worth the effort. Quote: ----------- org.eclipse.update.core is left in master because it is still used in some of our tests. org.eclipse.update.configurator is still shipped in the platform because it it used quite extensively in PDE and I felt it was not worth the effort to migrate. It is only 3 API classes so a tiny fraction of old update. ------------ So IMHO , we should be fine to delete these. Alex, WDYT? Other PMC members, any opinion?
There’s one public api and we should still be able to reimplement it without needing to keep the bundle. I don’t think we should block the removal of the bundle but there’s an orthogonal aspect of whether the API here is changed or not. If we can remove it, great. But I think we can put in an emulator shim for that method and allow us to remove the update configuration and dependency on PDE. I’m working on a new version of the change that Julian commented on to see if we can do this.
(In reply to Alex Blewitt from comment #54) > There’s one public api and we should still be able to reimplement it without > needing to keep the bundle. I don’t think we should block the removal of the > bundle but there’s an orthogonal aspect of whether the API here is changed > or not. > > If we can remove it, great. But I think we can put in an emulator shim for > that method and allow us to remove the update configuration and dependency > on PDE. I’m working on a new version of the change that Julian commented on > to see if we can do this. This sounds like perfect solution to me. I vote for keeping explicit notifications for things to be removed.
I've rebased and resubmitted the patches; they all test/build clean now. I've left the public API in place but put in a re-implementation for the transitional period between now and the subsequent removal. Lars/Julian did you send out the removal process notification? Should we have a separate bug for the removal of org.eclipse.pde.core.plugin.TargetPlatform::createPlatformConfiguration and the UpdateManagerHelperDeprecated introduced in https://git.eclipse.org/r/#/c/164038/11/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/UpdateManagerHelperDeprecated.java
Gerrit change https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/164034 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=5b2dbe0e80306bdba613235ea6525d72e41dfd20
Gerrit change https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/164036 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=500acb2d18dca530c850b50bff032b64933c6207
Gerrit change https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/164035 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=a113ae7bf4dc063e902ba7a1e700e80d8f40570b
Gerrit change https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/164037 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=e3dfdbd23673d77d18a016b1bee0c4a593b42888
Gerrit change https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/165560 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=cda7ad1ba8e4297e3b7a46bf6abc4863e69a1741
Gerrit change https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/164038 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=11a09692b07fd57ba1e9e85f225edff215eb5a0f
Gerrit change https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/164039 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=386e461afa5eefb733cb0a2ac864dc230b0fe05b
*** Bug 539475 has been marked as a duplicate of this bug. ***
The update.configurator dependency is now gone thanks to Alex's great work. I'll close this bug once we have a PMC decision on the TargetPlatform API. I've opened bug 564760 to remove update.configurator from the RCP.
(In reply to Julian Honnen from comment #65) > The update.configurator dependency is now gone thanks to Alex's great work. Thanks Alex for fixing this long outstanding issue. > I'll close this bug once we have a PMC decision on the TargetPlatform API. Please open a new bug for this mark for deletion.
(In reply to Lars Vogel from comment #66) > Please open a new bug for this mark for deletion. Done: bug 564763
The changes for this bug have caused a major regression. Details of the investigation are described here: https://www.eclipse.org/forums/index.php/mv/msg/1105083/1831927/#msg_1831927 The fundamental problem is that the org.eclipse.pde.internal.core.PluginPathFinder.getFeaturePaths(String) method's implementation only works for installations with a collocated bundle pool, i.e., it only finds features in the features folder of the installation. An installation that uses a shared bundle pool does not have such a folder. I believe the implementation should be reading this file configuration/org.eclipse.update/platform.xml But with all these changes, it's not clear if the intent is for this file to continue to exist and if not, where the information about the installation's features should properly come from. This file is still present in the EPP package distros... Furthermore, there are more complex scenarios such as shared read-only installations with cascaded configurations where the user could install features in their surrogate and I expect the current implementation does not work properly for these either. I suppose we should open a new bug for this regression, right?
Thanks for the analysis, Ed! Please do open a new bug. Seems like update.configurator was scheduled for removal without a clear replacement (related: bug 565239). I don't see many options here other than restoring the previous behavior. Rather than depending on update.configurator again, we could parse the platform.xml ourself or try to get the required information from IBundleGroupProvider.
I've opened this follow up: https://bugs.eclipse.org/bugs/show_bug.cgi?id=566642
For the record, another regression is bug 567318.
