Improve how the plugin is passed configuration information (iteration 2)

[AdamBJ]

Kevin has made some changes to how the plugin reads configuration information from the projects it is applied to. He's also added new tests. See d249ce1

There are a number of issues to resolve before we can merge these changes into master. Roughly:

1. Add the Eclipse plugin to our plugin's build.gradle file. Do whatever initialization is necessary to get it working.
2. Add the comment Kevin mentioned here d249ce1#commitcomment-21371251 that emphasizes that the umple configuration closure is independent of the SourceSets in the project (it can be used to configure the source sets, but it is not a part of the source sets)
3. Add an overridable default output file path for generated files. While this may not be something most users of the plugin require, we'll need it for the Umple build!
4. Try to cut down on the amount of duplicate code in the test files

We're also going to need to settle on a final configuration syntax. This is still up in the air, and will depend on the answer to a question Kevin has asked about source set configuration on the Gradle forum

[AdamBJ]

Referencing @vahdat-ab so that he knows what we're currently working on. I'm going to try and work on this issue in tandem with the umple compiler jar stuff we're discussing in #10

[vahdat-ab]

Great, thanks.
Having Eclipse plugin as a part of the build is really important. Currently, it's done manually by me. Furthermore, the Eclipse plugin you need to work on is org.cruise.umple.eclipse.plugin. The other eclipse projects are obsolete.

[AdamBJ]

@vahdat-ab @Nava2 To be clear, what I was talking about was adding the Gradle Eclipse Plugin to our Gradle plugin's build.umple file so that people can develop the Umple Gradle plugin in Eclipse if they want to. I'm wasn't talking about building the Umple Eclipse Plugin with Gradle, although I think that's a good idea. @vahdat-ab I'm going to create a new issue for that.

[Nava2]

@AdamBJ Check this out: http://stackoverflow.com/a/43047325/1748595 (the comments particularly)

[AdamBJ]

I've had a look at the comments. Seems like a good idea, though there's a lot of stuff that's new to me. Hard to wrap my head around. Here's a couple things I'm trying to figure out:

In your commit, you add the configuration defaults like this:

project.extensions.add("umple", DefaultUmpleOptions)

According to what the SO guy said, we should also be using a DslObject to somehow add a bunch of properties and methods to our SourceSets. Like this:

new DslObject(sourceSet).getConvention().getPlugins().put("umple", umpleSourceSet);

Does that mean we're supposed to be adding the defaults via the DslObject now? Or should we keep both steps? And how do we access the umpleSourceSet after we've added it to the DslObject?

If we get rid of the extensions stuff we're going to need to change at least the getGlobals() method in UmpleGradleTask, as it refers to the project's extension property.

I'm going to have another look at everything tomorrow. On the plus side, I've got to the point where I can run the compileUmple task. But at run time it errors out because of issues with default configuration.

[AdamBJ]

Made a bit more progress today, though the going is still slow. There's a lot of stuff I still don't fully understand, like how the DSLObject works. This guide has been helpful for general understanding. Hopefully I make more progress tomorrow.

FYI this is the error I've been battling when I try to run the compileUmple task:

Some problems were found with the configuration of task ':sub2:compileUmple'.
> No value has been specified for property 'sourceSet'.
> File 'src\umple\Master.ump' specified for property 'umpleFile' does not exist.

The second one is being caused because the default value for umpleFile isn't being overridden properly. I'm not sure what's causing the first one, since sourceSet isn't one of the properties we add. As far as I can tell, the error is referring to the sourceSet iterator from UmpleGradlePlugin.groovy.

[Nava2]

That is a good resource. Is the code pushed in the branch? I'm finishing up writing my final exam for my course tonight, but I should have some time to look.

[Nava2]

Just checked, no code is up on github, could you push what you've been working on? I can take a look later.

[AdamBJ]

I've pushed my changes. They're still fairly rough (spacing etc will need to be fixed). The main changes I made are:

