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

Sourcekit-lsp go-to-def not working for modules built from diff toolchain version #1626

Open
jkl0619 opened this issue Aug 15, 2024 · 12 comments
Labels
bug Something isn't working

Comments

@jkl0619
Copy link

jkl0619 commented Aug 15, 2024

Swift version

swift 5

Platform

Linux 5.19

Editor

Visual studio with swift-tools-version 5.9

Does the issue reproduce with Swift 6?

I didn’t try

Description

Creating this issue as I was asked here

Hello, this is using VSCode + sourcekit-lsp

I have a swift library that is built using toolchain sdk version of 13.4.

In this code, I have import modules, import A and import B

The code builds, and I am also able to find the go-to-defs for items in A. However, I am unable to find anything for items in B when using go-to-defs. I've took a look at the LSP logging, and can verify that both output directory of the lib modules are included in the key.compilerargs with the -XCC flag

B uses sdk version of 15.2, A uses version 13.0

Is there something with the sdk versions that causes sourcekit-lsp to be able to not link the symbols correctly? Is this a known issue? Or am I totally on the wrong track here and the issue is elsewhere?

Thanks in advance.

Steps to Reproduce

Unfortunately I don't really have a simple way to reproduce this, as I can't share this project.

Logging

Unfortunately contains internal information, but if you'd like specific information I can do my best to send it

@jkl0619 jkl0619 added the bug Something isn't working label Aug 15, 2024
@jkl0619 jkl0619 changed the title Sourcekit-lsp go-to-def not working Sourcekit-lsp go-to-def not working for modules built from diff toolchain version Aug 15, 2024
@ahoppen
Copy link
Member

ahoppen commented Aug 15, 2024

Synced to Apple’s issue tracker as rdar://133965202

@ahoppen
Copy link
Member

ahoppen commented Aug 15, 2024

Is this a SwiftPM project, a project with compile_commands.json or a build server?

Did you build your project when you’re seeing this issue? SourceKit bundled with Swift 5.9 doesn’t support background indexing and we need a build to update the index.

B uses sdk version of 15.2, A uses version 13.0

Are they just built with different SDKs or also different toolchains / Swift compilers?

@jkl0619
Copy link
Author

jkl0619 commented Aug 15, 2024

Is this a SwiftPM project, a project with compile_commands.json or a build server?

build server

Did you build your project when you’re seeing this issue? SourceKit bundled with Swift 5.9 doesn’t support background indexing and we need a build to update the index.

Yes, I am able to build it. What's weird is that for the module where the go-to-defs don't work, clicking on the import B go-to-def will take me to a .swiftinterface in /tmp/sourcekit-lsp/GeneratedInterfaces/. However, clicking on the actual symbols from B will lead to no-definition-found

Are they just built with different SDKs or also different toolchains / Swift compilers?

Different SDK Versions as far as I am aware. They all use the same build server.

Thanks for looking into

@jkl0619 jkl0619 closed this as completed Aug 15, 2024
@jkl0619 jkl0619 reopened this Aug 15, 2024
@ahoppen
Copy link
Member

ahoppen commented Aug 15, 2024

I assume that we are missing index data, which causes cross-module jump-to-definition not to work. Jumping to definition on the import itself uses a slightly different code path because we don’t need to figure out in which module a type was defined.

Could you ensure that you:

  • Set -index-store-path in the compiler invocations that build your project
  • Make sure that we have the index store and index database path by either
    • Return the path to the index store as indexStorePath from your build server and returning indexDatabasePath to any directory in which SourceKit-LSP can build indexstore-db (
      // see if index store was set as part of the server metadata
      if let indexDbPath = readReponseDataKey(data: response.data, key: "indexDatabasePath") {
      self.indexDatabasePath = try AbsolutePath(validating: indexDbPath, relativeTo: self.projectRoot)
      }
      if let indexStorePath = readReponseDataKey(data: response.data, key: "indexStorePath") {
      self.indexStorePath = try AbsolutePath(validating: indexStorePath, relativeTo: self.projectRoot)
      }
      )
    • Alternatively, you can set these options as -index-store-path and -index-db-path when launching sourcekit-lsp.

@jkl0619
Copy link
Author

jkl0619 commented Aug 16, 2024

Hey thanks for your response

Set -index-store-path in the compiler invocations that build your project

I'm not sure if I understand this correctly. Should I be overriding it to my current projects path?
or should it be something like
-index-store-path ${SOURCEKIT_LSP_CMAKE_BINARY_DIR}/index?

@ahoppen
Copy link
Member

