[project] Retaining Mars API compatibility

[eclipse-ocl-bot]

| --- | --- | | Bugzilla Link | 471114 | | Status | RESOLVED FIXED | | Importance | P3 major | | Reported | Jun 26, 2015 06:27 EDT | | Modified | Nov 24, 2020 14:28 EDT | | Depends on | 512403, 543621, 543870 | | See also | 470734, 541857, 473672, 443003 | | Reporter | Ed Willink |

Description

Mars sees the promotion of examples functionality to API compliance, so at least org.eclipse.ocl.pivot must observe API tooling.

The refactoring of the Evaluator/EvaluationVisitor into Executor+EvaluationVisitor required an unpleasant number of Extension interfaces to avoid detected API violations. (Of course this is logically still internal API and so only QVTd should be broken by the hidden semantic API changes.)

Bug 468156 cleans up the Lookup model, which unfortunately was not fully removed from Mars, so the changes appear as significant breakages. (Of course the Mars partial code was unuseable so the breakages cannot realistically break anyone.)

Evolution of the OCL 2.5 models is just about guaranteed to break something.

---

Do we go straight to org.eclipse.ocl.pivot 2.0.0 and lose any API tooling assistance in retaining the primary API?

Do we struggle with API tooling and continue with org.eclipse.ocl.pivot 1.1.0

---

Since we made big mistakes in failing to properly internalize API for Mars SR0, should we correct these mistakes in SR1 by adding many @noimplements, x-friends? Retracting API is bad, but ensuring toling understands internal API is surely good?

[eclipse-ocl-bot]

By Ed Willink on Jun 26, 2015 07:33

Bug 471118 gives another challenge. Do we remove the _ identifier from PivotTables that Java 8 deprecates? Nearly all the PivotTables identifiers are internal, so shouldn't they be more tightly defined?

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jun 26, 2015 07:38

Hi Ed,

If I don't misunderstand, the mistake with the internal API was, that in the manifest, packages were not marked as internal, right ?. Even though we didn't do that, the package segment name "internal" is meaningful enough to state that it's internal API, therefore I see reasonable to add API breakage filters in those situations.

For the specific case of the lookup model generated itfs/classes (Bug 468156), I think it's even more justified since they are new, and not thought to be used by clients.

I don't dislike your suggestion of including the x-internal bits were appropriate, then we could use Mars SR1 as the APIBaseLine so the API filters can be removed.

On the other hand if QVTd needs some internal API, surely Acceleo and QVTo might need it, and any other language interested in Unified OCL. So in those cases, publishing the API might be something to consider. Since I presume that rethinking/reworking the API is not something that you/we want to spend too much time now, if you prefer to add the x-friend bit in favour of QVTd, that's OK to me as well.

BTW, we should set Mars API baseline for Neon development. Is OK to you if I do that now ?

Regards,\ Adolfo.

[eclipse-ocl-bot]

By Ed Willink on Jun 26, 2015 08:01

