Re: [platform-ui-dev] Smart Import concerns

[Mickael Istria <mistria@xxxxxxxxxx>]

Hi Stefan,

Thank you for your feedback. I'm glad to tell you that most of your concerns are already handled by the Smart Importer. However, I admit the approach is very disruptive for contributors compared to existing import scenarios, it's not trivial to understand so it requires some (a lot of) explanations.

On 05/15/2016 01:54 PM, Stefan Oehme wrote:

As far as I understand the logic goes as follows:

1. The user selects a root folder

2. The Smart Importer recursively walks the file tree

3. It asks every contributor "Is this folder an Eclipse project?"

4. It then displays the list of possible projects to the user for selection

5. The user selects and clicks Finish

6. The Smart Importer imports all those projects as plain Eclipse projects with almost no configuration

7. It then tells every contributor to configure those imported projects

This was what was happening with earlier versions. Here is what's not happening
1. User selects root folder
2. Each "active" contributor returns a list of project proposals from this project root.
3. All the proposals are shown to users, users can select which one to include/exclude and press Finish
4. The Smart Importer imports all this projects as plain Eclipse projects without almost no configuration
5. It tells every contributor to configure those imported projects.

The first problem is that this assumes that there is a 1:1 mapping from physical folders to Eclipse projects.

This is actually a (fair) limitation from the Resource model: 1 project maps to a single folder and vice-versa. The workspace doesn't allow you to have multiple projects at the same location, nor to have a project with multiple locations.

The ProjectConfigurator API still makes it possible to have 1 given directory resolve to multiple project proposals. See for example the Maven import, or a hierarchy of Eclipse projects: the input is a single directory and the output is a set of projects.
I'll go into more concrete explanation trying to answer you questions step by step. Make sure you have the code of ProjectConfigurator ( http://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/wizards/datatransfer/ProjectConfigurator.java ) nearby to look at it while I reference some parts.

Also, to determine which sub-projects are contained in a Gradle build, you need to load the model, which can be a long-running operation (e.g. you might have to download the right Gradle distro first).

The Gradle configurator, implementing ProjectConfigurator, can load this Gradle model for the given input directory in the findConfigurableLocations method. This can be a long running one and it's the one used in order to populate the proposals.
An semi implementation for Gradle would look like (I imagined some API that I suppose have equivalent in BuildShip):
  public Set<File> findConfigurableLocations(File root, IProgressMonitor monitor) {
    SubMonitor subMonitor = SubMonitor.convert(monitor...);
    GradleModelBuilder gradleModelBuilder = GradleModelBuilder.getInstance(subMonitor.split(...));
    Set<File> gradleFiles = findChildrenWithName("build.gradle", subMonitor.split(...));
    Set<File> allGradleComponents = new HashSet<>();
    for (File gradleFile : gradleFiles) {
      GradleModel gradleModel = gradleModelBuilder.buildModel(gradleFile, subMonitor.split(...));
      allGradleComponents.addAll(gradleModel.getResolvedComponentLocations());
    }
    return allGradleComponents;
  }
     

The next problem comes when the Smart Importer goes and imports all those projects itself. As far as I've seen it'll just use the folder name. This won't work for any larger project. It is very common to have a physical structure like this

root

 |- sub1

    |- api

    |- impl

 |- sub2

   |- api

   |-impl

If you try to import these, you'll get an exception because of duplicate names. Both Gradle and Maven handle this properly in their own importers, because they actually use a richer model. For Gradle there is a de-duplication algorithm, for Maven the artifactIds are used (they are unique within a multi-module project)

This one is indeed an issue, but the importer will manage to import the example structures. Although project in sub2/api will be likely to be called api_(0) and sub2/impl likely to be called impl_(0). http://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/SmartImportJob.java#n554

The next problem is that the user has no way of specifying technology-specific options. Both the Gradle and Maven wizards have options like which version of the tool to use etc. All of these are lost in the generic wizard.

Initially, a while ago, I thought ability to contribute specific wizard page for a technology was necessary. However, as I implemented several import strategies and read/listened several user reports about the complexity of the "regular" import, it seems to me that such pages at import time are not a high priority for users.
Users expect a good import to import the project "as best", in a working state. In the vast majority of cases, they are not interested in selecting a runtime version, a default set of APIs. In some cases, they're not even able to make those changes by themselves because they're not familiar enough with the technology.

So the idea of allowing tech-specific wizard in a generic import was dropped. There is enough cleverness in Eclipse and in the build/project technologies such as Gradle or Maven to infer good values for everything.
In cases of user diverging from the "mainstream" configurations, then we can easily expect users to tweak their projects via the Project > Properties menu.

