Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce 'user' preference scope located in <user.home>/.eclipse #446

Merged
merged 1 commit into from
Jan 10, 2024

Conversation

HannesWell
Copy link
Member

This PR introduces a user preferences scope API to provide a unified way to store and read per-user-global preferences under <user.home>/.eclipse.

At the moment plugins that want to store settings in a user-global scope need their own implementations to write and read those preferences.
With this PR a new UserScope is introduced to store user-preferences in a standard way and the user scope is added to the PreferencesService.DEFAULT_DEFAULT_LOOKUP_ORDER in order to read from the user location if the instance or configuration scopes don't provide any value for a key out of the box.

In the first of the two commits in this PR the existing ConfigurationPreferences and InstancePreferences have their common code moved into a new super-class SingletonEclipsePreferences, that is then also used by the new UserPreferences.

Copy link

github-actions bot commented Dec 17, 2023

Test Results

   25 files  +1     25 suites  +1   10m 54s ⏱️ -31s
2 156 tests +6  2 111 ✅ +6  45 💤 ±0  0 ❌ ±0 
2 200 runs  +6  2 155 ✅ +6  45 💤 ±0  0 ❌ ±0 

Results for commit b067575. ± Comparison against base commit 5ba8030.

♻️ This comment has been updated with latest results.

@iloveeclipse
Copy link
Member

I wonder why that wasn't provided from very beginning. @tjwatson : any clue?

Some questions :

  1. Tests should be added / adopted for new scope/preferences.
  2. I haven't checked how current code manages parallel read/write access to the files, in case of shared common user preferences this should be guarded by some inter-process locks (like workspace access).
  3. Some preferences make sense for all application installations, some not. Different applications installed for same user would even have opposite requirements for same preference. For example, for a "typical" IDE saving last used workspaces into user scope would be nice feature for seamless update. Same however is exact the opposite for IDE based applications that have incompatible workspaces in different versions (or require explicit one way upgrade on using old workspace with new version), so sharing last used workspaces list in user preferences would be considered as major bug (because user will see incompatible workspaces in the workspace selection dialog). How that is supposed to be handled?

@laeubi
Copy link
Member

laeubi commented Dec 17, 2023

Just wanted to note that Java already has support for system/user global settings:
https://docs.oracle.com/javase/8/docs/api/java/util/prefs/Preferences.html

@HannesWell
Copy link
Member Author

I wonder why that wasn't provided from very beginning.

Me too.

Some questions :

1. Tests should be added / adopted for new scope/preferences.

Yes, I will add them.

2. I haven't checked how current code manages parallel read/write access to the files, in case of shared common user preferences this should be guarded by some inter-process locks (like workspace access).

Currently the default implementation for writes is

if (Files.exists(preferenceFile)) {
// Write new file content to a temporary file first to not loose the old content
// in case of a failure. If everything goes OK, it is moved to the right place.
Path tmp = preferenceFile.resolveSibling(preferenceFile.getFileName() + BACKUP_FILE_EXTENSION);
Files.writeString(tmp, fileContent, StandardCharsets.UTF_8);
Files.move(tmp, preferenceFile, StandardCopyOption.REPLACE_EXISTING);
} else {
Files.writeString(preferenceFile, fileContent, StandardCharsets.UTF_8);
}

But one can also provide an AbstractPreferenceStorage via the ScopeDescriptor to customize the storing.

As it can be seen in the default implementation it already has some basic considerations of parallel writes, but for example overwrites/ignores changes done to the preferences since the file was read and until it is written by one application.

But the Configuration-Scope already faces the concurrency problem and while the instance-scope (aka workspace) is guarded by the workspace lock, theoretically even the project scope could have that problem if a project is at the same time imported into multiple workspaces (which is of course a questionable usage).

If a more advanced mechanism that handles concurrent writes more precisely a new set of questions arise, especially:

  • How to record which nodes have been changed between reading a node from disk and flushing it to disk?
  • How to handle keys that have been changed differently in different applications?
    Is it ok to let the last one win? Will the other application be notified somehow or will it only notice it when reading the node again (will probably not happen before a restart, at least at the moment).

I'm not saying concurrent changes should be handled in a more precise way, it just will be more complex.

