search

eclipse / xtext

Eclipse Xtext™ is a language development framework
http://www.eclipse.org/Xtext
Eclipse Public License 2.0
753 stars 317 forks source link

Xtext and (binary) backwards compatibility #2668

Open cdietrich opened 1 year ago

cdietrich commented 1 year ago

Given the ongoing discussions in https://github.com/eclipse/xtext/issues/2056 https://github.com/eclipse/xtext/issues/2176 https://github.com/eclipse/xtext/issues/2101

how much do you as Xtext consumer value binary backwards compatibility regardings our dependencies? what is ok to break and what is not

ewillink commented 1 year ago

https://bugs.eclipse.org/bugs/show_bug.cgi?id=460677 and linked bugs discuss the breaking change made to Xcore.ecore and consequently the undesirability of using *.xtextbin. I contributed a solution that avoids the incompatibility and almost certainly improves editor activation speed, but the contribution remains unadopted.

This change and the avoidance of other new Xtext features enable OCL and QVTd Xtext editors to be usable over well over 5 years worth of Xtext releases.

cdietrich commented 1 year ago

changing Xcore.ecore i wont consider a "dependency" change

HannesWell commented 1 year ago

how much do you as Xtext consumer value binary backwards compatibility regardings our dependencies? what is ok to break and what is not

At least for us, binary backwards compatibility regarding dependencies is not important. Our product is a more or less closed world and we try to use the latest releases if possible and can adapt our code base if necessary. So as long as other Eclipse projects like Xcore can follow (I'm willing to help if work needs to be done) we would have no problem if binary backward compatibility breaks.

rubenporras commented 1 year ago

Similar for us, we can adapt our code base if necessary, it might require coordination with other teams in our company and might prevent us upgrading for one or two months, but it is doable. So if libraries are upgraded because they bring benefits, that is generally fine for us.

cdietrich commented 1 year ago

please note: this is not only about bumping libs. its also about removing reexports so that you might have to adjust your deps. we had this e.g. in the past when we were deprecating the old generator/xpand

rubenporras commented 1 year ago

You mean adding the dependencies to our MANIFESTs? Or is there something else involved?

HannesWell commented 1 year ago

please note: this is not only about bumping libs. its also about removing reexports so that you might have to adjust your deps.

Yes, if possible I would just remove all reexports. While reexporting a required-bundle might be convenient at first sight it can really become a blocker for libraries like Xtext on the long run as we can see in https://github.com/eclipse/xtext/issues/2176. When a Plugin/Bundle reexports a bundle it effectively becomes part of the API of the reexporting Plugin and therefore has to be handled as such. This increases the API size and at the same time it is a dependency which the Plugin maintainers maybe not control and even if they control it it can make evolving the reexported Plugin more difficult.

In a closed world, e.g. if one also builds the final product the implications of reexported bundles might not be an issue, but for a library/framework like Xtext I think the implications are far worse on the long run than the convenience gains.

For platform I think we should also work towards removing the reexports, which would save us changes like https://github.com/eclipse-platform/eclipse.platform/pull/454, but that might become even more bureaucratic and is another topic.

If downstream consumers really need a reexport in the middle of the dependency chain, they can probably add it to one of their Plugins.

You mean adding the dependencies to our MANIFESTs? Or is there something else involved?

No, that should be the only necessary change for consumers.

danielwinkels commented 1 year ago

I did not encounter any problems. At least none i am aware of.

ewillink commented 1 year ago

Removing re-exports creates unpleasant surprises, but is relatively easily fixed at compile-time. If necessary it can be 'patched' at run-time by an additional dummy plugin that ensures that plugins are loaded. However if something like EObject appears in an Xtext API, it seems pretty daft to remove org.eclipse.emf.ecore from the re-exports; there is no possibility of Xtext migrating away from EObject without a major version change. Conversely e.g. ASM classes should not appear in Xtext APIs and so should not be re-exported; Xtext should be able to change to a completely different sub-tool without affecting users.

cdietrich commented 11 months ago

we have now updated to new guice and guava, but still depend old javax.inject for now

cdietrich commented 10 months ago

we will drop javax.inject with Xtext 2.33. we also will drop xpand support (Version to be defined)

HannesWell commented 10 months ago

we will drop javax.inject with Xtext 2.33. we also will drop xpand support (Version to be defined)

Great. Do you also already have plans for other re-exports like guava?

cdietrich commented 10 months ago

No plans yet

HannesWell commented 10 months ago

Would be great if this is considered in the future.

szarnekow commented 10 months ago

I just wanted to let you know that it's still under consideration. One option is to use import-package for Guava and only re-export the packages that contain types that are used in signatures in the Xtext codebase.

HannesWell commented 10 months ago

I just wanted to let you know that it's still under consideration. One option is to use import-package for Guava and only re-export the packages that contain types that are used in signatures in the Xtext codebase.

Thanks for considering it. Imported-Packages cannot be re-exported, only required bundles can. IIRC you only have to Import the package of a type if you call the method that contains the type in its signature. So callers actually have to import the type's packages already because they are for example create an instance of it and pass it to that method or declare a variable that gets the returned value assigned. At the moment this does not need to be done because xtext plugins rexports it.

As said elsewhere, those that can't adapt a Plug-ins code probably still could create a fragment to re-add the re-export to the corresponding Xtext Plugin or just as Required-Bundle to the Plugin whose Manifest that can't be adjusted.

Btw. for PDE I'm contemplating to add an option to ignore re-exports in the Workspace build so that one can prepare for a future removal https://github.com/eclipse-pde/eclipse.pde/issues/728.

szarnekow commented 10 months ago

Imported-Packages cannot be re-exported

This is not how I understand https://docs.osgi.org/specification/osgi.core/7.0.0/framework.module.html#framework.module-import.export.same.package

HannesWell commented 10 months ago

Imported-Packages cannot be re-exported

This is not how I understand https://docs.osgi.org/specification/osgi.core/7.0.0/framework.module.html#framework.module-import.export.same.package

This applies to bundles that export the package they contain and import the package as well (to e.g. allow more recent versions as replacement). I'm relatively certain that this does not work to 're-export' a package from another bundle.

szarnekow commented 10 months ago

This applies to bundles that export the package they contain and import the package as well (to e.g. allow more recent versions as replacement). I'm relatively certain that this does not work to 're-export' a package from another bundle.

Ok, understood. You're right, it's not possible to re-export such a package.