Another concern is performance. I can only speak for Gradle here, but I assume the Maven guys have also done some heavy optimizations. Buildship fetches the model for a root project *once* and then does all the configuration within a single transaction. It suppresses both resource change notifications as well as JDT notifications until the import is complete. As a result, Buildship can import 500 sub-projects in just 10 seconds! Without these optimizations the same can take up to 5 minutes.

For Maven, the strategy that was used is to run Maven configuration asynchronously. So the modules get imported by the importer, and the Maven configurator populates a set of IProject to process for Maven configuration.
The actual configuration is performed asynchronously and in a non-blocking way, resulting in a decent user experience. See https://github.com/eclipse/m2e-core/blob/master/org.eclipse.m2e.importer/src/org/eclipse/m2e/importer/internal/MavenProjectConfigurator.java#L258 .

Finally, it seems to me that all configurators will be run. This will lead to very awkward situations for many users. For instance, if you are doing a build system migration, you will usually have build files for both build systems in your projects for the transition period. Let's say, you might have both pom.xml and build.gradle files. With the current approach, this would lead to a project that is configured by both the Maven and Gradle configurator, which would definitely lead to a completely broken project.

Can you please provide an example?
Most natures work well together. A project that has both pom.xml and build.gradle is simultaneously a Maven and a Gradle project. It's totally consistent to import it with both natures enabled in the IDE. Then users should be able to disable the nature they don't want (Gradle or Maven) later, while using the IDE.
The "regular" import wizards would still be available for such tricky case when user need an exclusive import strategy.

Also, here are a few questions to consider about the mvn vs gradle issue at import:
* how would a user on a new project know whether it's best to import with Maven or Gradle?
* should it be the role of the user to select one or another?
* since Maven and Gradle work together on CLI, why aren't they working together in the IDE.

Most importantly, there may be very good reason why the user removed the facet. Maybe it doesn't work for him and it produces unhelpful warnings/errors.

In case the facet is not desired, then just leave the .project in the folder, commit it to Git (as it's usually recommended to commit IDE-specific metadata), and then the importer will only rely on it and won't perform more analysis.
The project would be imported without check for Facet, nor Maven, not anything. Instead it will be imported as specified by the .project file and nothing more.

I don't know about Maven here, but Gradle already has a rich model about all the facets of WTP projects. The user configures those in his build and Buildship will then transport that configuration into the Eclipse project. So we definitely don't want another configurator to the configuration that Buildship did. This would just lead to users reporting bugs against Buildship and we'd have to explain that this is caused by another configurator in the Smart Importer.

So I think these "Quickfix" configurators really should be a different extension point.

There is already a mechanism to make a distinction between "structuring" configurators and "decorating"/"QuickFix" ones so that the second ones wouldn't override the work of the previous one.
Basically, if "shouldBeAnEclipseProject" http://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/wizards/datatransfer/ProjectConfigurator.java#n107 returns true for one or more configurators, then only those ones would be executed. The other ones would be skipped as assuming that this "strongly typing" configurators manage it all. That was specifically designed for the use-case you mentioned.
WTP supports does always return false for shouldBeAnEclipseProject. It will only decorate the new project if nothing better was found.

Here's what I expected when I just read the abstract description of the "Smart Import" feature:

1. The user selects a root directory

2. All the configurators are asked "Can you import this root?"

3. The user is presented with a choice which configurator to use, e.g. "Plain Eclipse Project" or "Gradle Build" or "Maven Build"

4. The user is then taken to the appropriate technology-specific wizard. There he can benefit from all the hard work those projects put into their own wizards, like extra options, fast imports, name de-duplication etc.
Not only does this seem simpler from the users perspective to me

Actually, you proposal is more like a way to filter the "Import Wizards" list according to the input folder. It would have been nice too.

3 and 4 are IMO too much complexity for users. Think about what users really want at import: it's not that much choice they want, it's something that goes as fast as possible and without effort from a directory to a set of projects to work on. The purpose of Smart Import has also been to remove those steps 3 and 4.

I'd be very happy for some feedback on this, because our users keep asking us about EGit support and we would really love to tell them "Just use Smart Import".

Buildship is not far from being able to say "just use smart import". The remaining steps seems to be to Implement the project detection mentioned higher in this mail, and (depending on actual performances) to Implement configuration asynchronously like Maven did.

HTH

--
Mickael Istria
Eclipse developer at JBoss, by Red Hat
My blog - My Tweets