dlsc-software-consulting-gmbh / jfxcentral2

Version 2 of JFXCentral.
74 stars 10 forks source link

Please review this patch for desktop packaging #190

Closed mikehearn closed 1 year ago

mikehearn commented 1 year ago

I didn't open a PR because I don't want to accidentally expose the repo by forking it - maybe I'm just being a scaredy cat and it's actually not a problem.

This patch does the following

If it all looks OK I'll upload to a private location that's not used by the current jfxcentral downloads.

diff --git a/.gitignore b/.gitignore
index 00ec4f9..37b00bd 100644
--- a/.gitignore
+++ b/.gitignore
@@ -38,3 +38,6 @@ build/
 .DS_Store
 /app/RUNNING_PID
 /app/logs/
+
+### Conveyor ###
+output/
diff --git a/app/pom.xml b/app/pom.xml
index 64b1f1f..304e53f 100644
--- a/app/pom.xml
+++ b/app/pom.xml
@@ -131,6 +131,27 @@
                    <mainClassName>com.dlsc.jfxcentral2.app.JFXCentral2App</mainClassName>
                </configuration>
            </plugin>
+
+           <!-- For Conveyor: Copy all dependencies excluding JavaFX which will be jlinked in for each platform. -->
+           <plugin>
+               <groupId>org.apache.maven.plugins</groupId>
+               <artifactId>maven-dependency-plugin</artifactId>
+               <executions>
+                   <execution>
+                       <id>copy-dependencies</id>
+                       <phase>prepare-package</phase>
+                       <goals>
+                           <goal>copy-dependencies</goal>
+                       </goals>
+                       <configuration>
+                           <outputDirectory>target/classpath-jars</outputDirectory>
+                           <includeScope>runtime</includeScope>
+                           <excludeGroupIds>org.openjfx</excludeGroupIds>
+                           <overWriteReleases>true</overWriteReleases>
+                       </configuration>
+                   </execution>
+               </executions>
+           </plugin>
        </plugins>
    </build>
 </project>
diff --git a/app/src/main/java/com/dlsc/jfxcentral2/app/JFXCentral2App.java b/app/src/main/java/com/dlsc/jfxcentral2/app/JFXCentral2App.java
index 5549335..f963748 100644
--- a/app/src/main/java/com/dlsc/jfxcentral2/app/JFXCentral2App.java
+++ b/app/src/main/java/com/dlsc/jfxcentral2/app/JFXCentral2App.java
@@ -36,6 +36,7 @@ import javafx.beans.property.ObjectProperty;
 import javafx.beans.property.SimpleObjectProperty;
 import javafx.scene.Scene;
 import javafx.scene.paint.Color;
+import javafx.stage.Stage;
 import javafx.util.Callback;
 import one.jpro.routing.Request;
 import one.jpro.routing.Response;
@@ -54,6 +55,14 @@ public class JFXCentral2App extends RouteApp {
         DataRepository.getInstance().loadData();
     }

+    @Override
+    public void start(Stage stage) {
+        super.start(stage);
+
+        // Can't use Platform.exit because something is leaking a non-daemon thread pool.
+        stage.setOnCloseRequest(t -> System.exit(0));
+    }
+
     @Override
     public Route createRoute() {

diff --git a/conveyor.conf b/conveyor.conf
index e69de29..bed4464 100644
--- a/conveyor.conf
+++ b/conveyor.conf
@@ -0,0 +1,50 @@
+// Before packaging do the build like this:
+//
+// ./mvnw install
+// ./mvnw -pl app prepare-package
+
+include required("/stdlib/jdk/19/openjdk.conf")
+
+// Read some version numbers.
+include "#!=javafx.version ./mvnw -q help:evaluate -Dexpression=javafx.version -DforceStdout"
+include "#!=pom-version ./mvnw -q help:evaluate -Dexpression=project.version -DforceStdout"
+include "#!=commit-height git rev-list --count --first-parent HEAD"
+
+// Import JavaFX jmods for each platform, hosted by Gluon.
+include required("/stdlib/jvm/javafx/from-jmods.conf")
+
+app {
+  fsname = jfxcentral
+  display-name = JFXCentral
+  license = Apache 2   // Is this right?
+  vcs-url = github.com/dlemmermann/jfxcentral2
+
+  // The pom-version currently has a -SNAPSHOT suffix, but this isn't a valid version for package managers.
+  // Version 2 because version 1 already has a release at the same URL.
+  version = 2.0.${commit-height}
+
+  inputs += "app/target/classpath-jars"
+  inputs += "app/target/app-*.jar"
+
+  jvm {
+    gui = com.dlsc.jfxcentral2.app.JFXCentral2App
+
+    modules = ${app.jvm.modules} [
+      "javafx.{controls,web,media,swing,fxml}"
+
+      // Icon font packs need to be on the module path. TODO: Why?
+      "org.kordamp.ikonli.{core,javafx,bootstrapicons,evaicons,ionicons4,material2,material,materialdesign2,materialdesign}"
+      "com.dlsc.jfxcentral2.iconfont"
+    ]
+  }
+
+  windows.console = true
+
+  // The app is hosted by Hydraulic.
+  site {
+    base-url = downloads.hydraulic.dev/jfxcentral2   // TODO: Set back to 1 when we release.
+    copy-to = "//hq.hydraulic.software/var/www/downloads.hydraulic.dev/jfxcentral2"
+  }
+}
+
+conveyor.compatibility-level = 9
dlemmermann commented 1 year ago

Hi Mike ... I have no idea how to apply patches like that. I have never done that. Why don't you simply create a feature branch and provide a PR with these changes? Also, the repo will eventually be public anyways, so no worries.

mikehearn commented 1 year ago

Gah, please accept my apologies, somehow I've ended up accidentally pushing my changes directly into your repository (develop branch). Apparently I have write access, didn't realize and pushed the wrong button.

I don't think I can force push over the branch, but you should be able to if you'd like to undo the merge. Then I can open a PR for review. Alternatively, review and make changes in place on top. Sorry once again. I'm not quite sure why I can write to this repo.

mikehearn commented 1 year ago

A test package is here:

https://downloads.hydraulic.dev/jfxcentral2/download.html

There are some conf changes. I'll send a PR (properly this time) for those.