diff --git a/Sources/PackageLoading/PackageBuilder.swift b/Sources/PackageLoading/PackageBuilder.swift index 1c00be9eac5..0d486cd4ea3 100644 --- a/Sources/PackageLoading/PackageBuilder.swift +++ b/Sources/PackageLoading/PackageBuilder.swift @@ -678,19 +678,12 @@ public final class PackageBuilder { throw ModuleError.invalidPublicHeadersDirectory(potentialModule.name) } - // Exclude public headers path directory from source searching if it's not - // the target root. - // FIXME: This means we'll try to assign rules to header files - // which is currently not handled by the sources builder. - let extraExcludes = publicHeadersPath != potentialModule.path ? [publicHeadersPath] : [] - let sourcesBuilder = TargetSourcesBuilder( packageName: manifest.name, packagePath: packagePath, target: manifestTarget, path: potentialModule.path, additionalFileRules: additionalFileRules, - extraExcludes: extraExcludes, toolsVersion: manifest.toolsVersion, fs: fileSystem, diags: diagnostics diff --git a/Sources/PackageLoading/TargetSourcesBuilder.swift b/Sources/PackageLoading/TargetSourcesBuilder.swift index 622af244281..2aceefe4e08 100644 --- a/Sources/PackageLoading/TargetSourcesBuilder.swift +++ b/Sources/PackageLoading/TargetSourcesBuilder.swift @@ -30,6 +30,9 @@ public struct TargetSourcesBuilder { /// The path of the target. public let targetPath: AbsolutePath + /// The list of declared sources in the package manifest. + public let declaredSources: [AbsolutePath]? + /// The rules that can be applied to files in the target. public let rules: [FileRuleDescription] @@ -49,7 +52,6 @@ public struct TargetSourcesBuilder { target: TargetDescription, path: AbsolutePath, additionalFileRules: [FileRuleDescription] = [], - extraExcludes: [AbsolutePath] = [], toolsVersion: ToolsVersion = .currentToolsVersion, fs: FileSystem = localFileSystem, diags: DiagnosticsEngine @@ -63,7 +65,19 @@ public struct TargetSourcesBuilder { self.toolsVersion = toolsVersion self.fs = fs let excludedPaths = target.exclude.map{ path.appending(RelativePath($0)) } - self.excludedPaths = Set(excludedPaths + extraExcludes) + self.excludedPaths = Set(excludedPaths) + + let declaredSources = target.sources?.map{ path.appending(RelativePath($0)) } + if let declaredSources = declaredSources { + // Diagnose duplicate entries. + let duplicates = declaredSources.spm_findDuplicateElements() + if !duplicates.isEmpty { + for duplicate in duplicates { + diags.emit(warning: "found duplicate sources declaration in the package manifest: \(duplicate.map{ $0.pathString }[0])") + } + } + } + self.declaredSources = declaredSources?.spm_uniqueElements() #if DEBUG validateRules(self.rules) @@ -92,8 +106,10 @@ public struct TargetSourcesBuilder { pathToRule[path] = findRule(for: path) } - // Emit an error if we found files without a matching rule in tools version >= 5.2 - if toolsVersion >= .v5_2 { + // Emit an error if we found files without a matching rule in + // tools version >= vNext. This will be activated once resources + // support is complete. + if toolsVersion >= .vNext { let filesWithNoRules = pathToRule.filter{ $0.value == .none } if !filesWithNoRules.isEmpty { var error = "found \(filesWithNoRules.count) file(s) which are unhandled; explicitly declare them as resources or exclude from the target\n" @@ -109,7 +125,7 @@ public struct TargetSourcesBuilder { let resources: [Resource] = pathToRule.compactMap { switch $0.value { - case .compile, .none, .modulemap: + case .compile, .none, .modulemap, .header: return nil case .processResource: return Resource(rule: .process, path: $0.key) @@ -157,14 +173,20 @@ public struct TargetSourcesBuilder { } // Match any sources explicitly declared in the manifest file. - if let declaredSources = target.sources { - for declaredSource in declaredSources { - let sourcePath = self.targetPath.appending(RelativePath(declaredSource)) + if let declaredSources = self.declaredSources { + for sourcePath in declaredSources { if path.contains(sourcePath) { if matchedRule != .none { diags.emit(.error("Duplicate rule compile found for \(path)")) } - matchedRule = .compile + + // Check for header files as they're allowed to be mixed with sources. + if let ext = path.extension, + FileRuleDescription.header.fileTypes.contains(ext) { + matchedRule = .header + } else { + matchedRule = .compile + } } } } @@ -294,6 +316,9 @@ public struct FileRuleDescription { /// The modulemap rule. case modulemap + /// A header file. + case header + /// Sentinal to indicate that no rule was chosen for a given file. case none } @@ -363,12 +388,22 @@ public struct FileRuleDescription { ) }() + /// The rule for detecting header files. + public static var header: FileRuleDescription = { + .init( + rule: .header, + toolsVersion: .minimumRequired, + fileTypes: ["h"] + ) + }() + /// List of all the builtin rules. public static let builtinRules: [FileRuleDescription] = [ swift, clang, asm, modulemap, + header, ] } diff --git a/TSC/Sources/TSCBasic/CollectionExtensions.swift b/TSC/Sources/TSCBasic/CollectionExtensions.swift index ec015044a5a..b0ba5df6dc0 100644 --- a/TSC/Sources/TSCBasic/CollectionExtensions.swift +++ b/TSC/Sources/TSCBasic/CollectionExtensions.swift @@ -23,3 +23,20 @@ extension Collection { } } } + +extension Collection where Element: Hashable { + /// Returns a new list of element removing duplicate elements. + /// + /// Note: The order of elements is preseved. + /// Complexity: O(n) + public func spm_uniqueElements() -> [Element] { + var set = Set() + var result = [Element]() + for element in self { + if set.insert(element).inserted { + result.append(element) + } + } + return result + } +} diff --git a/TSC/Tests/TSCBasicTests/CollectionExtensionsTests.swift b/TSC/Tests/TSCBasicTests/CollectionExtensionsTests.swift index 217f32056c1..bf84b6fdb9f 100644 --- a/TSC/Tests/TSCBasicTests/CollectionExtensionsTests.swift +++ b/TSC/Tests/TSCBasicTests/CollectionExtensionsTests.swift @@ -17,4 +17,10 @@ class CollectionExtensionsTests: XCTestCase { XCTAssertEqual([42].spm_only, 42) XCTAssertEqual([42, 24].spm_only, nil) } + + func testUniqueElements() { + XCTAssertEqual([1, 2, 2, 4, 2, 1, 1, 4].spm_uniqueElements(), [1, 2, 4]) + XCTAssertEqual([1, 2, 2, 4, 2, 1, 1, 4, 9].spm_uniqueElements(), [1, 2, 4, 9]) + XCTAssertEqual([3, 2, 1].spm_uniqueElements(), [3, 2, 1]) + } } diff --git a/Tests/PackageLoadingTests/PackageBuilderTests.swift b/Tests/PackageLoadingTests/PackageBuilderTests.swift index 92e1a90bd29..718b8ab176b 100644 --- a/Tests/PackageLoadingTests/PackageBuilderTests.swift +++ b/Tests/PackageLoadingTests/PackageBuilderTests.swift @@ -215,6 +215,37 @@ class PackageBuilderTests: XCTestCase { } } + func testPublicIncludeDirMixedWithSources() throws { + let fs = InMemoryFileSystem(emptyFiles: + "/Sources/clib/nested/nested.h", + "/Sources/clib/nested/nested.c", + "/Sources/clib/clib.h", + "/Sources/clib/clib.c", + "/Sources/clib/clib2.h", + "/Sources/clib/clib2.c", + "/done" + ) + + let manifest = Manifest.createV4Manifest( + name: "MyPackage", + targets: [ + TargetDescription( + name: "clib", + path: "Sources", + sources: ["clib", "clib"], + publicHeadersPath: "." + ), + ] + ) + PackageBuilderTester(manifest, in: fs) { package, diags in + diags.check(diagnostic: "found duplicate sources declaration in the package manifest: /Sources/clib", behavior: .warning) + package.checkModule("clib") { module in + module.check(c99name: "clib", type: .library) + module.checkSources(root: "/Sources", paths: "clib/clib.c", "clib/clib2.c", "clib/nested/nested.c") + } + } + } + func testDeclaredExecutableProducts() { // Check that declaring executable product doesn't collide with the // inferred products. diff --git a/Tests/PackageLoadingTests/XCTestManifests.swift b/Tests/PackageLoadingTests/XCTestManifests.swift index d327bcbba10..897f2192959 100644 --- a/Tests/PackageLoadingTests/XCTestManifests.swift +++ b/Tests/PackageLoadingTests/XCTestManifests.swift @@ -50,6 +50,7 @@ extension PackageBuilderTests { ("testPlatforms", testPlatforms), ("testPredefinedTargetSearchError", testPredefinedTargetSearchError), ("testPublicHeadersPath", testPublicHeadersPath), + ("testPublicIncludeDirMixedWithSources", testPublicIncludeDirMixedWithSources), ("testResolvesSystemModulePackage", testResolvesSystemModulePackage), ("testSpecialTargetDir", testSpecialTargetDir), ("testSpecifiedCustomPathDoesNotExist", testSpecifiedCustomPathDoesNotExist),