JavaPackager icon indicating copy to clipboard operation
JavaPackager copied to clipboard

Disable jre lookup when bundling with `why`

Open ykrasik opened this issue 3 years ago • 18 comments

I'm submitting a…

  • [ ] bug report
  • [x] feature request
  • [ ] other

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:

  • JavaPackager version: 1.6.7
  • OS version: Windows 11
  • JDK version: 1.8.0_271
  • Build tool:
    • [ ] Maven
    • [x] Gradle

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

ykrasik avatar Aug 02 '22 17:08 ykrasik

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.

fvarrui avatar Aug 14 '22 07:08 fvarrui

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?

ykrasik avatar Aug 14 '22 09:08 ykrasik

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 avatar Aug 14 '22 11:08 fvarrui

I fixed my previous comment

fvarrui avatar Aug 15 '22 14:08 fvarrui

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

ykrasik avatar Aug 15 '22 15:08 ykrasik

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

fvarrui avatar Aug 17 '22 18:08 fvarrui

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

fvarrui avatar Aug 17 '22 18:08 fvarrui

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.

ykrasik avatar Aug 18 '22 17:08 ykrasik

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.

fvarrui avatar Aug 20 '22 19:08 fvarrui

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

ykrasik avatar Aug 23 '22 13:08 ykrasik

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

fvarrui avatar Sep 04 '22 11:09 fvarrui

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

ykrasik avatar Sep 05 '22 11:09 ykrasik

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.

fvarrui avatar Sep 13 '22 11:09 fvarrui

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

ykrasik avatar Sep 13 '22 16:09 ykrasik

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

keastrid avatar Sep 13 '22 22:09 keastrid

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

fvarrui avatar Sep 14 '22 14:09 fvarrui

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 avatar Sep 14 '22 14:09 fvarrui

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

fvarrui avatar Sep 14 '22 14:09 fvarrui

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

ykrasik avatar Oct 23 '22 21:10 ykrasik

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 avatar Dec 05 '22 14:12 fvarrui

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

fvarrui avatar Jan 08 '23 17:01 fvarrui

JavaPackager 1.7.0 released to Maven Central

fvarrui avatar Feb 08 '23 02:02 fvarrui