Simplify project synchronization #196
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Up until now, the build command updater could not handle two build commands with the same id but different arguments. It is now simplified to the following behavior: - If the Gradle model does not provide build commands, leave them alone - If the Gradle model provides build commands, overwrite the existing ones with those This makes sure that the project accurately matches the Gradle model while not breaking users on older versions.
This ensures that the Eclipse project faithfully matches the Gradle project. If the Gradle version does not provide project nature information, then the user-defined natures stay untouched.
Writing them to the .prefs file is confusing our users. They think that they can customize them or that they are somehow important. In fact, they are just internal state which doesn't make sense to expose to the user and also should not be checked into version control. Linked resources are workspace metadata, so persistent properties are a good fit, since their lifecycle is also bound to the workspace. The size limitation of persistent properties (2KB) should be acceptable, since we store relative paths, not absolute ones. If we actually encounter a real world project with so many linked resources that this becomes an issue, we can use a custom file inside our plugin's state location, similar to our classpath cache.
Writing them to the .prefs file is confusing our users. They think that they can customize them or that they are somehow important. In fact, they are just internal state which doesn't make sense to expose to the user and also should not be checked into version control. Derived resources are workspace metadata, so persistent properties are a good fit, since their lifecycle is also bound to the workspace. The size limitation of persistent properties (2KB) should be acceptable, since we store relative paths, not absolute ones. If we actually encounter a real world project with so many derived resources that this becomes an issue, we can use a custom file inside our plugin's state location, similar to our classpath cache.
The source folder updater used to keep source folders that were not defined in Gradle. This could lead to a situation where the user defined a folder that is in conflict with the Gradle model and can now no longer synchronize. The user then needs to know how to remove the source folder manually before the synchronize will work again. The updater now removes folders that were not defined in Gradle. This makes sure the Eclipse project matches the Gradle configuration.
The classpath container updater will now overwrite all containers if Gradle supports container information. In case Gradle does not provide container information, it will only update the JRE container based on the Java source settings. This removes the need to remember which containers were user defined and makes the project configuration match the Gradle configuration as closely as possible.
Doing the whole synchronization in a single transaction lead to lots of memory pressure in bigger projects. We originally introduced this transaction because the tasks view could not handle rapid updates. This has been fixed in the meantime and the view now behaves sanely even when there are hundreds of "project added" events in short succession.
Adding or removing projects from the workspace should not trigger a reload of the whole Gradle model. It should only be re-fetched if the user explicitly requests it. Otherwise we'd reload the model twice during project import.
The following has been changed by this commit: - Use consistent naming in ClasspathContainerUpdaterTest
The new preference implementation automatically deletes the preferences for a deleted project, but the tests expected that they remain at their location.
- Rename ProjectPluginStatePreferenceStore and ProjectPluginStatePreferences to ModelPersistence and PersistentModel - Rename loadProjectPrefs() method to loadModel() - Merge how project change events are handled in the core and in the ui plugin - Add test coverage for ProjectChangeListener - Add test coverage for ModelPersistence
Projects with existing descriptors are automatically imported with the IMPORT_AND_MERGE flag.
Re-introduce the previously removed assertion This reverts commit 5ce6123
oehme
approved these changes
Dec 9, 2016
| /** | ||
| * Persists all changes on the disk. | ||
| */ | ||
| void flush(); |
There was a problem hiding this comment.
This should be on ModelPersistence
There was a problem hiding this comment.
I chose this because this way we don't have to use instanceof when saving the state in DefaultModelPersistence.
| * | ||
| * @author Donat Csikos | ||
| */ | ||
| public abstract class PersistentUpdater { |
There was a problem hiding this comment.
I think this should be a helper instead of a base class.
There was a problem hiding this comment.
Frankly, I like the superclass better because it states that from all the existing updaters only the subclasses persist their state.
This was referenced Feb 4, 2017
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.