Bug 349972

Comment 1 Louis Rose 2011-06-21 14:06:46 EDT

This bug might be related to the following code in ModelRepository#getModelByName, which I think was introduced for EUnit:

// Note: EUnit's model bindings depend on having "" aliased to the first model.
// If you change this, don't forget to change EUnitModule#runSuite!
if (modelName.length() == 0){
	if (models.size() > 0) {
	//if (models.size() == 1) {
		return models.get(0);
	}
	else return null;
}

I think reinstating the old code (below) should fix this, but will break EUnit. Is there a way we can localise this requirement in the EUnit code.

if (modelName.length() == 0){
	if (models.size() == 1) {
		return models.get(0);
	}
	else return null;
}

Antonio, what do you think?

Comment 2 Dimitris Kolovos 2011-06-21 14:15:17 EDT

I think this precedes EUnit and was actually intentional so that the first model in the configuration could be treated as "primary". Let's have a chat about this in the next telecon so that I can explain the rationale behind this and perhaps we can come up with a better solution.

Comment 3 Louis Rose 2011-06-21 14:19:42 EDT

(In reply to comment #2)
> I think this precedes EUnit and was actually intentional so that the first
> model in the configuration could be treated as "primary". Let's have a chat
> about this in the next telecon so that I can explain the rationale behind this
> and perhaps we can come up with a better solution.

To be clear, I think that this primary behaviour is fine: when there's only one model there's no room for ambiguity.

The problem is that the current code (below) allows a model name of "" to be used when there's more than one model in the model repository. When there's more than one model in the repository, there can be type ambiguity, and this can be confusing.

// Note: EUnit's model bindings depend on having "" aliased to the first model.
// If you change this, don't forget to change EUnitModule#runSuite!
if (modelName.length() == 0){
    if (models.size() > 0) {
    //if (models.size() == 1) {
        return models.get(0);
    }
    else return null;
}

Comment 4 Antonio Garcia-Dominguez 2011-06-21 14:20:33 EDT

"svn blame" says I only added the two comment lines there to make sure anyone touching that code had EUnit in mind. That did work, didn't it? :-D

I think Dimitris introduced that code in 11th September 2008 (revision 8). Perhaps you should ask him before reverting that change.

In any case, even we change it, it shouldn't be that big of an issue. In fact, it should simplify EUnit quite a bit. Right now, we have to treat the first model differently from the rest, because of this "default model" concept.

We should also think about breaking existing EOL scripts. I think we should throw a warning, instead of an error. We could switch to an error a few releases down the road.

Comment 5 Louis Rose 2011-06-21 14:27:23 EDT

(In reply to comment #4)
> "svn blame" says I only added the two comment lines there to make sure anyone
> touching that code had EUnit in mind. That did work, didn't it? :-D
> 
> I think Dimitris introduced that code in 11th September 2008 (revision 8).
> Perhaps you should ask him before reverting that change.
> 
> In any case, even we change it, it shouldn't be that big of an issue. In fact,
> it should simplify EUnit quite a bit. Right now, we have to treat the first
> model differently from the rest, because of this "default model" concept.
> 
> We should also think about breaking existing EOL scripts. I think we should
> throw a warning, instead of an error. We could switch to an error a few
> releases down the road.

D'oh! I misinterpreted the comment, sorry Antonio. I thought this was a recent change for EUnit, rather than a long-standing feature of EOL!

I agree with Antonio, we should avoid making changes that will break existing EOL scripts: a warning would be a good idea, if the type is ambiguous. Let's discuss this together on Thursday.

Comment 6 Louis Rose 2011-06-23 07:10:47 EDT

We discussed this in person today. In summary...

Currently:
- EOL permits unqualified types when there is only one model in the repository.
- Consequently, users have (in the past) written sets of EOL operations (with unqualified types) that they wish to import into an Epsilon program that uses more than one model (e.g. a transformation). Therefore, EOL *always* resolves unqualified types in the first model in a repository.

This is perhaps unpredictable behaviour from a user's perspective. This behaviour is particularly problematic in Flock because the Original and Migrated models often conform to metamodels that share a lot of types.

We decided that the minimum we need to do is issue a warning when an unqualified reference is used for a type that is defined by more than model in the repository. This bug will cover that change.

We will also investigate a better mechanism for importing EOL operations into other programs (e.g. transformations).

Comment 7 Louis Rose 2011-07-14 10:31:33 EDT

(In reply to comment #6)
> We decided that the minimum we need to do is issue a warning when an
> unqualified reference is used for a type that is defined by more than model in
> the repository. This bug will cover that change.

I've checked in to SVN a fix that issues warnings when ambiguous types are used.

Comment 8 Dimitris Kolovos 2011-07-25 08:18:41 EDT

Fixed in 0.9.1