Skip to content

[Issue #5923] Fix resource-related build failure for package containing Objective-C and C sources #5924

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 7 commits into from
Dec 5, 2022
Merged
Show file tree
Hide file tree
Changes from 6 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
7 changes: 7 additions & 0 deletions Fixtures/Resources/Simple/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,12 @@ let package = Package(
.copy("foo.txt"),
]
),

.target(
name: "MixedClangResource",
resources: [
.copy("foo.txt"),
]
),
]
)
6 changes: 6 additions & 0 deletions Fixtures/Resources/Simple/Sources/MixedClangResource/Foo.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#import <Foundation/Foundation.h>

#import "Foo.h"

@implementation Foo
@end
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
#include "bar.h"
6 changes: 6 additions & 0 deletions Fixtures/Resources/Simple/Sources/MixedClangResource/bar.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#ifndef foo_h
#define foo_h

#include <stdio.h>

#endif /* foo_h */
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#import <Foundation/Foundation.h>

@interface Foo : NSObject
@end
10 changes: 8 additions & 2 deletions Sources/Build/BuildPlan.swift
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,10 @@ public final class ClangTargetBuildDescription {

/// Builds up basic compilation arguments for a source file in this target; these arguments may be different for C++ vs non-C++.
/// NOTE: The parameter to specify whether to get C++ semantics is currently optional, but this is only for revlock avoidance with clients. Callers should always specify what they want based either the user's indication or on a default value (possibly based on the filename suffix).
public func basicArguments(isCXX isCXXOverride: Bool? = .none) throws -> [String] {
public func basicArguments(
isCXX isCXXOverride: Bool? = .none,
isC: Bool = false
) throws -> [String] {
// For now fall back on the hold semantics if the C++ nature isn't specified. This is temporary until clients have been updated.
let isCXX = isCXXOverride ?? clangTarget.isCXX

Expand Down Expand Up @@ -419,7 +422,10 @@ public final class ClangTargetBuildDescription {
// Add arguments from declared build settings.
args += try self.buildSettingsFlags()

if let resourceAccessorHeaderFile = self.resourceAccessorHeaderFile {
// Include the path to the resource header unless the arguments are
// being evaluated for a C file. A C file cannot depend on the resource
// accessor header due to it exporting a Foundation type (`NSBundle`).
if let resourceAccessorHeaderFile = self.resourceAccessorHeaderFile, !isC {
args += ["-include", resourceAccessorHeaderFile.pathString]
}

Expand Down
4 changes: 3 additions & 1 deletion Sources/Build/LLBuildManifestBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,9 @@ extension LLBuildManifestBuilder {

for path in try target.compilePaths() {
let isCXX = path.source.extension.map{ SupportedLanguageExtension.cppExtensions.contains($0) } ?? false
var args = try target.basicArguments(isCXX: isCXX)
let isC = path.source.extension.map { $0 == SupportedLanguageExtension.c.rawValue } ?? false

var args = try target.basicArguments(isCXX: isCXX, isC: isC)

args += ["-MD", "-MT", "dependencies", "-MF", path.deps.pathString]

Expand Down
75 changes: 75 additions & 0 deletions Tests/BuildTests/BuildPlanTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3952,6 +3952,81 @@ final class BuildPlanTests: XCTestCase {
])
}

func testClangBundleAccessor() throws {
let fs = InMemoryFileSystem(emptyFiles:
"/Pkg/Sources/Foo/include/Foo.h",
"/Pkg/Sources/Foo/Foo.m",
"/Pkg/Sources/Foo/bar.h",
"/Pkg/Sources/Foo/bar.c",
"/Pkg/Sources/Foo/resource.txt"
)

let observability = ObservabilitySystem.makeForTesting()

let graph = try loadPackageGraph(
fileSystem: fs,
manifests: [
Manifest.createRootManifest(
name: "Pkg",
path: .init(path: "/Pkg"),
toolsVersion: .current,
targets: [
TargetDescription(
name: "Foo",
resources: [
.init(
rule: .process(localization: .none),
path: "resource.txt"
)
]
)
]
)
],
observabilityScope: observability.topScope
)

XCTAssertNoDiagnostics(observability.diagnostics)

let plan = try BuildPlan(
buildParameters: mockBuildParameters(),
graph: graph,
fileSystem: fs,
observabilityScope: observability.topScope
)
let result = try BuildPlanResult(plan: plan)

let buildPath: AbsolutePath = result.plan.buildParameters.dataPath.appending(component: "debug")

let fooTarget = try result.target(for: "Foo").clangTarget()
XCTAssertEqual(try fooTarget.objects.map(\.pathString).sorted(), [
buildPath.appending(components: "Foo.build", "Foo.m.o").pathString,
buildPath.appending(components: "Foo.build", "bar.c.o").pathString,
buildPath.appending(components: "Foo.build", "resource_bundle_accessor.m.o").pathString
].sorted())

let resourceAccessorDirectory = buildPath.appending(components:
"Foo.build",
"DerivedSources"
)

let resourceAccessorHeader = resourceAccessorDirectory
.appending(component: "resource_bundle_accessor.h")
let headerContents: String = try fs.readFileContents(resourceAccessorHeader)
XCTAssertMatch(
headerContents,
.contains("#define SWIFTPM_MODULE_BUNDLE Foo_SWIFTPM_MODULE_BUNDLE()")
)

let resourceAccessorImpl = resourceAccessorDirectory
.appending(component: "resource_bundle_accessor.m")
let implContents: String = try fs.readFileContents(resourceAccessorImpl)
XCTAssertMatch(
implContents,
.contains("NSBundle* Foo_SWIFTPM_MODULE_BUNDLE() {")
)
}

func testShouldLinkStaticSwiftStdlib() throws {
let fs = InMemoryFileSystem(emptyFiles:
"/Pkg/Sources/exe/main.swift",
Expand Down
13 changes: 13 additions & 0 deletions Tests/CommandsTests/TestToolTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,19 @@ final class TestToolTests: CommandsTestCase {
}
}

// TODO: This test should be moved into `ResourceTests.swift` in the
// `FunctionalTests` scheme when the `FunctionalTests` scheme is re-enabled.
func testResourcesInMixedClangPackage() throws {
#if !os(macOS)
// Running swift-test fixtures on linux is not yet possible.
try XCTSkipIf(true, "test is only supported on macOS")
#endif

try fixture(name: "Resources/Simple") { fixturePath in
XCTAssertBuilds(fixturePath, extraArgs: ["--target", "MixedClangResource"])
}
}

func testList() throws {
try fixture(name: "Miscellaneous/TestDiscovery/Simple") { fixturePath in
let (stdout, stderr) = try SwiftPMProduct.SwiftTest.execute(["list"], packagePath: fixturePath)
Expand Down