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

Add vscode-kotlin extension #611

Closed
wants to merge 3 commits into from
Closed

Conversation

blues-man
Copy link

@blues-man blues-man commented Sep 9, 2020

Signed-off-by: Natale Vinto nvinto@redhat.com

What does this PR do?

Add language support for Kotlin using VS Code extension fwcd.kotlin

TODO

  • Resolve issue on extension activation

@che-bot
Copy link
Contributor

che-bot commented Sep 9, 2020

Can one of the admins verify this patch?

1 similar comment
@che-bot
Copy link
Contributor

che-bot commented Sep 9, 2020

Can one of the admins verify this patch?

@blues-man
Copy link
Author

blues-man commented Sep 9, 2020

Hello, reopening this with new extension from fwcd.

I would like to discuss here this issue I found in extension activation giving this error:
image

Activating extension 'Kotlin' failed: ENOENT: no such file or directory,
mkdir '/home/theia/home/theia/fwcd.kotlin'

Adding logs from Workspace: https://pastebin.com/RDMmmz9g
If you want to try it from PR code:

quay.io/bluesman/che-plugin-registry:nightly
quay.io/bluesman/che-devfile-registry:nightly

It looks like it is trying to create the extension directory into Theia $HOME$HOME, but it is not using mkdir -p

Is that expected?

vscode-extensions.json Outdated Show resolved Hide resolved
@gattytto
Copy link
Contributor

gattytto commented Sep 9, 2020

@blues-man you have the extension trying to create a folder after being initialized by che:
https://github.com/fwcd/vscode-kotlin/blob/bd965d60fc20a08a32d80f3466c6227dd1b221aa/src/serverDownloader.ts#L61

