Simplifies/Unifies settings + JUnit tests + auto JDK download

[Osiris-Team]

Contains following breaking changes:

• Gradle users must encapsulate their task settings inside a javapackager extension, except for the win/linux/mac configs, manfiest and scripts closures.
  Show example

task packageMyApp(type: io.github.fvarrui.javapackager.GradlePackageTask, dependsOn: build) {
    javapackager{
        // mandatory
        mainClass = 'path.to.your.mainClass'
        // optional
        bundleJre = true|false
        generateInstaller = true|false
        administratorRequired = true|false
        platform = auto|linux|mac|windows
        additionalResources = [ file('file path'), file('folder path'), ... ]
    }
    linuxConfig {
        ...
    }
    macConfig {
        ...
    }
    winConfig {
        ...
    }
    manifest {
        ....
    }
    scripts {
        ...
    }
    ...
}

• Maven users would need to rename their name property to appName.

[fvarrui]

Please check PackageTask class for TODO not sure about those default values.

Ok! I'll check it ASAP. Thanks so much for your hard work! 👏 👏 👏

[Osiris-Team]

Btw, there would be breaking changes for gradle users at least, so maybe this should be a major release?

[fvarrui]

Btw, there would be breaking changes for gradle users at least so maybe this should be a major release?

Yes, I've been checking your changes and I'm trying to avoid breaking the old behaviour. If I manage to do it, I'll release my changes. I've also resolved your TODOs. I'll tell you something ASAP.

[fvarrui]

Hi @Osiris-Team!

I'm not sure if it's a good idea breaking the plugin's behaviour due to refactoring, but I would like to find a solution to this as you have had a great idea trying to merge the Maven and Gradle settings. I haven't tried it yet, but I think Velocity templates would not be working right now in Gradle, because the properties are in the task property instead of in the packager. They should be adapted too.

This problem could be solved if GradlePackageTask could extend DefaultTask (Gradle task) and PackageTask (plugin settings), but, as we already know, this is not possible in Java 😢

I've also seen that instead of extending the AbstractMojo class in MavenPackageTask (MOJO), you implemented the Mojo interface in order to extend PackageTask. Why don't we do the same with GradlePackageTask? Maybe this class could extend the PackageTask class and implements the Task interface. This way we don't have to use the proxy pattern as it's done with the task property and the old behaviour will not be broken.

Sorry, I hope I've explained myself well 😅 ... what do you think?

[Osiris-Team]

breaking the plugin's behaviour due to refactoring

What do you mean? Everything still should work as expected. With refactoring, do you mean that the Packager doesn't extend PackageTask anymore? (if you mean that, I can revert it, don't remember why I changed that)

merge the Maven and Gradle settings

Yeah I tried everything imaginable to get that done. The problem is that gradle requires you to extend their DefaultTask class which prevents us from extending our settings class. Thus the only workaround/solution I found was to use the settings class as a global extension that gets given to the packaging task(s).

but I think Velocity templates would not be working right now in Gradle, because the properties are in the task property instead of in the packager

Oh snap didn't know about that. Isn't it possible to provide an extension instead? (dunno a thing about this stuff, gotta do some research)

I've also seen that instead of extending the AbstractMojo class in MavenPackageTask (MOJO), you implemented the Mojo interface in order to extend PackageTask. Why don't we do the same with GradlePackageTask?

Yeah, that was one of the first things I tried, and it actually was possible and the project compiled. The problem was that gradle requires tasks to extend directly from DefaultTask and throws an error if they are not doing that :/

I did so much testing in the end that I am pretty sure this is the only way of having a simple, unified, easy-to-maintain settings class. Keep in mind that for maven users everything stays the same, only gradle is giving us a hard time.

[Osiris-Team]

I also tried settings the tasks' settings at runtime via task.getInputs().properties(newProperties). But gradle ignores that, throws errors and says they are not present :/

[fvarrui]

breaking the plugin's behaviour due to refactoring

What do you mean? Everything still should work as expected. With refactoring, do you mean that the Packager doesn't extend PackageTask anymore? (if you mean that, I can revert it, don't remember why I changed that)

I mean that now we have to use javackager extension inside the packaging task:

import io.github.fvarrui.javapackager.GradlePackageTask;
task packageForWindows(type: GradlePackageTask, dependsOn: build) {
    javapackager{
                [...]
    }
}

