[evaluator] Change the default Workspace Preference to Pivot

[eclipse-ocl-bot]

| --- | --- | | Bugzilla Link | 403595 | | Status | CLOSED WONTFIX | | Importance | P3 normal | | Reported | Mar 18, 2013 04:19 EDT | | Modified | Jul 10, 2021 04:00 EDT | | See also | 574038 | | Reporter | Ed Willink |

Description

The evaluator workspace preference was added in Juno to allow users to select the Pivot evaluator, but preserve classic behaviour.

This preference is mostly only of interest to interactive users who are using interactive tools that all use the Xtext/Pivot based functionality. So there can be some confusion when evaluation as part of model validation malfunctions.

With Papyrus also exploiting the Xtext editor, so that Class etc are available, there is even more confusion for users. The classic evaluator supports EClass rather than Class.

Even though the Pivot is not being promoted for Kepler, I think the time has come to change the evaluation preference default so that, if the Examples and Editors are installed, evaluation uses the same parser as the editors until users specify otherwise.

Comments?

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Mar 18, 2013 10:43

For Papyrus could be important having the delegates working with the new pivot-based evaluator.

I don't know the impact for QVTo, but I don't think that there is any.

I don't know the impact for Acceleo, but I think that some Acceleo developer must participate in.

In principle, and giving that this impacts some behaviour provided by exclusive OCL delegates, I don't see any inconvenience of doing that. Any nuisance in kepler due to the new pivot-based delegates could arise overseen problems, and who knows if improving Pivot-based implementation before promoting. Besides, any nuisance can easily workarounded by changing the preference. In any case, I'd make some Acceleo developer participate in the discussion.

Regards,\ Adolfo.

[eclipse-ocl-bot]

By Ed Willink on Mar 18, 2013 11:34

It should have zero impact on anyone using an underlying Java access since the Java access is hard coded to the particular OCL. Classic for QVTo, Acceleo, OCL Console; Pivot for editors, Xtext Console, Papyrus.

It's solely for validation, and when EMF delegates work perfectly, a tool using e.g the Classic-binding of OCL is unaware that e.g the Pivot binding is used during validation.

An Acceleo user using dynamic EMF and embedded OCL delegates could see a difference. Previously they may have prepared their delegates using OCLinEcore and been confused by package resolutiion differences when Acceleo executes them With the default change they will execute in their development style. Probably worth an explicit test.

[eclipse-ocl-bot]

By Ed Willink on Mar 23, 2013 11:38

Change of default pushed to edw/403595.

Retesting standalone classpath execution revealed that the prototype of this fix and the Bug 403567 fix broken standalone execution. Fixed.

I've improved "org.eclipse.ocl.ecore.tests (Standalone Classpath)" so that it is now portable; no longer has my C:/Tools/Eclipse/... path. But it still has qualifiers on JARs so would need updating every milestone if we run it on Hudson. Any suggesrtions on cleverer use of 'variables'?

Anyway. This is now ready for review for M7.

[eclipse-ocl-bot]

By Ed Willink on Mar 23, 2013 11:41