3. Some preferences make sense for all application installations, some not. Different applications installed for same user would even have opposite requirements for same preference. For example, for a "typical" IDE saving last used workspaces into user scope would be nice feature for seamless update. Same however is exact the opposite for IDE based applications that have incompatible workspaces in different versions (or require explicit one way upgrade on using old workspace with new version), so sharing last used workspaces list in user preferences would be considered as major bug (because user will see incompatible workspaces in the workspace selection dialog). How that is supposed to be handled?

For each preference it the store scope can be chosen individually. Most often it is the INSTANCE (aka workspace) scope, that is, depending on the way/API chosen to manage preferences, is even the default.
So I would say, the USER scope should only be used for preferences where it makes sense to have the same settings for all instances of the same user.

In your example it might be a solution to have additional means to filter incompatible workspaces.

Just wanted to note that Java already has support for system/user global settings: https://docs.oracle.com/javase/8/docs/api/java/util/prefs/Preferences.html

Yes, but I would say it should be integrated into the known Eclipse preferences mechanism, which is based on the org.osgi.service.prefs.Preferences.

@laeubi
Copy link
Member

laeubi commented Dec 18, 2023

Yes, but I would say it should be integrated into the known Eclipse preferences mechanism, which is based on the org.osgi.service.prefs.Preferences.

The backing store for org.osgi.service.prefs.Preferences is not specified so it can be anything and actually they are quite similar.

@tjwatson
Copy link
Contributor

I wonder why that wasn't provided from very beginning. @tjwatson : any clue?

Sorry, I don't have any recollection of a user scope concept for Eclipse preferences. The Eclipse preferences predated OSGi preferences usages in the Eclipse project and then the Eclipse team at the time merged the two into what we have today. IIRC they simply took the existing concepts of what they had and kept them in the new implementation and tried to rebase them as much as they could on the OSGi preferences service.

One thing that I find confusing is that OSGi does have a concept of "user" preferences also: https://docs.osgi.org/specification/osgi.cmpn/8.1.0/service.prefs.html#org.osgi.service.prefs.PreferencesService.getUserPreferences-String-

But I believe that is in name-only when compared to what is proposed here.

@laeubi
Copy link
Member

laeubi commented Dec 18, 2023

One thing that I find confusing is that OSGi does have a concept of "user" preferences also

The spec says this:

For each bundle, there is a separate tree of nodes for each user, and one for system preferences. The precise description of "user" and "system" will vary from one bundle to another. Typical information stored in the user preference tree might include font choice, and color choice for a bundle which interacts with the user via a servlet. Typical information stored in the system preference tree might include installation data, or things like high score information for a game program.

So the "user" preferences are still local to the framework or more formally the storage, while this here is more to select where it is stored, I think similar to what Oomph does.

@iloveeclipse
Copy link
Member

this here is more to select where it is stored, I think similar to what Oomph does.

@merks : FYI.

@merks
Copy link
Contributor

merks commented Dec 18, 2023

Yes, I'm watching. With Oomph's preference tasks recorded in the user.setup, one can copy any preferences to every workspace, thereby achieving a similar effect, when desired.

One consideration here when deciding on the user's behalf to make some specific preference user-scoped in some specific framework component, it will no longer be possible to set the preference differently in different workspaces or even differently in different applications that the reuse some underlying framework component that has decided its preferences should be user-scoped...

@HannesWell
Copy link
Member Author

For each bundle, there is a separate tree of nodes for each user, and one for system preferences. The precise description of "user" and "system" will vary from one bundle to another. Typical information stored in the user preference tree might include font choice, and color choice for a bundle which interacts with the user via a servlet. Typical information stored in the system preference tree might include installation data, or things like high score information for a game program.

So the "user" preferences are still local to the framework or more formally the storage, while this here is more to select where it is stored, I think similar to what Oomph does.

Yes, the intention to provide a new scope for the existing Eclipse preferences system. While it is (at least from an interface POV) based on the OSGi preferences I would not like to try to combine both in this change, especially as you mentioned the OSGi preferences don't specify the exact was.

Yes, I'm watching. With Oomph's preference tasks recorded in the user.setup, one can copy any preferences to every workspace, thereby achieving a similar effect, when desired.

Yes and this is even inspired by the great and handy Oomph user-setup. The main difference is that it obviously works without Oomph and the plugin developer can decide where to store the value and it has not (and cannot be decided by the user).

