Bug 128866 - M5: Behavior change when computing the ID of project natures
Summary: M5: Behavior change when computing the ID of project natures
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Runtime (show other bugs)
Version: 3.2   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.2 M6   Edit
Assignee: Oleg Besedin CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 139307 (view as bug list)
Depends on:
Blocks: 128838 128904
  Show dependency tree
 
Reported: 2006-02-21 13:55 EST by Marcelo Paternostro CLA
Modified: 2006-05-01 06:40 EDT (History)
20 users (show)

See Also:


Attachments
Patch for maintaining backward compatibility for IDs with dots (5.29 KB, patch)
2006-02-22 10:52 EST, Oleg Besedin CLA
no flags Details | Diff
updated patch (8.87 KB, patch)
2006-02-22 14:00 EST, DJ Houghton CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Marcelo Paternostro CLA 2006-02-21 13:55:50 EST
For ages the ID of the following nature had been computed to <plugin-id>.jet.IJETNature

  <extension point="org.eclipse.core.resources.natures" id="jet.IJETNature">
    <runtime>
      <run class="org.eclipse.emf.codegen.jet.JETNature" />
    </runtime>
  </extension>

At some point between the I20060118-0800 and M5 drivers, the ID computation has changed so that nature IDs with a '.' (and maybe other characters) are not prefixed with the plugin ID.