As you suggested, this way we should upgrade the major version ... and just for refactoring if I'm not wrong.

merge the Maven and Gradle settings

Yeah I tried everything imaginable to get that done. The problem is that gradle requires you to extend their DefaultTask class which prevents us from extending our settings class. Thus the only workaround/solution I found was to use the settings class as a global extension that gets given to the packaging task(s).

Yes, you have done a great work, and I know what I'm saying looks trivial, but I'd love to avoid it if it would be possible.

but I think Velocity templates would not be working right now in Gradle, because the properties are in the task property instead of in the packager

Oh snap didn't know about that. Isn't it possible to provide an extension instead? (dunno a thing about this stuff, gotta do some research)

Oh ... no no, sorry, I was wrong. I confused packager with MOJO/task.

I've also seen that instead of extending the AbstractMojo class in MavenPackageTask (MOJO), you implemented the Mojo interface in order to extend PackageTask. Why don't we do the same with GradlePackageTask?

Yeah, that was one of the first things I tried, and it actually was possible and the project compiled. The problem was that gradle requires tasks to extend directly from DefaultTask and throws an error if they are not doing that :/

Oh! I was not sure about this ... I thought that implementing the Task interface would be enough.

I did so much testing in the end that I am pretty sure this is the only way of having a simple, unified, easy-to-maintain settings class. Keep in mind that for maven users everything stays the same, only gradle is giving us a hard time.

Yes, as I said, you have done a great work!! I also like so much the unit tests ... great point!

I wonder if we can have only one package task class ... I mean, a class that was Task and MOJO at the same time. ❓ ... maybe we could call it TOJO 🤣

[Osiris-Team]

Yeah I tried "TOJO" variant too. Gradle works fine in that case. The issue then is maven and the mojo context already bound exception. I wasnt able to figure out exactly the issue for that and a solution. But it was something related to dependencies, since in that case I had to shade the gradle api classes (only necesary ones with minimize option, also note that shading somehow then breaks gradle), since otherwise the maven task gives class not found exception, since then PackageTask has additional gradle imports.

[Osiris-Team]

I will give implementing only the Task interface tomorrow a try.

[fvarrui]

Yeah I tried "TOJO" variant too. Gradle works fine in that case. The issue then is maven and the mojo context already bound exception. I wasnt able to figure out exactly the issue for that and a solution. But it was something related to dependencies, since in that case I had to shade the gradle api classes (only necesary ones with minimize option, also note that shading somehow then breaks gradle), since otherwise the maven task gives class not found exception, since then PackageTask has additional gradle imports.

Yes, Gradle API is not available on Maven Central

[Osiris-Team]

Tried implementing only the Task, but that also fails with a class cast exception at the line above: at org.gradle.api.internal.project.taskfactory.TaskFactory.create(TaskFactory.java:79)

As you can see, it specifically requires the class to be extended by AbstractTask.

[Osiris-Team]

I got a question about these in my gradle task:

Didnt quite understand what closures are and why they are necessary.

Not sure if it works right now, since those methods are inside the gradle package task class and not in the actual extension/settings.

[fvarrui]

Didnt quite understand what closures are and why they are necessary.

Not sure if it works right now, since those methods are inside the gradle package task class and not in the actual extension/settings.

Closures make easier to initialize complex objects, such as WinConfig, MacConfig, LinuxConfig, Scripts, ...:

task packageForMac(type: GradlePackageTask, dependsOn: build) {
        description = 'Packages the application as a native Mac OS app and bundles it in a tarball'
    javapackager{
        platform = 'mac'
        createTarball = true
        description = 'Hello World app'
    }
        scripts {
                bootstrap = file('assets/bootstrap.sh')
        }
        macConfig {
                infoPlist.additionalEntries = '''
                        <key>LSUIElement</key>
                        <true/>
                '''
        }
}

Closures are defined in the Task, not in the JavaPackager extension, so, they should be specified this way.

Task description and plugin description property are not the same thing ... so, name and description properties had to be renamed to appName and appDescription in PackagerSettings.

[Osiris-Team]

Ah ok gotta update the docs.

[fvarrui]

I plan to add new MOJOs and Tasks, in order to split packaging logic in several goals/tasks (create-dmg, create-pkg, create-mac-app, create-linux-app, create-deb, create-rpm, create-win-app, create-setup, create-msi), so it's possible to use this funtionality independiently, allowing developers to define their own packaging workflow. It allows to make changes in the generated app or create your own app structure, before package it in an installation artifact. I mean, devs can create the Mac OS app, make changes on it, and then generate a DMG file. Right now it's not possible.

