Skip to content

Support String Catalog symbol generation by adding String Catalogs as both Sources and Resources #8815

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 25, 2025
Merged

Support String Catalog symbol generation by adding String Catalogs as both Sources and Resources #8815

merged 1 commit into from
Jun 25, 2025

Conversation

matthewseaman
Copy link
Contributor

@matthewseaman matthewseaman commented Jun 12, 2025 •
edited
Loading

  • Explanation:
    This change adds support for String Catalog symbol generation by ensuring that String Catalogs get added to the Sources build phase in the same way that Asset Catalogs do.

Depends on: swiftlang/swift-build#582

  • Scope:
    Packages that include String Catalogs.
  • Issue:
    rdar://147474472
  • Risk:
    Change is already tested and in use within Xcode.
  • Testing:
    Unit tests added here, plus usage within Xcode.

@matthewseaman matthewseaman self-assigned this Jun 12, 2025
@plemarquand
Copy link
Contributor

@swift-ci please test

@matthewseaman
Copy link
Contributor Author

Will fail in CI until SwiftBuild portion lands via swiftlang/swift-build#582

@bkhouri
Copy link
Contributor

bkhouri commented Jun 16, 2025

Thanks for this change, but would it be possible to add, or enabled, one or more automated tests that would validate the SwiftPM behaviour?

@matthewseaman
Copy link
Contributor Author

Thanks for this change, but would it be possible to add, or enabled, one or more automated tests that would validate the SwiftPM behaviour?

It would be the same as the Asset Catalog logic on the lines above. Are there existing tests for these code paths somewhere that it would make sense to add to?

@matthewseaman matthewseaman changed the title Support String Catalog symbol generation by always treating String Catalogs as Sources Support String Catalog symbol generation by adding String Catalogs as both Sources and Resources Jun 21, 2025
@matthewseaman
Copy link
Contributor Author

@swift-ci please test

@matthewseaman
Copy link
Contributor Author

@swift-ci test

@matthewseaman
Copy link
Contributor Author

matthewseaman commented Jun 21, 2025 •
edited
Loading

Added tests in XCBuildSupportTests module.
Interestingly, I don't see a SwiftBuildSupportTests module.

matthewseaman
matthewseaman commented Jun 21, 2025
View reviewed changes
Tests/XCBuildSupportTests/PIFBuilderTests.swift
@@ -2216,15 +2222,15 @@ final class PIFBuilderTests: XCTestCase {
Manifest.createRootManifest(
displayName: "Foo",
path: "/Foo",
toolsVersion: .v5_3,
toolsVersion: .v5_9,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bumped to the first tools version that supports xcstrings at all.

@matthewseaman
Copy link
Contributor Author

@swift-ci test windows

@matthewseaman matthewseaman changed the title Support String Catalog symbol generation by adding String Catalogs as both Sources and Resources [6.2] Support String Catalog symbol generation by adding String Catalogs as both Sources and Resources Jun 24, 2025
@matthewseaman matthewseaman force-pushed the mseaman/string-catalog-symbols branch from 70ec07d to 0006fd6 Compare June 24, 2025 18:55
@matthewseaman matthewseaman changed the base branch from release/6.2 to main June 24, 2025 18:55
@matthewseaman matthewseaman changed the title [6.2] Support String Catalog symbol generation by adding String Catalogs as both Sources and Resources Support String Catalog symbol generation by adding String Catalogs as both Sources and Resources Jun 24, 2025
@matthewseaman
Copy link
Contributor Author

@swift-ci test

@dschaefer2
Copy link
Member

@swift-ci please test windows

@dschaefer2 dschaefer2 merged commit d8bc782 into main Jun 25, 2025
6 checks passed
@dschaefer2 dschaefer2 deleted the mseaman/string-catalog-symbols branch June 25, 2025 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jakepetroules jakepetroules jakepetroules approved these changes

@pmattos pmattos pmattos approved these changes

@dschaefer2 dschaefer2 Awaiting requested review from dschaefer2 dschaefer2 is a code owner

@shawnhyam shawnhyam Awaiting requested review from shawnhyam shawnhyam is a code owner

@bripeticca bripeticca Awaiting requested review from bripeticca bripeticca is a code owner

@plemarquand plemarquand Awaiting requested review from plemarquand plemarquand is a code owner

@owenv owenv Awaiting requested review from owenv owenv is a code owner

@bkhouri bkhouri Awaiting requested review from bkhouri bkhouri is a code owner

@cmcgee1024 cmcgee1024 Awaiting requested review from cmcgee1024 cmcgee1024 is a code owner

@daveyc123 daveyc123 Awaiting requested review from daveyc123 daveyc123 is a code owner

Assignees

@matthewseaman matthewseaman

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@matthewseaman @plemarquand @bkhouri @dschaefer2 @jakepetroules @pmattos