This has broken our builder in M5 (and also the nature's property page displayed in the project's properties dialog).  Although the fix was trivial (fully qualifying the nature's ID), it took us a couple of hours to identify the reason why our code was not working.

Was this behavior change intentional or is this a side effect of another change? If it was intentional, I would suggest documenting it before others have the same problem.
Comment 1 Pascal Rapicault CLA 2006-02-21 13:59:57 EST
Oleg, could you please advise?
Comment 2 John Arthorne CLA 2006-02-21 14:20:47 EST
I don't think any changes have been made on the resources side that could effect this.
Comment 3 Oleg Besedin CLA 2006-02-21 14:26:56 EST
Yes, the change was intentional - see bug 112856 ("namespace separation").

In short, the ID used for this extension point was invalid. The "." was not a valid character for the ID; the ID containing "." should have produced error in the PDE wizard and warning for the plugin.xml file itself. 

Note that using the "." in the ID prior to the namespace separation enhancement would have produced some interesting side effects in the search and update mechanism for those objects. (I am trying to say that were were some real reasons why we didn't want "."s to be used in the IDs :-)).

Late in M5 we added ability to contribute to other namespace. If the ID has a ".", portion after the last "." is interprested as the simple name and the rest is interpreted as a namespace.

Yes, we still need to update PDE UI and Javadocs to catch up with this change.

Could you specify as to why you need the Id to be "<plugin-id>.jet.IJETNature" rather than "<plugin-id>.IJETNature"? 
Comment 4 Marcelo Paternostro CLA 2006-02-21 15:50:29 EST
Well... the truth is that this nature has this name for quite a while (for sure since Eclipse 2.0 and maybe since Eclipse 1.0).  I was not kidding when I wrote "for ages".  And it has always worked.

I wouldn't be able to say right now why we chose this name 4 years ago.  Maybe the fact that '.' is a valid character in XML Schema's ID definition has helped not raising a flag.
Comment 5 Doug Schaefer CLA 2006-02-21 15:56:03 EST
Get ready for the avalanch :)

We (QNX internal) have the same thing. Having dots in the id names has never appeared as a warning, and given the change you have made, this should have been an error.

The reason why this is now a huge problem is that all of our customers have projects with natures that have dots in their ids since we've done this since Eclipse 1.0. It won't be simple to change.
Comment 6 Doug Schaefer CLA 2006-02-21 16:12:58 EST
Actually, if you use the feature as Oleg has built, all you need to do is put the fully qualified name as the id of the nature extension, i.e. prefix it with the plugin id. Worked for us.
Comment 7 Marcelo Paternostro CLA 2006-02-21 16:26:25 EST
Yeap...  As we were debugging the code yesterday, we've noticed that fully qualifying the nature's id would fix the problem given that the .project files refers to the computed id.

I've opened this bug to find out why this has happened (it could be a regression) and to suggest that some highly visible documentation may be required.  We didn't get any message in the log file so this is a pretty though problem to identify - my initial thought was that my builder was broken.
Comment 8 Pascal Rapicault CLA 2006-02-21 17:18:12 EST
We had not realized how many people were using that even though it was forbidden (the 1.0 doc is not super clear but let it understand). That said, as usual, your friends from the runtime team will make everything possible to not break you. 

Therefore, here is what we will do:
- For all plugin.xml that have the tag  <?eclipse version="3.0"?>  or don't have it at all, the behavior will stay unchanged. This means that if you specified <extension point="org.eclipse.core.resources.natures" id="jet.IJETNature"> then the fully qualified name of the extension will be "{pluginName}.jet.IJETNature". This will preserver backward compatibility with 3.1 and before.

- All plug-ins willing to take advantage of the separation of namespace will have to state in their plugin.xml or fragment.xml <?eclipse version="3.2"?> instead of <?eclipse version="3.0"?>. This means that if you specified <extension point="org.eclipse.core.resources.natures" id="jet.IJETNature"> then the fully qualified name of the extension will be "jet.IJETNature". 

We are actively working on a patch. Stay tuned.
Comment 9 Thomas Watson CLA 2006-02-21 17:21:18 EST
Pascal, with this affect the plugin converter from the framework if you add a new version 3.2?  Will we have to handle that?
Comment 10 Thomas Watson CLA 2006-02-21 17:29:19 EST
The code for the plugin parser in the framework does not seem to care what the version is.  If it is specified then it is assumed >= 3.0.  If it is not specified then the plugin.xml is assumed to be pre-3.0.  So I think I've answered my own question.  Having a version 3.2 will not break the plugin converter.  Pascal, please confirm.
Comment 11 Pascal Rapicault CLA 2006-02-21 17:34:02 EST
This does not require any change from the plug-in converter.
The only changes required are in the registry code and in PDE. I have open the bug #128904 for the latter.
Comment 12 Marcelo Paternostro CLA 2006-02-22 09:50:01 EST
I don't believe Eclipse would respin M5 to include this fix, right?  So if we want/need to release our components in M5 we will need to fully qualify the id, and we will also need to change the version number to avoid future breakage.

What are the other implications of changing the version of the plugin.xml?  For example, does it affect how IExtension.getNamespace() (or, better, getNamespaceIdentifier() and getContributor()) behave?
Comment 13 Pascal Rapicault CLA 2006-02-22 10:38:52 EST
Respinning M5 is being discussed. We will let you know.
Comment 14 Oleg Besedin CLA 2006-02-22 10:52:50 EST
Created attachment 35147 [details]
Patch for maintaining backward compatibility for IDs with dots

The patch implements approach specified by Pascal. It looks like we will have a rebuild for M5.

To avoid problems, I'd advise to either stick with pre-M5 version and old IDs until we get a new build, or fix IDs to be fully qualified AND update version if the eclipse in the plugin.xml to 3.2.

Please note that, unless we go some different approach, if you updated your IDs to be fully qualified, you have to update versions of the plugin.xml to 3.2 to avoid breakage once this patch is added.

To summarize the migration process: if you have “.”s in the IDs of extensions / extension points, the plugin.xml file needs to be modified to continue working with the current build. The next build (and we might have a rebuild of M5) should contain this patch that is going to eliminate the compatibility problem.

If you decide to pick up the current build and/or update plugin.xml to the new version: 
1) change ID to be fully qualified. Fully qualified Id = <plugin Id> + “.” + <your current ID with a ‘dot’ in it>.
and
2) change version of the “eclipse” tag to 3.2 to avoid breakage in the future versions. Instead of
<?eclipse version="3.0"?>
(this typically is the second line of the plugin.xml), put
<?eclipse version="3.2"?>

Just a reminder that, as a rule, it is best to avoid using qualified IDs. There are some situations there it is desirable or necessary (and supporting backward compatibility is high on that list), but the “default” mode should be using a “simple” ID (no dots :-).
Comment 15 Oleg Besedin CLA 2006-02-22 12:27:17 EST
To answer the question on the getNamespace() / gateNamespaceIdentifier() / getContributor() methods (the answer applied to both IExtension and IExtensionPoint):