So, I have a question for you ... how would you implement those new tasks or MOJOs?

[fvarrui]

Maybe, instead of merge all settings in one class,

Ah ok gotta update the docs.

I think it's not clear ... should those closures be inside javapackager extension?

[Osiris-Team]

Well since we now got a lot of stuff simplified you could, easily duplicate the GradlePackageTask for example, rename it to GradleCreateDmgTask, and then register it in the PackagePlugin class.

For compatibility, I would still have a single package task that does all the stuff like before.

[fvarrui]

Well since we now got a lot of stuff simplified you could, easily duplicate the GradlePackageTask for example, rename it to GradleCreateDmgTask, and then register it in the PackagePlugin class.

For compatibility, I would still have a single package task that does all the stuff like before.

But this task doesn't need all those properties ... just a few and/or expose internal artifact generator properties?

[Osiris-Team]

I think it's not clear ... should those closures be inside javapackager extension?

Well they don't really need to, I think it would make the refactoring for users easier since then there arent any exceptions aka settings that need to be inside of the task scope and not the extension scope.

Idk if that would work anyways since then we might run into maven problems again.

[fvarrui]

Maybe we can use an object mapper like JMapper or Dozer to copy all properties from Task/Mojo to the packager, instead of use the same settings for all tasks

[fvarrui]

Sorry, I know you have been working hard on this stuff, but I need to think more about all this, because I implemented this part of the plugin (to support Gradle/Maven) months or even years ago and I need to refresh my ideas and understand why things were done the way they were done at the time, and see if the new changes are worth it or not. Thanks!!!

[fvarrui]

I know I'm stubborn, but I don't like the idea that refactoring the plugin will change the way you use it

[Osiris-Team]

Maybe we can use an object mapper like JMapper or Dozer to copy all properties from Task/Mojo to the packager, instead of use the same settings for all tasks

Never used an object mapper, but if you say its possible then it should be ^^ The changes I did should at least make it easier to implement a customizable package task.

Sorry, I know you have been working hard on this stuff, but I need to think more about all this, because I implemented this part of the plugin (to support Gradle/Maven) months or even years ago and I need to refresh my ideas and understand why things were done the way they were done at the time, and see if the new changes are worth it or not. Thanks!!!

Yeah, no worries ;)

[Osiris-Team]

I know I'm stubborn, but I don't like the idea that refactoring the plugin will change the way you use it

There is still the possibility of generating java source code, aka generating all the individual maven/gradle classes from a single class (or json file etc.) with defaults. I tried that, but it was way more work than it was worth.

It would be like a plugin to generate unified maven/gradle plugins.

[fvarrui]

Hi @Osiris-Team!

There is still the possibility of generating java source code, aka generating all the individual maven/gradle classes from a single class (or json file etc.) with defaults. I tried that, but it was way more work than it was worth.

It would be like a plugin to generate unified maven/gradle plugins.

I'm pretty sure your changes have been a great effort to make the plugin easier to maintain, but for now I don't want to cross this line: change the way the plugin is used. As I already told you, I plan to add new tasks/MOJOs, and each one should have its own properties.

So, I prefer the option of generating code from a model, as it will make easier to create new tasks and MOJOs. I think we only have to generate an abstract class for each task and MOJO with the same properties, and then implement these classes, so we don't have to touch them, as they will be regenerated each time the plugin is validated by Gradle, or something so (just before the code is compiled). Maybe we can use javapoet, jcodemodel, or simply Velocity templates.

I'll do some more research about this and share my conclusions here ASAP.

I'm going to merge only the part of the code used to download JDKs and skip the refactored part for now, because there are quite a few pending issues to be resolved and I'm a bit stuck.

Thanks so much!!!

[fvarrui]

Sorry ... I don't know what the hell I did. I merged this PR in the wrong branch (devel) and then reverted changes, so it created a new branch called revert-258-pr-248, but I'm not sure if your changes are there. 🤦

[Osiris-Team]

Its ok Im gonna delete this and the reverted branch (u gotta do this), since I added the changes to the devel branch of my fork. I think they are better placed there anyways. https://github.com/Osiris-Team/JavaPackager/tree/devel