Skip to content

Commit

Permalink
[PackageLoading] Load only the declared executable product
Browse files Browse the repository at this point in the history
Don't infer an executable target when there is a declared product with
same name.

-- https://bugs.swift.org/browse/SR-3562
  • Loading branch information
ankitspd committed Mar 23, 2017
1 parent 2d23788 commit 63bcbd6
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 9 deletions.
16 changes: 11 additions & 5 deletions Sources/PackageLoading/PackageBuilder.swift
Expand Up @@ -658,9 +658,17 @@ public struct PackageBuilder {
products.append(product)
}

// Create executables.
func createExecutables() {
// Auto creates executable products from executables targets if there
// isn't already a product with same name.
func createExecutables(declaredProducts: Set<String> = []) {
for module in modules where module.type == .executable {
// If this target already has a product, skip generating a
// product for it.
if declaredProducts.contains(module.name) {
// FIXME: We should probably check and warn in case this is
// not an executable product.
continue
}
let product = Product(name: module.name, type: .executable, modules: [module])
products.append(product)
}
Expand All @@ -681,16 +689,14 @@ public struct PackageBuilder {
}

case .v4(let package):

// Only create implicit executables for root packages in v4.
if isRootPackage {
createExecutables()
createExecutables(declaredProducts: Set(package.products.map{$0.name}))
}

for product in package.products {
switch product {
case let p as PackageDescription4.Product.Executable:
// FIXME: We should handle/diagnose name collisions between local and vended executables (SR-3562).
let modules = try modulesFrom(targetNames: p.targets, product: p.name)
products.append(Product(name: p.name, type: .executable, modules: modules))
case let p as PackageDescription4.Product.Library:
Expand Down
40 changes: 36 additions & 4 deletions Tests/PackageLoadingTests/ConventionTests.swift
Expand Up @@ -229,6 +229,35 @@ class ConventionTests: XCTestCase {
}
}

func testDeclaredExecutableProducts() {
// Check that declaring executable product doesn't collide with the
// inferred products.
let fs = InMemoryFileSystem(emptyFiles:
"/Sources/exec/main.swift",
"/Sources/foo/foo.swift"
)
let package = PackageDescription4.Package(
name: "pkg",
products: [
.executable(name: "exec", targets: ["exec", "foo"]),
]
)
PackageBuilderTester(package, in: fs) { result in
result.checkModule("foo") { _ in }
result.checkModule("exec") { _ in }
result.checkProduct("exec") { productResult in
productResult.check(type: .executable, modules: ["exec", "foo"])
}
}
PackageBuilderTester("pkg", in: fs) { result in
result.checkModule("foo") { _ in }
result.checkModule("exec") { _ in }
result.checkProduct("exec") { productResult in
productResult.check(type: .executable, modules: ["exec"])
}
}
}

func testDotSwiftSuffixDirectory() throws {
var fs = InMemoryFileSystem(emptyFiles:
"/hello.swift/dummy",
Expand Down Expand Up @@ -1024,6 +1053,7 @@ class ConventionTests: XCTestCase {
static var allTests = [
("testCInTests", testCInTests),
("testCompatibleSwiftVersions", testCompatibleSwiftVersions),
("testDeclaredExecutableProducts", testDeclaredExecutableProducts),
("testDotFilesAreIgnored", testDotFilesAreIgnored),
("testDotSwiftSuffixDirectory", testDotSwiftSuffixDirectory),
("testMixedSources", testMixedSources),
Expand Down Expand Up @@ -1071,8 +1101,9 @@ class ConventionTests: XCTestCase {
/// - Throws: ModuleError, ProductError
private func loadPackage(_ package: Manifest.RawPackage, path: AbsolutePath, in fs: FileSystem, warningStream: OutputByteStream) throws -> PackageModel.Package {
let manifest = Manifest(path: path.appending(component: Manifest.filename), url: "", package: package, version: nil)
// FIXME: We should allow customizing root package boolean.
let builder = PackageBuilder(
manifest: manifest, path: path, fileSystem: fs, warningStream: warningStream, isRootPackage: false)
manifest: manifest, path: path, fileSystem: fs, warningStream: warningStream, isRootPackage: true)
return try builder.construct()
}

Expand Down Expand Up @@ -1180,10 +1211,11 @@ final class PackageBuilderTester {
guard case .package(let package) = result else {
return XCTFail("Expected package did not load \(self)", file: file, line: line)
}
guard let product = package.products.first(where: {$0.name == name}) else {
return XCTFail("Product: \(name) not found", file: file, line: line)
let foundProducts = package.products.filter{$0.name == name}
guard foundProducts.count == 1 else {
return XCTFail("Couldn't get the product: \(name). Found products \(foundProducts)", file: file, line: line)
}
body?(ProductResult(product))
body?(ProductResult(foundProducts[0]))
}

final class ProductResult {
Expand Down

0 comments on commit 63bcbd6

Please sign in to comment.