Users need a way to override contributed property sets

[lwrage]

Contributed property sets (and packages) used to be copied into the workspace, which we considered bad practice. In #527 we changed that, such that contributed AADL files are read from the contributing plugins. However, now it is impossible to fix errors in such a file without updating OSATE. There should be some way to update just a single contributed file.

We have a specialized mechanism that works for AADL_Project.aadl only.

Who updates the file contents? End user or update mechanism? What's the scope of the change? Workspace or installation?

[Etienne13]

As mentioned in #1669, solving this issue would help to experiment AADL tools that need to go beyond some limitations in the current version of some property sets (including modeling ARINC653 systems on multicores).

[Etienne13]

I now face this issue again because of the "pok_properties.aadl" file, which is contributed by the osate-ocarina plugin and my own plugin (RAMSES) with two different contents. My current workaround is to uninstall osate-ocarina but is is not satisfactory for my end-users: each time they install RAMSES for a new version of OSATE, they have to uninstall osate-ocarina.

Here is a proposal for this issue: define an order of contributed property sets such that property set N+1 overrides property set N;

• the default order is computed according to the history of installed plugins: the latest is pushed back in the queue and thus overrides the previous ones
  • the order can be viewed and changed by end-users

This would be a solution at the installation level which could be changed at the workspace level (or at the project level if needed).

It would not allow to mix (merge/override) property set contents but at least property set files...

[jjhugues]

Pok no longer exists as one os, there are several forks, and I do not have the ressource to maintain thid

I can suppress the property set

Le 5 juil. 2019 à 12:32, Etienne Borde notifications@github.com<mailto:notifications@github.com> a écrit :

I now face this issue again because of the "pok_properties.aadl" file, which is contributed by the osate-ocarina plugin and my own plugin (RAMSES) with two different contents. My current workaround is to uninstall osate-ocarina but is is not satisfactory for my end-users: each time they install RAMSES for a new version of OSATE, they have to uninstall osate-ocarina.

Here is a proposal for this issue: define an order of contributed property sets such that property set N+1 overrides property set N;

• the default order is computed according to the history of installed plugins: the latest is pushed back in the queue and thus overrides the previous ones
• the order can be viewed and changed by end-users

This would be a solution at the installation level which could be changed at the workspace level (or at the project level if needed).

It would not allow to mix (merge/override) property set contents but at least property set files...

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/osate/osate2/issues/1134?email_source=notifications&email_token=AL7JLSOO555EB7XS7JSA2ALP55ZS7A5CNFSM4E23UR6KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZJ5KVA#issuecomment-508810580, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AL7JLSJG2NZ7D7IJW7U3VLTP55ZS7ANCNFSM4E23UR6A.

[Etienne13]

Hi Jérome,

yes, that would at least solve my problem for now.

Thanks! Etienne.

[lwrage]

Add workspace scoped preference to configure which contributed AADL is used. Details TBD.

[AaronGreenhouse]

Need more info here.

Processing of contributed AADL files seems to be handled in several places, making this potentially tricky, including:

• org.osate.xtext.aadl2.properties.ui.builder.PropertiesToBeBuiltComputerContribution --- I think this inserts the files into the build process?
• org.osate.xtext.aadl2.ui.containers.Aadl2ProjectsStateHelper --- Not really sure what this does
• org.osate.xtext.aadl2.ui.resource.Aadl2Storage2UriMapper --- Not really sure what this does
• org.osate.ui.navigator.AadlContributionContentProvider --- Adds the contributed files to the AADL navigator tree

Current uses all rely on the method PluginSupportUtil.getContributedAadl(). This method reads all the contributions from the plugin.xml files. If there are duplicates here, then they will simply be propogated. If we want to override a contributed file with a file from the OSATE workspace, then it will be missed here. We would have to replace uses of this method with a new method that returns a manipulated list of contributed files. (This new method would not be applicable to replacing getCongtributedAadlAsClasspath() that is used by stand-alone (non-Eclipse) applications. They would have to there own mechanism for this.)

[AaronGreenhouse]

What is the plan here? I think what is wanted is a way to override a contributed resource with a resource from the workspace, in effect, treating that workspace file as a contributed resource.

I don't think we need to deal with the case of multiple contributed resources with the same name.

So we want an Eclipse workspace preference (and associated UI pane) that allows a file from the workspace to be inserted into the list of contributed resources and possibly replace an existing resource.

Probably better (simpler to think about) is this:

• The preference allows contributed resources to be selected to be ignored (not reported)
• The preference allows files from the workspace to be added to the list of contributed resources.
• The preference panel should not allow the overall list of "contributed resources" - "ignore resources" + "workspace contributions" to contain packages/property sets with duplicate names.

[AaronGreenhouse]

Looked at the special case of AADL_Project.aadl, which can be replaced by an identically named file in the workspace. This works by having the path of the AADL_Project file that should be used saved in a workspace preference. Aadl2ProjectsStateHelper uses the value of this preference instead of the path of any AADL_Project file that is in the contributed resources.

[AaronGreenhouse]

The problem with the current approach (see above) is that the UI doesn't reflect what is happening at all.

I think that ideally in the AADL Navigator, we should have "Plug-in Contributions" and "Workspace Contributions". The first should show anything contributed by a plug-in minus any item that is removed via the workspace preferences. The second show any contributions from the workspace (as set in the preferences).

[AaronGreenhouse]

Steps

1. Review the workspace preferences mechanism again.
2. Figure out how to save the preference information that I need (List of suppressed contributions, list of workspace contributions)
3. Create UI preference pane of the preferences
4. Update PluginSupportUtil.getContributedAadl() and friends

[AaronGreenhouse]

For 1 & 3 above: https://www.vogella.com/tutorials/EclipsePreferences/article.html

[AaronGreenhouse]

To avoid having to parse lists, etc., I think the best way to manage the preferences for the plug-in contributed resources is use the URI of the contributed resource as part of the preference name, and to treat them all as a set of booleans. Name should be something like "contributed.resource.isVisible.URI"

Going to alter my steps a bit. Going to work on just suppressing contributed resources first:

1. Create preference pane for suppressing resources
2. Update PluginSupportUtil.getContributedAadl() and friends
3. Verify that the AADL Navigator (which uses getContributedAadl() is updated. May need to generate refresh events of some sort?
4. Verify that the builder process ignores the suppressed resources. Can test this by having packages that make reference to contributed resources and seeing what happens when I make them go away.

[AaronGreenhouse]

This is useful to know: https://www.codeaffine.com/2016/03/01/swt-scrolledcomposite/

[AaronGreenhouse]

Note: preferences are not available in the stand-alone (headless) environment. Will need to provide a clean way to handle this in headless mode, but after I get it working in Eclipse.

[AaronGreenhouse]

Come back to updating Aadl2Storage2UriMapper, ContributedAadlEditorInputFactory

[AaronGreenhouse]

Added preference pane "Contributed Resources". Can select with contributed resources are visible. Added method PredeclaredProperties.getVisibleContributedResources() that filters out the invisible resources. Updated the callsites of PluginResourceUtil.getContributedAadl() to use this new method as appropriate.

The preference pane triggers a workspace build when in performOk(). This seems to make everything work okay, but

1. Issue #1824 has not been merged yet which makes things confusing, and
2. I need to be more selective about when to perform the build, such as when the user didn't actually change anything.

[AaronGreenhouse]

Updated the preference pane to allow workspace contributions.

Still need to

1. Save these in the preferences
2. Incorporate these into the visible contributed resources
3. Have the preference pane check for duplicates, both duplicate URIs and packages with the same name.

[AaronGreenhouse]

Did (1), above.

Should do (3) next, and then (2)

[AaronGreenhouse]

Regarding (3), above:

• It is not possible to have duplicate URIs in the plug-in contributions, or to have workspace contributions whose URI duplicates a plug-in contribution URI. The URIs effectively exist in different name spaces.
• It is not possible to add duplicates in an "add" operation because the ResourceSelectionDialog doesn't allow it.
• Therefore we only have to check that none of the URIs being added duplicate those that are already in the workspace contribution list.

The check for duplicate names must occur across all the contributed resources.

• It is possible for plug-in contributions to have the same name.
• It is possible to add identically named contributions in a single "add" operation
• It is possible to add a workspace contribution with the same name as a plug-in contribution.

Notably, this check needs to be done when the pane is opened to make sure that the plug-in contributions don't have duplicates.

[AaronGreenhouse]

Look into what the Restore Defaults button is really supposed to do.

[AaronGreenhouse]

Silly me, it's not necessary to test for duplicate URI explicitly because duplicate URI will have duplicate package name.

[AaronGreenhouse]

The preference pane is done and seems to work. The UI is bit clunky and ugly, but it's not worth making too pretty.

Need to incorporate the workspace contributions into the contributions returned by PredeclaredProperties.getVisibleContributedResources().

I expect there is going to be some craziness in project A if that project contributes a resource R. Just have to try it and see what happens.

In the AADL Navigator I would ideally like to have a Workspace Contributions item just like we have a Plug-in Contributions item. It's that too time-consuming to add just put them under Plug-in Contributions.

[AaronGreenhouse]

Things are not working correctly. I updated the classes Aadl2ProjectsStateHelper and PropertiesToBeBuiltComputerContribution. The workspace rebuilds after the preferences are updated, and they use the new values for the contributions, but references are not always resolved correctly, and usually get weird exceptions:

ENTRY org.apache.log4j 4 0 2019-08-05 16:37:38.501
!MESSAGE org.eclipse.xtext.builder.clustering.CopiedResourceDescription  - java.lang.IllegalStateException: getReferenceDescriptions platform:/resource/00foobar/Other.aadl

!STACK 0
java.lang.IllegalStateException: getReferenceDescriptions platform:/resource/00foobar/Other.aadl
    at org.eclipse.xtext.builder.clustering.CopiedResourceDescription.getReferenceDescriptions(CopiedResourceDescription.java:84)
    at org.eclipse.xtext.resource.DescriptionUtils.collectOutgoingReferences(DescriptionUtils.java:29)
    at org.eclipse.xtext.ui.editor.DirtyStateEditorSupport.isReparseRequired(DirtyStateEditorSupport.java:658)
    at org.eclipse.xtext.ui.editor.DirtyStateEditorSupport$UpdateEditorStateJob$1.exec(DirtyStateEditorSupport.java:161)
    at org.eclipse.xtext.ui.editor.DirtyStateEditorSupport$UpdateEditorStateJob$1.exec(DirtyStateEditorSupport.java:1)
    at org.eclipse.xtext.resource.OutdatedStateManager.exec(OutdatedStateManager.java:91)
    at org.eclipse.xtext.ui.editor.model.XtextDocument$XtextDocumentLocker.internalReadOnly(XtextDocument.java:527)
    at org.eclipse.xtext.ui.editor.model.XtextDocument$XtextDocumentLocker.readOnly(XtextDocument.java:499)
    at org.eclipse.xtext.ui.editor.model.XtextDocument.readOnly(XtextDocument.java:138)
    at org.eclipse.xtext.ui.editor.DirtyStateEditorSupport$UpdateEditorStateJob.run(DirtyStateEditorSupport.java:146)
    at org.eclipse.core.internal.jobs.Worker.run(Worker.java:63)

Here platform:/resource/00foobar/Other.aadl is file from the workspace that has been set to be contributed in the preferences.

[AaronGreenhouse]

I need to make sure that files are not processed twice, once because they are contributed and then again in their home project. This is in the old Aadl2ProjectStateHandler

    public String initHandle(URI uri) {
        if (uri.lastSegment().contentEquals(PredeclaredProperties.AADL_PROJECT)) {
            if (uri.toString().contentEquals(PredeclaredProperties.getAADLProjectPreference())) {
                return AADL_PROJECT_HANDLE;
            } else {
                return null;
            }
        } else if (CONTRIBUTED_AADL.contains(uri)) {
            return CONTRIBUTED_HANDLE;
        } else {
            return super.initHandle(uri);
        }
    }

I did look at this, but somehow kept missing the significance of return null.

I need to update so we can check a file name against all the contributed names.

[AaronGreenhouse]

Updated initHandle to

    public String initHandle(final URI uri) {
        if (PredeclaredProperties.hasSameNameAsVisibleContributedResource(uri)) {
            if (PredeclaredProperties.getVisibleContributedResources().contains(uri)) {
                return CONTRIBUTED_HANDLE;
            } else {
                // Don't include files that
                return null;
            }
        } else {
            return super.initHandle(uri);
        }
    }

The method PredeclaredProperties.hasSameNameAsVisibleContributedResource is

    public static boolean hasSameNameAsVisibleContributedResource(final URI testMe) {
        final String fileName = testMe.lastSegment();
        for (final URI uri : getVisibleContributedResources()) {
            if (uri.lastSegment().equalsIgnoreCase(fileName)) {
                return true;
            }
        }
        return false;
    }

This seems to fix the strange exception I was getting above. I assumed the problem was adding the URI for the workspace contribution more than once, but it seems to be caused by having duplicate AADL_Project property sets. (In general, the problem would be a visible plug-in contribution and a workspace file with the same name.)

[AaronGreenhouse]

Rebuilding the owrkspace is not a reliable way to fix references and build state after a change to the contributed resources. I have no idea why this is. I thought forcing a restart of Eclipse/OSATE would help. But it does not. What does seem to works is closing and reopening the project. I am going to try to make so that instead of rebuilding, all the open projects are closed and reopened.

[AaronGreenhouse]

Don't forget to update the docs

[AaronGreenhouse]

Updated the preference pane to close and reopen all the open projects instead of doing an explicit rebuild. For reasons that I don't understand, this seems to make things work beautifully and is also faster than running a rebuild or clean operation.

Note: This opening/closing of projects must be done in a regular Job not a WorkspaceJob. Doing it atomically as a WorkspaceJob causes events to be eaten or optimized away, and then the rebuilds that we want to have happen do not happen.

Note: This isn't a flawless way to do things. Sometimes there are exception traces generated in the error log, but they do not seem to break anything, and the end result in the workspace seems to be correct.

[AaronGreenhouse]

Updated the AADL Navigator to have separate Plug-in Contributions and Workspace Contributions headings.

[AaronGreenhouse]

Updated the OSATE user guide.

[AaronGreenhouse]

Current approach is too complicated. Should just present a list of all the currently contributed property sets and allow the user to choose which one to override. Overriding set must have the same name.

No "workspace contributions" in the AADL navigator. Instead show overridden property sets in italic. Show information in the status bar about overridding.

[AaronGreenhouse]

Redid this.

Property pane now shows a list of all the contributed resources. You can select one, and click on an override button. This allows you select a file with the same name in the workspace to replace it with. Alternatively, can use the "restore" button to return it to normal.

Within the AADL navigator, the location of the overriding (workspace) resource is used to locate the resource under "Plug-in Contributions". But selecting and overridden resource shows the location of the original and replacement uris in the description line.

(It is too hard to force the navigator to show the location of the original file, but still use the location of the replacement resource internally.)

Still need to update the help docs, but waiting for some more feedback before I do that.

[AaronGreenhouse]

More comments from @lwrage:

• AADL navigator should always show the location of the plug-in contributed file; The status bar should show the location of the overriding file (if any). The status bar should show the contributing plug-in in the normal case.

• AADL navigator should show in italics if the resource is overridden

• Browser launched by the preference pane should use the filter to only show eligible files (if possible). Otherwise, at least filter out closed projects and non-AADL projects.

• Ideally the list in the preference pane should be a tree that resembles the AADL Navigator. (Hand this off to @asazonova )

• May need a IResourceChangeListener of some sort to handle the case of the location of the overriding resource being changed. Not sure where this listener should come from and how to make it installed in a timely manner.

• When revising the help/docs make sure to mention the recommendation that overriding files should be in their own projects without any other files that depend on them. The projects should be depended upon by any other projects.

[AaronGreenhouse]

It is not possible to change the text style of the navigator labels. The existing (old) way of doing things does not change the text style either. We could add a decorator to the icon, but then I have to get artistic and that seems hard. We can add to the label of the item though, either postfix " (overridden)", or just enclosing the name in brackets of some sort.

[AaronGreenhouse]

Updated so that a contributed property set that is overridden is bracketed by "<...>" in the AADL Navigator.

Changed the selection dialog in the contributed resource preference pane to only show acceptable overriding resources.

[AaronGreenhouse]

Updated the status bar to show the contributing plug-in

[AaronGreenhouse]

Updated so that the AADL navigator always shows a subtree based on the contributed resources. If a URI is overridden, the label is marked " (overridden)" and the status line shows the overriding uri.

[AaronGreenhouse]

Need an IResourceChangeListener installed by PluginSupportPlugin that deals with overriding workspace resources being moved or deleted. (Looking at ProjectRenameHandler for inspiration.)

[AaronGreenhouse]

PluginSupportPlugin adds aIResourceChangeListener` to watch for changes to the workspace resources that override plug-in contributions. If a file is moved, then the overrides mapping is updated. If a file is deleted or renamed then it is removed the mapping.

Had to be careful to shutdown the listener during the "project closing and opening" phase otherwise it breaks everything because closing a project counts as deleting a file.

Also had to be careful to avoid spurious rebuilds of the state.

[AaronGreenhouse]

Need to redo the display in the preference pane. Should pass that task to @asazonova .

[AaronGreenhouse]

For @asazonova:

ContributedResourcesPreferencePage currently uses a bland ListViewer to display the list of contributed resources:

We should switch to a tree viewer that is more similar to the AADL navigator:

The only catch is that when a resource is overridden by a workspace file the label should change to indicate this. Right now I have an "(Overridden)" suffix. If you have a better idea that is good too.

[AaronGreenhouse]

Had a problem today where the overridding wasn't working. Seems that my .prefs file in the workspace was messed up. When I deleted the prefs for org.osate.pluginsupport and tried again, everything worked fine.

I updated PredeclaredProperties.setOverriddenResources() to clean up the old settings first. Hopefully this prevents problems in the future.

    public static synchronized void setOverriddenResources(final Map<URI, URI> replaced) {
        selfUpdating = true;

        /*
         * First clean up the old settings. This isn't strictly necessary, but things can bet confusing if the
         * number of overrides shrinks but the old key and value preferences are still left hanging around.
         */
        final int oldSize = preferenceStore.getInt(NUMBER_OF_WORKSPACE_OVERRIDES);
        for (int i = 0; i < oldSize; i++) {
            final String keyName = WORKSPACE_OVERRIDE_KEY_PREFIX + i;
            preferenceStore.setToDefault(keyName);

            final String valueName = WORKSPACE_OVERRIDE_VALUE_PREFIX + i;
            preferenceStore.setToDefault(valueName);
        }
        preferenceStore.setToDefault(NUMBER_OF_WORKSPACE_OVERRIDES);

        /* Now set the new values */
        final int size = replaced.size();
        preferenceStore.setValue(NUMBER_OF_WORKSPACE_OVERRIDES, size);
        int i = 0;
        for (Map.Entry<URI, URI> entry : replaced.entrySet()) {
            final String keyName = WORKSPACE_OVERRIDE_KEY_PREFIX + i;
            preferenceStore.setValue(keyName, entry.getKey().toString());

            final String valueName = WORKSPACE_OVERRIDE_VALUE_PREFIX + i;
            preferenceStore.setValue(valueName, entry.getValue().toString());
        }
        isChanged = true;
        selfUpdating = false;
    }

[AaronGreenhouse]

Merged tree viewer changes from Anna. Added a double-click listener to the tree: double-clicking a directory will expand/collapse the tree. Double-clicking a file node is like clicking the "override" button.