-
Notifications
You must be signed in to change notification settings - Fork 48
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
Detect JVM installs at startup #231
Detect JVM installs at startup #231
Conversation
org.eclipse.jdt.launching/launching/org/eclipse/jdt/internal/launching/LaunchingPlugin.java
Outdated
Show resolved
Hide resolved
org.eclipse.jdt.launching/launching/org/eclipse/jdt/internal/launching/LaunchingPlugin.java
Outdated
Show resolved
Hide resolved
org.eclipse.jdt.launching/launching/org/eclipse/jdt/internal/launching/LaunchingPlugin.java
Outdated
Show resolved
Hide resolved
org.eclipse.jdt.launching/launching/org/eclipse/jdt/internal/launching/LaunchingPlugin.java
Outdated
Show resolved
Hide resolved
org.eclipse.jdt.launching/launching/org/eclipse/jdt/internal/launching/LaunchingPlugin.java
Outdated
Show resolved
Hide resolved
org.eclipse.jdt.launching/launching/org/eclipse/jdt/internal/launching/LaunchingPlugin.java
Outdated
Show resolved
Hide resolved
org.eclipse.jdt.launching/launching/org/eclipse/jdt/internal/launching/LaunchingPlugin.java
Outdated
Show resolved
Hide resolved
org.eclipse.jdt.launching/launching/org/eclipse/jdt/internal/launching/LaunchingPlugin.java
Outdated
Show resolved
Hide resolved
org.eclipse.jdt.launching/launching/org/eclipse/jdt/internal/launching/LaunchingPlugin.java
Show resolved
Hide resolved
caaa072
to
b96a86a
Compare
fd33f8f
to
1de400f
Compare
All tests are now passing. I had to adjust some of them because those were assuming that no other JRE is configured for proper test executions; which is now wrong on CI as the infra has numerous JVMs that are now detected, some of them without sources. Adding extra checks on the tests make them work more reliably on any infra. |
@mickaelistria
|
Yes.
It adds the VMs so JDT known them, then JDT may then use it or not in order to run particular programs or resolve execution environments. I did not touch the code that is responsible for choosing the VM to use by default when computing a project build path or when running it. So if it's hardcoded somewhere than the one in eclipse.ini should have priority, it should still happen.
The patch doesn't affect the Eclipse launch in any way. So what is defined in eclipse.ini and how it is used in order to start the Eclipse product is not at all modified by this patch. |
org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/core/ClasspathVariableTests.java
Show resolved
Hide resolved
org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/sourcelookup/Bug565462Tests.java
Outdated
Show resolved
Hide resolved
org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/jres/JREsPreferencePage.java
Outdated
Show resolved
Hide resolved
org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/jres/JREsPreferencePage.java
Show resolved
Hide resolved
...pse.jdt.launching/launching/org/eclipse/jdt/internal/launching/DetectVMInstallationsJob.java
Show resolved
Hide resolved
...pse.jdt.launching/launching/org/eclipse/jdt/internal/launching/DetectVMInstallationsJob.java
Show resolved
Hide resolved
...pse.jdt.launching/launching/org/eclipse/jdt/internal/launching/DetectVMInstallationsJob.java
Show resolved
Hide resolved
...pse.jdt.launching/launching/org/eclipse/jdt/internal/launching/DetectVMInstallationsJob.java
Outdated
Show resolved
Hide resolved
...pse.jdt.launching/launching/org/eclipse/jdt/internal/launching/DetectVMInstallationsJob.java
Show resolved
Hide resolved
...pse.jdt.launching/launching/org/eclipse/jdt/internal/launching/DetectVMInstallationsJob.java
Outdated
Show resolved
Hide resolved
d81cb13
to
71f0d24
Compare
@iloveeclipse Any remaining issues you think need to be considered before we merge? |
Indeed, failing test :-) |
Thanks for the reminder, I had forgotten about it and will try to fix it shortly. |
71f0d24
to
bbcb52c
Compare
No objections. However, I would prefer to postpone this and plan for 4.29 M1. |
cf060d2
to
2e0d0eb
Compare
Hi, |
@jjohnstn Would you please review this one? |
/rebase |
2e0d0eb
to
225a51c
Compare
...pse.jdt.launching/launching/org/eclipse/jdt/internal/launching/DetectVMInstallationsJob.java
Show resolved
Hide resolved
1b474c6
to
8a23f4f
Compare
In light of all conversations being resolved and Mac support being relegated to another issue, the changes have been addressed.
@mickaelistria Please add a N&N entry. Patch approved. |
Note: we see eclipse-jdt/eclipse.jdt.core#1150 today, may be caused by this PR. |
* Remove use of OPTION_JdtDebugCompileMode with OPTION_IgnoreUnnamedModuleForSplitPackage This change replaces use of OPTION_JdtDebugCompileMode with OPTION_IgnoreUnnamedModuleForSplitPackage, since the latter does what we need better and doesn't cause the side effects that the first option causes. Goal is to later on remove OPTION_JdtDebugCompileMode in JDT core. Fixes: #260 Signed-off-by: Simeon Andreev <simeon.danailov.andreev@gmail.com> * Detect JVM installs at startup (#231) * Detect JVM installs at startup Fixes #230 * Delete classpath argument files upon shutdown. Fixes #240 (#242) Store classpath argument files in bundle state location, not in workspace - Avoids that workspace is contaminated with temporary files. - Delete classpath argument files upon shutdown. - Version bump org.eclipse.jdt.launching. See https://github.com/eclipse-platform/eclipse.platform.debug/issues/84 Fixes #240 * Avoid reusing of not thread-safe SimpleDateFormat SimpleDateFormat.format(new Date()); returns same string as immutable and thread-safe DateTimeFormatter.format(new Date().toInstant()) * Avoid potential Threadlocal.set(null) memory leak (#249) Threadlocal.set(null) keeps a reference to ThreadLocal.this see https://rules.sonarsource.com/java/RSPEC-5164 Co-authored-by: Jörg Kubitz <jkubitz-eclipse@gmx.de> * Change how JrtPackageFragmentRoots are printed in Source tab (#266) - currently all JrtPackageFragmentRoots are printed using the path which is the same for all modules extracted from a jrt-fs.jar - change this to print the module name followed by the full jrt-fs.jar path - fixes #633 * JavaSnippetEditor: fix missing synchronize (#259) * JavaSnippetEditor: fix missing synchronize * JavaSnippetEditor: Adding "this." for non static field access --------- Co-authored-by: Jörg Kubitz <jkubitz-eclipse@gmx.de> * LaunchingPlugin: use try-with-resource (#252) to close streams in case of Exceptions. Co-authored-by: Jörg Kubitz <jkubitz-eclipse@gmx.de> * Add missing "static" modifier for constants (#256) To reduce memory per instance Co-authored-by: Jörg Kubitz <jkubitz-eclipse@gmx.de> * [trivial] Fixed "Redundant specification of type arguments" warnings Reported by official SDK build since 4.29 for whatever reason See for example https://download.eclipse.org/eclipse/downloads/drops4/I20230621-1800/compilelogs/plugins/org.eclipse.jdt.debug.jdi.tests_1.1.0.v20230328-1614/@dot.html#OTHER_WARNINGS * Records with inner records breaks breakpoint toggling fixes #270 Boundary for Breakpoint should be checked * Touch bundles affected by the new ecj version See eclipse-platform/eclipse.platform.releng.aggregator#1184 * Fix html links in VMInstall(Type) Extension-Point documentation * Re-throw underlying exceptions in OpenFromClipboardTests #27 (#279) Replace call to Display::syncExec with call to Display::syncCall in the private method getJavaElementMatches. snycCall does not hide exceptions, it re-throws them to the calling thread (which helps debugging). Don't catch exceptions in the helper class "Accessor", re-throw them so the calling tests can show a more meaningful stack-trace when they fail. * Use typed for-each loop Avoiding type-casts makes code easier to read. * Avoid wrapping primitives for toString() see https://rules.sonarsource.com/java/RSPEC-2131 * ReferenceTypeImpl: compare Array content instead of instance comparison. https://stackoverflow.com/questions/8777257/equals-vs-arrays-equals-in-java * VMInstallTests: fix random ConcurrentModificationException #202 by using threadsafe datastructures. * Set console stream encoding for java >= 19 - eclipse.platform.debug#124 https://github.com/eclipse-platform/eclipse.platform.debug/issues/124 * FileHashing: avoid getCanonicalFile() getCanonicalFile() is slow on windows / JDK17 - can costs even more then calculating the SHA1 of the file content, which it is supposed to avoid. * Instead use normalized path. It does not matter that multiple files could be the same due to symbolic links, it will just recalculate their SHA1 for all of them - which is still faster. * Also read all file attributes at once. * Refactor XML parsing * Fix path decoding of URL to File containing space (fixes #64) A space in an URL is encoded as "%20" and has to be decoded to " ". Otherwise a debugged JAR rooted in a path containing space is not opening class file but java file editor. * Add MacOS support for detecting VM installs at start up (#276) - fixes #265 * require org.eclipse.core.runtime 3.29.0 --------- Signed-off-by: Simeon Andreev <simeon.danailov.andreev@gmail.com> Co-authored-by: Simeon Andreev <simeon.danailov.andreev@gmail.com> Co-authored-by: Mickael Istria <mistria@redhat.com> Co-authored-by: Diethard Ohrt <130975899+diti0023@users.noreply.github.com> Co-authored-by: Jörg Kubitz <jkubitz-eclipse@gmx.de> Co-authored-by: Jörg Kubitz <51790620+jukzi@users.noreply.github.com> Co-authored-by: Jeff Johnston <jjohnstn@redhat.com> Co-authored-by: Andrey Loskutov <loskutov@gmx.de> Co-authored-by: Hannes Wellmann <wellmann.hannes1@gmx.net> Co-authored-by: Federico Jeanne <2205684+fedejeanne@users.noreply.github.com>
* Remove use of OPTION_JdtDebugCompileMode with OPTION_IgnoreUnnamedModuleForSplitPackage This change replaces use of OPTION_JdtDebugCompileMode with OPTION_IgnoreUnnamedModuleForSplitPackage, since the latter does what we need better and doesn't cause the side effects that the first option causes. Goal is to later on remove OPTION_JdtDebugCompileMode in JDT core. Fixes: #260 Signed-off-by: Simeon Andreev <simeon.danailov.andreev@gmail.com> * Detect JVM installs at startup (#231) * Detect JVM installs at startup Fixes #230 * Delete classpath argument files upon shutdown. Fixes #240 (#242) Store classpath argument files in bundle state location, not in workspace - Avoids that workspace is contaminated with temporary files. - Delete classpath argument files upon shutdown. - Version bump org.eclipse.jdt.launching. See https://github.com/eclipse-platform/eclipse.platform.debug/issues/84 Fixes #240 * Avoid reusing of not thread-safe SimpleDateFormat SimpleDateFormat.format(new Date()); returns same string as immutable and thread-safe DateTimeFormatter.format(new Date().toInstant()) * Avoid potential Threadlocal.set(null) memory leak (#249) Threadlocal.set(null) keeps a reference to ThreadLocal.this see https://rules.sonarsource.com/java/RSPEC-5164 Co-authored-by: Jörg Kubitz <jkubitz-eclipse@gmx.de> * Change how JrtPackageFragmentRoots are printed in Source tab (#266) - currently all JrtPackageFragmentRoots are printed using the path which is the same for all modules extracted from a jrt-fs.jar - change this to print the module name followed by the full jrt-fs.jar path - fixes #633 * JavaSnippetEditor: fix missing synchronize (#259) * JavaSnippetEditor: fix missing synchronize * JavaSnippetEditor: Adding "this." for non static field access --------- Co-authored-by: Jörg Kubitz <jkubitz-eclipse@gmx.de> * LaunchingPlugin: use try-with-resource (#252) to close streams in case of Exceptions. Co-authored-by: Jörg Kubitz <jkubitz-eclipse@gmx.de> * Add missing "static" modifier for constants (#256) To reduce memory per instance Co-authored-by: Jörg Kubitz <jkubitz-eclipse@gmx.de> * [trivial] Fixed "Redundant specification of type arguments" warnings Reported by official SDK build since 4.29 for whatever reason See for example https://download.eclipse.org/eclipse/downloads/drops4/I20230621-1800/compilelogs/plugins/org.eclipse.jdt.debug.jdi.tests_1.1.0.v20230328-1614/@dot.html#OTHER_WARNINGS * Records with inner records breaks breakpoint toggling fixes #270 Boundary for Breakpoint should be checked * Touch bundles affected by the new ecj version See eclipse-platform/eclipse.platform.releng.aggregator#1184 * Fix html links in VMInstall(Type) Extension-Point documentation * Re-throw underlying exceptions in OpenFromClipboardTests #27 (#279) Replace call to Display::syncExec with call to Display::syncCall in the private method getJavaElementMatches. snycCall does not hide exceptions, it re-throws them to the calling thread (which helps debugging). Don't catch exceptions in the helper class "Accessor", re-throw them so the calling tests can show a more meaningful stack-trace when they fail. * Use typed for-each loop Avoiding type-casts makes code easier to read. * Avoid wrapping primitives for toString() see https://rules.sonarsource.com/java/RSPEC-2131 * ReferenceTypeImpl: compare Array content instead of instance comparison. https://stackoverflow.com/questions/8777257/equals-vs-arrays-equals-in-java * VMInstallTests: fix random ConcurrentModificationException #202 by using threadsafe datastructures. * Set console stream encoding for java >= 19 - eclipse.platform.debug#124 https://github.com/eclipse-platform/eclipse.platform.debug/issues/124 * FileHashing: avoid getCanonicalFile() getCanonicalFile() is slow on windows / JDK17 - can costs even more then calculating the SHA1 of the file content, which it is supposed to avoid. * Instead use normalized path. It does not matter that multiple files could be the same due to symbolic links, it will just recalculate their SHA1 for all of them - which is still faster. * Also read all file attributes at once. * Refactor XML parsing * Fix path decoding of URL to File containing space (fixes #64) A space in an URL is encoded as "%20" and has to be decoded to " ". Otherwise a debugged JAR rooted in a path containing space is not opening class file but java file editor. * Add MacOS support for detecting VM installs at start up (#276) - fixes #265 * require org.eclipse.core.runtime 3.29.0 --------- Signed-off-by: Simeon Andreev <simeon.danailov.andreev@gmail.com> Co-authored-by: Simeon Andreev <simeon.danailov.andreev@gmail.com> Co-authored-by: Mickael Istria <mistria@redhat.com> Co-authored-by: Diethard Ohrt <130975899+diti0023@users.noreply.github.com> Co-authored-by: Jörg Kubitz <jkubitz-eclipse@gmx.de> Co-authored-by: Jörg Kubitz <51790620+jukzi@users.noreply.github.com> Co-authored-by: Jeff Johnston <jjohnstn@redhat.com> Co-authored-by: Andrey Loskutov <loskutov@gmx.de> Co-authored-by: Hannes Wellmann <wellmann.hannes1@gmx.net> Co-authored-by: Federico Jeanne <2205684+fedejeanne@users.noreply.github.com>
Fixes #230
What it does
This adds a (opt-out) startup job that looks up installed JRE is some standard directores. So people usually don't need to configure JRE by hand. It's useful both to Eclipse IDE and JDT-LS with VSCode.
=## How to test
Start a fresh workspace, go to Installed JREs preference page, enjoy that some JREs are detected without any effort.
Author checklist