- [deprecated] getNamespace() - returns plugin Id regardless of version (both pre-M5 and post-M5)
- [new] getNamespaceIdentifier() – returns namespace “name” that is version dependent. 
For versions < 3.2 it will return plugin id, for versions >= 3.2 it will return plugin id for elements with Id like “myId” and “foo” for the Ids like “foo.myId”
- [new] getContributor() – returns IContributor that boils down to the plugin Id regardless of version

(If fragments are contributing, plugin Id of the hosting plugin is returned.) 

Internally, implementations of extension points and extensions are different and it would require some time to figure out what exactly happens with registry elements with dots in IDs. I suspect that there might be problems with updating such plugins (as updater uninstalls old plugin.xml and installs new plugin.xml) and with discovering such registry elements (searching by namespace or qualified name).

If having dots in names worked for you, chances are that it is still going to work. However, chances are that at some point an oddity in the search or update will show up (or it might be already showing up, but being rare or insignificant in the normal user workflow).

Now that I had a minute to think about it :-), I would recommend to change to a fully qualified Id and update version number as this is the usage scenario that we are testing and supporting. While we will try to maintain backward compatibility, there is no guarantee that processing of the registry elements with dots in simple names is accurate (pre-M5 or post-M5).
Comment 16 DJ Houghton CLA 2006-02-22 14:00:56 EST
Created attachment 35174 [details]
updated patch

This is an updated patch after the review.
Comment 17 Jeff McAffer CLA 2006-02-22 17:38:03 EST
Just so there is no confusion here, we have ALWAYS said that extension ids are "simple" (that is, no '.').  Here is a snippet from the plugin manifest specification doc included in the help from March 2002. 

    SimpleToken := sequence of characters from ('a-z','A-Z','0-9') 
    ExtensionId := SimpleToken
    ExtensionPointId := SimpleToken

The doc goes on to say that the various ids are simple and in some cases explicitly restates the fact that '.'s are not allowed. (Note that at some point we added '_' to the token definition)

There are some parts of the registry API and implementation that rely on this characteristic.  The fact that anything non-conformant has worked for you in the past is pure chance.

Having said that, it appears that there are several groups that did not pick up on this part of the API and have been happily violating it "for ages".  Since there appear to be several avenues to enabling you and still moving forward with the namespace separation, we should address this situation for 3.2.

Seems like one of the following should work
1) upping the plugin.xml version number 
2) reverting to the explicit namespace attribute approach that was discussed when designing the new function.

Personally I prefer #1 but if it is going to cause alot of special cases in PDE and elsewhere then perhaps #2 is an alternative.

There is also the question of patch vs respin for M5.  For Callisto this is a non-issue.  The affected Callisto teams should immediately update their code to not use '.' in their ids.  That is an API violation and you have been caught by it.  You have to fix your bugs for 3.2 so do it now and get it overwith.  So the current M5 build is fine for Callisto.

For the rest of the community that is affected, we can make a patched version of the M5 Registry plugin and the affected teams can point their users at this bundle directing their users to add it to their configuration.
Comment 18 Ed Merks CLA 2006-02-23 11:03:37 EST
Jeff,

The advice you give is not workable.  If we remove the use of ".", we'd change the ID, and we'd break every existing project using this nature.  Instead what we've done is fully qualified the ID *and* upped the version number. This way we will work with the current M5 *and* if the behavior is changed again, we'll work after that change too. 

This seems like the only workable approach.  Do you agree?
Comment 19 Mike Wilson CLA 2006-02-23 11:11:38 EST
As much as I hate to suggest respinning M5, I believe we should do so to implement option 1) in comment #17. Obviously, the Callisto teams should fix their API violations, but requiring this before they can step up to M5 is too harsh -- it's clear that the fallout from this can be significant. 

The blame for this situation should lie at least partially with us, for not making the discussion that lead to the change sufficiently visible to the community. 
Comment 20 Doug Schaefer CLA 2006-02-23 11:42:28 EST
I just loaded up Eclipse 1.0, which was used to build our plugin, and found said docs dated March 2001. Unfortunately for us the developer didn't read that section and since no error was raised, he thought it was fine to use a dot in the extension id.

It does point out that we need a good communication mechanism to get informed and be involved in such behavior changes. It's pretty tricky, though, to find a solution that scales with the size of the full Eclipse community. My feeling is that these problems are inevitable and the Eclipse plugin developer has to be vigilant in ensuring his stuff works on the latest Eclipse builds (which is how I found out our plugins had this problem).
Comment 21 DJ Houghton CLA 2006-02-23 11:44:47 EST
There will be a rebuild.

