Skip to content
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

Support using 'javac' as internal compiler. #3167

Merged
merged 6 commits into from
Oct 16, 2024

Conversation

fbricon
Copy link
Contributor

@fbricon fbricon commented May 27, 2024

The idea is to run

mvn clean verify -Pjavac

But for some reason I run into:

[ERROR] Failed to execute goal org.eclipse.tycho:tycho-compiler-plugin:4.0.8:validate-classpath (default-validate-classpath) on project org.eclipse.jdt.ls.tests: Execution default-validate-classpath of goal org.eclipse.tycho:tycho-compiler-plugin:4.0.8:validate-classpath failed: org.osgi.framework.BundleException: Bundle org.eclipse.jdt.ls.tests cannot be resolved:org.eclipse.jdt.ls.tests [293]
[ERROR] Unresolved requirement: Require-Bundle: org.eclipse.jdt.core.javac

@akurtakov @mickaelistria what am I missing?

@fbricon fbricon force-pushed the javac-poc-tests branch 3 times, most recently from 046db5d to db2819f Compare May 27, 2024 22:36
@fbricon
Copy link
Contributor Author

fbricon commented May 27, 2024

I got the tests running in https://ci.eclipse.org/ls/job/jdt-ls-javac/

@fbricon fbricon force-pushed the javac-poc-tests branch 4 times, most recently from ea04e38 to 46fe57f Compare May 30, 2024 10:01
@fbricon fbricon force-pushed the javac-poc-tests branch 2 times, most recently from 182472f to 7ff2b84 Compare July 10, 2024 10:22
Comment on lines +8 to +11
Bundle-RequiredExecutionEnvironment: JavaSE-23
Import-Package: org.osgi.framework;version="1.3.0",
org.eclipse.jdt.internal.javac,
org.eclipse.jdt.internal.javac.dom
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of tweaking the MANIFEST.MF, you can consider adding the javac bundle as extraRequirement with target-platform-configuration block in the pom.xml. This bit of customization could also be encapsulated in the javac profile, so it can be easier to merge later.

pom.xml Outdated
<profile>
<id>javac</id>
<properties>
<tycho.testArgLine>--add-opens jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED -DICompilationUnitResolver=org.eclipse.jdt.core.dom.JavacCompilationUnitResolver -DCompilationUnit.DOM_BASED_OPERATIONS=true -DAbstractImageBuilder.compiler=org.eclipse.jdt.internal.javac.JavacCompiler</tycho.testArgLine>
Copy link
Contributor

Choose a reason for hiding this comment

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

This now also needs

--add-opens jdk.javadoc/jdk.javadoc.internal.doclets.formats.html.taglets.snippet=ALL-UNNAMED --add-opens jdk.javadoc/jdk.javadoc.internal.doclets.formats.html.taglets=ALL-UNNAMED

Comment on lines +24 to +43
<compilerArgs combine.self="override">
<arg>--add-exports</arg>
<arg>java.base/java.lang=ALL-UNNAMED</arg>
<arg>--add-exports</arg>
<arg>jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED</arg>
<arg>--add-exports</arg>
<arg>jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED</arg>
<arg>--add-exports</arg>
<arg>jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED</arg>
<arg>--add-exports</arg>
<arg>jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED</arg>
<arg>--add-exports</arg>
<arg>jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED</arg>
<arg>--add-exports</arg>
<arg>jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED</arg>
<arg>--add-exports</arg>
<arg>jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED</arg>
<arg>--add-exports</arg>
<arg>jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED</arg>
</compilerArgs>
Copy link
Contributor

Choose a reason for hiding this comment

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

This might possibly be removed

@rgrunber
Copy link
Contributor

A fix to ensure completion actually produces results. The monitor field in https://github.com/eclipse-jdtls/eclipse-jdt-core-incubator/blob/dom-with-javac/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/DOMCompletionEngine.java never calls beginTask or anything other than isCancelled. We use beginTask in the monitor to initialize the time at which we will call setCancelled(true). This fixes things for now.

diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionHandler.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionHandler.java
index 2610ec8a..f7861c1e 100644
--- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionHandler.java
+++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionHandler.java
@@ -268,8 +268,9 @@ public class CompletionHandler{
                        IBuffer buffer = unit.getBuffer();
                        if (buffer != null && buffer.getLength() >= offset) {
                                IProgressMonitor subMonitor = new ProgressMonitorWrapper(monitor) {
-                                       private long timeLimit;
                                        private final long TIMEOUT = Long.getLong("completion.timeout", 5000);
+                                       // beginTask need not be called so initialize here just in case
+                                       private long timeLimit = System.currentTimeMillis() + TIMEOUT;
 
                                        @Override
                                        public void beginTask(String name, int totalWork) {

@mickaelistria
Copy link
Contributor

@rgrunber are you sure it's related to this issue? At the moment, the DOMCompletionEngine shouldn't be turned on with other Javac feature (soon, but not yet), so the behavior shouldn't be impacted.
Can you please report the issue about the monitor to the jdt-core-incubator repo? cc @datho7561

@datho7561
Copy link
Contributor

I'll fix it in the DOMCompletionEngine; I have a PR up right now since it's a ~2-3 line change

@rgrunber
Copy link
Contributor

rgrunber commented Oct 11, 2024

It's all disabled by default. However, if a user were to turn on the completion through DOM, they would get no results every time.

datho7561 added a commit to datho7561/eclipse.jdt.core that referenced this pull request Oct 11, 2024
datho7561 added a commit to eclipse-jdtls/eclipse-jdt-core-incubator that referenced this pull request Oct 11, 2024
@datho7561
Copy link
Contributor

I've merged the fix to call beginTask() and done() on the progress monitor

@testforstephen
Copy link
Contributor

@rgrunber Anything blocking? Can we merge it now?

@fbricon
Copy link
Contributor Author

fbricon commented Oct 15, 2024

mmm

[ERROR] Cannot resolve target definition:
[ERROR]   Software being installed: org.eclipse.jdt.core 3.38.0.v20240515-1634
[ERROR]   Software being installed: org.eclipse.jdt.core.manipulation 1.21.300.v20240918-0710
[ERROR]   Only one of the following can be installed at once: [org.eclipse.jdt.core 3.40.0.v20240919-0625, org.eclipse.jdt.core 3.38.0.v20240515-1634, org.eclipse.jdt.core 3.39.0.v20240807-1632]
[ERROR]   Cannot satisfy dependency: org.eclipse.jdt.core.manipulation 1.21.300.v20240918-0710 depends on: osgi.bundle; org.eclipse.jdt.core [3.39.0,4.0.0)
[ERROR]
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for JDT Language Server :: Parent 1.41.0-SNAPSHOT:
[INFO]
[INFO] JDT Language Server :: Parent ...................... SUCCESS [  0.024 s]
[INFO] JDT Language Server :: Target Platform ............. SUCCESS [  0.004 s]
[INFO] JDT Language Server :: Core ........................ FAILURE [  3.625 s]

@rgrunber rgrunber force-pushed the javac-poc-tests branch 2 times, most recently from 7a85299 to cfd7f2d Compare October 16, 2024 02:37
- Report setting through 'telemetry/event' notification
- Use a JDK 23 for building
- Bump ECJ to 3.39.0.v20240820-0604
- Bump target platform to match new jdt-core-incubator site

Signed-off-by: Roland Grunberg <[email protected]>
@rgrunber rgrunber merged commit 494c2f4 into eclipse-jdtls:master Oct 16, 2024
6 of 7 checks passed
rgrunber pushed a commit to eclipse-jdtls/eclipse-jdt-core-incubator that referenced this pull request Oct 18, 2024
mickaelistria pushed a commit to mickaelistria/eclipse.jdt.core that referenced this pull request Oct 23, 2024
mickaelistria pushed a commit to eclipse-jdtls/eclipse-jdt-core-incubator that referenced this pull request Nov 12, 2024
mickaelistria pushed a commit to eclipse-jdtls/eclipse-jdt-core-incubator that referenced this pull request Dec 5, 2024
mickaelistria pushed a commit to mickaelistria/eclipse.jdt.core that referenced this pull request Dec 6, 2024
mickaelistria pushed a commit to mickaelistria/eclipse.jdt.core that referenced this pull request Dec 13, 2024
datho7561 added a commit to eclipse-jdtls/eclipse-jdt-core-incubator that referenced this pull request Dec 13, 2024
mickaelistria pushed a commit to eclipse-jdtls/eclipse-jdt-core-incubator that referenced this pull request Dec 15, 2024
mickaelistria pushed a commit to eclipse-jdtls/eclipse-jdt-core-incubator that referenced this pull request Jan 6, 2025
mickaelistria pushed a commit to eclipse-jdtls/eclipse-jdt-core-incubator that referenced this pull request Jan 7, 2025
mickaelistria pushed a commit to eclipse-jdtls/eclipse-jdt-core-incubator that referenced this pull request Jan 10, 2025
robstryker pushed a commit to eclipse-jdtls/eclipse-jdt-core-incubator that referenced this pull request Jan 15, 2025
robstryker pushed a commit to eclipse-jdtls/eclipse-jdt-core-incubator that referenced this pull request Jan 15, 2025
robstryker pushed a commit to eclipse-jdtls/eclipse-jdt-core-incubator that referenced this pull request Jan 22, 2025
datho7561 added a commit to eclipse-jdtls/eclipse-jdt-core-incubator that referenced this pull request Jan 22, 2025
datho7561 added a commit to eclipse-jdtls/eclipse-jdt-core-incubator that referenced this pull request Jan 27, 2025
mickaelistria pushed a commit to eclipse-jdtls/eclipse-jdt-core-incubator that referenced this pull request Jan 29, 2025
datho7561 added a commit to eclipse-jdtls/eclipse-jdt-core-incubator that referenced this pull request Jan 29, 2025
robstryker pushed a commit to robstryker/eclipse.jdt.core that referenced this pull request Jan 31, 2025
robstryker pushed a commit to eclipse-jdtls/eclipse-jdt-core-incubator that referenced this pull request Jan 31, 2025
robstryker pushed a commit to robstryker/eclipse.jdt.core that referenced this pull request Jan 31, 2025
robstryker pushed a commit to eclipse-jdtls/eclipse-jdt-core-incubator that referenced this pull request Jan 31, 2025
robstryker pushed a commit to robstryker/eclipse.jdt.core that referenced this pull request Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants