Skip to content

Commit c94bd6f

Browse files
authored
Merge pull request #446 from CodaFi/lighting-fixtures
[NFC] Plumb Through Explicit FileSystem to the Integrator
2 parents 22e13b9 + 0becbbc commit c94bd6f

File tree

7 files changed

+103
-33
lines changed

7 files changed

+103
-33
lines changed

Sources/SwiftDriver/IncrementalCompilation/IncrementalCompilationState.swift

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,8 @@ public class IncrementalCompilationState {
110110
outputFileMap: outputFileMap,
111111
parsedOptions: &driver.parsedOptions,
112112
remarkDisabled: Diagnostic.Message.remark_incremental_compilation_has_been_disabled,
113-
reporter: reporter)
113+
reporter: reporter,
114+
fileSystem: driver.fileSystem)
114115
else {
115116
return nil
116117
}
@@ -550,7 +551,7 @@ extension IncrementalCompilationState {
550551
Set(
551552
job.primaryInputs.flatMap {
552553
input -> [TypedVirtualPath] in
553-
if let found = moduleDependencyGraph.findSourcesToCompileAfterCompiling(input) {
554+
if let found = moduleDependencyGraph.findSourcesToCompileAfterCompiling(input, on: self.driver.fileSystem) {
554555
return found
555556
}
556557
self.reporter?.report("Failed to read some swiftdeps; compiling everything", path: input)

Sources/SwiftDriver/IncrementalCompilation/ModuleDependencyGraph.swift

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ extension ModuleDependencyGraph {
5959
outputFileMap: OutputFileMap?,
6060
parsedOptions: inout ParsedOptions,
6161
remarkDisabled: (String) -> Diagnostic.Message,
62-
reporter: IncrementalCompilationState.Reporter?
62+
reporter: IncrementalCompilationState.Reporter?,
63+
fileSystem: FileSystem
6364
) -> (ModuleDependencyGraph, inputsWithMalformedSwiftDeps: [(TypedVirtualPath, VirtualPath)])?
6465
where Inputs.Element == TypedVirtualPath
6566
{
@@ -100,7 +101,8 @@ extension ModuleDependencyGraph {
100101
into: graph,
101102
input: input,
102103
reporter: reporter,
103-
diagnosticEngine: diagnosticEngine)
104+
diagnosticEngine: diagnosticEngine,
105+
fileSystem: fileSystem)
104106
return changes == nil ? (input, swiftDepsFile) : nil
105107
}
106108
return (graph, inputsWithMalformedSwiftDeps)
@@ -147,11 +149,13 @@ extension ModuleDependencyGraph {
147149
/// Used to schedule the 2nd wave.
148150
/// Return nil in case of an error.
149151
func findSourcesToCompileAfterCompiling(
150-
_ source: TypedVirtualPath
152+
_ source: TypedVirtualPath,
153+
on fileSystem: FileSystem
151154
) -> [TypedVirtualPath]? {
152155
findSourcesToCompileAfterIntegrating(
153156
input: source,
154-
swiftDeps: sourceSwiftDepsMap[source])
157+
swiftDeps: sourceSwiftDepsMap[source],
158+
on: fileSystem)
155159
}
156160

157161
/// After a compile job has finished, read its swiftDeps file and return the source files needing
@@ -160,13 +164,15 @@ extension ModuleDependencyGraph {
160164
/// May return a source that has already been compiled.
161165
private func findSourcesToCompileAfterIntegrating(
162166
input: TypedVirtualPath,
163-
swiftDeps: SwiftDeps
167+
swiftDeps: SwiftDeps,
168+
on fileSystem: FileSystem
164169
) -> [TypedVirtualPath]? {
165170
Integrator.integrate(swiftDeps: swiftDeps,
166171
into: self,
167172
input: input,
168173
reporter: self.reporter,
169-
diagnosticEngine: diagnosticEngine)
174+
diagnosticEngine: diagnosticEngine,
175+
fileSystem: fileSystem)
170176
.map {
171177
findSwiftDepsToRecompileWhenNodesChange($0)
172178
.map {sourceSwiftDepsMap[$0]}

Sources/SwiftDriver/IncrementalCompilation/ModuleDependencyGraphParts/Integrator.swift

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,11 @@ extension ModuleDependencyGraph.Integrator {
6060
into destination: Graph,
6161
input: TypedVirtualPath,
6262
reporter: IncrementalCompilationState.Reporter?,
63-
diagnosticEngine: DiagnosticsEngine
63+
diagnosticEngine: DiagnosticsEngine,
64+
fileSystem: FileSystem
6465
) -> Changes? {
6566
guard let sfdg = try? SourceFileDependencyGraph.read(
66-
from: swiftDeps)
67+
from: swiftDeps, on: fileSystem)
6768
else {
6869
reporter?.report("Could not read \(swiftDeps)", path: input)
6970
return nil

Sources/SwiftDriver/IncrementalCompilation/SourceFileDependencyGraph.swift

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212
import Foundation
13+
import TSCBasic
1314
import TSCUtility
1415

1516
/*@_spi(Testing)*/ public struct SourceFileDependencyGraph {
@@ -115,10 +116,12 @@ extension SourceFileDependencyGraph {
115116
case unknownKind
116117
}
117118

118-
// FIXME: This should accept a FileSystem parameter.
119-
static func read(from swiftDeps: ModuleDependencyGraph.SwiftDeps
119+
static func read(
120+
from swiftDeps: ModuleDependencyGraph.SwiftDeps,
121+
on fileSystem: FileSystem
120122
) throws -> Self {
121-
try self.init(pathString: swiftDeps.file.name)
123+
try self.init(contentsOf: swiftDeps.file.absolutePath!,
124+
on: fileSystem)
122125
}
123126

124127
/*@_spi(Testing)*/ public init(nodesForTesting: [Node]) {
@@ -128,14 +131,18 @@ extension SourceFileDependencyGraph {
128131
allNodes = nodesForTesting
129132
}
130133

131-
// FIXME: This should accept a FileSystem parameter.
132-
/*@_spi(Testing)*/ public init(pathString: String) throws {
133-
let data = try Data(contentsOf: URL(fileURLWithPath: pathString))
134+
/*@_spi(Testing)*/ public init(
135+
contentsOf path: AbsolutePath,
136+
on filesystem: FileSystem
137+
) throws {
138+
let data = try filesystem.readFileContents(path)
134139
try self.init(data: data)
135140
}
136141

137-
/*@_spi(Testing)*/ public init(data: Data,
138-
fromSwiftModule extractFromSwiftModule: Bool = false) throws {
142+
/*@_spi(Testing)*/ public init(
143+
data: ByteString,
144+
fromSwiftModule extractFromSwiftModule: Bool = false
145+
) throws {
139146
struct Visitor: BitstreamVisitor {
140147
let extractFromSwiftModule: Bool
141148

@@ -244,7 +251,14 @@ extension SourceFileDependencyGraph {
244251
}
245252

246253
var visitor = Visitor(extractFromSwiftModule: extractFromSwiftModule)
247-
try Bitcode.read(stream: data, using: &visitor)
254+
try data.contents.withUnsafeBytes { buf in
255+
// SAFETY: The bitcode reader does not mutate the data stream we give it.
256+
// FIXME: Let's avoid this altogether and traffic in ByteString/[UInt8]
257+
// if possible. There's no real reason to use `Data` in this API.
258+
let baseAddr = UnsafeMutableRawPointer(mutating: buf.baseAddress!)
259+
let data = Data(bytesNoCopy: baseAddr, count: buf.count, deallocator: .none)
260+
try Bitcode.read(stream: data, using: &visitor)
261+
}
248262
guard let major = visitor.majorVersion,
249263
let minor = visitor.minorVersion,
250264
let versionString = visitor.compilerVersionString else {
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
//===-------- DriverExtensions.swift - Driver Testing Extensions ----------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2020 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
import TSCBasic
14+
15+
/// Contains helpers for retrieving paths to fixtures under the
16+
/// repo-local `TestInputs` directory.
17+
enum Fixture {
18+
/// Form a path to a file residing in the test fixtures directory under the
19+
/// root of this package.
20+
/// - Parameters:
21+
/// - file: The name of the fixture file to search for.
22+
/// - relativePath: The relative path of the subdirectory under
23+
/// `<package-root>/TestInputs`
24+
/// - fileSystem: The filesystem on which to search.
25+
/// - Returns: A path to the fixture file if the resulting path exists on the
26+
/// given file system. Else, returns `nil`.
27+
static func fixturePath(
28+
at relativePath: RelativePath,
29+
for file: String,
30+
on fileSystem: FileSystem = localFileSystem
31+
) -> AbsolutePath? {
32+
let packageRootPath = AbsolutePath(#file)
33+
.parentDirectory
34+
.parentDirectory
35+
.parentDirectory
36+
.parentDirectory
37+
let fixturePath = packageRootPath
38+
.appending(component: "TestInputs")
39+
.appending(relativePath)
40+
.appending(component: file)
41+
42+
// Check that the fixture is really there.
43+
guard fileSystem.exists(fixturePath) else {
44+
return nil
45+
}
46+
47+
return fixturePath
48+
}
49+
}

Tests/SwiftDriverTests/IncrementalCompilationTests.swift

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,10 @@ final class NonincrementalCompilationTests: XCTestCase {
6565
}
6666

6767
func testReadBinarySourceFileDependencyGraph() throws {
68-
let packageRootPath = URL(fileURLWithPath: #file).pathComponents
69-
.prefix(while: { $0 != "Tests" }).joined(separator: "/").dropFirst()
70-
let testInputPath = packageRootPath + "/TestInputs/Incremental/main.swiftdeps"
71-
let graph = try SourceFileDependencyGraph(pathString: String(testInputPath))
68+
let absolutePath = try XCTUnwrap(Fixture.fixturePath(at: RelativePath("Incremental"),
69+
for: "main.swiftdeps"))
70+
let graph = try SourceFileDependencyGraph(contentsOf: absolutePath,
71+
on: localFileSystem)
7272
XCTAssertEqual(graph.majorVersion, 1)
7373
XCTAssertEqual(graph.minorVersion, 0)
7474
XCTAssertEqual(graph.compilerVersionString, "Swift version 5.3-dev (LLVM f516ac602c, Swift c39f31febd)")
@@ -109,10 +109,10 @@ final class NonincrementalCompilationTests: XCTestCase {
109109
}
110110

111111
func testReadComplexSourceFileDependencyGraph() throws {
112-
let packageRootPath = URL(fileURLWithPath: #file).pathComponents
113-
.prefix(while: { $0 != "Tests" }).joined(separator: "/").dropFirst()
114-
let testInputPath = packageRootPath + "/TestInputs/Incremental/hello.swiftdeps"
115-
let graph = try SourceFileDependencyGraph(pathString: String(testInputPath))
112+
let absolutePath = try XCTUnwrap(Fixture.fixturePath(at: RelativePath("Incremental"),
113+
for: "hello.swiftdeps"))
114+
let graph = try SourceFileDependencyGraph(contentsOf: absolutePath,
115+
on: localFileSystem)
116116
XCTAssertEqual(graph.majorVersion, 1)
117117
XCTAssertEqual(graph.minorVersion, 0)
118118
XCTAssertEqual(graph.compilerVersionString, "Swift version 5.3-dev (LLVM 4510748e505acd4, Swift 9f07d884c97eaf4)")
@@ -160,10 +160,9 @@ final class NonincrementalCompilationTests: XCTestCase {
160160
}
161161

162162
func testExtractSourceFileDependencyGraphFromSwiftModule() throws {
163-
let packageRootPath = URL(fileURLWithPath: #file).pathComponents
164-
.prefix(while: { $0 != "Tests" }).joined(separator: "/").dropFirst()
165-
let testInputPath = packageRootPath + "/TestInputs/Incremental/hello.swiftmodule"
166-
let data = try Data(contentsOf: URL(fileURLWithPath: String(testInputPath)))
163+
let absolutePath = try XCTUnwrap(Fixture.fixturePath(at: RelativePath("Incremental"),
164+
for: "hello.swiftmodule"))
165+
let data = try localFileSystem.readFileContents(absolutePath)
167166
let graph = try SourceFileDependencyGraph(data: data, fromSwiftModule: true)
168167
XCTAssertEqual(graph.majorVersion, 1)
169168
XCTAssertEqual(graph.minorVersion, 0)
@@ -778,7 +777,7 @@ class CrossModuleIncrementalBuildTests: XCTestCase {
778777
let jobs = try driver.planBuild()
779778
try driver.run(jobs: jobs)
780779

781-
let data = try Data(contentsOf: path.appending(component: "main.swiftdeps").asURL)
780+
let data = try localFileSystem.readFileContents(path.appending(component: "main.swiftdeps"))
782781
let graph = try SourceFileDependencyGraph(data: data, fromSwiftModule: false)
783782
XCTAssertEqual(graph.majorVersion, 1)
784783
XCTAssertEqual(graph.minorVersion, 0)

Tests/SwiftDriverTests/ModuleDependencyGraphTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1034,7 +1034,7 @@ fileprivate struct SourceFileDependencyGraphMocker {
10341034

10351035
private mutating func mock() -> SourceFileDependencyGraph {
10361036
buildNodes()
1037-
return SourceFileDependencyGraph(nodesForTesting: allNodes )
1037+
return SourceFileDependencyGraph(nodesForTesting: allNodes)
10381038
}
10391039

10401040
private mutating func buildNodes() {

0 commit comments

Comments
 (0)