Skip to content

Commit bd6487b

Browse files
authored
[Issue #5923] Fix resource-related build failure for package containing Objective-C and C sources (#5924)
* Fix resource issue with mixed Clang sources
1 parent a89e99b commit bd6487b

File tree

10 files changed

+121
-3
lines changed

10 files changed

+121
-3
lines changed

Fixtures/Resources/Simple/Package.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,5 +36,12 @@ let package = Package(
3636
.copy("foo.txt"),
3737
]
3838
),
39+
40+
.target(
41+
name: "MixedClangResource",
42+
resources: [
43+
.copy("foo.txt"),
44+
]
45+
),
3946
]
4047
)
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#import <Foundation/Foundation.h>
2+
3+
#import "Foo.h"
4+
5+
@implementation Foo
6+
@end
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
#include "bar.h"
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#ifndef foo_h
2+
#define foo_h
3+
4+
#include <stdio.h>
5+
6+
#endif /* foo_h */

Fixtures/Resources/Simple/Sources/MixedClangResource/foo.txt

Whitespace-only changes.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#import <Foundation/Foundation.h>
2+
3+
@interface Foo : NSObject
4+
@end

Sources/Build/BuildPlan.swift

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,10 @@ public final class ClangTargetBuildDescription {
356356

357357
/// Builds up basic compilation arguments for a source file in this target; these arguments may be different for C++ vs non-C++.
358358
/// 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).
359-
public func basicArguments(isCXX isCXXOverride: Bool? = .none) throws -> [String] {
359+
public func basicArguments(
360+
isCXX isCXXOverride: Bool? = .none,
361+
isC: Bool = false
362+
) throws -> [String] {
360363
// For now fall back on the hold semantics if the C++ nature isn't specified. This is temporary until clients have been updated.
361364
let isCXX = isCXXOverride ?? clangTarget.isCXX
362365

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

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

Sources/Build/LLBuildManifestBuilder.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -769,7 +769,9 @@ extension LLBuildManifestBuilder {
769769

770770
for path in try target.compilePaths() {
771771
let isCXX = path.source.extension.map{ SupportedLanguageExtension.cppExtensions.contains($0) } ?? false
772-
var args = try target.basicArguments(isCXX: isCXX)
772+
let isC = path.source.extension.map { $0 == SupportedLanguageExtension.c.rawValue } ?? false
773+
774+
var args = try target.basicArguments(isCXX: isCXX, isC: isC)
773775

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

Tests/BuildTests/BuildPlanTests.swift

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3954,6 +3954,81 @@ final class BuildPlanTests: XCTestCase {
39543954
])
39553955
}
39563956

3957+
func testClangBundleAccessor() throws {
3958+
let fs = InMemoryFileSystem(emptyFiles:
3959+
"/Pkg/Sources/Foo/include/Foo.h",
3960+
"/Pkg/Sources/Foo/Foo.m",
3961+
"/Pkg/Sources/Foo/bar.h",
3962+
"/Pkg/Sources/Foo/bar.c",
3963+
"/Pkg/Sources/Foo/resource.txt"
3964+
)
3965+
3966+
let observability = ObservabilitySystem.makeForTesting()
3967+
3968+
let graph = try loadPackageGraph(
3969+
fileSystem: fs,
3970+
manifests: [
3971+
Manifest.createRootManifest(
3972+
name: "Pkg",
3973+
path: .init(path: "/Pkg"),
3974+
toolsVersion: .current,
3975+
targets: [
3976+
TargetDescription(
3977+
name: "Foo",
3978+
resources: [
3979+
.init(
3980+
rule: .process(localization: .none),
3981+
path: "resource.txt"
3982+
)
3983+
]
3984+
)
3985+
]
3986+
)
3987+
],
3988+
observabilityScope: observability.topScope
3989+
)
3990+
3991+
XCTAssertNoDiagnostics(observability.diagnostics)
3992+
3993+
let plan = try BuildPlan(
3994+
buildParameters: mockBuildParameters(),
3995+
graph: graph,
3996+
fileSystem: fs,
3997+
observabilityScope: observability.topScope
3998+
)
3999+
let result = try BuildPlanResult(plan: plan)
4000+
4001+
let buildPath: AbsolutePath = result.plan.buildParameters.dataPath.appending(component: "debug")
4002+
4003+
let fooTarget = try result.target(for: "Foo").clangTarget()
4004+
XCTAssertEqual(try fooTarget.objects.map(\.pathString).sorted(), [
4005+
buildPath.appending(components: "Foo.build", "Foo.m.o").pathString,
4006+
buildPath.appending(components: "Foo.build", "bar.c.o").pathString,
4007+
buildPath.appending(components: "Foo.build", "resource_bundle_accessor.m.o").pathString
4008+
].sorted())
4009+
4010+
let resourceAccessorDirectory = buildPath.appending(components:
4011+
"Foo.build",
4012+
"DerivedSources"
4013+
)
4014+
4015+
let resourceAccessorHeader = resourceAccessorDirectory
4016+
.appending(component: "resource_bundle_accessor.h")
4017+
let headerContents: String = try fs.readFileContents(resourceAccessorHeader)
4018+
XCTAssertMatch(
4019+
headerContents,
4020+
.contains("#define SWIFTPM_MODULE_BUNDLE Foo_SWIFTPM_MODULE_BUNDLE()")
4021+
)
4022+
4023+
let resourceAccessorImpl = resourceAccessorDirectory
4024+
.appending(component: "resource_bundle_accessor.m")
4025+
let implContents: String = try fs.readFileContents(resourceAccessorImpl)
4026+
XCTAssertMatch(
4027+
implContents,
4028+
.contains("NSBundle* Foo_SWIFTPM_MODULE_BUNDLE() {")
4029+
)
4030+
}
4031+
39574032
func testShouldLinkStaticSwiftStdlib() throws {
39584033
let fs = InMemoryFileSystem(emptyFiles:
39594034
"/Pkg/Sources/exe/main.swift",

Tests/FunctionalTests/ResourcesTests.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,17 @@ class ResourcesTests: XCTestCase {
4949
}
5050
}
5151

52+
func testResourcesInMixedClangPackage() throws {
53+
#if !os(macOS)
54+
// Running swift-test fixtures on linux is not yet possible.
55+
try XCTSkipIf(true, "test is only supported on macOS")
56+
#endif
57+
58+
try fixture(name: "Resources/Simple") { fixturePath in
59+
XCTAssertBuilds(fixturePath, extraArgs: ["--target", "MixedClangResource"])
60+
}
61+
}
62+
5263
func testMovedBinaryResources() throws {
5364
try fixture(name: "Resources/Moved") { fixturePath in
5465
var executables = ["SwiftyResource"]

0 commit comments

Comments
 (0)