Skip to content

BuildServerIntegration: thread a request purpose so editor and index build settings don't share a cache entry - #2782

Open
nkcsgexi wants to merge 1 commit into
swiftlang:mainfrom
nkcsgexi:sourcekit-options-purpose
Open

nkcsgexi wants to merge 1 commit into
swiftlang:mainfrom
nkcsgexi:sourcekit-options-purpose

Conversation

@nkcsgexi

@nkcsgexi nkcsgexi commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Add a purpose (editor vs index) to the TextDocumentSourceKitOptionsRequest that BuildServerManager sends. buildSettings defaults to .editor; background indexing (UpdateIndexStoreTaskDescription) passes .index. Because purpose is part of the request, editor and index requests occupy separate cachedAdjustedSourceKitOptions entries, so the index settings are computed after preparation and pick up the explicit-modules sidecar instead of reusing the editor's stale entry.

@nkcsgexi

nkcsgexi commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

swiftlang/swift-tools-protocols#86
@swift-ci please test

@nkcsgexi
nkcsgexi requested a review from owenv October 1, 2026 20:56
@nkcsgexi
nkcsgexi force-pushed the sourcekit-options-purpose branch from aa7bd1d to ea9479b Compare October 1, 2026 21:10
@nkcsgexi

nkcsgexi commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

swiftlang/swift-tools-protocols#86
@swift-ci please test

@nkcsgexi
nkcsgexi force-pushed the sourcekit-options-purpose branch from ea9479b to a20d5fb Compare October 1, 2026 21:45
@nkcsgexi

nkcsgexi commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

swiftlang/swift-tools-protocols#86
@swift-ci please test

…build settings don't share a cache entry

Add a purpose (editor vs index) to the TextDocumentSourceKitOptionsRequest that
BuildServerManager sends. The editor path passes .editor explicitly and background
indexing (UpdateIndexStoreTaskDescription) passes .index; buildSettings has no default
purpose, so every caller states its purpose. Because purpose is part of the request,
editor and index requests occupy separate cachedAdjustedSourceKitOptions entries, so the
index settings are computed after preparation and pick up the explicit-modules sidecar
instead of reusing the editor's stale entry.
@nkcsgexi
nkcsgexi force-pushed the sourcekit-options-purpose branch from a20d5fb to 15eefce Compare October 2, 2026 15:21
@nkcsgexi

nkcsgexi commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

swiftlang/swift-tools-protocols#86
@swift-ci please test

@nkcsgexi
nkcsgexi enabled auto-merge October 2, 2026 18:27

@ahoppen ahoppen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don’t fully understand the purpose of this. Indexing and editor functionality should be operating on the same build settings and if the explicit modules build has finished, shouldn’t the editor also pick up these modified settings?

@nkcsgexi

nkcsgexi commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor Author

shouldn’t the editor also pick up these modified settings?

You are right that we should also enable EBMs for editor build settings. However, we currently don't have a way to reliably enable EBM for editor functionalities since editor's build setting could be requested before the preparation build is done, and the preparation build is where all the module dependencies are generated. However, we could enable EBM reliably for background indexing build after this change since background indexing always happens after a preparation build.

@nkcsgexi

nkcsgexi commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

swiftlang/swift-tools-protocols#86
@swift-ci please test

@ahoppen

ahoppen commented Oct 5, 2026

Copy link
Copy Markdown
Member

Requesting editor build settings should be kicking off the file’s preparation. The diagnostics requests even waits for editor preparation. So I don’t see a reason why we can’t wait for EMB to finish for editor settings.

@nkcsgexi

nkcsgexi commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

reason why we can’t wait for EMB to finish for editor settings.

Thinking more about this, I think you are right Alex that the problem here is more about that now preparation can change the compiler arguments, however we never invalidate the stale cache after the preparation build is done. What do you think we call cachedAdjustedSourceKitOptions.clear(isolation: self) { targets.contains($0.target) } inside prepare?

@ahoppen

ahoppen commented Oct 11, 2026

Copy link
Copy Markdown
Member

What causes different compiler arguments to be returned by the build system once preparation has been done? Ie. how does the build system know that preparation has finished? Ideally, whatever triggers that change in behavior would also be the one that invalidates the build settings in the cache (probably by having the BSP server send a buildTarget/didChange notification to LSP, which invalidates the cache).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants