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

Xtext and (binary) backwards compatibility #2668

Open
cdietrich opened this issue May 17, 2023 · 19 comments
Open

Xtext and (binary) backwards compatibility #2668

cdietrich opened this issue May 17, 2023 · 19 comments
Assignees
Labels
Milestone

Comments

@cdietrich
Copy link
Contributor

Given the ongoing discussions in #2056 #2176 #2101

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

@cdietrich cdietrich added this to the Release_2.32 milestone May 17, 2023
@ewillink
Copy link

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
Copy link
Contributor Author

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

@HannesWell
Copy link
Contributor

HannesWell commented May 17, 2023

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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

@HannesWell
Copy link
Contributor

HannesWell commented May 17, 2023

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 #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 eclipse-platform/eclipse.platform#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
Copy link

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

@ewillink
Copy link

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
Copy link
Contributor Author

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

@cdietrich
Copy link
Contributor Author

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

@cdietrich cdietrich modified the milestones: Release_2.32, Release_2.33 Aug 27, 2023
@HannesWell
Copy link
Contributor

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
Copy link
Contributor Author

No plans yet

@HannesWell
Copy link
Contributor

Would be great if this is considered in the future.

@szarnekow
Copy link
Contributor

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
Copy link
Contributor

HannesWell commented Aug 30, 2023

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 eclipse-pde/eclipse.pde#728.

@szarnekow
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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.

@cdietrich cdietrich added this to the Release_2.35 milestone Feb 27, 2024
@cdietrich cdietrich modified the milestones: Release_2.35, Release_2.36 May 23, 2024
@cdietrich cdietrich modified the milestones: Release_2.36, Release_2.37 Aug 25, 2024
@cdietrich cdietrich modified the milestones: Release_2.37, Release_2.38 Nov 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants