Bug 560093 - Sirius loads Xtend files as models by default, breaking them if the Sirius/Xtext compatibility plug-in is not installed
Summary: Sirius loads Xtend files as models by default, breaking them if the Sirius/Xt...
Status: NEW
Alias: None
Product: Sirius
Classification: Modeling
Component: Core (show other bugs)
Version: 6.3.0   Edit
Hardware: All All
: P3 major (vote)
Target Milestone: ---   Edit
Assignee: Project inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 535199 547162
Blocks:
  Show dependency tree
 
Reported: 2020-02-13 09:24 EST by Rolf Theunissen CLA
Modified: 2020-02-21 05:42 EST (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Rolf Theunissen CLA 2020-02-13 09:24:48 EST
Have an project with Xtend files and a Sirius modeling nature with an AIRD file.

After some operations, rename of random Xtend file in the workspace or a git pull, Sirius gets the wonderful idea to start mangling Xtend files.
The contents of the Xtend files is replaced by Sirius with slightly different contents. Trying to revert these files to original is not possible, because as soon as the file is changed, the automatic destruction mechanism of Sirius is started that mangles the files again.

Sirius should *never ever* considering changing files without permission.
Comment 1 Pierre-Charles David CLA 2020-02-13 09:58:53 EST
Hi,

I understand this can be confusing, but this is the expected behavior of Modeling Projects (see [1]): when you add the 'Modeling' nature you basically tell Sirius to manage all the EMF models inside the project. Xtend being Xtext-based, .xtend files are detected as valid EMF models and loaded as part of the Sirius session, and then saved when the .aird is saved (which can happen behind the scene under some conditions).

If you do not want this behavior, there are two options:
* do not use de 'Modeling Project' nature at all, and manage the "aird" file manually, see [2] (despite its name in the documentation, this "legacy" mode is not deprecated and will always be supported, notably to handle this kind of scenario where the default "discover and manage all EMF models" behavior is not adapted).
* use the org.eclipse.sirius.resourceStrategy extension point to provide a custom ResourceStrategy which tells Sirius not to consider "*.xtend" files ad loadable EMF models (see the ResourceStrategy.isLoadableModel(URI uri, Session session) method).

Regards,

[1] https://www.eclipse.org/sirius/doc/user/general/Modeling%20Project.html#ModelsInMP
[2] https://www.eclipse.org/sirius/doc/user/general/Modeling%20Project.html#LegacyMode
Comment 2 Rolf Theunissen CLA 2020-02-13 10:21:19 EST
I can understand the behavior of the modeling project, and Xtend being EMF that they are included.

What I don't understand is that the modeling project changes (read *destroys*) the Xtend files. The modeling project somehow bypasses the full Xtend behavior, which results in parse-errors in for the AIRD linked file. Instead of ignoring these errors, the modeling projects forces an save action an the Xtend file. Which is changing the file.

It is *never* expected behavior if an IDE destroys files (and insists on keeping the broken version). There is *NO* way for the user to correct this, other then removing the modeling nature.
Comment 3 Pierre-Charles David CLA 2020-02-14 10:29:42 EST
(In reply to Rolf Theunissen from comment #2)
> I can understand the behavior of the modeling project, and Xtend being EMF
> that they are included.
> 
> What I don't understand is that the modeling project changes (read
> *destroys*) the Xtend files.

Unless you have installed the the org.eclipse.sirius.runtime.ide.xtext optional feature, Sirius loads the models it detects using a plain EMF ResourceSetImpl. Xtext (and thus Xtend) does not work well with this default ResourceSet. The org.eclipse.sirius.runtime.ide.xtext feature makes the Sirius sessions "Xtext-aware" by using the XtextResourceSet implementation and tweaking some load/save options to improve the behavior with Xtext models (not sure it would fix all your issues, but if you do not have it installed it should improve the situation).

The main issue in you case is that you do not want Sirius to even consider these Xtend files as models. The immediate solution is not to use the modeling nature. You'll lose some of the automatic behaviors of Sirius, but gain precise control on which files are managed (or not) by Sirius.

On our side, we could probably make it clearer in the documentation about this behavior, and warn that if a project contains EMF models you do not want Sirius to handle (notably Xtext ones), the "Legacy Mode" (which should be renamed "Manual Mode") should be prefered.

We have the infrastructure code in place internally to be more precise/discriminate on what files we consider as "loadable EMF models", and could be used to ignore "*.xtend" files even when using the "Modeling Project" nature, but configuring this is currently too cumbersome (one has to extend the resourceStrategy extension point mentioned above). There should probably be an easier mechanism to configure this for the common case of ignoring some file extensions (but what should be the default extensions to ignore? any file can be an EMF model given the proper resource implementation).
Comment 4 Rolf Theunissen CLA 2020-02-17 05:57:47 EST
(In reply to Pierre-Charles David from comment #3)
> (In reply to Rolf Theunissen from comment #2)
> > I can understand the behavior of the modeling project, and Xtend being EMF
> > that they are included.
> > 
> > What I don't understand is that the modeling project changes (read
> > *destroys*) the Xtend files.
> 
> Unless you have installed the the org.eclipse.sirius.runtime.ide.xtext
> optional feature, Sirius loads the models it detects using a plain EMF
> ResourceSetImpl. Xtext (and thus Xtend) does not work well with this default
> ResourceSet. The org.eclipse.sirius.runtime.ide.xtext feature makes the
> Sirius sessions "Xtext-aware" by using the XtextResourceSet implementation
> and tweaking some load/save options to improve the behavior with Xtext
> models (not sure it would fix all your issues, but if you do not have it
> installed it should improve the situation).

I did not have this plugin installed. Indeed, it does solve many of the issues that I did observe. 
Though opening the first Xtend file is slowed down a lot, while the AIRD file is being opened. At least it is obvious where the slow-down comes from.

> The main issue in you case is that you do not want Sirius to even consider
> these Xtend files as models. The immediate solution is not to use the
> modeling nature. You'll lose some of the automatic behaviors of Sirius, but
> gain precise control on which files are managed (or not) by Sirius.

I have another issue with Xtend files not being considered, that is the other bug about. Main reason for this bug is that in the default configuration there are some major issues with Xtend/Sirius inter-operation. But you have a plugin for that.
It would be nice if Eclipse had some way to automatically suggest to install that plugin if both Xtend and Sirius are installed. I don't know if that is possible by OSGi/Equinox.

> On our side, we could probably make it clearer in the documentation about
> this behavior, and warn that if a project contains EMF models you do not
> want Sirius to handle (notably Xtext ones), the "Legacy Mode" (which should
> be renamed "Manual Mode") should be prefered.

Renaming to "Manual Mode" would be very good. By naming it "Legacy Mode" users (me) are under the impression that some (new) functionality is missing.

> We have the infrastructure code in place internally to be more
> precise/discriminate on what files we consider as "loadable EMF models", and
> could be used to ignore "*.xtend" files even when using the "Modeling
> Project" nature, but configuring this is currently too cumbersome (one has
> to extend the resourceStrategy extension point mentioned above). There
> should probably be an easier mechanism to configure this for the common case
> of ignoring some file extensions (but what should be the default extensions
> to ignore? any file can be an EMF model given the proper resource
> implementation).

Lets track that in Bug 547162
Comment 5 Pierre-Charles David CLA 2020-02-20 08:51:35 EST
(In reply to Rolf Theunissen from comment #4)
> I did not have this plugin installed. Indeed, it does solve many of the
> issues that I did observe. 
> Though opening the first Xtend file is slowed down a lot, while the AIRD
> file is being opened. At least it is obvious where the slow-down comes from.

The slow down is most probably because Xtend integrates with the Java type system, which makes Xtext expose all the available Java types as EMF models, and because they are dependencies of the Xtend file, Sirius loads them all when initializing its session/ResourceSet).

> > The main issue in you case is that you do not want Sirius to even consider
> > these Xtend files as models. The immediate solution is not to use the
> > modeling nature. You'll lose some of the automatic behaviors of Sirius, but
> > gain precise control on which files are managed (or not) by Sirius.
> 
> I have another issue with Xtend files not being considered, that is the
> other bug about. Main reason for this bug is that in the default
> configuration there are some major issues with Xtend/Sirius inter-operation.
> But you have a plugin for that.
> It would be nice if Eclipse had some way to automatically suggest to install
> that plugin if both Xtend and Sirius are installed. I don't know if that is
> possible by OSGi/Equinox.

Not sure either. The thing is, you had the issue with Xtend, but it actually concerns all Xtext-based languages. We do not want Sirius to have a hard-dependency on Xtext, so the plug-in can not be part of the core Sirius components, but if it is not installed, by the time we try to discover potential models to load from inside a project, if it's an Xtext resource then i) it's too late to install the plug-in, ii) I'm not even sure Sirius can detect if an EMF model is an org.eclipse.xtext.resource.XtextResource without depending on Xtext (maybe with some reflection, or optional dependencies, but everything I can think of seem tricky and brittle).

I guess we could consider Xtend a special case, as in pratice it is used more as a programming language than as models. The easiest way would be to add a special case in org.eclipse.sirius.tools.internal.resource.ModelingProjectFileQuery.isPotentialSemanticResource() to ignore "*.xtend", but that feels ugly.

> > On our side, we could probably make it clearer in the documentation about
> > this behavior, and warn that if a project contains EMF models you do not
> > want Sirius to handle (notably Xtext ones), the "Legacy Mode" (which should
> > be renamed "Manual Mode") should be prefered.
> 
> Renaming to "Manual Mode" would be very good. By naming it "Legacy Mode"
> users (me) are under the impression that some (new) functionality is missing.

I was about to enter a new bug for this, but actually we already got bug 547162. We just never got around to work on it. Adding the dependency.

> > We have the infrastructure code in place internally to be more
> > precise/discriminate on what files we consider as "loadable EMF models", and
> > could be used to ignore "*.xtend" files even when using the "Modeling
> > Project" nature, but configuring this is currently too cumbersome (one has
> > to extend the resourceStrategy extension point mentioned above). There
> > should probably be an easier mechanism to configure this for the common case
> > of ignoring some file extensions (but what should be the default extensions
> > to ignore? any file can be an EMF model given the proper resource
> > implementation).
> 
> Lets track that in Bug 547162