My first use case for this is the 'Windows Defender Auto-fix' development effort of the the Eclipse IDE WG I'm currently working on where I would like to add a Never ask again button that should work across installations and should therefore be persisted in the user-scope. I'll hope to provide a PR for that in the next days. I'll link it here so that those that are interested can have a look.

One consideration here when deciding on the user's behalf to make some specific preference user-scoped in some specific framework component, it will no longer be possible to set the preference differently in different workspaces or even differently in different applications that the reuse some underlying framework component that has decided its preferences should be user-scoped...

It still can be overridden in narrower scopes if the same qualifier (usually the bundle-id) and key are used. When using the PreferencesService this works seamlessly as the service will first look for a qualifier+key in the instance/workspace scope, then in the configuration/installation scope and if no value was found in these scopes, with this change, it will only then look up the value in the new user-scope. As last resort the default scope will be queried:

private static List<String> DEFAULT_DEFAULT_LOOKUP_ORDER = List.of( //
InstanceScope.SCOPE, //
ConfigurationScope.SCOPE, //
DefaultScope.SCOPE);

@HannesWell
Copy link
Member Author

In order to simplify the review of this PR, I have now created #449, that only contains the refactoring done in this first commit.
Reviews are welcome.

@iloveeclipse
Copy link
Member

service will first look for a qualifier+key in the instance/workspace scope, then in the configuration/installation scope and if no value was found in these scopes, with this change, it will only then look up the value in the new user-scope. As last resort the default scope will be queried

Product customization still supported, I assume? Where is it in this order above? Default scope?

@HannesWell
Copy link
Member Author

service will first look for a qualifier+key in the instance/workspace scope, then in the configuration/installation scope and if no value was found in these scopes, with this change, it will only then look up the value in the new user-scope. As last resort the default scope will be queried

Product customization still supported, I assume? Where is it in this order above? Default scope?

Yes the default Scope looks up in the product customization. But as before, when using the IPreferencesService a value is first looked up in the narrower scopes, instance, configuration and with this also user.
Because the default scope is immutable for a user, I would keep it as last scope.

@vogella
Copy link
Contributor

vogella commented Dec 20, 2023

I just wanted to say thanks to you @HannesWell for addressing so many things in platform and elsewhere, very impressive

@vogella
Copy link
Contributor

vogella commented Dec 20, 2023

One very useful thing might be to have a way to move all instance scope to user scope in the preference. I would guess in 80% of the cases I know people would like to use the same settings for all their workspaces and installations. So having a easy way to move everything out of instance scope to the new user scope would IMHO be very nice. Of course having the option to have different settings in different WS is also very important to get compliant and support the other % of use cases.

@iloveeclipse
Copy link
Member

One very useful thing might be to have a way to move all instance scope to user scope in the preference

That's exact why I have my concerns regarding different (incompatible) application settings for different applications running for same user. There should be no such "thing" by default and if it would be added, it should be possible for applications to disable that "thing".

@vogella
Copy link
Contributor

vogella commented Dec 20, 2023

Yes @iloveeclipse that for repeating that

@HannesWell
Copy link
Member Author

My first use case for this is the 'Windows Defender Auto-fix' development effort of the the Eclipse IDE WG I'm currently working on where I would like to add a Never ask again button that should work across installations and should therefore be persisted in the user-scope. I'll hope to provide a PR for that in the next days. I'll link it here so that those that are interested can have a look.

For those interested there is now a PR

One very useful thing might be to have a way to move all instance scope to user scope in the preference

That's exact why I have my concerns regarding different (incompatible) application settings for different applications running for same user. There should be no such "thing" by default and if it would be added, it should be possible for applications to disable that "thing".

In general I would say that the decision to save a preference in the user-scope should be done with great care.
In my use case where I want to implement an Ignore all installations button, choosing that would of course mean that all Eclipse based applications are affected, may it be different Eclipse packages like Eclipse for Java Developers, Eclipse Modelling Tools or Eclipse CDT or some other custom third-party Eclipse based applications.

So yes, there is indeed some potential for confusion, some people might not even know that their application is based on Eclipse and yes there could definitely be desire to handle different products groups differently. For example ignore all Eclipse installations (which ever product it is exactly) but consider my custom product.

