Community
Participate
Working Groups
It seems that the p2 preferences are being stored in the instance data area scope, whereas it may be more adequate to store them in the config scope.
Created attachment 129754 [details] Patch Moves the Preferences used by org.eclipse.equinox.p2.ui.sdk and org.eclipse.equinox.p2.ui.sdk.scheduler to the profile scope. Along with the scope change I've migrated the classes to use org.osgi.service.prefs.Preferences instead of the depreciated org.eclipse.core.runtime.Preferences which fixes a number of warnings.
Does it include preference migration?
Created attachment 130253 [details] Updated patch with preference migration
Fixed in HEAD > 20090413. To be sure, I'm going to recheck various migration scenarios on tomorrow's i-build. There were several problems with the patch, some of them dating back to bogus code in 3.4. - the org.eclipse.equinox.p2.ui.sdk prefs were moved to preference scope, and the pref page accessed them, but there was no migration code in the patch. The only migration code was for the preferences in org.eclipse.equinox.p2.ui.sdk.scheduler, so the older instance scope values were being ignored. - the migration code for org.eclipse.equinox.p2.ui.sdk.scheduler was added to "ClassicUpdateInitializer," a preference initializer. I noticed while testing/debugging, that this code was never called, so the prefs were not being migrated. The problem is that a PreferenceInitializer is only called when the default pref scope is accessed. So the auto updater tried to retrieve the profile-scoped prefs and since this had nothing to do with a default scope, the migration code didn't run. I talked to DJ about this because I don't fully understand when/how the default scope is accessed. He said it was probably coincidence that the ClassicUpdateInitializer ever did a proper migration. The UI startup code was accessing preferences and as a side-effect, the default scope got initialized. He suggested that I call specific preference migration methods in the start methods of the bundles involved. This is what I ended up doing. I verified that the migration code is now running, but still need to take a clean build and verify that various update scenarios still migrate properly: - 3.5 instance scope -> 3.5 profile scope - 3.4 instance scope -> 3.5 profile scope - 3.3 UM instance scope -> 3.5 profile scope
Tested preference migration from: 3.3 workspace with UM prefs 3.3 workspace with no UM prefs 3.4 workspace with p2 prefs 3.4 workspace with no p2 prefs 3.5 workspace before profile scoping 3.5 fresh workspace One caveat: Because of the change in profile scope, users may be used to having workspace-specific preferences for automatic update, and this is no longer true. Once the pref value has been established (by migration or default value), it is not obtained from a workspace. Of course, that is the reason to move to this scope, but it should be noted that this is a change in behavior for the end user. (When testing these various migration scenarios, I had to clean out the profile scoped pref files in order to verify that the various migrations happened.)
Reopening. I noticed while testing the fix for 268142 that the method AutomaticUpdater.getPreferenceStore() still remains, so much of the automatic update code is accessing the old preferences, thus ignoring any future changes.
Same problem with ProvSDKUIActivator. The policy is being driven by the old preference store. I'm working on a fix.
Fixed in HEAD >20090422. There was UI client code that ultimately would map back to this pattern: someUIPlugin.getPreferenceStore() The problem here is that the AbstractUIPlugin.getPreferenceStore() creates a preference store using the instance scope. Even if we changed all the UI client code to stop using this method, it could always creep back in. It seemed dangerous to have an instance-scoped preference store being returned here. So instead, I override getPreferenceStore() in the UI plug-ins to create a profile-scoped preference store. This allows all the UI code to traffic in preference stores as before and get the right stuff. I removed the static methods from Matt's patch and switched to using either getPreferenceStore() where applicable or using the ProfileScope to obtain the correct node. This also meant I had to move ProfileScope into an internal.provisional package.
I looked at the changes and they look good, thanks Susan. I released a couple of minor things but more for consistency, they won't change the behaviour. The only thing that I wonder is if we should be calling #setSearchContexts in the ScopedPreferenceStore instances that you are creating. By default when you do a look-up, it will look in the "profile" and then the "default" scopes. If we want to include other scopes in the look-up (instance, config...) then we need to call that method to set them. I didn't change the code because I wasn't sure of the behaviour you wanted.
(In reply to comment #9) > The only thing that I wonder is if we should be calling #setSearchContexts in > the ScopedPreferenceStore instances that you are creating. By default when you > do a look-up, it will look in the "profile" and then the "default" scopes. If > we want to include other scopes in the look-up (instance, config...) then we > need to call that method to set them. I didn't change the code because I wasn't > sure of the behaviour you wanted. > Thanks for looking at this, DJ. I don't think we need to set the search contexts, but let me explain and then you tell me if I'm wrong. The preference stores are for the UI prefs (show latest version, etc.) and the automatic update prefs. We do have to migrate from the instance scope of the plug-in (used in 3.5 until now) to the profile scope. However the preference page now sets the user preferences in the profile scope, so I don't think we need to include an instance scope in the normal operation of the store. Now, if we wanted to support the user being able to have both workspace-level and profile-level preferences, we might do such a thing, but we'd need some UI affordance in the pref page to say where the settings should be applied, right? Or...if another product/app wanted to insert another scope for preferences, they could create their own pref store that included their scope as well as the profile scope. Does that sound about right?
Yep, that sounds right. It sounds like we should keep things the way they are right now and change later if needed. Thanks!
