Patch available to make .classpath entries portable - relative to ECLIPSE_HOME

• Key: DERBY-2376
• Type: Sub-task
• Status: Resolved
• Resolution: Fixed
• Priority: Major
• Assignee: Aaron Tarter
• Reporter: Aaron Tarter
• Component/s: Eclipse Plug-in
• Affects Version/s: 10.2.2.0
• Fix Version/s: 10.2.2.1, 10.3.1.4

Description

This patch modifies the DerbyUtils class to add variable entries relative to ECLIPSE_HOME as described in the comments below, so that eclipse projects with Derby Nature can be committed to an SCM without causing build path errors. I did not think any of the derby functional tests were applicable to this ui action, so I manually tested the code by trying the following:
1) Adding Derby Nature to a java project
2) Starting and stopping a database with the derby nature
3) Removing the Derby Nature from a java project

The code modification is only required on the add and not on the remove since the current remove looks for any entry that ends with the correct JAR names.

Comments

[Aaron Tarter]

The current behavior of adding four derby lib entries with absolute path references to the .classpath is problematic. It prevents easy sharing through an SCM repository like SVN or CVS, since the references are not relative (i.e. portable). The solution suggested above would be to use an eclipse IClasspathContainer (like the JRE System Library). Define a container in the plugin that loads the derby jars and then just add that container to the .classpath when the user clicks "Add Derby Nature". Another simpler, but perhaps less pretty, approach is to modify the "Add Derby Nature" popup action to add var entries relative to the ECLIPSE_HOME variable. So, in this case they would still appear as non-collapsable individual entries, but at least they would be portable. The entries in the .classpath would then be:
<classpathentry kind="var" path="ECLIPSE_HOME/plugins/org.apache.derby.core_10.2.2/derby.jar"/>
<classpathentry kind="var" path="ECLIPSE_HOME/plugins/org.apache.derby.core_10.2.2/derbyclient.jar"/>
<classpathentry kind="var" path="ECLIPSE_HOME/plugins/org.apache.derby.core_10.2.2/derbynet.jar"/>
<classpathentry kind="var" path="ECLIPSE_HOME/plugins/org.apache.derby.core_10.2.2/derbytools.jar"/>

[A B]

I have zero experience in this particular are of the Derby code and I've never used the UI plugin, so I don't have much to offer in the way of review. Just the following relatively unimportant stylistic notes:

1) It'd be great if you could add comments to the code that explain the justification behind the change. You've mentioned why this is needed in the Jira and also in your email to derby-dev, but the code itself has no explanation.

 2) Would it make sense to use a constant variable for "ECLIPSE_HOME/plugins" instead of using the literal twice? Not by any means a requirement, just a suggestion.

And one question: looking at this patch I assume that, currently, whenever derby lib entires are added the absolute path *always* starts with the expanded form of "ECLISPE_HOME/plugins". Is that correct? Is it ever possible for the path to start with something else? (maybe showing my ignorance of Eclipse plugins right now...sorry). Or put another way, is there any chance that existing users of the Derby UI plugin might see altered behavior as a result of this change? If so, what are the scenarios in which that might happen?

[Aaron Tarter]

Updated as recommended by Army:
1) Added code comments to describe the reason for adding a var entry
2) Added constant to reference ECLIPSE_HOME/plugins

[Aaron Tarter]

Thanks for your comments Army:
I made the source changes that you recommended. Now, to answer your question - your assumption is correct that the absolute path *always* starts with the expanded form of the variable. It would never start with anything else in the current implementation. There is no chance that the existing users of the Derby UI plugin will see altered behavior.

[Nell Gawor]

I am experiencing this problem and it is a major pain point as I can't share any projects with the Derby nature without manually removing all the hard-coded classpath entries.

[Rajesh Kartha]

I applied the patch on trunk (the patch is with respect to the trunk/plugins/eclipse/org.apache.derby.ui, so need to be applied there - will not work from trunk, which has been the normal way of applying patches).

In any case, I applied the patch and was able to create and run the plugins. The fix does addess the issue of avoiding actual jar path entries in the project's .classpath file. With the new plugin the .classpath for any project now shows up as:

<?xml version="1.0" encoding="UTF-8"?>
<classpath>
<classpathentry kind="src" path=""/>
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER"/>
<classpathentry kind="var" path="ECLIPSE_HOME/plugins/org.apache.derby.core_10.2.2/derby.jar"/>
<classpathentry kind="var" path="ECLIPSE_HOME/plugins/org.apache.derby.core_10.2.2/derbyclient.jar"/>
<classpathentry kind="var" path="ECLIPSE_HOME/plugins/org.apache.derby.core_10.2.2/derbytools.jar"/>
<classpathentry kind="var" path="ECLIPSE_HOME/plugins/org.apache.derby.core_10.2.2/derbynet.jar"/>
<classpathentry kind="output" path=""/>
</classpath>

