Skip to content

Stop passing -resource-dir to the Frontend if the flag wasn't passed in on the command-line #1827

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

Merged
merged 1 commit into from
Mar 19, 2025
Merged

Stop passing -resource-dir to the Frontend if the flag wasn't passed in on the command-line #1827

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
14 changes: 9 additions & 5 deletions Sources/SwiftDriver/Jobs/FrontendJobHelpers.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,11 +392,15 @@ extension Driver {
try addPathArgument(.absolute(workingDirectory), to: &commandLine, remap: jobNeedPathRemap)
}

// Resource directory.
try addPathOption(option: .resourceDir,
path: VirtualPath.lookup(frontendTargetInfo.runtimeResourcePath.path),
to: &commandLine,
remap: jobNeedPathRemap)
// Only pass in a resource directory to the frontend if one was passed to
// swift-driver. Make an exception for scan-dependencies jobs for now till
// we figure out a remaining problem with in-process scanning.
if parsedOptions.hasArgument(.resourceDir) || kind == .scanDependencies {
Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't pass it for scanDependencies jobs either initially, then a single test ExplicitModuleBuildTests.testExplicitLinkLibraries failed. It appears that a swift-frontend -scan-dependencies job alone does not find the resource-dir properly, so I set this flag for that kind of job for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

What was the failure? It's possible this job relies on being able to look up linked libraries relative to resource directory.

Copy link
Member Author

Choose a reason for hiding this comment

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

What was the failure?

It complains that it didn't find a single library:

/home/finagolfin/swift-driver/Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift:549: error: ExplicitModuleBuildTests.testExplicitLinkLibraries : XCTUnwrap failed: expected non-nil value of type "LinkLibraryInfo" -

The test, which you just added in #1622 last summer, looks in the Swift resource directory next to whatever compiler was used, as these swift-driver tests require a Swift toolchain to be in the PATH when they're run, and makes sure various parts of the stdlib are available. This currently works because this swift-driver queries the frontend with -print-target-info then passes that Swift -resource-dir to all subsequent jobs, like the -scan-dependencies frontend job that this test invokes.

However, this seems like a frontend bug that this swift-driver is covering up, as it is the frontend that finds the Swift resource-dir for all other jobs anyway. I will look into why it does not do so only for -scan-dependencies in the frontend and fix that first, just ran this through the CI to see if any other tests broke on other platforms and they didn't.

It's possible this job relies on being able to look up linked libraries relative to resource directory.

Yep, the only question is who should supply the path to that Swift resource directory? We cannot easily do it in the test because it is just picking up the Swift frontend and resource-dir from the PATH, ie it could be anywhere.

I see no reason why the swift-frontend -scan-dependencies job can't find it, whereas the frontend always easily finds it for all other jobs, so I will look into fixing this in the frontend before coming back to this pull. If I'm missing anything, let me know.

Copy link
Member Author

@finagolfin finagolfin Mar 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

OK, I ran some experiments and it appears the problem is not in the frontend at all, but in this repo when trying to scan dependencies in-process by using libswiftScan. If I disable that, as you can see here, the failing test passes on linux, but some other caching and explicit module tests that probably require that in-process scanning then fail. If I simply disable two tests instead, as shown in the latest version of this pull, then the rest of the macOS tests pass, while other scan-dependencies tests still fail on linux for the lack of a resource-dir.

@artemcm, I see two options here:

  1. Only disable this resource-dir flag for non-scan-dependencies jobs, as I did initially, until we figure out why that in-process scanning works differently.
  2. Figure out why in-process scanning works differently now- my guess is that it's passed a different PATH in its environment- and try to fix that too.

Let me know what you'd like to do: if you choose 2., let me know any ideas you have for the difference.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the investigation! It makes sense that this would be due to a difference with the in-process scanner. And your theory on the different PATH seems plausible. I am okay with passing resource-dir here to the scanner invocation only in the meantime, to make up for that difference. And I will see if I can discern making the in-process scanner potentially not need that.

try addPathOption(option: .resourceDir,
path: VirtualPath.lookup(frontendTargetInfo.runtimeResourcePath.path),
to: &commandLine,
remap: jobNeedPathRemap)
}

if self.useStaticResourceDir {
commandLine.appendFlag("-use-static-resource-dir")
Expand Down
2 changes: 2 additions & 0 deletions Tests/SwiftDriverTests/SwiftDriverTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -860,8 +860,10 @@ final class SwiftDriverTests: XCTestCase {
let plannedJobs = try driver1.planBuild().removingAutolinkExtractJobs()
XCTAssertEqual(plannedJobs.count, 3)
XCTAssertEqual(plannedJobs[0].outputs.count, 1)
XCTAssert(!plannedJobs[0].commandLine.contains(.flag("-resource-dir")))
XCTAssertTrue(matchTemporary(plannedJobs[0].outputs.first!.file, "foo.o"))
XCTAssertEqual(plannedJobs[1].outputs.count, 1)
XCTAssert(!plannedJobs[1].commandLine.contains(.flag("-resource-dir")))
XCTAssertTrue(matchTemporary(plannedJobs[1].outputs.first!.file, "bar.o"))
XCTAssertTrue(plannedJobs[2].tool.name.contains(executableName("clang")))
XCTAssertEqual(plannedJobs[2].outputs.count, 1)
Expand Down