(In reply to Ed Willink from comment #0)

Do we struggle with API tooling and continue with org.eclipse.ocl.pivot 1.1.0

Probably. Bug 471118 (deprecated _) fixed by a commented API filter.

Lookup changes seem to be a case for commented API filters too. (Comments referring to the Bugzilla).

(In reply to Adolfo Sanchez-Barbudo Herrera from comment #2)

BTW, we should set Mars API baseline for Neon development. Is OK to you if I do that now ?

Yes, but check carefully. The QVTd baseline definition at least was/is rubbish.

[eclipse-ocl-bot]

By Ed Willink on Jun 26, 2015 09:51

(In reply to Adolfo Sanchez-Barbudo Herrera from comment #2)

On the other hand if QVTd needs some internal API, surely Acceleo and QVTo might need it, and any other language interested in Unified OCL.

QVTd is not a problem, we are can change.

QVTo has no released Pivot usage so not a problem.

I used the Acceleo/Papyrus usage as a bit of a guide to reduce internal API, but both use org.eclipse.ocl.examples.xtext.console. There is relatively little other internal API in use, but as always, I did not have enough time to do things properly.

Bug 470734 is there to gather ideas for more public API.

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jun 26, 2015 10:40

(In reply to Ed Willink from comment #4)

(In reply to Adolfo Sanchez-Barbudo Herrera from comment #2)

On the other hand if QVTd needs some internal API, surely Acceleo and QVTo might need it, and any other language interested in Unified OCL.

QVTd is not a problem, we are can change.

QVTo has no released Pivot usage so not a problem.

I used the Acceleo/Papyrus usage as a bit of a guide to reduce internal API, but both use org.eclipse.ocl.examples.xtext.console. There is relatively little other internal API in use, but as always, I did not have enough time to do things properly.

\ What I meant is that if you have to use X-friend bit in OCL packages for QVTd access, that means that the API could be public rather than internal.

Regards,\ Adolfo.

[eclipse-ocl-bot]

By Ed Willink on Jul 03, 2015 03:58

Adding/refining Pivot WFRs leads to over a thousand @since/removed/added API problems. We must establish an alternate baseline in which EMF/xxxTables changes are permitted. (I begin to understand why EMF does not use API tooling.)

[eclipse-ocl-bot]

By Ed Willink on Jul 03, 2015 04:05

Raised to critical while API tooling is disabled for org.eclipse.ocl.pivot.

[eclipse-ocl-bot]

By Ed Willink on Jul 10, 2015 05:38

The reality is that the Pivot was promoted to 1.0.0 before OCL 2.5 was stable. Maintaining strong API compatibility for Neon will be incredibly time consuming. Even providing a pseudo retrofot baseline may be hard. We must have a major version chanage for Neon and we must learn to declare our API narrowly and have auto-generation of @noimplement/@noreference/@noextend.

In practice the major APIs are stable and we can work hard to retain the very limited internal API used by Acceleo/Papyrus giving them the chance to accept 1.0.0,3.0.0. If xtext.console is promoted to non-examples then Acceleo/Papyrus must change anyway.

If we do a +0.5.0 change now, we warn of big changes without inhibiting installation of Neon milestones on Mars evolutions.

If we have a ...pivot.internal and/or a ...xtext.internal plugin that is an x-friend, then perhaps it can re-export everything allowing e.g. QVTd to require ...pivot.internal and so get at all the internal API without requiring QVTd to be a nominated x-friend; I don't want any QVTd-helpful bias in the declarations. So anyone who requires ...pivot.internal undertakes to rebuild for each new minor version.

[eclipse-ocl-bot]

By Ed Willink on Feb 01, 2016 07:16

(In reply to Ed Willink from comment #7)

Raised to critical while API tooling is disabled for org.eclipse.ocl.pivot.

The API tooling was never disabled on master. Back to "major".

(In reply to Ed Willink from comment #6)

Adding/refining Pivot WFRs leads to over a thousand @since/removed/added API problems. We must establish an alternate baseline in which EMF/xxxTables changes are permitted. (I begin to understand why EMF does not use API tooling.)

Continuing from Bug 486853#c3. We can use @noreference/@noimplement/@noextend to silence most of the problems associated with additions. Removals/renames are of course a very genuine API problem. There could be genuine references from CGed user code.

Bug 486853#c3 highlights a problem where addition of an explicit opposite causes a rename of the name for an implicit opposite. In principle the CG must have a name-choosing policy that is as stable as possible for additions. We could solve the implicit->explicit change by automatically synthesizing synonym references from all explicit opposite names to their implicit opposites; tedious bloat. Or we could CG wrt an API reference so we only synthesize the names we need. Or we keep doing manual fixups. Or we prohibit many useful changes.

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Feb 01, 2016 07:22

Hi Ed,

Since most of the "problems" you are mentioning are generated by EMF perhaps the following comment is irrelevant, but I've discovered quite interesting Java 8 feature called default[1]. I find it useful for API evolution (adding methods to interfaces to avoid breakages). Just to make you aware of it, in case you weren't.

[1] https://docs.oracle.com/javase/tutorial/java/IandI/defaultmethods.html

Regards,\ Adolfo.

[eclipse-ocl-bot]

By Ed Willink on Feb 01, 2016 07:36

(In reply to Adolfo Sanchez-Barbudo Herrera from comment #10)

Java 8 feature called default

Interesting. Certainly a CG (EMF or OCL) aware of its API reference could automatically synthesize defaults. But it requires API reference awareness. In principle it just needs a load of e.g. Pivot.ecore to be accompanied by a load of Pivot.reference.ecore.

If the CG is defined by a suite of Model2Java OCL equations, the necessary difference synthesis can perhaps be extracted for re-use.

[eclipse-ocl-bot]

By Ed Willink on Dec 03, 2018 07:23

(In reply to Adolfo Sanchez-Barbudo Herrera from comment #10)

Java 8 feature called default

After a bit of playing, default indeed seems useful for manually maintained interfaces, and potentially for EMF auto-generated interfaces since the default is preserved fort @generated NOT.

However it just doesn't work for true API compatibility. Any removal is obviously bad, but any (non-private) addition is also bad because it clashes with the possible pre-exisgting use of the new name by an old derived class.

API tooling diagnostics can be silenced by arbitrarily (and unhelpully) declaring an interface as @noimplement, or arbitarily but more helpfully filtering all addition problems.

[eclipse-ocl-bot]

By Ed Willink on Dec 04, 2018 03:52

(In reply to Ed Willink from comment #8)

If we have a ...pivot.internal and/or a ...xtext.internal plugin

Bottom line:

• using API tooling for the Pivot EMF model is a waste of time and actually damaging since it encourages a filter-everything culture.

• using API tooling for the Pivot as a whole is Eclipse policy and has significant benefits.

The only practical solution avoiding circular dependencies seems to be a three layer 'plugin'.

a) base/interface plugin - with API tooling - a few public interfaces\ b) model plugin - no API tooling - the EMF auto-generated model interfaces and impls\ c) exec/implementation plugin - with API tooling - the bulk of the code

The base plugin also 'solves' the problem of an API facade since we would like it to specify everything that the model plugin needs and the exec plugin provides. Perhaps PivotObject may be enough to avoid a third public variant of Class/ClassImpl.

(In reply to Ed Willink from comment #0)

Do we go straight to org.eclipse.ocl.pivot 2.0.0 and lose any API tooling assistance in retaining the primary API?

If the exec plugin retains the symbol-name, the refactoring may be invisible. The major version change can be deferred until the 'OCL 2.5' alignment.

[eclipse-ocl-bot]

By Ed Willink on Dec 07, 2018 05:03

(In reply to Ed Willink from comment #13)

The only practical solution avoiding circular dependencies seems to be a three layer 'plugin'.

This might be practical for the 'OCL 2.5' refactoring. It won't do for the current now-quarterly releases.

A quick experiment at introducing three layers was promising but reveals a number of areas such as IdManager where there is a missing intercace/implementation and of course PIvotTables.STR_xxx (Bug 47362). Attempting to genmodel to multiple plugins fails; not what the suffixes were intended to do, so any plugin partitioning must use a manual relocation.

The unmodified EMF interfaces could potentially go direct into an 'interface'/'api' plugin provided the *Factory/Package/Tables are directed to a 'meta'-package that could be placed elsewhere. This avoids the need for an Element/Element2 duality for separate interfaces in api/model plugins.

?? Could a plugin + two fragments avoid a multi-plugin but suppor partial API tooling ??

Caution: it was really hard to stop the API tooling reporting hideous numbers of errors while experimenting; looks like something isn't switch off properly. Had to switch off all baselines and remove all API natures/builders

[eclipse-ocl-bot]

By Ed Willink on Dec 07, 2018 05:57

(In reply to Ed Willink from comment #14)

(In reply to Ed Willink from comment #13) This might be practical for the 'OCL 2.5' refactoring. It won't do for the current now-quarterly releases.

Another idea. After each quarterly release we fork a "baseline/x.y.z" branch in which only model changes and their autogenerations occur. Changes to the "baseline/x.y.z" are carefully reviewed to preserve 'reasonable' API fidelity. A "baseline/x.y.z" build is used as the baseline for "master" so that model changes do not need API filtering. Since a baseline build comprises the models for the 'next' release but the support code for the 'previous' release, needing a successful baseline build will make it hard to get away with deleting model declarations.

Rather than cherry-picking from "baseline/x.y.z" to "master", we can perhaps regularly merge "baseline/x.y.z" into "master". This should be relatively painless if we avoid undue code development in auto-generated classes.

If we want to maintain longer timescale baselines, "baseline/6.1.0" has release 6.1.0 plus all subsequent model changes and "baseline/6.5.0" has release 6.5.0 plus all subsequent model changes, we must 'choose' whether to make the next model change on "baseline/6.1.0" or "baseline/6.5.0" and then 're-use' on the other. "baseline/6.1.0" can be merged into "baseline/6.5.0" but not vice-versa since "baseline/6.5.0" has all the changes of "baseline/6.1.0". We should therefore make all model changes to the oldest baseline and merge into newer baselines.

A typical development is therefore\ a) commit model improvements to oldest baseline branch\ b) commit autogenerations to oldest baseline branch\ c) commit necessary fixups to oldest baseline branch\ d) optionally build long term baseline\ e) merge oldest baseline into current baseline\ f) build current baseline\ g) merge current baseline into master\ h) commit developments exploiting model improvements to master

d) and e) are the overhead for supporting a long term baseline

This is clearly applicable to the pivot plugin, but what about Xtext plugins? Currently there is no Xtext plugin API tooling so a bad change to e.g. BaseCSLeft2RightVisitor is undetected. If we use this baseline policy for Xtext too, then although there may be some churn for Xtext grammar regeneration, we should significantly reduce residual breakage of derived plugins that have been regenerated.

Check this works for pivot first, then extend to xtext plugins, which can probably have significantly trim to 'private' now that what is needed for QVTd extensibility is clear.

[eclipse-ocl-bot]

By Ed Willink on Jan 19, 2019 16:22

(In reply to Ed Willink from comment #15)

Check this works for pivot first.

It worked for a minor evolution, bit as soon as we add another constraint, the old code gets test failures unless we put all the new support code in the baseline.

a) abandon\ b) all code in baseline is pretty pointless\ c) new/problem functionality is stubbed in the baseline - we only care about API. Distinctive "API-preserving-stub" comments should easy GIT merge resolution, post merge checking.\ d) variant JET templates givning a more API stable formulation\ e) new M2T altogether