ahoppen commented Aug 17, 2024

It’s hard to tell without knowing what your build server does. Do you have an example of build settings that are provided by your build server?

@jkl0619
Copy link
Author

jkl0619 commented Aug 19, 2024

For the build setting, are you referring to the project, or the sourcekit-lsp?

Just as a general info:
So I dug a little bit deeper into the build etc

We use the buck build system

The sourcekit-lsp is built via MAKE that has a specific .mk file to specify configuration, looks like its built using swift bunch of commands and configurations

It does not have an index-store-path in the set of configurations from what I can tell, but I assume it just uses some sort of default set path if thats the case.

The other modules (aka the imports / project) are built using buck (which underlying I believe uses swift compiler, uses the apple_library macro)

The reason why I was under the impression that the index store is included is because I have
the following from the sourcekit logging when opening the problem project file

sourcekit: [2:handleRequest-before:49.4170] {
  key.request: source.request.editor.open,
  key.name: "/some/directory/problemProject.swift",
  key.enablesyntaxmap: 0,
  key.enablesubstructure: 0,
  key.enablediagnostics: 0,
  key.syntactic_only: 1,
  key.compilerargs: [
  ...

`"-Xcc", "buck-out/.swift-vscode/blah/blah-blah/B/swift-extended_symlink_tree"`,
`"-Xcc", "buck-out/.swift-vscode/blah/blah-blah/A/exported_symlink_tree"`,
...

With that being said, I examined both directories and they both had their respective A-Swift.h and B-Swift.h files in those places. Below are few other settings that may be(?) relevant, so including it here. They are still part of the key.compilerargs

    "-Xfrontend",
    "-emit-clang-header-nonmodular-includes",
    "-Xfrontend",
    "-disable-cxx-interop-requirement-at-import",
    "-Xfrontend",
    "-enable-upcoming-feature",
    "-Xfrontend",
    "ImportObjcForwardDeclarations",
    "-Xfrontend",
    "-explicit-swift-module-map-file",
    "-Xfrontend",
    "buck-out/.swift-vscode/projectFileName.swift_module_map.json",
    ```

@ahoppen
Copy link
Member

ahoppen commented Aug 19, 2024

For the build setting, are you referring to the project, or the sourcekit-lsp?

I am referring to your project’s build settings.

It does not have an index-store-path in the set of configurations from what I can tell, but I assume it just uses some sort of default set path if thats the case.

You need to pass a -index-store-path so that we can build an index of your project during a build, which can then be used by SourceKit-LSP to provide cross-module functionality. Opening the file in sourcekitd doesn’t use the index and provides mostly functionality that’s contained within a single module. sourcekitd needs to co-operate with the index to provide cross-module functionality.

@jkl0619
Copy link
Author

jkl0619 commented Aug 20, 2024

I will look into being able to do that and report back when I can.

That does bring me another question however.

Clicking on a symbol from B will still take me to the generated interface file of .swiftinterface (Even without generating it first via clicking on import B go to def)

Are there any guesses as to why symbols from B will go to generated-interface, but say no definition found for A symbols, even if go-to-def for that import will take me to a .swiftinterface file as well? I'm trying to follow through in the sourcekit-lsp code but can't seem to understand why such case might happen

@ahoppen
Copy link
Member

ahoppen commented Aug 20, 2024

I haven’t fully understood that part yet. It is a little hard to reason about without a reproducing example project. But I’m pretty sure that the lack of index data is the root cause here.

@jkl0619
Copy link
Author

jkl0619 commented Aug 20, 2024

Looks like buck model and index store is very incompatible and they are currently working on coming up with a solution for it, so I guess at the time being the cross-module jump functionality is kind of thrown out the window. I do apologize that I'm unable to just provide everything (e.g. reproducible example project) so that you can also easily see whats going on.

I'd still like to discover why the go-to-def works for some symbols but not others to at least direct uesrs to .swiftinterface files. I see #747 seems to have added the functionality for it, but I guess it must be failing for some while succeeding for others. I guess I'll have to keep digging, but yeah, my initial thought was something with the toolchain version since they are both in the key.compilerargs: ....

@ahoppen
Copy link
Member

ahoppen commented Aug 20, 2024

To figure out why it works in one module but not the other, I would use SourceKit-LSP from a Swift 6.0 toolchain (because it has way more logging than 5.10), enable extended logging and check if there’s anything interesting in the log, as shown here.

If that doesn’t show anything, I would try building a debug version of SourceKit-LSP and debugging it, especially the SourceKitLSPServer.definition function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants