Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 346031

Summary:	[generator] Genmodel configuration is redundant
Product:	[Modeling] TMF	Reporter:	Knut Wannheden <knut.wannheden>
Component:	Xtext	Assignee:	Project Inbox <tmf.xtext-inbox>
Status:	CLOSED FIXED	QA Contact:
Severity:	normal
Priority:	P3	CC:	moritz.eysholdt, sebastian.zarnekow, sven.efftinge
Version:	2.0.0	Flags:	sven.efftinge: indigo+
Target Milestone:	RC2
Hardware:	All
OS:	All
Whiteboard:

Description Knut Wannheden

2011-05-17 02:20:47 EDT

When creating a new Xtext project (using the new experimental generator configuration) and running the workflow the generated AbstractMyDslSemanticSequencer contains errors. It has references to a non existant sequencer of the terminals grammar:

	@Inject
	protected Provider<TerminalsSemanticSequencer> superSequencerProvider;
	 
	protected TerminalsSemanticSequencer superSequencer; 
	
Somehow this doesn't seem to happen for Xtype.

(I also think we may want to remove or comment out the ParseTreeConstructorFragment from the experimental generator configuration, as we can only ever have one ISerializer.)

Comment 1 Sven Efftinge

2011-05-17 02:22:49 EDT

(In reply to comment #0)
> (I also think we may want to remove or comment out the
> ParseTreeConstructorFragment from the experimental generator configuration, as
> we can only ever have one ISerializer.)

Yes, of course. Please go ahead.

Comment 2 Sebastian Zarnekow

2011-05-17 02:26:21 EDT

(In reply to comment #0)
> (I also think we may want to remove or comment out the
> ParseTreeConstructorFragment from the experimental generator configuration, as
> we can only ever have one ISerializer.)

Thanks for the hint. Pushed to master.

Comment 3 Knut Wannheden

2011-05-17 02:40:15 EDT

(In reply to comment #2)
> 
> Thanks for the hint. Pushed to master.

That was quick :-)

I was just about to do it myself. I also thought about adding two comments to the StandaloneSetup:

    bean = StandaloneSetup {
        scanClassPath = true
        platformUri = "${runtimeProject}/.."
        // registerGenModelFile = "platform:/resource/org.eclipse.xtext.xbase/model/Xbase.genmodel"
        // registerGenModelFile = "platform:/resource/org.eclipse.xtext.common.types/model/JavaVMTypes.genmodel"
    }

But wouldn't it actually make more sense if the EcoreGeneratorFragment also registered its referencedGenModels (provided that they haven't already been registered)? That would save the developer some duplication (and trouble!).

If I currently don't add corresponding registerGenModelFile elements to StandaloneSetup I may get errors like the following error when running the workflow:

5135 ERROR CompositeGeneratorFragment - No GenModel for EPackage 'http://www.eclipse.org/xtext/xbase/Xbase' is registered.
java.lang.RuntimeException: No GenModel for EPackage 'http://www.eclipse.org/xtext/xbase/Xbase' is registered.
	at org.eclipse.xtext.generator.GenModelAccess.getGenPackage(GenModelAccess.java:81)
	at org.eclipse.xtext.generator.serializer.AbstractSemanticSequencer.genMethodCreateSequence(AbstractSemanticSequencer.java:463)
	at org.eclipse.xtext.generator.serializer.AbstractSemanticSequencer.getFileContents(AbstractSemanticSequencer.java:409)
	at org.eclipse.xtext.generator.serializer.SerializerFragment.generate(SerializerFragment.java:92)

Comment 4 Sebastian Zarnekow

2011-05-17 03:11:51 EDT

Knut,

I was probably too fast.
Please update / fix the experimental configuration to make it as easy as possible to create a language that reuses Xbase.

Thanks!

Comment 5 Knut Wannheden

2011-05-17 03:20:35 EDT

(In reply to comment #4)
> I was probably too fast.
> Please update / fix the experimental configuration to make it as easy as
> possible to create a language that reuses Xbase.
> 

Currently it would help to add the proposed comments to StandaloneSetup. But then you end up with the same list of referenced GenModels (in comments) for both the StandaloneSetup and the EcoreGeneratorFragment!

So I'm wondering if it wouldn't make more sense to change the EcoreGeneratorFragment to additionally also register all referenced GenModels in EcorePlugin.getEPackageNsURIToGenModelLocationMap(). Then the developer would not have to add them to the StandaloneSetup (and we wouldn't have to change the configuration again :-)).

Any other thoughts?

Comment 6 Sven Efftinge

2011-05-17 03:46:02 EDT

(In reply to comment #5)
> So I'm wondering if it wouldn't make more sense to change the
> EcoreGeneratorFragment to additionally also register all referenced GenModels
> in EcorePlugin.getEPackageNsURIToGenModelLocationMap(). Then the developer
> would not have to add them to the StandaloneSetup (and we wouldn't have to
> change the configuration again :-)).

Yes, I think we should do that. Would make it more consistent. 

All the components needing genmodels should retrieve them from the EMF singleton.

The EcoreGeneratorFragment should register dynamically created and explicitly configured genmodels in the singelton. The configuration of genmodels in any generator component should be marked Deprecated and the use of the StandaloneSetup should be encouraged.

Does that work? Moritz?

Comment 7 Sebastian Zarnekow

2011-05-17 03:53:00 EDT

I moved the actually reported issue to a new ticket since this one was highjacked by the genmodel-situation

Comment 8 Moritz Eysholdt

2011-05-17 09:33:50 EDT

(In reply to comment #6)
> (In reply to comment #5)
> > So I'm wondering if it wouldn't make more sense to change the
> > EcoreGeneratorFragment to additionally also register all referenced GenModels
> > in EcorePlugin.getEPackageNsURIToGenModelLocationMap(). Then the developer
> > would not have to add them to the StandaloneSetup (and we wouldn't have to
> > change the configuration again :-)).
> 
> Yes, I think we should do that. Would make it more consistent. 
> 
> All the components needing genmodels should retrieve them from the EMF
> singleton.
> 
> The EcoreGeneratorFragment should register dynamically created and explicitly
> configured genmodels in the singelton. The configuration of genmodels in any
> generator component should be marked Deprecated and the use of the
> StandaloneSetup should be encouraged.
> 
> Does that work? Moritz?

yes. I'll provide an implementation.

Comment 9 Moritz Eysholdt

2011-05-18 09:52:59 EDT

fixed; pushed to master;

The EcoreGeneratorFragment now collects all referenced EPackages from the to-be-generated EPackage. Then, the corresponding GenModel from the registry are obtained and added to the list of "usedGenModels". If no GenModel is found, a warning is logged but generation continues.

EcoreGeneratorFragment.setReferencedGenModels(String) is now deprecated. For backward compatibility, its GenModels are now added to the registry on EcoreGeneratorFragment.generate() and before the GenModel is created.

Comment 10 Moritz Eysholdt

2011-05-19 06:21:05 EDT

throw an error if no GenModel is found for an EPackage instead of logging a warning.

Comment 11 Moritz Eysholdt

2011-05-19 10:55:50 EDT

fixed; pushed to master;

Comment 12 Karsten Thoms

2017-09-19 16:56:57 EDT

Closing all bugs that were set to RESOLVED before Neon.0

Comment 13 Karsten Thoms

2017-09-19 17:08:11 EDT

Closing all bugs that were set to RESOLVED before Neon.0