Try c) for a bit.

Redoing EMF's genmodel is unattractive. Yet another OCL 'component' to maintain, but we only use a stylized part, so discarding all the bits we don't use will simplify and we cn do some of the things we want to right.

[eclipse-ocl-bot]

By Ed Willink on Jan 19, 2019 17:14

(In reply to Ed Willink from comment #16)

d) variant JET templates givning a more API stable formulation e) new M2T altogether

Currently EMF generates e.g.

public boolean eIsSet(int featureID) {\
    switch (featureID)\
    {\
        case PivotPackage.IF_EXP__ANNOTATING_COMMENTS:

with an unstable IF_EXP__ANNOTATING_COMMENTS per class-feature as an unstable public API integer.

If instead:

public boolean eIsSet(int featureID) {\
    switch (featureID - PivotRootClass_BASE_ID)\
    {\
        case 5:

where PivotRootClass_BASE_ID is the least derived class in PivotPackage's first feature. "5" is the relative index wrt to the PivotRootClass_BASE_ID. "5" etc are recom[uted by a regenerate.

This requires very few public constants, and they can probably have accessors to hide instability.

[eclipse-ocl-bot]

By Ed Willink on Jan 20, 2019 06:39

(In reply to Ed Willink from comment #17)

Currently EMF generates e.g.

public boolean eIsSet(int featureID) { switch (featureID) { case PivotPackage.IF_EXP__ANNOTATING_COMMENTS:

Bug 543621 outlines a rather simple change to EMF auto-generation that avoids publishing unstable integers.

[eclipse-ocl-bot]

By Ed Willink on Jan 20, 2019 12:13

(In reply to Ed Willink from comment #16)

Try c) for a bit.

It works but aggravated by Bug 543623 and many typos while trying to reconstruct the development on the baseline, this took a day elapsed time. Inserting "API-stdub" in the stubs helped, but didn't stop a few errors. Eventually a few bad GIT merge comments needed fixing. Also occasionally difference editing corrupts. All very painful just to 'prettify' done work.

[eclipse-ocl-bot]

By Ed Willink on Jan 27, 2019 10:39

(In reply to Ed Willink from comment #18)

Bug 543621 outlines a rather simple change to EMF auto-generation that avoids publishing unstable integers.

In order to exploit this, we need an ability to change/augment the EMF JET templates. Unfortunately this only works for dynamicTemplates which does not work standalone, from MWE2.

We can therefore define a new GenModel nsURI, which unfortunately due to a naive Generator.getPackageID() requires every significant GenModel class to be derived; just like UML. All quite painful.

Or, Bug 543870, we can fix the operation of staticTemplates in EMF. We can then just have OCL genmodel mention org.eclipse.ocl.examples.build/templates and they use replacement functionality, assuming not all of kit is promoted into EMF. If Bug 543870 is won't fixed, it should be possible for MWE2 to just register the 'wrong' GenModelAdapterFactory. To make it work interactively we would need a new OCLGenModel nsURI and all the associated model/edit/editor support.

[eclipse-ocl-bot]

By Ed Willink on Jan 31, 2019 05:58

(In reply to Ed Willink from comment #18)

Bug 543621 outlines a rather simple change to EMF auto-generation that avoids publishing unstable integers.

This works nicely for the AS and CG, but fails for the CS since Xtext's auto-generated AbstractXXXXXSemanticSequencer uses the ClassId ints for its own switch dispatcher. Re-implementing as an EMF Switch might be possible but would be API breaking so very unattractive to the Xtext developers. A tweaked SemanticSequencer template shouldn't be that hard, or even a post-processor. But all more to track. For now the Xtext API is so tightly coupled that we don't use API tooling anyway. Eliminating the Feature/Operation ints would have some benefits, but the Class ints are amongst the worst for disproportionate API change.

---

For the AS, the Bug 89325 workaround and 'smarter' lists required XXXPackage.YYY_ZZZ to be changed to XXXPackage.Literals.YYY_ZZZ.getFeatureId(). A very minor loss of performance on a light usage construction path.

SemanticSequencer problem resolved by adding a "Generate Classifier ints" GenModel EAnnotation so that for CS models we keep the classifier but at least save the feature/operation bloat.

To help QVTd that currently requires AdapterFactoryClass and SwitchClass at run-time, all customized .javajet moves from org.eclipse.ocl.examples.build to org.eclipse.ocl.examples.codegen and .genmodel just specigoes the org.eclipse.ocl.examples.codegen/templates templateDirectory.

Until EMF Bug 543870 is fixed, the extra functionality is only available when MWE2 specfies to replace EMF's GenModelGeneratorAdapterFactory by OCLBuildGenModelGeneratorAdapterFactory.

[eclipse-ocl-bot]

By Ed Willink on Feb 06, 2019 12:22

(In reply to Ed Willink from comment #21)

OCLBuildGenModelGeneratorAdapterFactory.

renamed to OCLGenModelGeneratorAdapterFactory now that it is no longer in the codegen rather than build plugin.

Unfortunately use by QVTd failed for Tycho on Jenkins but not locally. Eventually resolved to a failure to use canonical file names for locating matching class path entry. Fix necessitated an OCL 6.7.0M2a rebuild for QVTd.

New templates and workaround GeneratorAdapterFactory pushed to master for M2(a).

Still need to stabilize XXXTables. Possibly by using the EOperation literal rather than an extra custom String.

[eclipse-ocl-bot]

By Ed Willink on Feb 25, 2019 10:47

(In reply to Ed Willink from comment #22)

Still need to stabilize XXXTables. Possibly by using the EOperation literal rather than an extra custom String.

This is pretty easy. Pushed to master for M3a.

This renders many of the STR_xxx constants in XXXTables redundant.

An experiment reveals that all XXXTables global constants can be moved to package private in XXXTablesImpl.

For the rest of XXXTables, only the ._Types and _Operations names are actually referenced to define inheritance/dispatch tables. Relatibely few names are actually referenced so these could be computed programmatically (a linear / binary name search) and cached in XXXTablesImpl to avoid repeated searches.

With those declarations moved to package private in XXXTablesImpl, we might be left with just XXXTables.{LIBRARY/PACKAGE} as public API. A model change has zero API impact in PivotTables/PivotTablesImpl.java.

A bit late to do this for 2019-03.

[eclipse-ocl-bot]

By Ed Willink on Nov 24, 2020 14:28

The changes were pushed to master on 28-March-2019.