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

Java LS "Classpath is incomplete" warning when loading petclinic #13427

Open
l0rd opened this issue May 27, 2019 · 42 comments
Open

Java LS "Classpath is incomplete" warning when loading petclinic #13427

l0rd opened this issue May 27, 2019 · 42 comments
Assignees
Labels
area/plugins e2e-test/failure Issues that is related to a test failures reported by our CI platform and our QE. kind/bug Outline of a bug - must adhere to the bug report template. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. status/blocked Issue that can’t be moved forward. Must include a comment on the reason for the blockage.

Comments

@l0rd
Copy link
Contributor

l0rd commented May 27, 2019

Description

After I have created a workspace using the following devfile

---
specVersion: 0.0.1
name: petclinic-dev-environment
projects:
  - name: petclinic
    source:
      type: git
      location: 'https://github.com/spring-projects/spring-petclinic.git'
components:
  - type: kubernetes
    alias: mvn-container
    reference: https://gist.githubusercontent.com/l0rd/52eda1d1b57878a218cba17c0c93a477/raw/9a2d56e777d22e77c25c45744caea35bb62d8688/petclinic.yaml
    selector:
      app.kubernetes.io/component: webapp
  - type: kubernetes
    reference: https://gist.githubusercontent.com/l0rd/52eda1d1b57878a218cba17c0c93a477/raw/9a2d56e777d22e77c25c45744caea35bb62d8688/petclinic.yaml
    selector:
      app.kubernetes.io/component: database
  - type: chePlugin
    id: redhat/java/0.45.0
  - type: chePlugin
    id: redhat/vscode-yaml/0.4.0
commands:
  - name: build
    actions:
      - type: exec
        component: mvn-container
        command: mvn package
        workdir: /projects/spring-petclinic

a classpath incomplete warning is shown every time a Java file is opened:

image

Reproduction Steps

Start a workspace using this devfile:

chectl workspace:start --devfile=https://gist.githubusercontent.com/l0rd/52eda1d1b57878a218cba17c0c93a477/raw/8ef7ff447a7164215395d6bc7d70b49363ebae86/devfile.yaml

OS and version:

macOS / minikube / nightly che-server / chectl / helm / jtd ls 0.45.0

@l0rd l0rd added kind/bug Outline of a bug - must adhere to the bug report template. team/languages labels May 27, 2019
@rhopp rhopp mentioned this issue May 28, 2019
25 tasks
@l0rd
Copy link
Contributor Author

l0rd commented Jun 3, 2019

cc @tsmaeder @tolusha

@l0rd l0rd added the severity/P1 Has a major impact to usage or development of the system. label Jun 5, 2019
@svor
Copy link
Contributor

svor commented Jun 6, 2019

It looks like the project which was cloned after initializing jdt.ls workspace, isn't recognized as maven/gradel project. After reloading the browser tab or restarting jdt.ls (Clean the Java Language server workspace command), it works well.
It is also reproducible in local theia instance: just need to start Theia with empty workspace folder and clone some java project.

@l0rd
Copy link
Contributor Author

l0rd commented Jun 6, 2019

So it looks like a concurrency problem: if the project takes time to be cloned, and JDT LS in the meantime get started, the project won't be indexed correctly. A possible solution would be to guarantee that a LS is started only once the cloning operation has completed: @evidolob @benoitf what's your opinion?

@tolusha
Copy link
Contributor

tolusha commented Jun 7, 2019

From my point of view, JDT LS should assume that project might not be ready when it starts and try to initialize it later

@benoitf
Copy link
Contributor

benoitf commented Jun 7, 2019

@l0rd I think the LS is receiving multiple events. I'm unsure that it's possible to wait clone before sending events. All is asynchronous so you could have multiple clones over the lifecycle and you may have finished to clone a project before cloning a new one so it's better to work totally asynchronously.

@tsmaeder
Copy link
Contributor

@benoitf @l0rd this is really the expected behaviour: many language servers (jdt.ls being one) will not pick up on new projects being created in the workspace root (in our case: /projects). The user is expected to reopen the folder or "add folder to workspace". Expecting the language servers to pick this case up consistently is going to be a losing battle, IMO.
We currently have some kind of franken-UI: we still have the "Open Folder" menu, but we really expect projects to be in /projects and changes there to be picked up automatically.
I think there are a couple of things we could do:

  1. When we start a workspace with projects, do the cloning before we start up the IDE. If we can't do this at the che container engine level, we might put a wait into the IDE for this condition. The code that clones the projects would have to notify that it's done.
  2. We could detect folder creation in /projects in the IDE and offer the user the option to reload the window. This would restart all plugins and make them pick up the changes.

@tsmaeder
Copy link
Contributor

Alternative would be to treat each folder in /projects as a workspace root. When detecting a new folder, we would notify the IDE that a new root has been added. But:

  1. This would still be timing dependent: the LS needs metadat (pom.xml, etc.) to detect a project
  2. We would have to analyse how multi-root is different from single-root for language servers. Is the beaviour the one we want for common language servers?

@l0rd
Copy link
Contributor Author

l0rd commented Jun 13, 2019

@tsmaeder I don't want to clone the git repo before the IDE is started because that means that the start of a workspace may take even more. What about restarting Theia (just restarting the pod or restarting the node app) once the cloning is done?

@tsmaeder
Copy link
Contributor

tsmaeder commented Jun 13, 2019

We don't need to restart the pod or back end, a reload of the window is sufficient (reopen the /projects folder programmatically, even).

@l0rd l0rd mentioned this issue Jun 27, 2019
85 tasks
@l0rd l0rd added severity/blocker Causes system to crash and be non-recoverable or prevents Che developers from working on Che code. target/che7GA and removed target/che7GA severity/P1 Has a major impact to usage or development of the system. labels Jun 27, 2019
@fbricon
Copy link

fbricon commented Jul 1, 2019

Please test eclipse-jdtls/eclipse.jdt.ls#901, ask @snjeza for details

@tsmaeder
Copy link
Contributor

tsmaeder commented Jul 4, 2019

We've done some experiments, and the verdict is mixed:
the following language servers pick up new projects: PHP, Go, Typescript
the following do not: Java, Python, C#

The picture is evenly split, I don't think we can expect language servers to pick up changes in general. That means we'll have to restart the language servers, which usually is done by doing a reload of the front end (which will trigger new copies of the back end plugins)

@l0rd
Copy link
Contributor Author

l0rd commented Jul 5, 2019

@tsmaeder Java, with @snjeza fix, is now is picking up the new projects. But we still have Python and C# that do not behave as we would expect. So we should decide if, having an automatic refresh when one of these 2 LS is enabled, should be considered mandatory or not for Che 7 GA right?

@tsmaeder
Copy link
Contributor

tsmaeder commented Jul 8, 2019

@l0rd jdt.ls does not detect this: there is a long-standing PR, but it would need some serious attention and testing before it can be merged. And as I argued above, it's not a solution for any other language servers out there.
I would be strongly opposed to any solution that involves knowledge of a particular language server: we need to get away from the assumption that we know the set of plugins that are running on Che. So if we do a refresh, we need to do it for every type of project.

@l0rd
Copy link
Contributor Author

l0rd commented Jul 8, 2019

@tsmaeder what's your proposal to mitigate the problem for Che 7.0.0?

@l0rd l0rd removed the severity/blocker Causes system to crash and be non-recoverable or prevents Che developers from working on Che code. label Sep 13, 2019
@tsmaeder tsmaeder modified the milestones: 7.2.0, Backlog - Languages Sep 23, 2019
@tsmaeder tsmaeder added the status/blocked Issue that can’t be moved forward. Must include a comment on the reason for the blockage. label Sep 24, 2019
@tsmaeder
Copy link
Contributor

We need to wait for a new release of vscode-languageserver-node and a rebuild of vscode-java in order to proceed with moving to a multi-root based che workspace

@tsmaeder tsmaeder removed the status/blocked Issue that can’t be moved forward. Must include a comment on the reason for the blockage. label Oct 15, 2019
@tsmaeder
Copy link
Contributor

Reactivating: I suspect with the later activation of the plugins, our use cases should work. Retest and move towards a merge:
However: need to collect feedback from dev team about readiness for multi-root workspace.

@tsmaeder tsmaeder mentioned this issue Oct 15, 2019
26 tasks
@tsmaeder tsmaeder mentioned this issue Nov 4, 2019
25 tasks
@tsmaeder tsmaeder mentioned this issue Nov 21, 2019
26 tasks
@tsmaeder tsmaeder mentioned this issue Jan 8, 2020
35 tasks
@tsmaeder tsmaeder mentioned this issue Jan 23, 2020
36 tasks
@tsmaeder tsmaeder mentioned this issue Feb 18, 2020
34 tasks
@tsmaeder tsmaeder added status/blocked Issue that can’t be moved forward. Must include a comment on the reason for the blockage. and removed status/in-progress This issue has been taken by an engineer and is under active development. labels Feb 19, 2020
@tsmaeder
Copy link
Contributor

Blocked until tasks work with multi-root.

@che-bot
Copy link
Contributor

che-bot commented Feb 4, 2021

Issues go stale after 180 days of inactivity. lifecycle/stale issues rot after an additional 7 days of inactivity and eventually close.

Mark the issue as fresh with /remove-lifecycle stale in a new comment.

If this issue is safe to close now please do so.

Moderators: Add lifecycle/frozen label to avoid stale mode.

@che-bot che-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 4, 2021
@ericwill ericwill added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Feb 4, 2021
@ericwill
Copy link
Contributor

Blocked until tasks work with multi-root.

This is no longer the case right? I think we should be unblocked now cc @tsmaeder

@tsmaeder
Copy link
Contributor

This should be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/plugins e2e-test/failure Issues that is related to a test failures reported by our CI platform and our QE. kind/bug Outline of a bug - must adhere to the bug report template. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. status/blocked Issue that can’t be moved forward. Must include a comment on the reason for the blockage.
Projects
None yet
Development

No branches or pull requests