Skip to content

Use response files to index files if argument list exceeds maximum number of arguments #2032

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

Merged
merged 1 commit into from
Mar 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Sources/BuildSystemIntegration/BuildSettingsLogger.swift
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ package actor BuildSettingsLogger {
"""

let chunks = splitLongMultilineMessage(message: log)
for (index, chunk) in chunks.enumerated() {
// Only print the first 100 chunks. If the argument list gets any longer, we don't want to spam the log too much.
// In practice, 100 chunks should be sufficient.
for (index, chunk) in chunks.enumerated().prefix(100) {
logger.log(
level: level,
"""
Expand Down
2 changes: 2 additions & 0 deletions Sources/SKTestSupport/BuildServerTestProject.swift
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ package class BuildServerTestProject: MultiFileTestProject {
buildServerConfigLocation: RelativeFileLocation = ".bsp/sourcekit-lsp.json",
buildServer: String,
options: SourceKitLSPOptions? = nil,
enableBackgroundIndexing: Bool = false,
testName: String = #function
) async throws {
var files = files
Expand Down Expand Up @@ -92,6 +93,7 @@ package class BuildServerTestProject: MultiFileTestProject {
try await super.init(
files: files,
options: options,
enableBackgroundIndexing: enableBackgroundIndexing,
testName: testName
)
}
Expand Down
16 changes: 13 additions & 3 deletions Sources/SKTestSupport/INPUTS/AbstractBuildServer.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ def handle_message(self, message: Dict[str, object]) -> Optional[Dict[str, objec
return self.initialized(params)
elif method == "build/shutdown":
return self.shutdown(params)
elif method == "buildTarget/prepare":
return self.buildtarget_prepare(params)
elif method == "buildTarget/sources":
return self.buildtarget_sources(params)
elif method == "textDocument/registerForChanges":
Expand All @@ -80,6 +82,8 @@ def handle_message(self, message: Dict[str, object]) -> Optional[Dict[str, objec
return self.workspace_did_change_watched_files(params)
elif method == "workspace/buildTargets":
return self.workspace_build_targets(params)
elif method == "workspace/waitForBuildSystemUpdates":
return self.workspace_waitForBuildSystemUpdates(params)

# ignore other notifications
if "id" in message:
Expand Down Expand Up @@ -120,10 +124,8 @@ def initialize(self, request: Dict[str, object]) -> Dict[str, object]:
"version": "0.1",
"bspVersion": "2.0",
"rootUri": "blah",
"capabilities": {"languageIds": ["a", "b"]},
"capabilities": {"languageIds": ["swift", "c", "cpp", "objective-c", "objective-c"]},
"data": {
"indexDatabasePath": "some/index/db/path",
"indexStorePath": "some/index/store/path",
"sourceKitOptionsProvider": True,
},
}
Expand All @@ -144,6 +146,11 @@ def textdocument_sourcekitoptions(
def shutdown(self, request: Dict[str, object]) -> Dict[str, object]:
return {}

def buildtarget_prepare(self, request: Dict[str, object]) -> Dict[str, object]:
raise RequestError(
code=-32601, message=f"'buildTarget/prepare' not implemented"
)

def buildtarget_sources(self, request: Dict[str, object]) -> Dict[str, object]:
raise RequestError(
code=-32601, message=f"'buildTarget/sources' not implemented"
Expand All @@ -157,6 +164,9 @@ def workspace_build_targets(self, request: Dict[str, object]) -> Dict[str, objec
code=-32601, message=f"'workspace/buildTargets' not implemented"
)

def workspace_waitForBuildSystemUpdates(self, request: Dict[str, object]) -> Dict[str, object]:
return {}


class LegacyBuildServer(AbstractBuildServer):
def send_sourcekit_options_changed(self, uri: str, options: List[str]):
Expand Down
71 changes: 70 additions & 1 deletion Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import TSCExtensions
import struct TSCBasic.AbsolutePath
import class TSCBasic.Process
import struct TSCBasic.ProcessResult
import enum TSCBasic.SystemError
#else
import BuildServerProtocol
import BuildSystemIntegration
Expand All @@ -38,6 +39,11 @@ import TSCExtensions
import struct TSCBasic.AbsolutePath
import class TSCBasic.Process
import struct TSCBasic.ProcessResult
import enum TSCBasic.SystemError
#endif

#if os(Windows)
import WinSDK
#endif

private let updateIndexStoreIDForLogging = AtomicUInt32(initialValue: 1)
Expand Down Expand Up @@ -415,7 +421,7 @@ package struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
let result: ProcessResult
do {
result = try await withTimeout(timeout) {
try await Process.run(
try await Process.runUsingResponseFileIfTooManyArguments(
arguments: processArguments,
workingDirectory: workingDirectory,
outputRedirection: .stream(
Expand Down Expand Up @@ -464,3 +470,66 @@ package struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
}
}
}

fileprivate extension Process {
/// Run a process with the given arguments. If the number of arguments exceeds the maximum number of arguments allows,
/// create a response file and use it to pass the arguments.
static func runUsingResponseFileIfTooManyArguments(
arguments: [String],
workingDirectory: AbsolutePath?,
outputRedirection: OutputRedirection = .collect(redirectStderr: false)
) async throws -> ProcessResult {
do {
return try await Process.run(
arguments: arguments,
workingDirectory: workingDirectory,
outputRedirection: outputRedirection
)
} catch {
let argumentListTooLong: Bool
#if os(Windows)
if let error = error as? CocoaError {
argumentListTooLong =
error.underlyingErrors.contains(where: {
return ($0 as NSError).domain == "org.swift.Foundation.WindowsError"
&& ($0 as NSError).code == ERROR_FILENAME_EXCED_RANGE
})
} else {
argumentListTooLong = false
}
#else
if case SystemError.posix_spawn(E2BIG, _) = error {
argumentListTooLong = true
} else {
argumentListTooLong = false
}
#endif

guard argumentListTooLong else {
throw error
}

logger.debug("Argument list is too long. Using response file.")
let responseFile = FileManager.default.temporaryDirectory.appendingPathComponent(
"index-response-file-\(UUID()).txt"
)
defer {
orLog("Failed to remove temporary response file") {
try FileManager.default.removeItem(at: responseFile)
}
}
FileManager.default.createFile(atPath: try responseFile.filePath, contents: nil)
let handle = try FileHandle(forWritingTo: responseFile)
for argument in arguments.dropFirst() {
handle.write(Data((argument.spm_shellEscaped() + "\n").utf8))
}
try handle.close()

return try await Process.run(
arguments: arguments.prefix(1) + ["@\(responseFile.filePath)"],
workingDirectory: workingDirectory,
outputRedirection: outputRedirection
)
}
}
}
2 changes: 0 additions & 2 deletions Sources/SourceKitLSP/Swift/DocumentFormatting.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import TSCExtensions

import struct TSCBasic.AbsolutePath
import class TSCBasic.Process
import func TSCBasic.withTemporaryFile
#else
import Foundation
import LanguageServerProtocol
Expand All @@ -37,7 +36,6 @@ import TSCExtensions

import struct TSCBasic.AbsolutePath
import class TSCBasic.Process
import func TSCBasic.withTemporaryFile
#endif

fileprivate extension String {
Expand Down
82 changes: 82 additions & 0 deletions Tests/SourceKitLSPTests/BackgroundIndexingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2040,6 +2040,88 @@ final class BackgroundIndexingTests: XCTestCase {
return hoverAfterAddingDependencyDeclaration != nil
}
}

func testUseResponseFileIfTooManyArguments() async throws {
// The build system returns too many arguments to fit them into a command line invocation, so we need to use a
// response file to invoke the indexer.

let project = try await BuildServerTestProject(
files: [
// File name contains a space to ensure we escape it in the response file.
"Test File.swift": """
func 1️⃣myTestFunc() {}
"""
],
buildServer: """
class BuildServer(AbstractBuildServer):

def initialize(self, request: Dict[str, object]) -> Dict[str, object]:
return {
"displayName": "test server",
"version": "0.1",
"bspVersion": "2.0",
"rootUri": "blah",
"capabilities": {"languageIds": ["swift", "c", "cpp", "objective-c", "objective-c"]},
"data": {
"indexDatabasePath": r"$TEST_DIR/index-db",
"indexStorePath": r"$TEST_DIR/index",
"prepareProvider": True,
"sourceKitOptionsProvider": True,
},
}

def workspace_build_targets(self, request: Dict[str, object]) -> Dict[str, object]:
return {
"targets": [
{
"id": {"uri": "bsp://dummy"},
"tags": [],
"languageIds": [],
"dependencies": [],
"capabilities": {},
}
]
}

def buildtarget_sources(self, request: Dict[str, object]) -> Dict[str, object]:
return {
"items": [
{
"target": {"uri": "bsp://dummy"},
"sources": [
{"uri": "$TEST_DIR_URL/Test.swift", "kind": 1, "generated": False}
],
}
]
}

def textdocument_sourcekitoptions(self, request: Dict[str, object]) -> Dict[str, object]:
return {
"compilerArguments": [r"$TEST_DIR/Test File.swift", "-DDEBUG", $SDK_ARGS] + \
[f"-DTHIS_IS_AN_OPTION_THAT_CONTAINS_MANY_BYTES_{i}" for i in range(0, 50_000)]
}

def buildtarget_prepare(self, request: Dict[str, object]) -> Dict[str, object]:
return {}
""",
enableBackgroundIndexing: true
)
try await project.testClient.send(PollIndexRequest())

let symbols = try await project.testClient.send(WorkspaceSymbolsRequest(query: "myTestFunc"))
XCTAssertEqual(
symbols,
[
.symbolInformation(
SymbolInformation(
name: "myTestFunc()",
kind: .function,
location: try project.location(from: "1️⃣", to: "1️⃣", in: "Test File.swift")
)
)
]
)
}
}

extension HoverResponseContents {
Expand Down