Skip to content

Conversation

@nkcsgexi
Copy link
Contributor

When using swift-driver as a library, we cannot assume compiler is adjacent to the
build system executable.

When using swift-driver as a library, we cannot assume compiler is adjacent to the
build system executable.
@nkcsgexi
Copy link
Contributor Author

@swift-ci please test

@artemcm
Copy link
Contributor

artemcm commented Feb 12, 2021

@nkcsgexi what are your thoughts on using this versus passing in a path to a toolchain "root"? We need many different things from the toolchain beyond a compiler executable so I wonder if it's better to find them relative to a toolchain itself, vs. relative to a compiler executable. Either way, we are beholden to a toolchain's directory structure being fixed.

fileSystem: FileSystem = localFileSystem,
executor: DriverExecutor,
integratedDriver: Bool = true,
compilerExecutableDir: AbsolutePath? = nil,
Copy link
Contributor

@cltnschlosser cltnschlosser Feb 12, 2021

Choose a reason for hiding this comment

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

It's not as explicitly typed, but you can already do this by changing the env that you provide the driver.
Using SWIFT_DRIVER_<TOOLNAME>_EXEC environment variables. It also happens sooner.

if let overrideString = envVar(forExecutable: executable) {
      return try AbsolutePath(validating: overrideString)
    } else if let toolDir = toolDirectory,
              let path = lookupExecutablePath(filename: executable, searchPaths: [toolDir]) {
      // Looking for tools from the tools directory.
      return path
    } else if let path = lookupExecutablePath(filename: executable, searchPaths: [executableDir]) {
      return path
    } else if let path = try? xcrunFind(executable: executable) {
      return path
    } else if !["swift-frontend", "swift"].contains(executable),
              let parentDirectory = try? getToolPath(.swiftCompiler).parentDirectory,
              parentDirectory != executableDir,
              let path = lookupExecutablePath(filename: executable, searchPaths: [parentDirectory]) {
      // If the driver library's client and the frontend are in different directories,
      // try looking for tools next to the frontend.
      return path
    } else if let path = lookupExecutablePath(filename: executable, searchPaths: searchPaths) {
      return path
    } else if executable == "swift-frontend" {
      // Temporary shim: fall back to looking for "swift" before failing.
      return try lookup(executable: "swift")
    } else if fallbackToExecutableDefaultPath {
      return AbsolutePath("/usr/bin/" + executable)
    } else {
      throw ToolchainError.unableToFind(tool: executable)
    }

Also using the -tools-directory option is of higher precedence as well.

You're overriding the executableDir, which is the 3rd if statement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both SWIFT_DRIVER_<TOOLNAME>_EXEC and -tools-directory are global settings that the build system will honor no matter which toolchain is currently under use. This parameter compilerExecutableDir is mostly for the build system to create different driver instances for different toolchains.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right you pass the args and environment to the driver as you create it. So you can modify them there. Or just append, the argument to the existing logic. Users of libSwiftDriver are probably already creating argument lists.

It seems weird that the Driver creator needs another way to override this path.

@nkcsgexi
Copy link
Contributor Author

@artemcm all tools are currently found with an anchor of the executable dir where swift-driver is installed, e.g. the scanner library. I think this still makes sense and more versatile in situations that we don't have a fully-defined toolchain (in build directories for example).

@nkcsgexi nkcsgexi merged commit 83c3282 into swiftlang:main Feb 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants