Bug 191130 - [api][breaking]Exception printed to stdout when starting RSE the first time
Summary: [api][breaking]Exception printed to stdout when starting RSE the first time
Status: RESOLVED FIXED
Alias: None
Product: Target Management
Classification: Tools
Component: RSE (show other bugs)
Version: 2.0   Edit
Hardware: PC Linux-GTK
: P2 normal (vote)
Target Milestone: 2.0   Edit
Assignee: David Dykstal CLA
QA Contact: Martin Oberhuber CLA
URL:
Whiteboard:
Keywords: api
Depends on:
Blocks:
 
Reported: 2007-06-05 17:52 EDT by Martin Oberhuber CLA
Modified: 2008-08-13 13:20 EDT (History)
1 user (show)

See Also:
mober.at+eclipse: pmc_approved+
dmcknigh: review+
mober.at+eclipse: review+


Attachments
patch to get rid of the exception printing to stdout at fresh startup (3.84 KB, patch)
2007-06-07 16:15 EDT, David Dykstal CLA
no flags Details | Diff
removes the exception printing and fixes usage of getRemoteSystemsProject() (14.17 KB, patch)
2007-06-08 11:05 EDT, David Dykstal CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Oberhuber CLA 2007-06-05 17:52:07 EDT
When starting RSE for the first time, close the welcome screen then press the upper right "switch perspective" button to open RSE perspective.

Following exception is printed to stdout:
Exception trying to test the natures of the project that fired a resource change event

This is in SystemResourceListener.java and might be a regression due to getting rid of the RemoteSystemsProject.

-----------Enter bugs above this line-----------
TM 2.0RC2 Testing
installation : eclipse-SDK-3.3RC3-linux-gtk-x86_64, cdt-4.0RC2,
     emf-sdo-runtime-2.3RC2.
     Download RSE-2.0RC2: RSE-SDK, examples, tests,discovery,terminal
java.runtime : Sun 1.5.0_10-b03, mixed mode, sharing 64-bit
os.name:     : OpenSUSE 10.2 64-bit
uname        : Linux osgiliath 2.6.18.8-0.3-default x86_64 GNU/Linux
------------------------------------------------
systemtype   : Linux-local / Windows-dstore (Daemon) / Unix-dstore (RExec)
targetos1    : Linux RHEL4, Sun 1.5.0_11
targetos2    : Windows XP SP1, Sun 1.4.2_13
targetos3    : Solaris-sparc 5.9, Sun 1.4.2_05
targetuname  : SunOS szg-anar 5.9 Generic_118558-06 sun4u sparc SUNW,Sun-Blade-1500
------------------------------------------------
Comment 1 David Dykstal CLA 2007-06-07 10:29:59 EDT
Getting rid of the print to standard output is relatively risk free for RC3. Why the exception occurs may be a different problem. I will investigate.
Comment 2 David Dykstal CLA 2007-06-07 16:06:59 EDT
There are three things going on here: 
1) the exception in SystemResourceListener is getting printed instead of logged
2) the remote systems project may not be accessible even if the handle is available - in this case events should be ignored
3) SystemTeamViewResourceAdapterFactory is attempting to force the creation of the remote systems project when asked to adapt the system registry to an IResource. This should use the new call that returns the project but doesn't force its creation. Forcing the creation is what lead to the code in SystemResourceListener to get invoked to begin with. The resulting project is only used for labelling purposes and a handle alone is sufficient for this.
Comment 3 David Dykstal CLA 2007-06-07 16:15:41 EDT
Created attachment 70574 [details]
patch to get rid of the exception printing to stdout at fresh startup

This patches SystemResourceListener to 
1) log the exception instead of printing it
2) listen only for the checked exception CoreException rather than all exceptions
3) check to see if the project is accessible before attempting to get its nature

It patches SystemTeamViewResourceAdapterFactory to not force the creation of the remote systems project when an adapter is requested.
Comment 4 David Dykstal CLA 2007-06-07 16:20:42 EDT
Risk Assessment:

The first part of the patch is low risk. Its what should be happening anyway.

The second part of the patch is should be medium risk, but since the creation of the project was being inhibited by the exception anyway I don't think we'd see any adverse effects since we haven't seen any up to this point.
Comment 5 Martin Oberhuber CLA 2007-06-08 01:33:42 EDT
All aspects of the patch look good.

But couldn't there be very similar problems elsewhere? I think I'd like to mark SystemResourceManager.getRemoteSystemsProject() deprecated (asking users to use the new version with the boolean parameter), review all our internal use of it, and replace it with using the new version. 

This should ensure that we really never accidentally create the project. I'm not sure if this request is worth a new bug entry for tracking it.
Comment 6 David Dykstal CLA 2007-06-08 09:17:14 EDT
Just marking it deprecated is a good idea and could be done in the scope of this defect. I shouldn't need to alter the patch for that. Does this report need to be marked as API then?

For 2.0.1 - as part of our internal cleanup - we can go through our own uses of it.
Comment 7 David Dykstal CLA 2007-06-08 10:34:20 EDT
I marked SystemResourceManager.getRemoteSystemsProject() as deprecated and followed up by searching for invocations in the workspace. There were a few and I've corrected those.

I think instead of deprecating it we should just remove this method and the unused getProfileFolder() method. I will be attaching a new patch in a few minutes with these changes. Please review when it appears.
Comment 8 David Dykstal CLA 2007-06-08 11:05:33 EDT
Created attachment 70697 [details]
removes the exception printing and fixes usage of getRemoteSystemsProject()

Does what the previous patch does but also fixes the remaining uses of getRemoteSystemsProject()
Comment 9 David Dykstal CLA 2007-06-08 11:06:40 EDT
Please review this latest patch.
Comment 10 David McKnight CLA 2007-06-08 11:24:22 EDT
Looks fine but could we get rid of those commented out methods (i.e. the old getRemoteSystemsProject() and getProfileFolder())?
Comment 11 David Dykstal CLA 2007-06-08 11:32:16 EDT
The commented out methods will disappear next week during cleanup.
Comment 12 David Dykstal CLA 2007-06-08 16:16:34 EDT
Increasing priority since this is an API change. Adding API tag. 
Comment 13 Martin Oberhuber CLA 2007-06-11 08:49:31 EDT
I'm ok with the patch, though I'm feeling slightly uneasy still changing API that late in the game.

If you do want to do it, please update the build notes with a clear statement what has changed and how to migrate. Otherwise, keep the old methods as deprecated -- it currently looks like we'll have a chance getting rid of deprecated stuff with TM 3.0.

My personal opinion is that we'll have less confusion and less work not changing the APIs any more, but I'll leave it for you to decide.
Comment 14 David Dykstal CLA 2007-06-11 13:00:38 EDT
Committed as patched above. I think this change should not be too disruptive so I decided to make the change and not just deprecate it.
Comment 15 Martin Oberhuber CLA 2008-08-13 13:20:54 EDT
[target cleanup] 2.0 RC3 was the original target milestone for this bug