Skip to content

Commit a9c0c70

Browse files
rbenegalrbenegal
committed
Make addLocalRpaths an enum to support different configs
addLocalRpaths can now be set to one of: .never, .debugOnly or .always to allow for selectively adding rpaths based on build requirements.
1 parent 2e6ba6e commit a9c0c70

8 files changed

Lines changed: 39 additions & 80 deletions

‎Sources/SwiftBuildSupport/PIFBuilder.swift‎

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,9 @@ package struct PIFBuilderParameters {
8181
/// Add rpaths which allow loading libraries adjacent to the current image at runtime. This is desirable
8282
/// when launching build products from the build directory, but should often be disabled when deploying
8383
/// the build products to a different location.
84-
let addLocalRpaths: Bool
84+
let addLocalRpaths: PackagePIFBuilder.AddLocalRpaths
8585

86-
/// Allow for turning off local rpaths in the release builds. Defaults to `true` but can be set to
87-
/// to `false` to prevent ld warnings and issues with performance, security and library relocation.
88-
let addLocalRpathsInReleaseConfiguration: Bool
89-
90-
package init(isPackageAccessModifierSupported: Bool, enableTestability: Bool, shouldCreateDylibForDynamicProducts: Bool, materializeStaticArchiveProductsForRootPackages: Bool, createDynamicVariantsForLibraryProducts: Bool, toolchainLibDir: AbsolutePath, pkgConfigDirectories: [AbsolutePath], supportedSwiftVersions: [SwiftLanguageVersion], pluginScriptRunner: PluginScriptRunner, disableSandbox: Bool, pluginWorkingDirectory: AbsolutePath, additionalFileRules: [FileRuleDescription], addLocalRPaths: Bool, addLocalRpathsInReleaseConfiguration: Bool = true) {
86+
package init(isPackageAccessModifierSupported: Bool, enableTestability: Bool, shouldCreateDylibForDynamicProducts: Bool, materializeStaticArchiveProductsForRootPackages: Bool, createDynamicVariantsForLibraryProducts: Bool, toolchainLibDir: AbsolutePath, pkgConfigDirectories: [AbsolutePath], supportedSwiftVersions: [SwiftLanguageVersion], pluginScriptRunner: PluginScriptRunner, disableSandbox: Bool, pluginWorkingDirectory: AbsolutePath, additionalFileRules: [FileRuleDescription], addLocalRpaths: PackagePIFBuilder.AddLocalRpaths) {
9187
self.isPackageAccessModifierSupported = isPackageAccessModifierSupported
9288
self.enableTestability = enableTestability
9389
self.shouldCreateDylibForDynamicProducts = shouldCreateDylibForDynamicProducts
@@ -100,8 +96,7 @@ package struct PIFBuilderParameters {
10096
self.disableSandbox = disableSandbox
10197
self.pluginWorkingDirectory = pluginWorkingDirectory
10298
self.additionalFileRules = additionalFileRules
103-
self.addLocalRpaths = addLocalRPaths
104-
self.addLocalRpathsInReleaseConfiguration = addLocalRpathsInReleaseConfiguration
99+
self.addLocalRpaths = addLocalRpaths
105100
}
106101
}
107102

@@ -443,7 +438,6 @@ public final class PIFBuilder {
443438
materializeStaticArchiveProductsForRootPackages: self.parameters.materializeStaticArchiveProductsForRootPackages,
444439
createDynamicVariantsForLibraryProducts: self.parameters.createDynamicVariantsForLibraryProducts,
445440
addLocalRpaths: self.parameters.addLocalRpaths,
446-
addLocalRpathsInReleaseConfiguration: self.parameters.addLocalRpathsInReleaseConfiguration,
447441
packageDisplayVersion: package.manifest.displayName,
448442
pkgConfigDirectories: self.parameters.pkgConfigDirectories,
449443
fileSystem: self.fileSystem,
@@ -570,8 +564,7 @@ public final class PIFBuilder {
570564
pluginWorkingDirectory: AbsolutePath,
571565
pkgConfigDirectories: [Basics.AbsolutePath],
572566
additionalFileRules: [FileRuleDescription],
573-
addLocalRpaths: Bool,
574-
addLocalRpathsInReleaseConfiguration: Bool = true,
567+
addLocalRpaths: PackagePIFBuilder.AddLocalRpaths,
575568
materializeStaticArchiveProductsForRootPackages: Bool,
576569
createDynamicVariantsForLibraryProducts: Bool
577570
) async throws -> PIFGenerationResult {
@@ -583,7 +576,6 @@ public final class PIFBuilder {
583576
pluginWorkingDirectory: pluginWorkingDirectory,
584577
additionalFileRules: additionalFileRules,
585578
addLocalRpaths: addLocalRpaths,
586-
addLocalRpathsInReleaseConfiguration: addLocalRpathsInReleaseConfiguration,
587579
materializeStaticArchiveProductsForRootPackages: materializeStaticArchiveProductsForRootPackages,
588580
createDynamicVariantsForLibraryProducts: createDynamicVariantsForLibraryProducts
589581

@@ -854,8 +846,7 @@ extension PIFBuilderParameters {
854846
disableSandbox: Bool,
855847
pluginWorkingDirectory: AbsolutePath,
856848
additionalFileRules: [FileRuleDescription],
857-
addLocalRpaths: Bool,
858-
addLocalRpathsInReleaseConfiguration: Bool = true,
849+
addLocalRpaths: PackagePIFBuilder.AddLocalRpaths,
859850
materializeStaticArchiveProductsForRootPackages: Bool,
860851
createDynamicVariantsForLibraryProducts: Bool
861852
) {
@@ -872,8 +863,7 @@ extension PIFBuilderParameters {
872863
disableSandbox: disableSandbox,
873864
pluginWorkingDirectory: pluginWorkingDirectory,
874865
additionalFileRules: additionalFileRules,
875-
addLocalRPaths: addLocalRpaths,
876-
addLocalRpathsInReleaseConfiguration: addLocalRpathsInReleaseConfiguration,
866+
addLocalRpaths: addLocalRpaths,
877867
)
878868
}
879869
}

‎Sources/SwiftBuildSupport/PackagePIFBuilder.swift‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -187,15 +187,20 @@ public final class PackagePIFBuilder {
187187
/// Create dynamic library variants for automatic library products, for use by development-time features such as Previews and Swift Playgrounds.
188188
let createDynamicVariantsForLibraryProducts: Bool
189189

190+
/// Controls whether local rpaths are imparted to package consumers and in which configurations.
191+
public enum AddLocalRpaths: Sendable {
192+
/// Do not add local rpaths.
193+
case never
194+
/// Add rpaths only in the Debug configuration.
195+
case debugOnly
196+
/// Add rpaths in both Debug and Release configurations.
197+
case always
198+
}
199+
190200
/// Add rpaths which allow loading libraries adjacent to the current image at runtime. This is desirable
191201
/// when launching build products from the build directory, but should often be disabled when deploying
192-
/// the build products to a different location.
193-
let addLocalRpaths: Bool
194-
195-
/// Whether to also impart local rpaths in the Release configuration. Defaults to `true` for
196-
/// native-build-system parity. Hosts that may produce OS dylibs (`install_name` under
197-
/// `/System/Library` or `/usr/lib`) should set this to `false`; ld rejects rpaths there.
198-
let addLocalRpathsInReleaseConfiguration: Bool
202+
/// the build products to a different location or building the release configuration.
203+
let addLocalRpaths: AddLocalRpaths
199204

200205
/// Package display version, if any (i.e., it can be a version, branch or a git ref).
201206
let packageDisplayVersion: String?
@@ -228,8 +233,7 @@ public final class PackagePIFBuilder {
228233
createDylibForDynamicProducts: Bool = false,
229234
materializeStaticArchiveProductsForRootPackages: Bool = false,
230235
createDynamicVariantsForLibraryProducts: Bool = true,
231-
addLocalRpaths: Bool = true,
232-
addLocalRpathsInReleaseConfiguration: Bool = true,
236+
addLocalRpaths: AddLocalRpaths = .always,
233237
packageDisplayVersion: String?,
234238
pkgConfigDirectories: [AbsolutePath],
235239
fileSystem: FileSystem,
@@ -248,7 +252,6 @@ public final class PackagePIFBuilder {
248252
self.fileSystem = fileSystem
249253
self.observabilityScope = observabilityScope
250254
self.addLocalRpaths = addLocalRpaths
251-
self.addLocalRpathsInReleaseConfiguration = addLocalRpathsInReleaseConfiguration
252255
}
253256

254257
public init(
@@ -260,8 +263,7 @@ public final class PackagePIFBuilder {
260263
createDylibForDynamicProducts: Bool = false,
261264
materializeStaticArchiveProductsForRootPackages: Bool = false,
262265
createDynamicVariantsForLibraryProducts: Bool = true,
263-
addLocalRpaths: Bool = true,
264-
addLocalRpathsInReleaseConfiguration: Bool = true,
266+
addLocalRpaths: AddLocalRpaths = .always,
265267
packageDisplayVersion: String?,
266268
pkgConfigDirectories: [AbsolutePath],
267269
fileSystem: FileSystem,
@@ -276,7 +278,6 @@ public final class PackagePIFBuilder {
276278
self.materializeStaticArchiveProductsForRootPackages = materializeStaticArchiveProductsForRootPackages
277279
self.createDynamicVariantsForLibraryProducts = createDynamicVariantsForLibraryProducts
278280
self.addLocalRpaths = addLocalRpaths
279-
self.addLocalRpathsInReleaseConfiguration = addLocalRpathsInReleaseConfiguration
280281
self.packageDisplayVersion = packageDisplayVersion
281282
self.pkgConfigDirectories = pkgConfigDirectories
282283
self.fileSystem = fileSystem

‎Sources/SwiftBuildSupport/PackagePIFProjectBuilder+Modules.swift‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -826,15 +826,15 @@ extension PackagePIFProjectBuilder {
826826
// An imparted build setting on C will propagate back to both B and A.
827827
// FIXME: -rpath should not be given if -static is
828828
var rpaths: [String] = impartedSettings[.LD_RUNPATH_SEARCH_PATHS] ?? []
829-
if pifBuilder.addLocalRpaths {
829+
if pifBuilder.addLocalRpaths != .never {
830830
rpaths.append("$(RPATH_ORIGIN)")
831-
if pifBuilder.addLocalRpathsInReleaseConfiguration {
831+
if pifBuilder.addLocalRpaths == .always {
832832
impartedSettings[.LD_RUNPATH_SEARCH_PATHS] = rpaths + ["$(inherited)"]
833833
}
834834
}
835835

836836
var impartedDebugSettings = impartedSettings
837-
if pifBuilder.addLocalRpaths {
837+
if pifBuilder.addLocalRpaths != .never {
838838
// FIXME: Why is this rpath only added to the debug config? We should investigate reworking this.
839839
rpaths.append("$(BUILT_PRODUCTS_DIR)/PackageFrameworks")
840840
impartedDebugSettings[.LD_RUNPATH_SEARCH_PATHS] = rpaths + ["$(inherited)"]

‎Sources/SwiftBuildSupport/PackagePIFProjectBuilder+Products.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ extension PackagePIFProjectBuilder {
135135
settings[.BUILD_SERVER_PROTOCOL_TARGET_TAGS, default: ["$(inherited)"]].append("test")
136136

137137
// FIXME: we shouldn't always include both the deep and shallow bundle paths here, but for that we'll need rdar://31867023
138-
if pifBuilder.addLocalRpaths {
138+
if pifBuilder.addLocalRpaths != .never {
139139
settings[.LD_RUNPATH_SEARCH_PATHS] = [
140140
"$(RPATH_ORIGIN)/Frameworks",
141141
"$(RPATH_ORIGIN)/../Frameworks",
@@ -1102,7 +1102,7 @@ extension PackagePIFProjectBuilder {
11021102

11031103
// A test-runner should always be adjacent to the dynamic library containing the tests,
11041104
// so add the appropriate rpaths.
1105-
if pifBuilder.addLocalRpaths {
1105+
if pifBuilder.addLocalRpaths != .never {
11061106
settings[.LD_RUNPATH_SEARCH_PATHS] = [
11071107
"$(inherited)",
11081108
"$(RPATH_ORIGIN)"

‎Sources/SwiftBuildSupport/SwiftBuildSystem.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1300,7 +1300,7 @@ public final class SwiftBuildSystem: SPMBuildCore.BuildSystem {
13001300
disableSandbox: self.pluginConfiguration.disableSandbox,
13011301
pluginWorkingDirectory: self.pluginConfiguration.workDirectory,
13021302
additionalFileRules: additionalFileRules,
1303-
addLocalRpaths: !self.buildParameters.linkingParameters.shouldDisableLocalRpath,
1303+
addLocalRpaths: self.buildParameters.linkingParameters.shouldDisableLocalRpath ? .never : .always,
13041304
materializeStaticArchiveProductsForRootPackages: materializeStaticArchiveProductsForRootPackages,
13051305
createDynamicVariantsForLibraryProducts: false,
13061306
),

‎Tests/SwiftBuildSupportTests/CGenPIFTests.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ import SwiftBuild
193193
graph: graph,
194194
parameters: try PIFBuilderParameters.constructDefaultParametersForTesting(
195195
temporaryDirectory: AbsolutePath.root,
196-
addLocalRpaths: true,
196+
addLocalRpaths: .always,
197197
pluginScriptRunner: pluginScriptRunner
198198
),
199199
fileSystem: fs,

‎Tests/SwiftBuildSupportTests/PIFBuilderTests.swift‎

Lines changed: 10 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ import Workspace
2828
extension PIFBuilderParameters {
2929
static func constructDefaultParametersForTesting(
3030
temporaryDirectory: Basics.AbsolutePath,
31-
addLocalRpaths: Bool,
32-
addLocalRpathsInReleaseConfiguration: Bool = true,
31+
addLocalRpaths: PackagePIFBuilder.AddLocalRpaths,
3332
shouldCreateDylibForDynamicProducts: Bool = false,
3433
pluginScriptRunner: PluginScriptRunner? = nil
3534
) throws -> Self {
@@ -50,16 +49,14 @@ extension PIFBuilderParameters {
5049
disableSandbox: false,
5150
pluginWorkingDirectory: temporaryDirectory.appending(component: "plugin-working-dir"),
5251
additionalFileRules: [],
53-
addLocalRPaths: addLocalRpaths,
54-
addLocalRpathsInReleaseConfiguration: addLocalRpathsInReleaseConfiguration
52+
addLocalRpaths: addLocalRpaths
5553
)
5654
}
5755
}
5856

5957
fileprivate func withGeneratedPIF(
6058
fromFixture fixtureName: String,
61-
addLocalRpaths: Bool = true,
62-
addLocalRpathsInReleaseConfiguration: Bool = true,
59+
addLocalRpaths: PackagePIFBuilder.AddLocalRpaths = .always,
6360
shouldCreateDylibForDynamicProducts: Bool = true,
6461
buildParameters: BuildParameters? = nil,
6562
hostBuildParameters: BuildParameters? = nil,
@@ -97,7 +94,6 @@ fileprivate func withGeneratedPIF(
9794
parameters: try PIFBuilderParameters.constructDefaultParametersForTesting(
9895
temporaryDirectory: fixturePath,
9996
addLocalRpaths: addLocalRpaths,
100-
addLocalRpathsInReleaseConfiguration: addLocalRpathsInReleaseConfiguration,
10197
shouldCreateDylibForDynamicProducts: shouldCreateDylibForDynamicProducts
10298
),
10399
fileSystem: localFileSystem,
@@ -388,7 +384,7 @@ struct PIFBuilderTests {
388384
graph: graph,
389385
parameters: try PIFBuilderParameters.constructDefaultParametersForTesting(
390386
temporaryDirectory: AbsolutePath.root.appending("tmp"),
391-
addLocalRpaths: true,
387+
addLocalRpaths: .always,
392388
),
393389
fileSystem: fs,
394390
observabilityScope: observabilityScope.topScope,
@@ -717,7 +713,7 @@ struct PIFBuilderTests {
717713
}
718714
}
719715

720-
try await withGeneratedPIF(fromFixture: "Miscellaneous/Simple", addLocalRpaths: false) { pif, observabilitySystem in
716+
try await withGeneratedPIF(fromFixture: "Miscellaneous/Simple", addLocalRpaths: .never) { pif, observabilitySystem in
721717
#expect(observabilitySystem.diagnostics.filter {
722718
$0.severity == .error
723719
}.isEmpty)
@@ -742,10 +738,10 @@ struct PIFBuilderTests {
742738
}
743739
}
744740

745-
@Test func disablingLocalRpathsInReleaseConfiguration() async throws {
741+
@Test func debugOnlyLocalRpaths() async throws {
746742
try await withGeneratedPIF(
747743
fromFixture: "Miscellaneous/Simple",
748-
addLocalRpathsInReleaseConfiguration: false
744+
addLocalRpaths: .debugOnly
749745
) { pif, observabilitySystem in
750746
#expect(observabilitySystem.diagnostics.filter {
751747
$0.severity == .error
@@ -769,34 +765,6 @@ struct PIFBuilderTests {
769765
#expect(releaseConfig.impartedBuildProperties.settings[.LD_RUNPATH_SEARCH_PATHS] == nil)
770766
}
771767
}
772-
773-
try await withGeneratedPIF(
774-
fromFixture: "Miscellaneous/Simple",
775-
addLocalRpaths: false,
776-
addLocalRpathsInReleaseConfiguration: false
777-
) { pif, observabilitySystem in
778-
#expect(observabilitySystem.diagnostics.filter {
779-
$0.severity == .error
780-
}.isEmpty)
781-
782-
do {
783-
let debugConfig = try pif.workspace
784-
.project(named: "Foo")
785-
.target(named: "Foo")
786-
.buildConfig(named: .debug)
787-
788-
#expect(debugConfig.impartedBuildProperties.settings[.LD_RUNPATH_SEARCH_PATHS] == nil)
789-
}
790-
791-
do {
792-
let releaseConfig = try pif.workspace
793-
.project(named: "Foo")
794-
.target(named: "Foo")
795-
.buildConfig(named: .release)
796-
797-
#expect(releaseConfig.impartedBuildProperties.settings[.LD_RUNPATH_SEARCH_PATHS] == nil)
798-
}
799-
}
800768
}
801769

802770
@Test func warningSettingsInRemotePackage() async throws {
@@ -900,7 +868,7 @@ struct PIFBuilderTests {
900868
graph: graph,
901869
parameters: try PIFBuilderParameters.constructDefaultParametersForTesting(
902870
temporaryDirectory: AbsolutePath.root,
903-
addLocalRpaths: true
871+
addLocalRpaths: .always
904872
),
905873
fileSystem: fs,
906874
observabilityScope: observability.topScope
@@ -1038,7 +1006,7 @@ struct PIFBuilderTests {
10381006
graph: graph,
10391007
parameters: try PIFBuilderParameters.constructDefaultParametersForTesting(
10401008
temporaryDirectory: AbsolutePath.root.appending("tmp"),
1041-
addLocalRpaths: true
1009+
addLocalRpaths: .always
10421010
),
10431011
fileSystem: fs,
10441012
observabilityScope: observability.topScope
@@ -1132,7 +1100,7 @@ struct PIFBuilderTests {
11321100
graph: graph,
11331101
parameters: try PIFBuilderParameters.constructDefaultParametersForTesting(
11341102
temporaryDirectory: AbsolutePath.root.appending("tmp"),
1135-
addLocalRpaths: true
1103+
addLocalRpaths: .always
11361104
),
11371105
fileSystem: fs,
11381106
observabilityScope: observability.topScope

‎Tests/SwiftBuildSupportTests/PrebuiltsPIFTests.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ struct PrebuiltsPIFTests {
171171
let pifBuilder: PIFBuilder = PIFBuilder(
172172
graph: graph,
173173
parameters: try PIFBuilderParameters.constructDefaultParametersForTesting(
174-
temporaryDirectory: AbsolutePath.root, addLocalRpaths: true),
174+
temporaryDirectory: AbsolutePath.root, addLocalRpaths: .always),
175175
fileSystem: fs,
176176
observabilityScope: observability.topScope
177177
)
@@ -426,7 +426,7 @@ struct PrebuiltsPIFTests {
426426
graph: graph,
427427
parameters: try PIFBuilderParameters.constructDefaultParametersForTesting(
428428
temporaryDirectory: AbsolutePath.root,
429-
addLocalRpaths: true,
429+
addLocalRpaths: .always,
430430
pluginScriptRunner: MockPluginScriptRunner()
431431
),
432432
fileSystem: fs,

0 commit comments

Comments
 (0)