private async downloadServer(downloadUrl: string, version: string, status: Status): Promise<void> {
        if (!(await fsExists(this.installDir))) {
            await fs.promises.mkdir(this.installDir, { recursive: true });
        }

the commands (like mkdir) run by the extension are executed within its sidecar context, so if you mkdir -p the /home/theia/home/theia/fwcd.kotlin it results in this being downloaded inside the sidecar:
image
image

Conclusion: try to find the settings in the extensions package.json that will let you customize the path where the extension is trying to download the language server. Or debug the extension to see how you can "help" it figure the right path string (seeing what variables can be used as a base for the folder calculation).

@gattytto
Copy link
Contributor

gattytto commented Sep 9, 2020

if this to be solved serving the language and debug servers inside the sidecar, this is what I did:

  1. open the project using the kotlin extension, it will fail
  2. open a shell to the kotlin sidecar (now being based on java11), mkdir -p /home/theia/home/theia/fwcd.kotlin
  3. go to workspaces menu in che, and come back to the project (so extension reloads). Now it will offer to download stuff
  4. once stuff is downloaded to the sidecar it can be copied and referenced using the following settings: (locations are just an example)
{
    "kotlin.languageServer.path": "/home/theia/kotlin_ls/fwcd.kotlin/langServerInstall/server/bin/kotlin-language-server",
    "kotlin.debugAdapter.path": "/home/theia/kotlin_ls/fwcd.kotlin/debugAdapterInstall/adapter/bin/kotlin-debug-adapter"
}

image

Signed-off-by: Natale Vinto <nvinto@redhat.com>
@blues-man
Copy link
Author

@gattytto I don't get why langServerInstallDir here https://github.com/fwcd/vscode-kotlin/blob/7f2ef9c81bf6109d88a37397d3add54d0a6b1d18/src/languageSetup.ts#L20 is landing to this path /home/theia/home/theia containing /home/theia twice, is that expected path? That seems the issue

@blues-man
Copy link
Author

I was able to workaround it adding a volume in the sidecar into plugin's meta.yaml like this:

spec:
  containers:
    - image: "quay.io/eclipse/che-sidecar-java:11-7bd8c8c"
      name: vscode-kotlin
      memoryLimit: "1500Mi"
      volumes:
      - mountPath: "/home/theia/.m2"
        name: m2
      - mountPath: "/home/theia/home/theia"
        name: vscode-theia-workaround

Don't know if this could be a suitable option?

@gattytto
Copy link
Contributor

gattytto commented Sep 10, 2020

@gattytto I don't get why langServerInstallDir here https://github.com/fwcd/vscode-kotlin/blob/7f2ef9c81bf6109d88a37397d3add54d0a6b1d18/src/languageSetup.ts#L20 is landing to this path /home/theia/home/theia containing /home/theia twice, is that expected path? That seems the issue

for starters, the runtime execution provider that runs the mkdir command prepends /home/theia by default, then the extension tries to download a zip file to that location, which didn't get created for some reason.
all this behavior is not needed in che design, that part could be just stripped from the extension code and pre-allocate the binaries somewhere in the sidecar filesystem, then from within devfile (or plugin meta-yaml which is not implemented) send those settings to the extension.

this is an example devfile for making the extension bypass the check and just look for the binaries in the specific path, then you may need to make a sidecar that includes those files:

metadata:
  name: kotlin
projects:
  - name: kotlin-examples
    source:
      location: 'https://github.com/Kotlin/kotlin-examples'
      type: git
    clonePath: kotlin
components:
  - id: eclipse/che-theia/next
    type: cheEditor
  - type: chePlugin
    reference: >-
      https://raw.githubusercontent.com/gattytto/cheplugins/master/kotlin/0.2.18/meta.yaml
    alias:
      kotlin
    preferences:
      kotlin.languageServer.path: /home/theia/kotlin_ls/fwcd.kotlin/langServerInstall/server/bin/kotlin-language-server
      kotlin.debugAdapter.path: /home/theia/kotlin_ls/fwcd.kotlin/debugAdapterInstall/adapter/bin/kotlin-debug-adapter
apiVersion: 1.0.0

@gattytto
Copy link
Contributor

gattytto commented Sep 11, 2020

@blues-man I'm trying to build an android native bin using kotlin and gradlew and it's asking me for the and sdk, do you think there could be a branch of the cli image with android sdks included? how is the android dependencies to be dealt with when it comes to language-server and import tracking? I'm not good with java stuff and my android is rusty

about the "dependencies", this is what the author's language server repo says about the subject:
image

so I'm not sure there is an "imports" resolution here

image

@gattytto
Copy link
Contributor

gattytto commented Sep 11, 2020

I have now also tried to add the java extension as you did in your devfile-registry pull request, the extension has the same behavior around the home/theia duplication @ericwill this seems to affect both kotlin and java-quarkus extensions

metadata:
  name: kotlin
projects:
  - name: kotlin-examples
    source:
      location: 'https://github.com/Kotlin/kotlin-examples'
      type: git
    clonePath: kotlin
components:
  - id: redhat/quarkus-java11/latest
    type: chePlugin
  - mountSources: true
    memoryLimit: 2048Mi
    type: dockerimage
    volumes:
      - name: gradle
        containerPath: /home/gradle/.gradle
    image: 'quay.io/eclipse/che-java11-gradle:nightly'
    alias: gradle
    env:
      - value: /home/gradle/.gradle
        name: GRADLE_USER_HOME
      - value: /home/gradle
        name: HOME
      - value: >-
          -XX:MaxRAMPercentage=50 -XX:+UseParallelGC -XX:MinHeapFreeRatio=10
          -XX:MaxHeapFreeRatio=20 -XX:GCTimeRatio=4
          -XX:AdaptiveSizePolicyWeight=90 -Dsun.zip.disableMemoryMapping=true
          -Xms20m -Djava.security.egd=file:/dev/./urandom
        name: JAVA_OPTS
      - value: >-
          -XX:MaxRAMPercentage=50 -XX:+UseParallelGC -XX:MinHeapFreeRatio=10
          -XX:MaxHeapFreeRatio=20 -XX:GCTimeRatio=4
          -XX:AdaptiveSizePolicyWeight=90 -Dsun.zip.disableMemoryMapping=true
          -Xms20m -Djava.security.egd=file:/dev/./urandom
        name: JAVA_TOOL_OPTIONS
  - id: eclipse/che-theia/next
    type: cheEditor
  - type: chePlugin
    reference: >-
      https://raw.githubusercontent.com/gattytto/cheplugins/master/kotlin/0.2.18/meta.yaml
    alias: kotlin
apiVersion: 1.0.0

image

@blues-man
Copy link
Author

blues-man commented Sep 11, 2020

The workaround mentioned above worked, however I can't have javax.ws resolved, could you have that in your test @gattytto ?

image

@blues-man
Copy link
Author

blues-man commented Sep 12, 2020

It looks it misses mvn from the sidecar:

[Info  - 12:08:25 AM] main      Adding script definitions [ScriptTemplateWithArgs]
[Info  - 12:08:25 AM] main      Connected to client
[Info  - 12:08:25 AM] client    Adding workspace file:///projects to source path
[Info  - 12:08:25 AM] client    Adding .../rest/GreetingResource.kt, .../rest/ Greeting.kt, .../rest/GreetingResourceTest.kt, .../rest/NativeGreetingResourceIT.kt under /projects to source path
[Info  - 12:08:25 AM] client    Searching for dependencies and Java sources in workspace root /projects
[Warn  - 12:08:25 AM] client    Could not resolve classpath using Maven: Unable to find the 'mvn' command
[Info  - 12:08:25 AM] client    Could not resolve kotlin-stdlib using Maven: null
[Info  - 12:08:25 AM] client    Could not resolve kotlin-stdlib using Gradle: /home/theia/.gradle/caches
[Info  - 12:08:25 AM] client    Could not resolve kotlin-stdlib using Maven: null
[Info  - 12:08:25 AM] client    Could not resolve kotlin-stdlib using Gradle: /home/theia/.gradle/caches
[Info  - 12:08:25 AM] client    Update build script path

https://github.com/fwcd/kotlin-language-server/blob/64c369cbd081c46d310e6b817e17c5b48fc498b6/shared/src/main/kotlin/org/javacs/kt/classpath/MavenClassPathResolver.kt#L76

Is it possible to add it to the sidecar with some extension or should it be added to a new sidecar like che-sidecar-java-maven ?

Signed-off-by: Natale Vinto <nvinto@redhat.com>
@blues-man
Copy link
Author

blues-man commented Sep 12, 2020

I managed to have it working with 2 adds into meta.yaml:

  • Adding a volume in the sidecar for /home/theia/home/theia path
  • Adding a custom sidecar with maven extending che-sidecar-java with maven package: che-sidecar-maven

image
image

Signed-off-by: Natale Vinto <nvinto@redhat.com>
@gattytto
Copy link
Contributor

fwcd/vscode-kotlin#29

@gattytto
Copy link
Contributor

gattytto commented Sep 12, 2020

@blues-man can you test the imports with a gradle based project using the gradle standard quay.io/eclipse/che-sidecar-java:11-7bd8c8c?

@gattytto
Copy link
Contributor

I have tested the following changes to the plugin.yaml for having a working gradle example:

apiVersion: v2
publisher: fwcd
name: vscode-kotlin
version: 0.2.18
type: VS Code extension
displayName: Kotlin
title: Kotlin tooling for VS Code
description: This plug-in provides support for Kotlin development.
icon: https://raw.githubusercontent.com/JetBrains/logos/master/web/kotlin/kotlin.svg
repository: https://github.com/fwcd/vscode-kotlin/
category: Language
spec:
  containers:
    - image: "quay.io/eclipse/che-sidecar-java:11-7bd8c8c"
      name: vscode-quarkus-kotlin
      memoryLimit: "2500Mi"
      env:
      - value: "/home/theia/.gradle"
        name: GRADLE_USER_HOME
      - value: "/home/theia"
        name: HOME
      volumes:
      - mountPath: "/home/theia/.m2"
        name: m2
      - mountPath: "/home/theia/home/theia"
        name: vscode-theia-workaround
  extensions:
    - https://github.com/fwcd/vscode-kotlin/releases/download/0.2.18/kotlin-0.2.18.vsix

image

@blues-man
Copy link
Author

blues-man commented Sep 14, 2020

Hello @gattytto thanks for sharing it! I tried generating the Kotlin Quarkus quickstart as gradle project and that works fine with those env you tried, in this way we don't have to use any custom sidecar and any Devfile would work straightforward if and only if, they point to Gradle projects.

I can update the PR code with that, I would like to know what's the best option here @ericwill @svor ?

@gattytto
Copy link
Contributor

@blues-man I think the extension is not fully functional, but it's hard for me to reproduce, could you please come to mattermost so we can chat about it more fluidly?.

@ericwill
Copy link
Contributor

@blues-man I think the extension is not fully functional, but it's hard for me to reproduce

Which parts are missing?

repository: https://github.com/fwcd/vscode-kotlin/
category: Language
spec:
containers:
Copy link
Contributor

Choose a reason for hiding this comment

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

env:
      - value: "/home/theia/.gradle"
        name: GRADLE_USER_HOME

@gattytto
Copy link
Contributor

gattytto commented Sep 19, 2020

@blues-man I think the extension is not fully functional, but it's hard for me to reproduce

Which parts are missing?

outline, kotiln dsl (.kts files) linting.

image
image

@gattytto
Copy link
Contributor

there's a setting I have to try next 'kotlin.compiler.jvm.target' : "1.8" .. putting it here for the record

@gattytto
Copy link
Contributor

gattytto commented Sep 20, 2020

so far using the following devfile (and making sure the setting is corretly put in /home/theia/.theia/settings.json)

metadata:
  name: kotlin
projects:
  - name: kotlin-examples
    source:
      location: 'https://github.com/Kotlin/kotlin-examples'
      type: git
    clonePath: kotlin
components:
  - id: eclipse/che-theia/next
    type: cheEditor
  - type: chePlugin
    reference: >-
      https://raw.githubusercontent.com/gattytto/cheplugins/master/kotlin/0.2.18/meta.yaml
    preferences:
      kotlin.compiler.jvm.target: "1.8"
    alias: kotlin
apiVersion: 1.0.0

produces the following results: Although the command-line part works ok within the sidecar terminal (no need for a CLI), the extension doesn't resolve the dokka import.
making sure 'kotlin.compiler.jvm.target' : "1.8" is set in the theia settings makes other warnings around the build.gradle.kts files go away. The test also uses a specific sidecar (java11-gradle diversion), with the kotlinc binary added
image

SIDE NOTE:
building the extension from the "master" branch makes the syntax coloring stop working, this may be noted in case the next extension's release breaks around syntax colors.

@gattytto
Copy link
Contributor

this line helps to prevent the language server from breaking after issuing the F1=>"Kotlin: Restart the Language Server".

@gattytto
Copy link
Contributor

clicking an outline item (now works) proposes the fix that actually erases the word:
image
image

@blues-man
Copy link
Author

I think that this extension, notwithstanding some bugs yet to correct, is the most complete for VSCode, maybe we can ship the che plugin with the maven sidecar and the gradle envs, so it will support both, releasing this version and updating with new ones.

What do you think?

@gattytto
Copy link
Contributor

gattytto commented Sep 25, 2020

@blues-man I think that the cleaner solution would be to have this implemented first, to make the workarounds (like settings*2 for this and this bins to avoid downloads from the extension) maintainable in new versions of the extension and a kotlin specific sidecar with tags for both gradle and maven (and then maybe a java11 gradle/maven che images to use for base-image)

@sunix sunix mentioned this pull request Feb 4, 2021
@RomanNikitenko
Copy link
Member

FYI
About /home/theia/home/theia path problem: I faced with that issue as well and changed the behavior within eclipse-che/che#18548.

@gattytto
Copy link
Contributor

gladly, time to revive this @ericwill @blues-man

@gattytto
Copy link
Contributor

@blues-man I think the extension is not fully functional, but it's hard for me to reproduce

Which parts are missing?

outline, kotiln dsl (.kts files) linting.

image
image

TODO: check .kts support status

@openshift-ci
Copy link

openshift-ci bot commented Apr 13, 2021

@blues-man: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@benoitf benoitf closed this Mar 18, 2022
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.

6 participants