Align resource bundle accessor generation with SPM

[danibachar]

Resolves #6149

Short description 📝

Comply to SPM bundle accessors, both for internal and external module.
Following up on this PR with a more novel solution.

Currently Tuist have a mix of how it generates bundle accessors for both ObjC and Swift run time. This PR tries to unify them and comply to SPM logic.

How to test the changes locally 🧐

Include a set of steps for the reviewer to test the changes locally (see the documentation for reference).

Use the disableBundleAccessors: true option.

In a target with Swift code and Resources, code will be generated and linked to the target to support the Bundle.module accessor - similar to SPM.

In a target with Objective-C code and Resources, code will be generated and linked to the target to support the SWIFTPM_MODULE_BUNDLE accessor macro - similar to SPM.

Contributor checklist ✅

• The code has been linted using run mise run lint:fix
• The change is tested via unit testing or acceptance testing, or both
• The title of the PR is formulated in a way that is usable as a changelog entry
• In case the PR introduces changes that affect users, the documentation has been updated

Reviewer checklist ✅

• The code architecture and patterns are consistent with the rest of the codebase
• Reviewer has checked that, if needed, the PR includes the label changelog:added, changelog:fixed, or changelog:changed, and the title is usable as a changelog entry

[fortmarek]

@danibachar I think this is the right way to go about it. We should preserve the \(targetName)Resources accessor for internal targets, but there's no need to have it for external resources – for SPM dependencies, we should really closely follow whatever Apple is doing.

With this PR, we can also remove this line and it will resolve this issue.

Let me know if you have time to work on this, otherwise, I can take it over 🙏

[danibachar]

Hi, yes, I opened this PR which lead me to the understanding people are probably using. the objc accessor to access the target resources from the outside.

I thought about it and I came to the conclusion that to comply with SPM we need to distinguish internal from public accessors.
With SPM if one wishes to allow accessing resources from a certain target they should write explicit code for that, its not given by SPM out of the box.

I'm going to add these changes + some others into my original PR if that is ok with you...

I think in this way we can comply with SPM - i.e generate internal accessors for both objc and swift runtime. In addition to allow public accessor for both (with the same objc class) like today.

struct BundleAccessorOptions {
 let internal: Bool // if true will create the internal accessors
 let public: Bool // if true will create the public accessor class, I do wonder if we should leave those in the same file as the internal ones?
}

Then the backward compatibility will be simple.
we keep the disableBundleAccessors: Bool and add a deprecation warning.

let bundleAccessorOptions = BundleAccessorOptions(internal: !disableBundleAccessors, public: !disableBundleAccessors)

WDYT?

[fortmarek]

I'd do two PRs:

• One that for SPM creates only internal accessors
• Possibly second one that allows users to control that. However, I'm not sure we need that level of control – from your use-case, making SPM accessors internal only should be good enough?

[danibachar]

I'd do two PRs:

• One that for SPM creates only internal accessors
• Possibly second one that allows users to control that. However, I'm not sure we need that level of control – from your use-case, making SPM accessors internal only should be good enough?

It will introduce breaking changes though - wouldn't it? I mean we don't know if users are using the TargetNameResource.swift class for external swift packages.
If I'd add the isExternal check like we do for Objc it might cause disruption for those who are using it (even though it sounds a bit abusing...

I have one example Fixture that is doing that - the app_spm_dependencies - it defines a Swift Package, and the app is using the codegen accessors to use resources (like colors, fonts) from that Package.

But to your question - yes it will solve my issue

I do think that changes here are quite simple and comprehensive, keeping backwards compatibility, and are not that big?

[fortmarek]

It will introduce breaking changes though - wouldn't it? I mean we don't know if users are using the TargetNameResource.swift class for external swift packages.

We have never marketed that SPM resources can be available externally and I don't think our users should expect us to support that option when it's not available via vanilla integration. While technically a breaking change, I think it's something we can live with.

I do think that changes #6124 are quite simple and comprehensive, keeping backwards compatibility, and are not that big?

I'd still redo the PR and remove the ProjectDescription changes. If we don't have a use-case for the added granularity, I'd skip it. We can still consider it but on its own and as a follow-up.

[fortmarek]

Closing in favor of: #6124

[fortmarek]

Can we also add the dependency from this issue into the app_with_spm_dependencies fixture to confirm the bug is resolved (and that we won't introduce a regression for this in the future?)

Otherwise, just a couple of nits after which we can merge this.

[fortmarek]

This formatting is slightly odd. Is it actually treated as one comment? 🤔

[danibachar]

I didn't touch this formatting, mise lint didn't change it, but indeed its considered as one comment - i.e one line

Example from the app_with_spm_dependencies, the derived file Derived/Sources/TuistBundle+Styles

...
/// Since Styles is a staticFramework, the bundle containing the resources is copied into the final product.
...

[danibachar]

I can change it if you wish

[fortmarek]

no, it's fine as long as it's treated as a single line of comment 👍

[danibachar]

Note this following PR and related issue I have found with formatting the bundleName

[fortmarek]

Amazing work 👏