Skip to content

Commit 177c0de

Browse files
committed
[gardening] Refactor and rename some internal methods for clarity
* Split up setting calculation by language to match the flow of the implementation * Rename the internal setting methods for consistency and clarity * Add a few more doc comments * Formatting cleanup around the code that was touched
1 parent 1bdbd02 commit 177c0de

File tree

1 file changed

+121
-102
lines changed

1 file changed

+121
-102
lines changed

Sources/SKSwiftPMWorkspace/SwiftPMWorkspace.swift

Lines changed: 121 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ extension SwiftPMWorkspace: BuildSystem {
230230
}
231231

232232
if path.basename == "Package.swift" {
233-
return packageDescriptionSettings(path)
233+
return settings(forPackageManifest: path)
234234
}
235235

236236
if path.extension == "h" {
@@ -245,106 +245,26 @@ extension SwiftPMWorkspace {
245245

246246
// MARK: Implementation details
247247

248+
/// Retrieve settings for the given file, which is part of a known target build description.
248249
public func settings(
249250
for path: AbsolutePath,
250251
_ language: Language,
251-
_ td: TargetBuildDescription
252-
) -> FileBuildSettings? {
253-
254-
let buildPath = self.buildPath
255-
252+
_ td: TargetBuildDescription) -> FileBuildSettings?
253+
{
256254
switch (td, language) {
257255
case (.swift(let td), .swift):
258-
// FIXME: this is re-implementing llbuild's constructCommandLineArgs.
259-
var args: [String] = [
260-
"-module-name",
261-
td.target.c99name,
262-
"-incremental",
263-
"-emit-dependencies",
264-
"-emit-module",
265-
"-emit-module-path",
266-
buildPath.appending(component: "\(td.target.c99name).swiftmodule").asString,
267-
// -output-file-map <path>
268-
]
269-
if td.target.type == .library || td.target.type == .test {
270-
args += ["-parse-as-library"]
271-
}
272-
args += ["-c"]
273-
args += td.target.sources.paths.map{ $0.asString }
274-
args += ["-I", buildPath.asString]
275-
args += td.compileArguments()
276-
277-
return FileBuildSettings(
278-
preferredToolchain: nil,
279-
compilerArguments: args,
280-
workingDirectory: workspacePath.asString
281-
)
282-
256+
return settings(forSwiftFile: path, td)
283257
case (.clang, .swift):
284258
return nil
285-
286259
case (.clang(let td), _):
287-
// FIXME: this is re-implementing things from swiftpm's createClangCompileTarget
288-
289-
let compilePath = td.compilePaths().first(where: { $0.source == path })
290-
291-
var args = td.basicArguments()
292-
293-
if let compilePath = compilePath {
294-
args += [
295-
"-MD",
296-
"-MT",
297-
"dependencies",
298-
"-MF",
299-
compilePath.deps.asString,
300-
]
301-
}
302-
303-
switch language {
304-
case .c:
305-
if let std = td.clangTarget.cLanguageStandard {
306-
args += ["-std=\(std)"]
307-
}
308-
case .cpp:
309-
if let std = td.clangTarget.cxxLanguageStandard {
310-
args += ["-std=\(std)"]
311-
}
312-
default:
313-
break
314-
}
315-
316-
if let compilePath = compilePath {
317-
args += [
318-
"-c",
319-
compilePath.source.asString,
320-
"-o",
321-
compilePath.object.asString
322-
]
323-
} else if path.extension == "h" {
324-
args += ["-c"]
325-
if let xflag = language.xflagHeader {
326-
args += ["-x", xflag]
327-
}
328-
args += [path.asString]
329-
} else {
330-
args += [
331-
"-c",
332-
path.asString,
333-
]
334-
}
335-
336-
return FileBuildSettings(
337-
preferredToolchain: nil,
338-
compilerArguments: args,
339-
workingDirectory: workspacePath.asString
340-
)
341-
260+
return settings(forClangFile: path, language, td)
342261
default:
343262
return nil
344263
}
345264
}
346265

347-
func packageDescriptionSettings(_ path: AbsolutePath) -> FileBuildSettings? {
266+
/// Retrieve settings for a package manifest (Package.swift).
267+
func settings(forPackageManifest path: AbsolutePath) -> FileBuildSettings? {
348268
for package in packageGraph.packages where path == package.manifest.path {
349269
let compilerArgs = workspace.interpreterFlags(for: package.path) + [path.asString]
350270
return FileBuildSettings(
@@ -355,6 +275,7 @@ extension SwiftPMWorkspace {
355275
return nil
356276
}
357277

278+
/// Retrieve settings for a given header file.
358279
public func settings(forHeader path: AbsolutePath, _ language: Language) -> FileBuildSettings? {
359280
var dir = path.parentDirectory
360281
while !dir.isRoot {
@@ -365,8 +286,105 @@ extension SwiftPMWorkspace {
365286
}
366287
return nil
367288
}
289+
290+
/// Retrieve settings for the given swift file, which is part of a known target build description.
291+
public func settings(
292+
forSwiftFile path: AbsolutePath,
293+
_ td: SwiftTargetBuildDescription) -> FileBuildSettings?
294+
{
295+
// FIXME: this is re-implementing llbuild's constructCommandLineArgs.
296+
var args: [String] = [
297+
"-module-name",
298+
td.target.c99name,
299+
"-incremental",
300+
"-emit-dependencies",
301+
"-emit-module",
302+
"-emit-module-path",
303+
buildPath.appending(component: "\(td.target.c99name).swiftmodule").asString,
304+
// -output-file-map <path>
305+
]
306+
if td.target.type == .library || td.target.type == .test {
307+
args += ["-parse-as-library"]
308+
}
309+
args += ["-c"]
310+
args += td.target.sources.paths.map { $0.asString }
311+
args += ["-I", buildPath.asString]
312+
args += td.compileArguments()
313+
314+
return FileBuildSettings(
315+
preferredToolchain: nil,
316+
compilerArguments: args,
317+
workingDirectory: workspacePath.asString)
318+
}
319+
320+
/// Retrieve settings for the given C-family language file, which is part of a known target build
321+
/// description.
322+
///
323+
/// - Note: language must be a C-family language.
324+
public func settings(
325+
forClangFile path: AbsolutePath,
326+
_ language: Language,
327+
_ td: ClangTargetBuildDescription) -> FileBuildSettings?
328+
{
329+
// FIXME: this is re-implementing things from swiftpm's createClangCompileTarget
330+
331+
var args = td.basicArguments()
332+
333+
let compilePath = td.compilePaths().first(where: { $0.source == path })
334+
if let compilePath = compilePath {
335+
args += [
336+
"-MD",
337+
"-MT",
338+
"dependencies",
339+
"-MF",
340+
compilePath.deps.asString,
341+
]
342+
}
343+
344+
switch language {
345+
case .c:
346+
if let std = td.clangTarget.cLanguageStandard {
347+
args += ["-std=\(std)"]
348+
}
349+
case .cpp:
350+
if let std = td.clangTarget.cxxLanguageStandard {
351+
args += ["-std=\(std)"]
352+
}
353+
default:
354+
break
355+
}
356+
357+
if let compilePath = compilePath {
358+
args += [
359+
"-c",
360+
compilePath.source.asString,
361+
"-o",
362+
compilePath.object.asString
363+
]
364+
} else if path.extension == "h" {
365+
args += ["-c"]
366+
if let xflag = language.xflagHeader {
367+
args += ["-x", xflag]
368+
}
369+
args += [path.asString]
370+
} else {
371+
args += [
372+
"-c",
373+
path.asString,
374+
]
375+
}
376+
377+
return FileBuildSettings(
378+
preferredToolchain: nil,
379+
compilerArguments: args,
380+
workingDirectory: workspacePath.asString)
381+
}
368382
}
369383

384+
/// A SwiftPM-compatible toolchain.
385+
///
386+
/// Appropriate for both building a pacakge (Build.Toolchain) and for loading the package manifest
387+
/// (ManifestResourceProvider).
370388
private struct SwiftPMToolchain: Build.Toolchain, ManifestResourceProvider {
371389
var swiftCompiler: AbsolutePath
372390
var clangCompiler: AbsolutePath
@@ -410,9 +428,12 @@ extension ToolchainRegistry {
410428
}
411429
}
412430

413-
private func findPackageDirectory(containing path: AbsolutePath, _ fs: FileSystem) -> AbsolutePath? {
431+
/// Find a Swift Package root directory that contains the given path, if any.
432+
private func findPackageDirectory(
433+
containing path: AbsolutePath,
434+
_ fileSystem: FileSystem) -> AbsolutePath? {
414435
var path = path
415-
while !fs.isFile(path.appending(component: "Package.swift")) {
436+
while !fileSystem.isFile(path.appending(component: "Package.swift")) {
416437
if path.isRoot {
417438
return nil
418439
}
@@ -422,23 +443,21 @@ private func findPackageDirectory(containing path: AbsolutePath, _ fs: FileSyste
422443
}
423444

424445
public final class BuildSettingProviderWorkspaceDelegate: WorkspaceDelegate {
425-
public func packageGraphWillLoad(currentGraph: PackageGraph, dependencies: AnySequence<ManagedDependency>, missingURLs: Set<String>) {
426-
}
446+
public func packageGraphWillLoad(
447+
currentGraph: PackageGraph,
448+
dependencies: AnySequence<ManagedDependency>,
449+
missingURLs: Set<String>)
450+
{}
427451

428-
public func fetchingWillBegin(repository: String) {
429-
}
452+
public func fetchingWillBegin(repository: String) {}
430453

431-
public func fetchingDidFinish(repository: String, diagnostic: Basic.Diagnostic?) {
432-
}
454+
public func fetchingDidFinish(repository: String, diagnostic: Basic.Diagnostic?) {}
433455

434-
public func cloning(repository: String) {
435-
}
456+
public func cloning(repository: String) {}
436457

437-
public func removing(repository: String) {
438-
}
458+
public func removing(repository: String) {}
439459

440-
public func managedDependenciesDidUpdate(_ dependencies: AnySequence<ManagedDependency>) {
441-
}
460+
public func managedDependenciesDidUpdate(_ dependencies: AnySequence<ManagedDependency>) {}
442461
}
443462

444463
extension Basic.Diagnostic.Behavior {

0 commit comments

Comments
 (0)