OK. I'm reducing the severity on this one, which now boils down to 3 separate issues:

* Improve the documentation as stated in comment 3: tracked in 535199.
* Do not consider Xtend files as loadable models by default (or at least make it easy to ignore them without resorting to writing a Java extension): tracked in 547162.
* See if we could automatically suggest the installation of Sirius/Xtext optional plug-in when the user starts using Xtext models: this can stay on this ticket for now, maybe we'll create a separate one later.
Comment 6 Rolf Theunissen CLA 2020-02-21 04:43:17 EST
(In reply to Pierre-Charles David from comment #5)

> * See if we could automatically suggest the installation of Sirius/Xtext
> optional plug-in when the user starts using Xtext models: this can stay on
> this ticket for now, maybe we'll create a separate one later.

The other option would be to always have the Sirius/Xtext plugin installed, but only activated when Xtext is available.

The Sirius/Xtext plugin could have an non-greedy optional dependency on Xtext, if I am correct, this will not install Xtext when this plugin is installed.
However, this plugin will always be activated, because its non-optional required plugins are available. I don't know if activation can be prevented if Xtext is not available. But there should be ways to disable the plugin functionality when Xtext is not available.
Comment 7 Pierre-Charles David CLA 2020-02-21 05:42:24 EST
(In reply to Rolf Theunissen from comment #6)
> (In reply to Pierre-Charles David from comment #5)
> 
> > * See if we could automatically suggest the installation of Sirius/Xtext
> > optional plug-in when the user starts using Xtext models: this can stay on
> > this ticket for now, maybe we'll create a separate one later.
> 
> The other option would be to always have the Sirius/Xtext plugin installed,
> but only activated when Xtext is available.
> 
> The Sirius/Xtext plugin could have an non-greedy optional dependency on
> Xtext, if I am correct, this will not install Xtext when this plugin is
> installed.
> However, this plugin will always be activated, because its non-optional
> required plugins are available. I don't know if activation can be prevented
> if Xtext is not available. But there should be ways to disable the plugin
> functionality when Xtext is not available.

Yes, that's roughly what I was thinking of, but I'm not sure either about the behavior, thus the "tricky and brittle" remark.