With the current architecture the user-scoped value can be overwritten with a configuration/installation- or instance/workspace-scoped entry, but in the preferences accessible through the UI menu Window -> Preferences (at least by default/usually) only instance/workspace entries are set. To set a configuration/installation-entry the preference page has to be programmed to set that value explicitly, at least that's what I have done in my use-case mentioned above.
And looking into configuration/settings/*.prefs files in my Eclipse installations I use daily I only see:

  • org.eclipse.ui.ide.prefs
    which contains the history of recently used workspaces and a few entries associated with the workspace selection dialog. But the listed workspaces can be changed in the usual preference dialog.
  • org.eclipse.equinox.p2.garbagecollector.prefs
    which contains one entry to enable/disable P2's bundle garbage collector (which is probably set by Oomph to not clean-up the shared bundle pool too early).

With that, but without checking all usages of the configuration/installation-scope exhaustively, I have the impression that the config/installation scope is already used only in specific ways and the user-scope would be used similarly.

Alternatively the difference could be made more visible and the different scope levels could be made accessible through the UI explicitly, similar to Oomph which provides a workspace, installation and user scoped setup.

@merks
Copy link
Contributor

merks commented Dec 28, 2023

I do a lot of testing the installer and of Oomph in general. For that purpose (to replicate a new-user experience) I often use -Duser.home=<test-user-home> which works nicely for the installer but not for the installation it creates. So there is also support in the installer for -Doomph.setup.user.home.redirect=true which replicates the value of the user.home system property in the product's *.ini. I mention this because the following logic might use some other system property to determine the location and it might use <user.home>/.eclipse only as a default:

	static {
		String userHome = System.getProperty("user.home"); //$NON-NLS-1$
		USER_HOME_PREFERENCE_LOCATION = IPath.forWindows(userHome).append(".eclipse"); //$NON-NLS-1$
	}

This would potentially allow other applications/products to configure a different location in the product itself. Such a property might be documented here:

https://help.eclipse.org/latest/index.jsp?topic=%2Forg.eclipse.platform.doc.isv%2Freference%2Fmisc%2Fruntime-options.html

I'm not sure if there would be a reason/advantage to use org.osgi.framework.BundleContext.getProperty(String)...

@HannesWell
Copy link
Member Author

I'm not sure if there would be a reason/advantage to use org.osgi.framework.BundleContext.getProperty(String)...

Are you suggesting to use BundleContext.getProperty("user.home") instead of System.getProperty("user.home")?
I have tested if overwriting user.home via a VM property -Duser.home=C:\foo\bar works in a launch config and yes it does work as expected and I assume it also works if I specify a -Duser.home=... VM property in a product config respectively .ini file.
What would then be the advantage in using BundleContext.getProperty(String)?

@HannesWell
Copy link
Member Author

Hopefully everybody had relaxing holidays and a good start into this new year.

I would like to complete this PR soon, so are all concerns addressed or do you still have things that should be changed?

@HannesWell
Copy link
Member Author

I would like to complete this PR soon, so are all concerns addressed or do you still have things that should be changed?

Can I interpret no answers as silent consent?
I want to emphasize again, that this change alone does not change anything where preferences are stored. It just adds a way, consistent with the existing IPreferences API, that allows storing preferences in user.home. But this has to be done explicitly, like for example in eclipse-platform/eclipse.platform.ui#1453.

We can later have a discussion if and how this could be generallized to for example give users more control on which level preferences are stored, similar to how one can do it for example in Oomph with the user, installation and workspace setup.

@merks
Copy link
Contributor

merks commented Jan 9, 2024

Yes, silence is consent.

@HannesWell
Copy link
Member Author

Good, then this can be submitted :)

An interesting issue in which a generalization might be discussed is eclipse-platform/eclipse.platform#89.

@HannesWell
Copy link
Member Author

  1. Tests should be added / adopted for new scope/preferences.

I just added the requested tests that check if the scope-nodes are written to the expected location.

@vogella
Copy link
Contributor

vogella commented Jan 29, 2024

@HannesWell can you add this new feature and its usage also to the N&N worthy page? Otherwise it is really hard for users to discover this new feature. I assume most RCP clients would be very happy to start using user scope instead of instance scope preferences.

(sorry if it is already added to the N&N, but a quick check did not reveal them in the 4.31 notes for me). https://eclipse.dev/eclipse/news/4.31/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants