Skip to content

[xcodegen] A couple of minor tweaks #78401

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.

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 3 commits into from
Jan 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ struct SwiftTargets {

func getTargets(below path: RelativePath) -> [SwiftTarget] {
targets.filter { target in
guard let parent = target.buildRule?.parentPath, parent.hasPrefix(path)
guard let parent = target.buildRule?.parentPath, parent.starts(with: path)
else {
return false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@
struct ClangTarget {
var name: String
var parentPath: RelativePath
var sources: [Source]
var unbuildableSources: [Source]
var sources: [RelativePath]
var unbuildableSources: [RelativePath]
var headers: [RelativePath]

init(
name: String, parentPath: RelativePath,
sources: [Source], unbuildableSources: [Source] = [],
sources: [RelativePath], unbuildableSources: [RelativePath] = [],
headers: [RelativePath]
) {
self.name = name
Expand All @@ -30,13 +30,6 @@ struct ClangTarget {
}
}

extension ClangTarget {
struct Source {
var path: RelativePath
var inferArgs: Bool
}
}

extension RepoBuildDir {
func getCSourceFilePaths(for path: RelativePath) throws -> [RelativePath] {
try getAllRepoSubpaths(of: path).filter(\.isCSourceLike)
Expand All @@ -58,24 +51,15 @@ extension RepoBuildDir {
return nil
}

var sources: [ClangTarget.Source] = []
var unbuildableSources: [ClangTarget.Source] = []
var sources: [RelativePath] = []
var unbuildableSources: [RelativePath] = []
for path in sourcePaths {
let source: ClangTarget.Source? =
if try clangArgs.hasBuildArgs(for: path) {
.init(path: path, inferArgs: false)
} else if target.inferArgs {
.init(path: path, inferArgs: true)
} else {
nil
}
guard let source else { continue }

// If we're inferring arguments, or have a known unbuildable, treat as not
// If we have no arguments, or have a known unbuildable, treat as not
// buildable. We'll still include it in the project, but in a separate
// target that isn't built by default.
if source.inferArgs || knownUnbuildables.contains(path) {
unbuildableSources.append(source)
if try !clangArgs.hasBuildArgs(for: path) ||
knownUnbuildables.contains(path) {
unbuildableSources.append(path)
continue
}
// If we have no '.o' present for a given file, assume it's not buildable.
Expand All @@ -84,10 +68,10 @@ extension RepoBuildDir {
if target.mayHaveUnbuildableFiles,
try !clangArgs.isObjectFilePresent(for: path) {
log.debug("! Treating '\(path)' as unbuildable; no '.o' file")
unbuildableSources.append(source)
unbuildableSources.append(path)
continue
}
sources.append(source)
sources.append(path)
}

return ClangTarget(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,13 @@ struct ClangTargetSource {
var name: String
var path: RelativePath
var mayHaveUnbuildableFiles: Bool
var inferArgs: Bool

init(
at path: RelativePath, named name: String,
mayHaveUnbuildableFiles: Bool,
inferArgs: Bool
mayHaveUnbuildableFiles: Bool
) {
self.name = name
self.path = path
self.mayHaveUnbuildableFiles = mayHaveUnbuildableFiles
self.inferArgs = inferArgs
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ fileprivate final class ProjectGenerator {
private var groups: [RelativePath: CachedGroup] = [:]
private var files: [RelativePath: Xcode.FileReference] = [:]
private var targets: [String: Xcode.Target] = [:]
private var unbuildableSources: [ClangTarget.Source] = []
private var unbuildableSources: [RelativePath] = []
private var runnableBuildTargets: [RunnableTarget: Xcode.Target] = [:]

/// The group in which external files are stored.
Expand Down Expand Up @@ -154,8 +154,9 @@ fileprivate final class ProjectGenerator {
guard let path else { return true }

// Not very efficient, but excludedPaths should be small in practice.
guard let excluded = spec.excludedPaths.first(where: { path.hasPrefix($0.path) })
else {
guard let excluded = spec.excludedPaths.first(
where: { path.starts(with: $0.path) }
) else {
return true
}
if let description, let reason = excluded.reason {
Expand Down Expand Up @@ -213,10 +214,18 @@ fileprivate final class ProjectGenerator {
_ name: String, at parentPath: RelativePath?, canUseBuildableFolder: Bool,
productType: Xcode.Target.ProductType?, includeInAllTarget: Bool
) -> Xcode.Target? {
guard targets[name] == nil else {
log.warning("Duplicate target '\(name)', skipping")
return nil
}
let name = {
// If we have a same-named target, disambiguate.
if targets[name] == nil {
return name
}
var i = 2
var newName: String { "\(name)\(i)" }
while targets[newName] != nil {
i += 1
}
return newName
}()
var buildableFolder: Xcode.FileReference?
if let parentPath, !parentPath.components.isEmpty {
// If we've been asked to use buildable folders, see if we can create
Expand All @@ -230,7 +239,9 @@ fileprivate final class ProjectGenerator {
group(for: repoRelativePath.appending(parentPath)) != nil else {
// If this isn't a child of an explicitly added reference, something
// has probably gone wrong.
if !spec.referencesToAdd.contains(where: { parentPath.hasPrefix($0.path) }) {
if !spec.referencesToAdd.contains(
where: { parentPath.starts(with: $0.path) }
) {
log.warning("""
Target '\(name)' at '\(repoRelativePath.appending(parentPath))' is \
nested in a folder reference; skipping. This is likely an xcodegen bug.
Expand Down Expand Up @@ -310,12 +321,11 @@ fileprivate final class ProjectGenerator {
return false
}
let parent = clangTarget.parentPath
let sources = clangTarget.sources.map(\.path)
let hasConsistentArgs = try sources.allSatisfy {
let hasConsistentArgs = try clangTarget.sources.allSatisfy {
try !buildDir.clangArgs.hasUniqueArgs(for: $0, parent: parent)
}
guard hasConsistentArgs else { return false }
return try canUseBuildableFolder(at: parent, sources: sources)
return try canUseBuildableFolder(at: parent, sources: clangTarget.sources)
}

func canUseBuildableFolder(
Expand Down Expand Up @@ -383,15 +393,14 @@ fileprivate final class ProjectGenerator {
let sourcesToBuild = target.addSourcesBuildPhase()

for source in targetInfo.sources {
let sourcePath = source.path
guard let sourceRef = getOrCreateRepoRef(.file(sourcePath)) else {
guard let sourceRef = getOrCreateRepoRef(.file(source)) else {
continue
}
let buildFile = sourcesToBuild.addBuildFile(fileRef: sourceRef)

// Add any per-file settings.
var fileArgs = try buildDir.clangArgs.getUniqueArgs(
for: sourcePath, parent: targetPath, infer: source.inferArgs
for: source, parent: targetPath, infer: spec.inferArgs
)
if !fileArgs.isEmpty {
applyBaseSubstitutions(to: &fileArgs)
Expand Down Expand Up @@ -747,12 +756,7 @@ fileprivate final class ProjectGenerator {
let target = try buildDir.getClangTarget(
for: targetSource, knownUnbuildables: spec.knownUnbuildables
)
guard var target else { continue }
// We may have a Swift target with the same name, disambiguate.
// FIXME: We ought to be able to support mixed-source targets.
if targets[target.name] != nil {
target.name = "\(target.name)-clang"
}
guard let target else { continue }
try generateClangTarget(target)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,7 @@ extension ProjectSpec {
guard let path = mapPath(path, for: "Clang target") else { return }
let name = name ?? path.fileName
clangTargetSources.append(ClangTargetSource(
at: path, named: name,
mayHaveUnbuildableFiles: mayHaveUnbuildableFiles,
inferArgs: inferArgs
at: path, named: name, mayHaveUnbuildableFiles: mayHaveUnbuildableFiles
))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ public extension PathProtocol {
return exts.contains(where: { ext == $0.rawValue })
}

func hasPrefix(_ other: Self) -> Bool {
rawPath.hasPrefix(other.rawPath)
func starts(with other: Self) -> Bool {
self.storage.starts(with: other.storage)
}

var components: FilePath.ComponentView {
Expand Down