Skip to content

Relative rsp files #82

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
+86 −10
Closed

Relative rsp files #82

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 19 additions & 6 deletions Sources/SwiftDriver/Driver/Driver.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -524,13 +524,22 @@ extension Driver {
private static func expandResponseFiles(
_ args: [String],
diagnosticsEngine: DiagnosticsEngine,
visitedResponseFiles: inout Set<AbsolutePath>
visitedResponseFiles: inout Set<VirtualPath>
) throws -> [String] {
var result: [String] = []

var skipNext = false
// FIXME: This is very fragile
let forwardingOptions: Set<String> = Set([
Option.Xcc, .XclangLinker, .Xfrontend, .Xlinker, .Xllvm,
].map { $0.spelling })
// Go through each arg and add arguments from response files.
for arg in args {
if arg.first == "@", let responseFile = try? AbsolutePath(validating: String(arg.dropFirst())) {
defer { skipNext = forwardingOptions.contains(arg) }
if !skipNext, arg.first == "@" {
guard let responseFile = diagnosticsEngine
.wrap({ try VirtualPath(path: String(arg.dropFirst())) }) else {
continue
}
// Guard against infinite parsing loop.
guard visitedResponseFiles.insert(responseFile).inserted else {
diagnosticsEngine.emit(.warn_recursive_response_file(responseFile))
Expand All @@ -540,7 +549,11 @@ extension Driver {
visitedResponseFiles.remove(responseFile)
}

let contents = try localFileSystem.readFileContents(responseFile).cString
guard let contents = diagnosticsEngine.wrap({
try localFileSystem.readFileContents(responseFile).cString
}) else {
continue
}
let lines = tokenizeResponseFile(contents)
result.append(contentsOf: try expandResponseFiles(lines, diagnosticsEngine: diagnosticsEngine, visitedResponseFiles: &visitedResponseFiles))
} else {
Expand All @@ -556,7 +569,7 @@ extension Driver {
_ args: [String],
diagnosticsEngine: DiagnosticsEngine
) throws -> [String] {
var visitedResponseFiles = Set<AbsolutePath>()
var visitedResponseFiles = Set<VirtualPath>()
return try expandResponseFiles(args, diagnosticsEngine: diagnosticsEngine, visitedResponseFiles: &visitedResponseFiles)
}
}
Expand Down Expand Up @@ -714,7 +727,7 @@ extension Driver {
}

extension Diagnostic.Message {
static func warn_recursive_response_file(_ path: AbsolutePath) -> Diagnostic.Message {
static func warn_recursive_response_file(_ path: VirtualPath) -> Diagnostic.Message {
.warning("response file '\(path)' is recursively expanded")
}

Expand Down
71 changes: 67 additions & 4 deletions Tests/SwiftDriverTests/SwiftDriverTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -582,16 +582,19 @@ final class SwiftDriverTests: XCTestCase {
let diags = DiagnosticsEngine()
let fooPath = path.appending(component: "foo.rsp")
let barPath = path.appending(component: "bar.rsp")
let notAFilePath = path.appending(component: "nonexistent.rsp")
try localFileSystem.writeFileContents(fooPath) {
$0 <<< "hello\nbye\nbye\\ to\\ you\n@\(barPath.pathString)"
}
try localFileSystem.writeFileContents(barPath) {
$0 <<< "from\nbar\n@\(fooPath.pathString)"
$0 <<< "from\nbar\n@\(fooPath.pathString)\n@\(notAFilePath.pathString)"
}
let args = try Driver.expandResponseFiles(["swift", "compiler", "-Xlinker", "@loader_path", "@" + fooPath.pathString, "something"], diagnosticsEngine: diags)
XCTAssertEqual(args, ["swift", "compiler", "-Xlinker", "@loader_path", "hello", "bye", "bye to you", "from", "bar", "something"])
XCTAssertEqual(diags.diagnostics.count, 1)
XCTAssertEqual(diags.diagnostics.count, 2)
XCTAssert(diags.diagnostics.first!.description.contains("is recursively expanded"))
// FIXME: Error message about nonexistent response file could be improved
XCTAssertEqual(diags.diagnostics[1].description, "noEntry")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it better to change it in tools-support-core or just transform it in swift-driver code?

}
}

Expand All @@ -610,7 +613,6 @@ final class SwiftDriverTests: XCTestCase {
// this is another comment
but this is \\\\\a command
@\#(barPath.pathString)
@NotAFile
-flag="quoted string with a \"quote\" inside" -another-flag
"""#
<<< "\nthis line\thas lots \t of whitespace"
Expand All @@ -634,7 +636,7 @@ final class SwiftDriverTests: XCTestCase {
$0 <<< "swift\n--driver-mode=swiftc\n-v\r\n//comment\n\"the end\""
}
let args = try Driver.expandResponseFiles(["@" + fooPath.pathString], diagnosticsEngine: diags)
XCTAssertEqual(args, ["Command1", "--kkc", "but", "this", "is", #"\\a"#, "command", #"swift"#, "rocks!" ,"compiler", "-Xlinker", "@loader_path", "mkdir", "Quoted Dir", "cd", "Unquoted Dir", "@NotAFile", #"-flag=quoted string with a "quote" inside"#, "-another-flag", "this", "line", "has", "lots", "of", "whitespace"])
XCTAssertEqual(args, ["Command1", "--kkc", "but", "this", "is", #"\\a"#, "command", #"swift"#, "rocks!" ,"compiler", "-Xlinker", "@loader_path", "mkdir", "Quoted Dir", "cd", "Unquoted Dir", #"-flag=quoted string with a "quote" inside"#, "-another-flag", "this", "line", "has", "lots", "of", "whitespace"])
let escapingArgs = try Driver.expandResponseFiles(["@" + escapingPath.pathString], diagnosticsEngine: diags)
XCTAssertEqual(escapingArgs, ["swift", "--driver-mode=swiftc", "-v","the end"])
}
Expand Down Expand Up @@ -685,6 +687,67 @@ final class SwiftDriverTests: XCTestCase {
}
}

func testResponseFileExpansionIgnoresForwarded() throws {
try withTemporaryDirectory { path in
try assertNoDiagnostics { diags in
let forwardedRsp = path.appending(component: "foo.rsp")
let barPath = path.appending(component: "bar.rsp")
let nonexisting = path.appending(component: "nonexiting.rsp")
try localFileSystem.writeFileContents(forwardedRsp) {
$0 <<< "should_not_appear_in_args"
}
try localFileSystem.writeFileContents(barPath) {
$0 <<< """
-Xcc @\(forwardedRsp.pathString)
-Xclang-linker @\(forwardedRsp.pathString)
-Xcc @\(nonexisting.pathString)
-Xclang-linker @\(nonexisting.pathString)
"""
}
let args = try Driver.expandResponseFiles([
"swift", "compiler",
"-Xlinker", "@\(nonexisting.pathString)",
"-Xfrontend", "@\(forwardedRsp.pathString)",
"@\(barPath.pathString)",
], diagnosticsEngine: diags)
XCTAssertEqual(args, [
"swift", "compiler",
"-Xlinker", "@\(nonexisting.pathString)",
"-Xfrontend", "@\(forwardedRsp.pathString)",
"-Xcc", "@\(forwardedRsp.pathString)",
"-Xclang-linker", "@\(forwardedRsp.pathString)",
"-Xcc", "@\(nonexisting.pathString)",
"-Xclang-linker", "@\(nonexisting.pathString)",
])
}
}
}

func testResponseFile() throws {
try withTemporaryDirectory { path in
guard let cwd = localFileSystem
.currentWorkingDirectory else { fatalError() }
let responseFile1 = path.appending(component: "fileList1.rsp")
let responseFile2 = path.appending(component: "fileList2.rsp")
try localFileSystem.writeFileContents(responseFile1) {
$0 <<< "from1stList.swift"
}
try localFileSystem.writeFileContents(responseFile2) {
$0 <<< "from2ndList.swift"
}
let responseFile2Relative = responseFile2.relative(to: cwd)
let driver = try Driver(args: [
"swiftc",
"@\(responseFile1.pathString)",
"@\(responseFile2Relative)",
])
XCTAssertEqual(driver.inputFiles, [
.init(file: .relative(.init("from1stList.swift")), type: .swift),
.init(file: .relative(.init("from2ndList.swift")), type: .swift),
])
}
}

func testLinking() throws {
var env = ProcessEnv.vars
env["SWIFT_DRIVER_TESTS_ENABLE_EXEC_PATH_FALLBACK"] = "1"
Expand Down