instead of the actual paths to these jars.

I think it would be good to add information abt this JIRA entry in the comments

+1 to commit.

We may need to update some of the screen shots in the corresponding Doc plugin to reflect this (separate JIRA entry). Maybe then we can consider bumping the versions of the UI and DOC plugins from 1.1.0 to 1.1.1

 

[Andrew McIntyre]

Committed second DerbyUtils.diff patch with revision 515864. I scanned the doc plugin for pages that might need updating, although I didn't see any. Perhaps we should still bump the version to 1.1.1 and include a note about this change in the next release, though.

[Aaron Tarter]

I was just working on updating the images in the doc plugin. There very minor changes to the screen shots, where the JAR paths were visible. The following images show part of the path:
trunk/plugins/eclipse/org.apache.derby.plugin.doc/topics/images/completed_javaapp.GIF
trunk/plugins/eclipse/org.apache.derby.plugin.doc/topics/images/create_restaurant.GIF
trunk/plugins/eclipse/org.apache.derby.plugin.doc/topics/images/restaurant_editor.GIF
trunk/plugins/eclipse/org.apache.derby.plugin.doc/topics/images/restaurant_script.GIF
trunk/plugins/eclipse/org.apache.derby.plugin.doc/topics/images/run_javaapp.GIF

Although the changes will only noticeable to a very keen eye, I am in the process of recapturing these images, and I will post them tonight.

[Andrew McIntyre]

Wow, good eye! I hadn't noticed where these were showing up on the screenshots until you posted the list. Thanks, Aaron, I'll be sure to review the screenshots when you post them.

[Aaron Tarter]

I tried to mimic the original screen shot environment as best as I could to minimize changes, but I was working on a bare eclipse 3.2.2 installation instead of 3.0.2. The only difference is one menu option as far as I can tell.

This new svn diff is relative to the trunk (thanks for the pointer Rajesh) and it accounts for the original change to DerbyUtils.java, as well as five changed images and two HTML files. The HTML files had the image height/width of the original screen shots encoded and I changed the test to reflect the changed menu option. So, what was a one file change has grown to 8 files because of the doc changes.

[Andrew McIntyre]

Hi Aaron, sorry for the trouble and getting back around to this so late, but could you please reattach these images with the 'grant license to ASF' checkbox checked on the JIRA attachment page? That way we can commit them to the repository and include them in the next release of the plugin.

[Aaron Tarter]

Same images, just checking Grant ASF License

[Aaron Tarter]

Same images, just checking grant ASF License

[Aaron Tarter]

If I provide the svn diff against the 10.2 branch, what is the chance of getting this patch in a fix release? It would be nice to have something sooner.

[Bryan Pendleton]

Is this issue complete? Or is there still more work needed to be committed?

Also, I notice that there was a question about porting the fix to 10.2. Did that get resolved?

Just trying to ensure this didn't fall off the radar screens...

[Aaron Tarter]

The code is complete and committed to the trunk. The images for the eclipse help were reposted with the license granted to ASF as Andrew requested. If/when the images are committed, the issue will be complete. No answer on the 10.2 branch.

[Bryan Pendleton]

I've eye-balled the html diffs and I've clicked on the various images in my browser,
and they look OK to me. I intend to commit the 5 image files and the 2 HTML files
to subversion. If anybody else is reviewing these changes, please let us know.

[Andrew McIntyre]

Hi Bryan,

Sorry, this had dropped off my radar. If you are already looking at the updated files, please feel free to commit the changes to both trunk and 10.2. I had reviewed the previous versions and decided that they were ok, until I noticed they were attached without the "grant license to ASF" JIRA checkbox. Assuming the actual images are the same, then they're +1 to commit from me as well.

[Aaron Tarter]

Bryan, please let me know if you need an svn diff for the 10.2 branch.

[Bryan Pendleton]

Thanks Andrew and Aaron!

I committed the images and HTML files to the trunk as revision 531595.

I then merged both revision 515864 (the DerbyUtils change) and
revision 531595 (the docs change) to the 10.2 trunk using svn merge -r.
The merge was clean and simple, so I committed the change to the
10.2 branch as revision 531597.

Aaron, I think you should assign this Jira issue to yourself, since
you did the development of this change. Thanks for the patch!

[Aaron Tarter]

Thanks for committing the changes Bryan. I do not see an option to assign the issue. Is this a restricted operation?

[Andrew McIntyre]

Assigning issues is restricted to the derby-developers group in JIRA. Aaron, I have added you to the group and credited DERBY-2376 to you.

[Myrna van Lunteren]

I wonder, should we have updated the plugin version from 1.1.0 to 1.1.1?

[Myrna van Lunteren]

I bumped the version number - although after the fact - to 1.1.1 for doc & ui plugins; trunk: 556084, 10.3: 556087; 10.2: 556086.