Skip to content

Commit 5ac12f5

Browse files
matthewseamanmatthewseaman
authored
Copy Files phase should automatically use the correct lproj directory even if applying build rules (#880)
1 parent d92ea0b commit 5ac12f5

File tree

3 files changed

+103
-2
lines changed

3 files changed

+103
-2
lines changed

‎Sources/SWBTaskConstruction/TaskProducers/BuildPhaseTaskProducers/CopyFilesTaskProducer.swift‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,9 +169,15 @@ class CopyFilesTaskProducer: FilesBasedBuildPhaseTaskProducerBase, FilesBasedBui
169169
override func constructTasksForRule(_ rule: any BuildRuleAction, _ group: FileToBuildGroup, _ buildFilesContext: BuildFilesProcessingContext, _ scope: MacroEvaluationScope, _ delegate: any TaskGenerationDelegate) async {
170170
let dstFolder = computeOutputDirectory(scope)
171171

172-
// FIXME: Merge the region variant.
172+
// Merge the region variant.
173+
// Since this behavior was not always here, some people have hardcoded lproj directories in their destination folder path.
174+
// Only add an additional component if there isn't one already.
175+
var locDST = dstFolder
176+
if !locDST.containsRegionVariantPathComponent {
177+
locDST = dstFolder.join(group.regionVariantPathComponent)
178+
}
173179

174-
let cbc = CommandBuildContext(producer: context, scope: scope, inputs: group.files, isPreferredArch: buildFilesContext.belongsToPreferredArch, buildPhaseInfo: buildFilesContext.buildPhaseInfo(for: rule), resourcesDir: dstFolder, unlocalizedResourcesDir: dstFolder)
180+
let cbc = CommandBuildContext(producer: context, scope: scope, inputs: group.files, isPreferredArch: buildFilesContext.belongsToPreferredArch, buildPhaseInfo: buildFilesContext.buildPhaseInfo(for: rule), resourcesDir: locDST, unlocalizedResourcesDir: dstFolder)
175181
await constructTasksForRule(rule, cbc, delegate)
176182
}
177183

‎Sources/SWBUtil/Path.swift‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,24 @@ public struct Path: Serializable, Sendable {
431431
return nil
432432
}
433433

434+
/// `true` if the path contains any .lproj directories as path components.
435+
public var containsRegionVariantPathComponent: Bool {
436+
var path = self.join("File.strings") // since regionVariantName looks at parent dir
437+
while !path.isRoot && !path.isEmpty {
438+
if path.regionVariantName != nil {
439+
return true
440+
} else {
441+
let parent = path.dirname
442+
if parent == path {
443+
break
444+
} else {
445+
path = parent
446+
}
447+
}
448+
}
449+
return false
450+
}
451+
434452
/// Return true if the pathname is conformant to path restrictions on the platform.
435453
///
436454
/// Check the Unicode string representation of the path for reserved characters that cannot be represented as a path.

‎Tests/SWBTaskConstructionTests/InstallLocTaskConstructionTests.swift‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,6 +1014,83 @@ fileprivate struct InstallLocTaskConstructionTests: CoreBasedTests {
10141014
}
10151015
}
10161016

