fvarrui / JavaPackager

:package: Gradle/Maven plugin to package Java applications as native Windows, MacOS, or Linux executables and create installers for them.
GNU General Public License v3.0
1.07k stars 133 forks source link

Disable jre lookup when bundling with `why` #250

Closed ykrasik closed 1 year ago

ykrasik commented 2 years ago

I'm submitting a…

Short description of the issue/suggestion: Why uses a JRE lookup mechanism where it would search for a compatible JRE in a few places, one of which is the jvm_install specified in launcher.ini. In the case of bundling the jre, it would make sense to turn off this lookup by setting allow_system_java=false, allow_java_location_lookup=false and check_main_class=false in the generated launcher.ini (docs).

Now, this lookup will only run if why can't find a compatible JRE in the specified jvm_install path, however, I encountered a bug where this lookup ran regardless. Probably not a big deal if Why fix their bug, but overall feels like the more correct behavior.

Steps to reproduce the issue/enhancement:

javapackager {
    bundleJre = true
    jrePath = new File(System.properties['java.home'])
    winConfig {
        exeCreationTool = 'why'
    }
}

What is the expected behavior? Generate a launcher.ini with

allow_system_java=false
allow_java_location_lookup=false

What is the current behavior? Generates a launcher.ini without the above fields.

Do you have outputs, screenshots, demos or samples which demonstrate the problem or enhancement?

What is the motivation / use case for changing the behavior? There is no point in allowing why to perform a JRE lookup in the case of bundling a JRE with the app.

Please tell us about your environment:

Other information (e.g. related issues, suggestions how to fix, links for us to have context)

fvarrui commented 2 years ago

Hi @ykrasik! Yes, it makes sense, thanks for your proposal. This will be fixed in 1.7.0. Note that you can use your own template to generate the launcher.ini file, until I release version 1.7.0. Just copy and modify the current why-ini.vtl template into your project in ${assetsDir}/mac/why-ini.vtl, and JavaPackager will use this one.

ykrasik commented 2 years ago

Ah, that's good to know, thanks! Missed it in the docs.

P.S: The docs say the file should be called why-ini.vtl?

fvarrui commented 2 years ago

P.S: The docs say the file should be called why-ini.vtl?

Yes, sorry 😅 ... That is the template used to generate launcher.ini

fvarrui commented 2 years ago

I fixed my previous comment

ykrasik commented 2 years ago

The author of why just replied that he released a fix for that bug and it requires check_main_class=false as well, so updated original ticket with that as well

fvarrui commented 2 years ago

Fixed in issue-250 branch (JavaPackager 1.7.0-SNAPSHOT).

fvarrui commented 2 years ago

Please, try it and give me some feedback. Thanks!!

ykrasik commented 2 years ago

I'm sorry, is there any other repo where snapshots are published? I don't see one in maven. I tried the changes you added in that branch manually and that worked for me, though.

fvarrui commented 2 years ago

I'm sorry, is there any other repo where snapshots are published? I don't see one in maven.

No, there's no other repo. You should build and install the plugin manually in your local repo.

I tried the changes you added in that branch manually and that worked for me, though.

Great! It will be released to Maven Central ASAP as v1.7.0.

ykrasik commented 2 years ago

Got ya, thanks! Not sure this is the correct place to put this request, but could the 1.7.0 release include this fix in why? I see that you're using a static, precompiled version of why. Without this fix, bundling a jre with why doesn't really work: Issue Commit

fvarrui commented 2 years ago

So, do you mean we should replace why launcher with a newer version?

ykrasik commented 2 years ago

Yes, please. Latest version has an important fix for bundling the JRE

fvarrui commented 2 years ago

Yes, please. Latest version has an important fix for bundling the JRE

I've just check that JavaPackager was updated by @keastrid with Why 1.1.1 (latest version), so version 1.7.0-SNAPSHOT should be working fine.

ykrasik commented 2 years ago

Ah, I see. 1.1.1 was released on June 20, but the bugfix in Why was committed on Aug 13, so it hasn't been released, yet. I tried with the latest version of Why built from source and it worked. I'll talk to the author

keastrid commented 2 years ago

Was waiting to make sure it worked in this project. I've released a new version here: https://github.com/AstroImageJ/Why/releases/tag/1.1.2

fvarrui commented 2 years ago

Why Java Launcher updated to 1.1.2 in branch issue-250.

fvarrui commented 2 years ago

Was waiting to make sure it worked in this project. I've released a new version here: https://github.com/AstroImageJ/Why/releases/tag/1.1.2

Thanks!

fvarrui commented 2 years ago

I've just tested and it seems to be working fine

ykrasik commented 2 years ago

Should I close this task before or after 1.7.0 is released? :)

fvarrui commented 1 year ago

Should I close this task before or after 1.7.0 is released? :)

Don't worry, I'll close after 1.7.0 is released. Thanks!

fvarrui commented 1 year ago

Branch issue-250 merged into devel, ready to be released

fvarrui commented 1 year ago

JavaPackager 1.7.0 released to Maven Central