(In reply to comment #3)

Retesting standalone classpath execution revealed that the prototype of this fix and the Bug 403567 fix broken standalone execution. Fixed.

Correction Bug 403627's first attempt broke standalone.

[eclipse-ocl-bot]

By Ed Willink on Mar 23, 2013 13:54

12 Impact Analyzer tests fail.

The IA uses the virtual URI so gets switched to Pivot.

At first sight there are some enthusiastix references direcvt to INVOCATION_DELEGATES but actually very little is failing. Probably just a simple bug.

OCL-wise the IA has a very large body of code that is being run under the PIvot for the first time.

We could just set LPG explicitly in doSetup(), but fixing the subtle Pivot incompatibilities would be even better.

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Apr 05, 2013 06:23

Hi Ed,

The commit 17c77de4d39ae316ab483b4bb121f1cfce831759 looks good to me. But I'm not sure what you have done to sort out the IA test cases failures. Since the impact analyzer heavily relies on classic code, I'd make an explicitly use of the classic delegates (at least). On the other hand, I understand that enthusiastic of fixing those "incompatibilities" with the Pivot impl, but will this happen before M7 ?

Just a minor comment concerning the inner static class:

 * This code is factored into a separate static class to ensure that classes that are not
 * available standalone are not loaded before EMFPlugin.IS_ECLIPSE_RUNNING is checked.\
 */\
private static final class PreferenceListenerInstaller implements IPreferenceChangeListener\
\

We have a similar discussion at Bug 387973 (See for example Bug 387973 comment 49 and Bug 387973 comment 50). Why the inner class is necessary in this case ?

Cheers,\ Adolfo.

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Apr 05, 2013 06:35

(In reply to comment #6)

Hi Ed,

The commit 17c77de4d39ae316ab483b4bb121f1cfce831759 looks good to me. But I'm not sure what you have done to sort out the IA test cases failures. Since the impact analyzer heavily relies on classic code, I'd make an explicitly use of the classic delegates (at least). On the other hand, I understand that enthusiastic of fixing those "incompatibilities" with the Pivot impl, but will this happen before M7 ?

Oh, I've noticed the rebase now... the commit id changes to 2260ada644e60e4a7b69ed378842fbfadba2d15f

And now there are some extra commits to fix the test cases.... which don't deal with the own IA but the set up of the own test-case. I'll have a look to this...

Cheers,\ Adolfo.

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Apr 05, 2013 07:18

Ok,

So this is a clear example, in which clients are in somehow affected by the change of the "default workspace preference to pivot", and the client is in our own project... which worries me a lot...

Clients have to:

• either change the annotation on their Ecore metamodels to make the delegates annotation use the LPG one, rather than what you call the virtual one.
• or change the Eclipse preferences to use the classic evaluator.
• or programmatically change the default delegation mode to the classic one, if they don't have access to the eclipse preference (for instance, standalone mode)
• or change their OCL expressions to accommodate the Pivot subtle differences.

For a particular client any of them should be annoying.

This is quite embarrassing, specially without any announcement for the change, any migration guide, after M6, and what is worse in a minor service release increment.

On the one hand, I understand that the move is beneficial for the project (helps new users, its use will make the new approach improve), on the other hand this may be a disaster for clients which may expect for OCL 4.1 having everything working as it worked in OCL 4.0 (annoying for the current users).

Any thoughts ?

Cheers,\ Adolfo.

[eclipse-ocl-bot]

By Ed Willink on Apr 05, 2013 07:24

(In reply to comment #7)

Oh, I've noticed the rebase now... the commit id changes to 2260ada644e60e4a7b69ed378842fbfadba2d15f

Sorry. Forrgot to Save:

The IA tests fail when the Pivot is the default evaluator for a variety of reasons.

The pivot parser does not support the old policy of prefixed names. A fall-back has therefore been added so that an unresolved name stsrting with is retried with the _ prefix.

The IA creates some OCL expressions by invoking the LPG parser directly and then solving orphan problems bny putting such object directly as Resource.contents. When this Resource is presented as a meta-model it contains EPackages and other things that should not be there. The Pivot support has been improved to ignore and better diagnose these non-Ecore contributions.

However since the IA is clearlt tightly coupled to the LPG parser it is not surprising that there are failures. Therefore the IA tests have been improved to specify the LPG evaluator as their default.

[eclipse-ocl-bot]

By Ed Willink on Apr 05, 2013 07:29

(In reply to comment #8)

Any thoughts ?

New release, new code. It only affects Java clients. It does not affect them until they install the Examples and Editors. There is a one line fix. There is an interactive fix. They can specify the ...LPG URI explicitly. There is probably an eclipse.ini workaround too for the default preference.

On balance there are probably a minute number of users with tightly coupled LPG delegate functionality.

There may be a steadily growing number of Papyrus users. We cannot cripple the future for a hypothetical community that has easy workarounds.

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Apr 05, 2013 07:55

(In reply to comment #10)

(In reply to comment #8)

Any thoughts ?

New release, new code. It only affects Java clients. It does not affect them until they install the Examples and Editors. There is a one line fix. There is an interactive fix. They can specify the ...LPG URI explicitly. There is probably an eclipse.ini workaround too for the default preference.

There are easy fixes for my broken class (which I could fit in one line), when a method is added to a third party Interface which I implement...

On balance there are probably a minute number of users with tightly coupled LPG delegate functionality.

If I have my Juno-based tooling working using my constrained metamodels, and they stop to do what I expect to do because I've updated to the new OCL 4.1 ... this\ doesn't simply affect "a minute number of users with tightly coupled LPG delegate functionality"

There may be a steadily growing number of Papyrus users. We cannot cripple the future for a hypothetical community that has easy workarounds.

Papyrus users have the same easy fix than the broken users above.

Anyway, I'm not going to extend the discussion, nor block this enhancement which I find very useful to move forward. The code review has been successful, as committer I'm simply pointing out the risks I can foresee. I know that you are aware of Eclipse policies and you are the project leader in this case, who has the decision about what to do in this case for the benefits of the project ;)

If you finally decide to push this enhancement, I'd suggest announcing the "subtle" change in typical newsgroups (ocl, ocl-dev, cross-project) and creating some documentation about it (wiki, help, whatever...)

Cheers,\ Adolfo.

[eclipse-ocl-bot]

By Ed Willink on Apr 05, 2013 09:32

(In reply to comment #11)\

If you finally decide to push this enhancement, I'd suggest announcing the "subtle" change in typical newsgroups (ocl, ocl-dev, cross-project) and creating some documentation about it (wiki, help, whatever...)

After doing some more Papyrus debugging on Bug 404720, I'm not sure that the default setting makes that much difference to Papyrus users, so I'll commit all the changes except that the default default will remain LPG.

[eclipse-ocl-bot]

By Ed Willink on Apr 05, 2013 10:05

Changes to support a changed default, but no actual change in default pushed to master for M7.

[eclipse-ocl-bot]

By Ed Willink on May 27, 2014 09:43

CLOSED after more than a year in RESOLVED state.

[eclipse-ocl-bot]

By Ed Willink on May 27, 2014 09:52

and CLOSE

[eclipse-ocl-bot]

By Ed Willink on Jul 10, 2021 04:00

Revisiting 8 years on ...

(In reply to Ed Willink from comment #5)

12 Impact Analyzer tests fail.

...

OCL-wise the IA has a very large body of code that is being run under the PIvot for the first time.

very large => 130 line OCL expressions

3 "ImpactAnalysisTests with NavigationSteps and IA Annotations" tests fail due to package access failures. However opening e.g. data.ecore with the OCLinEcore editor gives no errors, so it appears that the Ecore2Pivot import deduction is 'better' than the IA test harness registry init. \

We could just set LPG explicitly in doSetup(),

This was done anyway.

but fixing the subtle Pivot incompatibilities would be even better.

The OCLinEcore for e.g. dataaccess::analytics:: provokes

import _'dataaccess.analytics' : 'dataaccess.ecore#//analytics';

so the problem is probably a nested package funny. Not a high priority.