beryx / badass-jlink-plugin

Create a custom runtime image of your modular application
https://badass-jlink-plugin.beryx.org
Apache License 2.0
385 stars 27 forks source link

JPackageImageTask property 'imageDir' specifies directory '/project/build/image' which doesn't exist #243

Closed bjorndarri closed 1 year ago

bjorndarri commented 1 year ago

Hey there,

I'm running into a problem with version 3.0.0 of this plugin, when I run the jpackage task I get the following output:

A problem was found with the configuration of task ':jpackageImage' (type 'JPackageImageTask'). In plugin 'org.beryx.jlink' type 'org.beryx.jlink.JPackageImageTask' property 'imageDir' specifies directory '/home/darri/minimal/build/image' which doesn't exist.

This problem is related to this line, where I set the imageName property in the jlink task:

jlink {
    imageName.set(project.name)
    ...
}

the problem disappears when I remove that line.

I've created a minimal project reproducing the error:

https://github.com/bjorndarri/jpackage-error

Just run the `jpackage' task.

I've traced the problem to the following commit (#237): 98a5ad1

I tried changing getImageDir() from @OutputDirectory back to @InputDirectory in both JPackageImageTask and JPackageTask, like it was before the above commit, while not touching getImageOutputDir(), and that solved the problem.

Regarding #237, I've been using the jpackage and jlinkZip tasks extensively for the last couple of years, and have never run into the problem described there. Could it be that this is a result of some misconfiguration, as opposed to a plugin bug?

Now, I'm no Gradle plugin expert and don't really know what I'm doing here, so we're going to have to figure this out somehow :smiley:.

ps. @airsquared thanks for picking up the torch and assisting with this plugin, it's an absolute life saver.

bjorndarri commented 1 year ago

I've created a fork https://github.com/bjorndarri/badass-jlink-plugin with my fix, I'd appreciate it if someone would check if the problem described in #237 has been re-introduced by this change.

bjorndarri commented 1 year ago

FYI, I tried testing this by cloning https://github.com/JabRef/jabref and running ./gradlew jpackage jlinkZip with my snapshot jlink plugin version, but ran into this issue: https://bugs.openjdk.org/browse/JDK-8240567 which turned out to be an interesting rabbit hole, but alas, I could not build.

bjorndarri commented 1 year ago

@koppor

I would appreciate it if you could test this, since all my jlink based builds broke after updating to v3.0.0.

I've been considering the problem, and have a feeling #237 is based on a misunderstanding.

These are the default dependencies between these tasks, as far as I can tell:

jlink -> jpackage (jpackage depends on the output of jlink)

jlink -> jlinkZip (jlinkZip depends on the output of jlink)

Making jlinkZip depend on jpackage seems quite a bit off, at least for the default case. I think your 'fix', adding the jlinkZip.dependsOn jpackage directive seems much more sensible, than modifying this plugin, to accommodate this unusual task dependency.

koppor commented 1 year ago

Which JDK do you have? 21u1 works. All before don't. Reason: We fixed the linked issue, but it takes time until the fix is present in a JDK release.

I can't currently check the availability of JDK 21u1. Our custom build https://files.jabref.org/jdks/ works.

bjorndarri commented 1 year ago

I managed to build JabRef successfully with the recently released JDK 21.0.1.

But I'm afraid the #237 problem remains, when the jlinkZip.dependsOn jpackage directive is missing, when using my snapshot plugin version.

I'm pretty sure that the changes made in #237 were not the right thing to do, when the jlinkZip.dependsOn jpackage directive was sufficient. I'm simply basing this on the fact that all my builds broke and that the default task dependencies in this plugin seem to be correct.

So, what to do?

I'm of the opinion that the changes made in #237 should be reverted and version 3.0.1 released, and that the JabRef build should simply keep using jlinkZip.dependsOn jpackage directive.

What do you guys think? @koppor @airsquared

koppor commented 1 year ago

since all my jlink based builds broke after updating to v3.0.0.

Is it possible to provide more details here?

I'm of the opinion that the changes made in https://github.com/beryx/badass-jlink-plugin/issues/237 should be reverted and version 3.0.1 released

This is fine with me. - However, I think, "proper" input and output dependencies would be great. Nevertheless, if I am the only one having issues (with an easy workaround), reverting is really fine!

bjorndarri commented 1 year ago

For details, see this issue, first comment.

airsquared commented 1 year ago

Try this patch:

Subject: [PATCH] inputs fix
---
Index: src/main/groovy/org/beryx/jlink/JPackageImageTask.groovy
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/main/groovy/org/beryx/jlink/JPackageImageTask.groovy b/src/main/groovy/org/beryx/jlink/JPackageImageTask.groovy
--- a/src/main/groovy/org/beryx/jlink/JPackageImageTask.groovy  (revision 9e3aae450f168a363a17ebc07ecfb605efd945d6)
+++ b/src/main/groovy/org/beryx/jlink/JPackageImageTask.groovy  (date 1698130694193)
@@ -56,8 +56,8 @@
     }

     @InputDirectory
-    Directory getImageDir() {
-        extension.imageDir.get()
+    File getImageInputDir() {
+        ((JlinkTask) project.tasks.getByName(JlinkPlugin.TASK_NAME_JLINK)).getImageDirAsFile()
     }

     @Nested
@@ -79,7 +79,7 @@
     void jpackageTaskAction() {
         def taskData = new JPackageTaskData()
         taskData.jlinkBasePath = jlinkBasePath
-        taskData.imageDir = imageName ? imageDirFromName : imageDir.asFile
+        taskData.imageDir = imageInputDir
         taskData.moduleName = moduleName
         taskData.customImageData = customImageData
         taskData.jpackageData = jpackageData
Index: src/main/groovy/org/beryx/jlink/JPackageTask.groovy
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/main/groovy/org/beryx/jlink/JPackageTask.groovy b/src/main/groovy/org/beryx/jlink/JPackageTask.groovy
--- a/src/main/groovy/org/beryx/jlink/JPackageTask.groovy   (revision 9e3aae450f168a363a17ebc07ecfb605efd945d6)
+++ b/src/main/groovy/org/beryx/jlink/JPackageTask.groovy   (date 1698130528834)
@@ -50,8 +50,8 @@
     }

     @Internal
-    Directory getImageDir() {
-        extension.imageDir.get()
+    File getImageInputDir() {
+        ((JlinkTask) project.tasks.getByName(JlinkPlugin.TASK_NAME_JLINK)).getImageDirAsFile()
     }

     @InputDirectory
@@ -73,7 +73,7 @@
     void jpackageTaskAction() {
         def taskData = new JPackageTaskData()
         taskData.jlinkBasePath = jlinkBasePath
-        taskData.imageDir = imageName ? imageDirFromName : imageDir.asFile
+        taskData.imageDir = imageInputDir
         taskData.moduleName = moduleName
         taskData.jpackageData = jpackageData
         taskData.mainClass = mainClass ?: defaultMainClass
bjorndarri commented 1 year ago

Thank you very much @airsquared, this patch fixed my problem (this issue, just to be clear)!

airsquared commented 1 year ago

I pushed the change to master; @koppor could you verify that this change doesn't break anything for you?

koppor commented 1 year ago

@airsquared I do not get any build errors.

Tested at https://github.com/koppor/jabref/pull/660

- jlinkZip.dependsOn jpackage

The binaries run well - thus, a +1 from my side!


Installation:

  1. ./gradlew shadowJar
  2. mvn install:install-file -Dfile=build/libs/badass-jlink-plugin-3.0.1.jar -DgroupId=org.beryx.jlink -DartifactId=org.beryx.jlink.gradle.plugin -Dversion=3.0.2 -Dpackaging=jar
  3. buildscript {
        repositories {
            mavenLocal()
        }
        dependencies {
            classpath 'org.beryx.jlink:org.beryx.jlink.gradle.plugin:3.0.2'
        }
    }
airsquared commented 1 year ago

Great, thanks for testing!

bjorndarri commented 1 year ago

I just tested with the current master and everything is working correctly.

bjorndarri commented 1 year ago

Closing this, thanks guys!