Skip to content

Detect JVM installs at startup#231

Merged
jjohnstn merged 4 commits intoeclipse-jdt:masterfrom
mickaelistria:auto-detect-jvm-installs
Jun 15, 2023
Merged

Detect JVM installs at startup#231
jjohnstn merged 4 commits intoeclipse-jdt:masterfrom
mickaelistria:auto-detect-jvm-installs

Conversation

@mickaelistria
Copy link
Copy Markdown
Contributor

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

@mickaelistria mickaelistria force-pushed the auto-detect-jvm-installs branch 4 times, most recently from caaa072 to b96a86a Compare April 11, 2023 08:46
@mickaelistria mickaelistria force-pushed the auto-detect-jvm-installs branch 10 times, most recently from fd33f8f to 1de400f Compare April 14, 2023 11:40
@mickaelistria
Copy link
Copy Markdown
Contributor Author

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.

@SarikaSinha
Copy link
Copy Markdown
Member

@mickaelistria
I will look at the code in the next few days. Had some basic queries :

  1. Does it handle only to populate the JREs in Installed JREs block or also uses them to launch if there was none defined in ini or in the folder under the eclipse directory?
  2. It does not break anything defined here https://wiki.eclipse.org/Eclipse.ini ?

@mickaelistria
Copy link
Copy Markdown
Contributor Author

Does it handle only to populate the JREs in Installed JREs block

Yes.

also uses them to launch if there was none defined in ini or in the folder under the eclipse directory?

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.

It does not break anything defined here https://wiki.eclipse.org/Eclipse.ini ?

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.

@mickaelistria mickaelistria force-pushed the auto-detect-jvm-installs branch 3 times, most recently from d81cb13 to 71f0d24 Compare April 17, 2023 15:58
@mickaelistria
Copy link
Copy Markdown
Contributor Author

@iloveeclipse Any remaining issues you think need to be considered before we merge?
@SarikaSinha IIRC, you're using a Mac, aren't you? If so, and if you feel like it, feel free to enrich this PR with whatever makes it work better on mac.

@iloveeclipse
Copy link
Copy Markdown
Member

Any remaining issues you think need to be considered before we merge?

Indeed, failing test :-)

https://ci.eclipse.org/jdt/job/eclipse.jdt.debug-github/job/PR-231/20/testReport/org.eclipse.jdt.debug.tests.sourcelookup/Bug565462Tests/testFindDuplicatesBug565462/

@mickaelistria
Copy link
Copy Markdown
Contributor Author

Indeed, failing test :-)

Thanks for the reminder, I had forgotten about it and will try to fix it shortly.

@mickaelistria mickaelistria force-pushed the auto-detect-jvm-installs branch from 71f0d24 to bbcb52c Compare April 18, 2023 13:33
@mickaelistria
Copy link
Copy Markdown
Contributor Author

Hi,
Can this be considered for a review/merge soon, so we get time to gather feedback during 4.29 cycle and adapt whatever needs to be adapted in a comfortable timeframe?

@akurtakov
Copy link
Copy Markdown
Contributor

@jjohnstn Would you please review this one?

@jjohnstn
Copy link
Copy Markdown
Contributor

/rebase

@github-actions github-actions bot force-pushed the auto-detect-jvm-installs branch from 2e0d0eb to 225a51c Compare June 12, 2023 18:44
@jjohnstn jjohnstn dismissed iloveeclipse’s stale review June 15, 2023 18:48

In light of all conversations being resolved and Mac support being relegated to another issue, the changes have been addressed.

@jjohnstn jjohnstn added this to the 4.29 M1 milestone Jun 15, 2023
@jjohnstn jjohnstn added enhancement New feature or request noteworthy Noteworthy feature labels Jun 15, 2023
@jjohnstn
Copy link
Copy Markdown
Contributor

@mickaelistria Please add a N&N entry. Patch approved.

@iloveeclipse
Copy link
Copy Markdown
Member

Note: we see eclipse-jdt/eclipse.jdt.core#1150 today, may be caused by this PR.

SarikaSinha added a commit that referenced this pull request Aug 1, 2023
* 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>
SarikaSinha added a commit that referenced this pull request Aug 1, 2023
* 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>
// particular VM installations
String javaHome = System.getenv("JAVA_HOME"); //$NON-NLS-1$
if (javaHome != null) {
directories.add(new File(javaHome));
Copy link
Copy Markdown
Contributor

@basilevs basilevs Mar 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

directories may be unmodifiable. See Collectors.toSet()

There are no guarantees on the type, mutability, serializability, or thread-safety of the Set returned; if more control over the returned Set is required, use toCollection(Supplier).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #758

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request noteworthy Noteworthy feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Detect Java installations from standard folders

7 participants