1017+
@Test(.requireSDKs(.macOS))
1018+
func copyFilesRulesMergeRegion() async throws {
1019+
let testProject = TestProject(
1020+
"aProject",
1021+
groupTree: TestGroup(
1022+
"SomeFiles", path: "Sources",
1023+
children: [
1024+
TestVariantGroup("Localizable.strings", children: [
1025+
TestFile("en.lproj/Localizable.strings", regionVariantName: "en"),
1026+
TestFile("ja.lproj/Localizable.strings", regionVariantName: "ja"),
1027+
TestFile("zh_TW.lproj/Localizable.strings", regionVariantName: "zh_TW"),
1028+
]),
1029+
]),
1030+
buildConfigurations: [
1031+
TestBuildConfiguration(
1032+
"Debug",
1033+
buildSettings: [
1034+
"PRODUCT_NAME": "$(TARGET_NAME)",
1035+
"GENERATE_INFOPLIST_FILE": "YES",
1036+
"APPLY_RULES_IN_COPY_FILES": "YES"
1037+
]),
1038+
],
1039+
targets: [
1040+
TestStandardTarget(
1041+
"CoreFoo", type: .framework,
1042+
buildPhases: [
1043+
TestCopyFilesBuildPhase([
1044+
"Localizable.strings",
1045+
], destinationSubfolder: .absolute, destinationSubpath: "/System/Library/Bundles/MyBundle.bundle", onlyForDeployment: true),
1046+
TestCopyFilesBuildPhase([
1047+
"Localizable.strings",
1048+
], destinationSubfolder: .builtProductsDir, destinationSubpath: "OtherProduct.bundle", onlyForDeployment: false),
1049+
]
1050+
)
1051+
])
1052+
let tester = try await TaskConstructionTester(getCore(), testProject)
1053+
1054+
// installloc single language
1055+
await tester.checkBuild(BuildParameters(action: .installLoc, configuration: "Release", overrides: ["INSTALLLOC_LANGUAGE": "ja"]), runDestination: .macOS) { results in
1056+
results.checkTarget("CoreFoo") { target in
1057+
results.checkTaskExists(.matchTarget(target), .matchRule(["Copy", "/tmp/Test/aProject/build/Debug/OtherProduct.bundle/ja.lproj/Localizable.strings", "/tmp/Test/aProject/Sources/ja.lproj/Localizable.strings"]))
1058+
results.checkTaskExists(.matchTarget(target), .matchRule(["Copy", "/tmp/aProject.dst/System/Library/Bundles/MyBundle.bundle/ja.lproj/Localizable.strings", "/tmp/Test/aProject/Sources/ja.lproj/Localizable.strings"]))
1059+
}
1060+
1061+
results.checkNoDiagnostics()
1062+
}
1063+
1064+
// installloc multi-language
1065+
await tester.checkBuild(BuildParameters(action: .installLoc, configuration: "Release", overrides: ["INSTALLLOC_LANGUAGE": "ja zh_TW"]), runDestination: .macOS) { results in
1066+
results.checkTarget("CoreFoo") { target in
1067+
results.checkTaskExists(.matchTarget(target), .matchRule(["Copy", "/tmp/Test/aProject/build/Debug/OtherProduct.bundle/ja.lproj/Localizable.strings", "/tmp/Test/aProject/Sources/ja.lproj/Localizable.strings"]))
1068+
results.checkTaskExists(.matchTarget(target), .matchRule(["Copy", "/tmp/aProject.dst/System/Library/Bundles/MyBundle.bundle/ja.lproj/Localizable.strings", "/tmp/Test/aProject/Sources/ja.lproj/Localizable.strings"]))
1069+
1070+
results.checkTaskExists(.matchTarget(target), .matchRule(["Copy", "/tmp/Test/aProject/build/Debug/OtherProduct.bundle/zh_TW.lproj/Localizable.strings", "/tmp/Test/aProject/Sources/zh_TW.lproj/Localizable.strings"]))
1071+
results.checkTaskExists(.matchTarget(target), .matchRule(["Copy", "/tmp/aProject.dst/System/Library/Bundles/MyBundle.bundle/zh_TW.lproj/Localizable.strings", "/tmp/Test/aProject/Sources/zh_TW.lproj/Localizable.strings"]))
1072+
}
1073+
1074+
results.checkNoDiagnostics()
1075+
}
1076+
1077+
// install
1078+
await tester.checkBuild(BuildParameters(action: .install, configuration: "Release"), runDestination: .macOS) { results in
1079+
results.checkTarget("CoreFoo") { target in
1080+
results.checkTaskExists(.matchTarget(target), .matchRule(["CopyStringsFile", "/tmp/Test/aProject/build/Debug/OtherProduct.bundle/en.lproj/Localizable.strings", "/tmp/Test/aProject/Sources/en.lproj/Localizable.strings"]))
1081+
results.checkTaskExists(.matchTarget(target), .matchRule(["CopyStringsFile", "/tmp/aProject.dst/System/Library/Bundles/MyBundle.bundle/en.lproj/Localizable.strings", "/tmp/Test/aProject/Sources/en.lproj/Localizable.strings"]))
1082+
1083+
results.checkTaskExists(.matchTarget(target), .matchRule(["CopyStringsFile", "/tmp/Test/aProject/build/Debug/OtherProduct.bundle/ja.lproj/Localizable.strings", "/tmp/Test/aProject/Sources/ja.lproj/Localizable.strings"]))
1084+
results.checkTaskExists(.matchTarget(target), .matchRule(["CopyStringsFile", "/tmp/aProject.dst/System/Library/Bundles/MyBundle.bundle/ja.lproj/Localizable.strings", "/tmp/Test/aProject/Sources/ja.lproj/Localizable.strings"]))
1085+
1086+
results.checkTaskExists(.matchTarget(target), .matchRule(["CopyStringsFile", "/tmp/Test/aProject/build/Debug/OtherProduct.bundle/zh_TW.lproj/Localizable.strings", "/tmp/Test/aProject/Sources/zh_TW.lproj/Localizable.strings"]))
1087+
results.checkTaskExists(.matchTarget(target), .matchRule(["CopyStringsFile", "/tmp/aProject.dst/System/Library/Bundles/MyBundle.bundle/zh_TW.lproj/Localizable.strings", "/tmp/Test/aProject/Sources/zh_TW.lproj/Localizable.strings"]))
1088+
}
1089+
1090+
results.checkNoDiagnostics()
1091+
}
1092+
}
1093+
10171094
@Test(.requireSDKs(.iOS))
10181095
func installLocForFramework() async throws {
10191096
let testProject = TestProject(

0 commit comments

Comments
 (0)