TomDmitriev / gradle-bundle-plugin

Apache License 2.0
47 stars 24 forks source link

Jar task CopySpec is ignored #68

Closed eskatos closed 7 years ago

eskatos commented 7 years ago

This plugin hardcodes the content of the produced jar.

For example only the main source set is considered when gathering sources & "resources". See https://github.com/TomDmitriev/gradle-bundle-plugin/blob/master/src/main/groovy/org/dm/gradle/plugins/bundle/BundleUtils.groovy#L73-L84 If supplementary source sets are added to the Jar, by e.g. jarTask.from sourceSets.another.output, then this plugin ignores that.

The Jar task defines a CopySpec that specifies it's intended content, build authors and other plugins will add to this CopySpec. This plugin should honor this instead of hardcoding what will be packaged in the resulting Jar.

Fixing this will allow usage of this plugin alongside other plugins that contribute content to the Jar like for example any source code generation plugin.

tonit commented 7 years ago

Hi @TomDmitriev, i am the original creator of the referenced ticket on the gradle side (https://github.com/gradle/gradle/issues/1046). I tried to fix the problem in your plugin and propose a PR by changing the lines at https://github.com/TomDmitriev/gradle-bundle-plugin/blob/master/src/main/groovy/org/dm/gradle/plugins/bundle/BundleUtils.groovy#L73-L84.

However, as written in the gradle ticket linked above, i wrestled more with unexpected Gradle Errors (yes, StackOverflowErrors..) than with anything else. I guess its easier for you to pick up the suggestions mentioned by @eskatos and apply them to your plugin. A fix would be highly appreciated! I'd be happy to supply more suggestions and test the fix. Enjoy, Toni

TomDmitriev commented 7 years ago

Hi @tonit, What part of BundleUtils did you try to alter? When it comes to building a jar using bndtools, which is what this plugin essentially does, the resources are usually added using instruction -includeresource, i.e.:

bundle {
    instruction '-includeresource', '${resource1Path},${resource2Path}'
}

As for the sources, I don't see any way to specify them using an instruction. Also, I think it's impossible to distinguish between source files and any other files using the API of JarTask. The solution that comes to my mind is to add a new property, sourceSets to BundleExtension, which by default would point to the main sourceSet only, and get the files from this property. That is in your case, you'd need to do add to your script something like

bundle {
    sourceSets << mySourceSet
}

Will it work for you?

tonit commented 7 years ago

Thanks Tom for your quick reply. I am aware of bnds endless amount of tricks and macros. Thats not the question here. Its really about setting better defaults so that this plugin works seemlessly with other tools. See, in line https://github.com/TomDmitriev/gradle-bundle-plugin/blob/master/src/main/groovy/org/dm/gradle/plugins/bundle/BundleUtils.groovy#L80 you specifically add the main sourceSet. However, the JarTaks allows to add additional sourceSets to be included as "resources". Other plugins build on that, but in your plugin you pick a single "default". I'd imagine that jarTask.from sourceSets.* gives you a list of all sourceSets configured. By default you still will only get "main" but in more complex cases you get more. So, this is really about being a good gradle citizen, not that its impossible to do with bnds macros.

Your suggestion would be a good "workaround" but it does not make it much better I'd say. I still need to manually add the extra sourceSet in your Extension. Not better than adding a bnd instruction in my .bnd file.

Anyway, thanks for picking this up. My main concern is that the simple solution i described in the linked gradle ticket raises a StackOverflow. Gradle guys say the plugin is not a good citizen. So I'd like to help but after been a very long Maven citizen, my 'gradle' is not good enough, yet.

Happy New Year, Tom! ;)

TomDmitriev commented 7 years ago

Happy New Year, Toni! Could you please check out branch issue-68 and test if the changes work for you? If they do, I will roll them out as a new release.

tonit commented 7 years ago

A first test looks good! However, we face an integration problem with IntelliJ now. When referencing that bundle project via compile project(":otherBundleProject") it appears to not pick the bundle as created by bnd but.. only the "main" source output? I still need to digg deeper. First guess is that your change is good! Maybe you also use IntelliJ and can reproduce?

tonit commented 7 years ago

This issue only appears when configuring intellij to do "create separate module per source set". If i deactivate it, its looks wonderful

tonit commented 7 years ago

Great, thanks, @TomDmitriev