If you are using the new namespace support, you must increment the Eclipse
version tag in your plugin or fragment xml to be 3.2.
  <?eclipse version="3.2"?>

If you are broken by M5 because you used a dot in your extension or extension
point id, build 3.2 M5a will expose the previous behaviour and you will be ok.
Going forward, you should conform to the API, fully qualify the ids which are
partially qualified and, update the eclipse version tag as mentioned above.
Comment 22 Ed Merks CLA 2006-02-23 12:08:53 EST
We must take some responsibility as well for not having properly and fully tested the readily available I builds.  Assuming that because all our JUnits run isn't always sufficient.  I'm sorry we didn't bring this issue to light sooner to have help prevent this problem.

It would sure be nice to be able to automate GUI testing more easily...
Comment 23 Ed Burnette CLA 2006-02-23 13:32:23 EST
Would it be out of the question to allow dots in the id (grandfather it into the API) and use some other character or way of specifying the new namespace stuff? 

Like:
   <extension id="foo" namespace="org.example">
or
   <extension id="org.example:foo">
or
   <extension id="foo_org.example">

Given i18n issues I'm surprised you're parsing out things in the id anyway.
Comment 24 Wassim Melhem CLA 2006-02-23 14:54:22 EST
I hope it's not too late but I like the following suggestion made in comment 23:

<extension id="org.example:foo">

Several reasons:
1. It's business as usual for all teams who don't want to take advantage of this new namespace feature.

2. No need for a new version in the <?eclipse?> processing instruction, and hence no work for PDE.

3. The ":" as a delimiter is the first thing that comes to my mind when I heard the word 'namespace'.

4. Even though the plugin.dtd has always specified that there are no dots allowed in the extension ID, every other ID in the plugin.xml does allow dots.
So in the name of consistency, "grandfathering" it into the API as Ed suggested seems like a reasonable thing if the ":" is adopted.

This would also allow whoever had dots in their extension ID to take advantage of the namespace feature in the future.
If we continue to implicitly parse IDs assuming everything up until the last dot is a plug-in ID, the EMF guys, for example, could never take advantage of this feature ever.
Comment 25 Gunnar Wagenknecht CLA 2006-02-23 15:10:58 EST
I second the requests made in comment 23 and comment 24. Personally I find <extension id="foo" namespace="org.example"> a little bit closer to the XML-way. ;)
Comment 26 Mel Martinez CLA 2006-02-23 15:24:01 EST
The colon (:) may or may not cause less havoc than the "." apparently has
caused, however, its still the same fundamental problem.  There is no logical
reason to exclude any character from the simple name, only legacy technical
reasons.

As discussed in https://bugs.eclipse.org/bugs/show_bug.cgi?id=112856, I would
prefer an explicit separation of namespace and id, as in the first suggestion
in comment #23.   By using an explicit and optional namespace="my.namespace"
attribute, you a) don't break the existing stuff, b) clearly identify the
intent to use a namespace id distinct from the plugin id and c) allow any
character to be in either component of the fully-qualified name.  You also
obtain some memory savings potential in the internal data model by intern()'ing
the namespace string.  

The problem this scheme presents is:  If the two are
logically separate, independent of any delimiter character, how does one
externalize the fully-qualified name?  That becomes important if you want to
re-consume such a string.  One could get around that by using a designated
character for the delimiter (such as : or (.), allowing for encoded use of that
character in the simple name when in fully-qualified externalized form.

That said, I realize that one has to be practical given the legacy constraints,
and one can't always get what one wants.  :-)
Comment 27 Oleg Besedin CLA 2006-02-23 15:48:50 EST
I think we are starting to mix two concerns here:
1) Using “.” in the simple IDs
2) How to describe a namespace of an extension/extension point

For (1): Keep in mind that having "." in simple name is not guaranteed to work in Eclipse - either per-M5 or post-M5. If it works today, that is just by chance. Internally, different registry elements have different processing routines. Some of those routines assume that what comes before the last "." in the qualified name is the namespace; some routines keep namespaces persisted in a separate slot.

So, no matter what outcome this discussion is going to have, the simple Ids with "." in the name have to be changed to fully qualified names. (Unless we undertake a major overhaul of the extension registry.)

