Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 354895 - Setting Target Platform doesn't update ECLIPSE_HOME until workbench is restarted
Summary: Setting Target Platform doesn't update ECLIPSE_HOME until workbench is restarted
Status: VERIFIED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.7   Edit
Hardware: All Mac OS X - Carbon (unsup.)
: P3 major (vote)
Target Milestone: 3.8.2   Edit
Assignee: Curtis Windatt CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-16 18:41 EDT by Alberto Ricart CLA
Modified: 2013-01-17 13:39 EST (History)
6 users (show)

See Also:
Michael_Rennie: review+


Attachments
Fix (1.08 KB, patch)
2011-08-19 12:52 EDT, Curtis Windatt CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alberto Ricart CLA 2011-08-16 18:41:35 EDT
Build Identifier: I20110519-1138

When changing the target platform the ECLIPSE_HOME environment variable fails to update. This makes projects that depend on classpath settings to have errors until the workbench is restarted.

Reproducible: Always

Steps to Reproduce:
1. Open preferences, see value of ECLIPSE_HOME in Java/Build Path/Classpath Variables
2. Change the target platform to another eclipse installation
3. Repeat step #1 - The value you'll see is the same as before. However it should have updated to the new eclipse installation location.

This was the behaviour in 3.4
Comment 1 Jay Arthanareeswaran CLA 2011-08-17 01:21:35 EDT
I can't reproduce this bug when I tested between 3.7 and 3.8 M1. Could you please try with newer builds?
Comment 2 Alberto Ricart CLA 2011-08-18 18:37:10 EDT
(In reply to comment #1)
> I can't reproduce this bug when I tested between 3.7 and 3.8 M1. Could you
> please try with newer builds?

Here's the scenario:
Eclipse SDK
Version: 3.7.0
Build id: I20110613-1736


1. Started Eclipse, look at the Classpath variables (ECLIPSE_HOME) in preferences under Java/Build Path/Classpath Variables.

2) Look at the target platform (Preferences Plug-in Development/Target Platform), the directory matches what you see in ECLIPSE_HOME.

3) Change the target platform to point to a different eclipse installation.

4) Reinspect ECLIPSE_HOME. The value still points to the original value, it should have changed to use the target platform location from step #3.

5) Go back to the target platform setting and restore the target platform to its pre step 3 setting.

6) Reinspect ECLIPSE_HOME again. Now the value has updated to the value it would have had in step #3, however this is wrong, because the platform has been reset to a different value.

In our code generation bundles we are referencing ECLIPSE_HOME for some of the Java dependencies to be set correctly. As you can see the value is not updated in sync with the setting of the target platform. Restarting eclipse after changing the target platform will yield a correct classpath.
Comment 3 Jay Arthanareeswaran CLA 2011-08-19 00:43:39 EDT
Thanks for the detailed steps. I must have missed something in the previous attempt to reproduce this. Now I see what you mean. I will investigate.
Comment 4 Jay Arthanareeswaran CLA 2011-08-19 01:02:40 EDT
The problem seem to be with the EclipseHomeInitializer, which for some reason uses the previous value of the target platform path while setting the ECLIPSE_HOME_VARIABLE. 

I am moving it to PDE.
Comment 5 Curtis Windatt CLA 2011-08-19 12:23:02 EDT
Caused by an ordering problem. The preference where we store this information is getting modified after the classpath variable is asked to reset.  Fix should be simple.

The problem has existed since 3.6.  We should consider backporting to 3.7.x.
Comment 6 Curtis Windatt CLA 2011-08-19 12:52:35 EDT
Created attachment 201816 [details]
Fix

We are in the process of migrating to git and 3.7.1 RC2 is next week, so I am not sure when this will be backported.

I am concerned that the ordering was done this way on purpose (certain preferences, including the platform path, are only set after the reset job runs). However, going through the operations on the reset job I don't see any reason why it should be called before all preferences are set.  This fix moves the reload operation until the end of loadPlugins() call.
Comment 7 Dani Megert CLA 2011-08-22 03:40:40 EDT
I didn't look at the patch but I agree (+1) that we should fix the problem for 3.7.1.

On a side note: I would deprecate that variable and replace it with something less confusing like 'TARGET_PLATFORM_LOCATION'.
Comment 8 Curtis Windatt CLA 2011-08-22 11:53:28 EDT
Ankur, can you please review this for 3.7.1?

(In reply to comment #7)
> On a side note: I would deprecate that variable and replace it with something
> less confusing like 'TARGET_PLATFORM_LOCATION'.

The deprecated variables still show up in the pref page right? I would think it would be confusing to see both variables there.
Comment 9 Dani Megert CLA 2011-08-22 11:56:48 EDT
(In reply to comment #8)
> Ankur, can you please review this for 3.7.1?
> 
> (In reply to comment #7)
> > On a side note: I would deprecate that variable and replace it with something
> > less confusing like 'TARGET_PLATFORM_LOCATION'.
> 
> The deprecated variables still show up in the pref page right? I would think it
> would be confusing to see both variables there.
Yes, but it will tell which variable to use instead.
Comment 10 Curtis Windatt CLA 2011-08-22 15:11:48 EDT
Mike, if you have time today, please review so it gets included when tagging for RC2.
Comment 11 Michael Rennie CLA 2011-08-22 16:52:55 EDT
Looks good for MASTER and 3.7.1
Comment 12 Curtis Windatt CLA 2011-08-22 17:51:09 EDT
Fixed in master and 3.7.1
Comment 13 Curtis Windatt CLA 2011-08-25 15:01:19 EDT
Verified in M20110825-0847
Comment 14 Vladimir Piskarev CLA 2012-10-02 02:55:19 EDT
This bug appears to come back in Eclipse Juno. The ordering fix made in LoadTargetDefininionJob was somehow lost in Juno. Steps to reproduce are the same.
Comment 15 Dani Megert CLA 2012-10-02 09:57:03 EDT
It looks like the fix for bug 347695 destroyed/ignored some changes.

When fixing this please also look for other possible destroyed fixes.
Comment 16 Curtis Windatt CLA 2012-10-02 11:54:43 EDT
It looks like when the LoadTargetDefinitionJob was moved to the API package, the file wasn't up to date.

http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?h=R3_8_maintenance&id=416bf2cb3b7375ac954afe12284a382d472eb5fe
Fix for 3.8.2

The fix for master is ready to go, but waiting for Bug 319744 to be merged.
Comment 18 Curtis Windatt CLA 2013-01-17 13:39:41 EST
Verified in M20130116-1800