1. Add the DSLObject inside the apply method of UmpleGradlePlugin.groovy
2. Changed UmpleSourceSet umple(Action<? super SourceDirectorySet> configureAction); to UmpleSourceSet umple(Action<DefaultUmpleOptions> configureAction); to try to match what the SO guy was saying about using an object that is capable of setting the configuration parameters.
3. Added configuration value that allows users to specify where they want the generated files outputted to
4. Added some comments/debugging stuff (we'll need to get rid of some of it before the PR).

I've pointed out 1) and 2) with comments on the commit page.

[AdamBJ]

I think one problem is that we're changing how we're using Conventions in the main plugin groovy file.

To configure the UmpleGenerate task in configureUmpleGenerate, we're retrieving the convention like this:

Convention umpleConvention = (Convention) InvokerHelper.getProperty(sourceSet, "convention")

We used to be setting the convention like this:

Convention sourceSetConvention = (Convention) InvokerHelper.getProperty(sourceSet, "convention") sourceSetConvention.plugins.put("umple", umpleSourceSet)

But we're using a DSLObject now instead. So when the configureUmpleGenerate method goes to retrieve the convention using the InvokerHelper, it doesn't get anything useful. We need to change configureUmpleGenerate to get the convention from the DSLObject (?) instead.

[AdamBJ]

@vahdat-ab @Nava2

After a lot of poking and prodding we've finally got a plan for how we can fix the issues with the ss-config-2 branch. The problem right now is that we can't seem to specify custom configuration values. For example:

sourceSets {
	   main {
	       umple{
	            umpleFilePath = file(test.ump')
	       }
	   }
	} 

Leads to a "Unknown Property Error" for umpleFilePath. To resolve this issue, we need to add umpleFilePath (and the other configuration values) to DefaultUmpleSourceSet and/or UmpleSourceSet (not sure if it's and or or at this point). While we're at it we should shorten the configuration option names a bit.

I'm going to try to apply these changes to the plugin tomorrow. Everyone keep their fingers crossed!

[AdamBJ]

I had a quick look at things before bed. Here are the remaining issues as far as I can tell:

We're currently only using the default values from DefaultUmpleOptions.groovy in the UmpleGenerateTask.groovy. To put that a different way, we're only looking to DefaultUmpleOptions.groovy in order to set

 private UmpleLanguage m_languageToGenerate
private File m_umpleFile
 private File m_outputDir

Inside UmpleGenerateTask.groovy. UmpleGenerateTask.groovy is our generation task now, so we need to change this. The good news is that we're now reading in the custom plugin parameters the user is specifying in the Umple closure:

sourceSets {
	   main {
	       umple{
	            umpleFilePath = file(test.ump')
	       }
	   }
	} 

umpleFilePath = file(test.ump') is now being read and stored in an UmpleSourceSet. What we need to do now is connect the UmpleSourceSet with the UmpleGenerateTask such that any fields that are set on the UmpleSourceSet are transferred over to UmpleGenerateTask where they override the default values. Haven't quite figured out how to do this yet, but I think if we can do that then we'll have a solution.

[Nava2]

I think what we do for the generate task is just change it two one of two ways:

1. Have it iterate over each sourceSet checking if there's umple work
2. Create a generate task per sourceSet

I think the Java plugin prefers 1, so I'm inclined that way. :)

[AdamBJ]

Ok, I'll look into adding the check within configureUmpleGenerate

[AdamBJ]

@Nava2, the problem is that the configuration of the umpleGenerateTask happens before we read the information from the umple closure. It's a nasty problem, because umpleGenerateTask executes when the java plugin is applied. We need to apply the java plugin before we can configure source sets via the umple closure though... the return of the dreaded chicken or egg problem. The other problem is that there's no way as far as I can see for the umpleSourceSet that gets created by the umple closure to access the umpleGenerateTask to overwrite the defaults.

[AdamBJ]

Think I got it! When we run configureUmpleGenerateTask I add the UmpleGenerateTask to the DefaultUmpleSourceSet as a member variable. Then, when we process the umple closure and fill in the fields of the DefaultUmpleSourceSet, we've got the UmpleGenerateTask available and can override the default values! It ain't pretty, but it works, and can be made a bit prettier with some refactoring.

Need to test this more fully, and see how it'll all work with the default umple closure. But it's a big step in the right direction 🎆 ! I'll try to push things tonight or tomorrow so you can have a look. Here are a couple snippets though:

In configureUmpleGenerateTask:

  final UmpleGenerateTask umpleGenerate = project.tasks.create(taskName, UmpleGenerateTask.class)
  UmpleSourceSet umpleSourceSet =  sourceSet.convention.plugins.umple
  umpleSourceSet.setUmpleGenerateTask(umpleGenerate)

and in UmpleSourceSet umple(Closure configureClosure):

  configure(configureClosure, umple)
  genTask.setUmpleFile umpleFilePath

The second bit will need some if/else logic so that a field only gets overwritten if we the user has specified a value for it.

[AdamBJ]

Not quite out of the woods yet as it turns out. Still have that > No value has been specified for property 'sourceSet'. error. Not sure what's causing that. I've renamed every variable called sourceSet to something else. sourceSet doesn't occur anywhere int he umple.gradle project, but the error is still there.

Update: this is driving me crazy! I've never had to specify a sourceSet property when using the java plugin before, and sourceSet literally appears nowhere in umple.gradle now, so it's not something we're adding.

[Nava2]

Can you push what you've got? Just tag the commit with skip ci (in square brackets, mobile keyboard doesn't have them...), and Travis won't run it. This all looks good, I'm happy to look at it today, finally got some time to myself.

[AdamBJ]

@Nava2 Pushed: 158bf03

[Nava2]

Check out the changes I made in ef3750d.

I noticed this message after running check:

Task property validation finished with warnings:
  - Warning: Task type 'cruise.umple.tasks.UmpleGenerateTask' declares property that is not annotated: 'compileConfigs'.

I think it leads from the bolded point below..

TODO:

1. Create a better testing suite -- the "properties-as-strings" approach is nasty, we need to do better
2. Re-enable integration testing
3. Verify what I did makes sense.. I think I may have excessively created the "config-style" as I'm not sure how many tasks are made per project. I think the old way the inputs for the task worked was okay, I guess just ripping out the "exec" loop and using that would be better.. not sure -- needs testing

[Nava2]

Random note: Gradle uses slf4j, so to properly log while running gradle you need to do:

    private static final logger = Logging.getLogger(MyClass)

and just use it... No idea why I didn't think about this earlier.

[Nava2]

Also... https://docs.gradle.org/3.3/userguide/test_kit.html#sub:test-kit-debug

asdfajshdfkajshdfasdflkjhasdfasd. There, I got out my frustration about not finding this until now.

[AdamBJ]

Do you mean we missed some functions that make working with a BuildResult easier? Or, were you hoping to test with other versions of Gradle, too?

[Nava2]

Debugging the actual plugin itself. I was trying to do it all via "println()" when there is support for debugging the plugin itself. I just missed it in the docs.

[AdamBJ]

Ah, yeah I see it at the bottom. That would have been handy! I'm going to see if I can get the debugger working in Eclipse.

[AdamBJ]

Ugh, nothing with Gradle is ever easy. Have you gotten the debugger working in IntelliJ? I think I'm close with Eclipse, but I'm getting a error that says the gradlePlugin() method is undefined. This is the guide I'm following. I'll fiddle around a bit more later tonight.

[Nava2]

I've got it working in IntelliJ, not Eclipse:

https://github.com/assertj/assertj-generator-gradle-plugin/blob/master/src/test/groovy/org/assertj/generator/gradle/SimpleBuild.groovy

This is how I was launching the debugger. Important line.