Hi everyone,
I just looked into the new Smart Import feature. Mickael told me that it may become the default wizard for EGit, which would make it really easy for users to import Gradle projects from Git.
First of all, this is a great initiative! Giving users a streamlined entry-point was long overdue. But I have some concerns about the way it was implemented, which I would like to share. Please keep in mind that I might have misunderstood concepts that just need some explanation. Also keep in mind that - even though this will be mostly a critical look at the feature - I'm very excited about the general idea behind it! Okay, here we go:
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
The first problem is that this assumes that there is a 1:1 mapping from physical folders to Eclipse projects. This won't be true for Gradle in the future. A single physical Gradle project will be able to contain several software components (e.g. a Java library and a set of integration tests. Or a set of Android product flavors). Each of those software components will most probably be mapped to a synthetic Eclipse project to better isolate things like their compile and runtime classpaths and other configuration. 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 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)
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.
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.
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.
I also looked at the other configurators (except the Maven one). None of them seem to have anything to do with importing. It seems more like they are "Quickfixes". E.g. there is one that adds the Java nature if you import a project with a .classpath file that misses the Java nature. There is another one that adds some WTP facets if you have some specific configuration in your web.xml. Again, this is a quickfix and a completely different concern from project import. 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. 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.
Okay, that was a lot of criticism, so I better give an alternative :) 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, it would also greatly simplify the API and the amount of code that integrators need to write. It would take full advantage of all the great features we already have in the existing wizards.
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".
Cheers,
Stefan
