Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
bjorndarri opened this issue Oct 10, 2023 · 14 comments
Labels
Pending Release This issue has been fixed in master and is awaiting a release

Comments

@bjorndarri
Copy link

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 😃.

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

@bjorndarri
Copy link
Author

bjorndarri commented Oct 11, 2023

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
Copy link
Author

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
Copy link
Author

@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
Copy link
Contributor

koppor commented Oct 18, 2023

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
Copy link
Author

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
Copy link
Contributor

koppor commented Oct 19, 2023

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 #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
Copy link
Author

For details, see this issue, first comment.

@airsquared
Copy link
Collaborator

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
Copy link
Author

bjorndarri commented Oct 24, 2023

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

@airsquared
Copy link
Collaborator

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

@airsquared airsquared added the Pending Release This issue has been fixed in master and is awaiting a release label Oct 24, 2023
@koppor
Copy link
Contributor

koppor commented Oct 24, 2023

@airsquared I do not get any build errors.

Tested at koppor/jabref#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
Copy link
Collaborator

Great, thanks for testing!

@bjorndarri
Copy link
Author

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

@bjorndarri
Copy link
Author

Closing this, thanks guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pending Release This issue has been fixed in master and is awaiting a release
Projects
None yet
Development

No branches or pull requests

3 participants