Skip to content

Disable jre lookup when bundling with why #250

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

Closed
2 of 5 tasks
ykrasik opened this issue Aug 2, 2022 · 22 comments
Closed
2 of 5 tasks

Disable jre lookup when bundling with why #250

ykrasik opened this issue Aug 2, 2022 · 22 comments
Labels
enhancement New feature or request fixed Issue fixed and release pending

Comments

@ykrasik
Copy link

ykrasik commented Aug 2, 2022

I'm submitting a…

  • bug report
  • 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
    • Gradle

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

@fvarrui
Copy link
Collaborator

fvarrui commented Aug 14, 2022

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 fvarrui added the enhancement New feature or request label Aug 14, 2022
@ykrasik
Copy link
Author

ykrasik commented Aug 14, 2022

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

fvarrui commented Aug 14, 2022

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

fvarrui commented Aug 15, 2022

I fixed my previous comment

@ykrasik
Copy link
Author

ykrasik commented Aug 15, 2022

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

fvarrui commented Aug 17, 2022

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

@fvarrui fvarrui added fixed Issue fixed and release pending feedback Waiting for feedback labels Aug 17, 2022
@fvarrui
Copy link
Collaborator

fvarrui commented Aug 17, 2022

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

@ykrasik
Copy link
Author

ykrasik commented Aug 18, 2022

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

fvarrui commented Aug 20, 2022

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

ykrasik commented Aug 23, 2022

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

fvarrui commented Sep 4, 2022

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

@ykrasik
Copy link
Author

ykrasik commented Sep 5, 2022

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

@fvarrui
Copy link
Collaborator

fvarrui commented Sep 13, 2022

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

ykrasik commented Sep 13, 2022

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

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

fvarrui commented Sep 14, 2022

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

@fvarrui
Copy link
Collaborator

fvarrui commented Sep 14, 2022

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

fvarrui commented Sep 14, 2022

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

@ykrasik
Copy link
Author

ykrasik commented Oct 23, 2022

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

@fvarrui
Copy link
Collaborator

fvarrui commented Dec 5, 2022

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

fvarrui commented Jan 8, 2023

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

@fvarrui fvarrui removed the feedback Waiting for feedback label Jan 25, 2023
@fvarrui
Copy link
Collaborator

fvarrui commented Feb 8, 2023

JavaPackager 1.7.0 released to Maven Central

@fvarrui fvarrui closed this as completed Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fixed Issue fixed and release pending
Projects
None yet
Development

No branches or pull requests

3 participants