Hopefully, as we established (1), let’s move on to (2).  Aside from the question of “explicit” vs. “implicit” specification of the namespace all those approaches are identical. Using Java approach to specify namespaces seems to be most natural from the registry view point. Unless incrementing a version in the plugin.xml produces a problem for PDE, I would prefer to keep it.
 
On the issue of I18N – do you use multi-byte characters in the IDs of extensions and extension points? 
Comment 28 Ed Merks CLA 2006-02-23 16:11:06 EST
I've tested the fix DJ provided.

Both the following work properly:

<?eclipse version="3.2"?>
...
<extension point="org.eclipse.core.resources.natures" id="org.eclipse.emf.codegen.jet.IJETNature">

and 

<?eclipse version="3.0"?>
....
<extension point="org.eclipse.core.resources.natures" id="jet.IJETNature">

Thanks DJ!
Comment 29 Wassim Melhem CLA 2006-02-23 16:19:40 EST
re comment 28

I am not sure how this one worked...

<?eclipse version="3.2"?>
...
<extension point="org.eclipse.core.resources.natures"
id="org.eclipse.emf.codegen.jet.IJETNature">


How is the ID parsed in this case?  Is the namespace "org.eclipse.emf.codegen.jet" or "org.eclipse.emf.codegen"?

If it's the latter, how did you know what dot to stop at?
If it's the former, I am not sure how things worked in the first place.
Comment 30 Ed Merks CLA 2006-02-23 16:41:29 EST
Wassim,

I don't know.  I didn't test anything programmatically.  I'm just testing whether the nature is registered, i.e., whether I'm getting the proper builder behavior that compiles JET templates automatically.  

I think you are asking what IExtension.getNamespace returns?  That method is now deprecated, so it matters less than before what it returns.  I assume that question is for DJ...
Comment 31 Oleg Besedin CLA 2006-02-23 17:07:26 EST
To answer question in the comment 29:

In M5, the namespace is everything before the last “.” (in your example, 
"org.eclipse.emf.codegen.jet").

“I am not sure how things worked in the first place” is exactly my statement :-). Pre-M5 different paths in the registry used two approaches to determine namespace: a) by storing contributor’s namespace in a persisted way; b) by forming qualified name and assuming that the portion before the last “.” is the namespace.

Each approach taken separately more-or-less works for the IDs with “.”s. However, it becomes much funnier if those approaches are used together.

Another note, is that some APIs we have either expect qualified name in the Java format (e.g., IExtensionRegistry#getExtensionPoint(String)) or provide qualified name in the Java format (e.g., IExtension# getUniqueIdentifier()). Introducing “:” to separate namespace from the simple name would be inconsistent with existing registry APIs.
Comment 32 DJ Houghton CLA 2006-02-23 17:25:10 EST
Patch released into M5a branch and HEAD. 
M5a build started.
Comment 33 Jeff McAffer CLA 2006-02-23 19:46:32 EST
Simple names have always been specified as not having '.'.  IMHO we should continue this.  So in the EMF scenario what would be happening is they would be using a different namespace from the "default".  That is, for the extension "ext" with id org.eclipse.emf.codegen.jet.IJETNature discussed in comment 29, the following calls should have the following results.

ext.getNamespaceIdentifier() = org.eclipse.emf.codegen.jet
ext.getSimpleIdentifier() = IJETNature
ext.getUniqueIdentifier() = org.eclipse.emf.codegen.jet.IJETNature

This seems fine for users and reduces the ripple.  Frankly with the separation of namespaces the use of simple ids becomes just a convenience factor so people have to type less.

As for i18n, the grammar for ids is ascii alphanumerics.  we can seek to extend that at some point in the future but that seems to be a separate issue.

Comment 34 DJ Houghton CLA 2006-02-24 09:57:57 EST
We have tested build I20060223-1656 and verified that it is good to be promoted to M5a. 
Comment 35 Doug Schaefer CLA 2006-02-24 10:27:43 EST
I can confirm that our QNX plugin works again with the new build. Thanks!
Comment 36 DJ Houghton CLA 2006-02-25 14:47:32 EST
Closing. Marking as M6 since there is no M5a target milestone in Bugzilla.
Comment 37 Pascal Rapicault CLA 2006-05-01 06:40:19 EDT
*** Bug 139307 has been marked as a duplicate of this bug. ***