Skip to content
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.

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

fix(PrivacyInfo) SPM/Cocoapods support for privacy manifest #487

Merged
merged 1 commit into from
Mar 22, 2024

Conversation

sdurban
Copy link
Contributor

@sdurban sdurban commented Mar 19, 2024

Summary

This PR adds exporting of the PrivacyInfo information added in version 8.18.0 for use via SPM and Cocoapods.

I have tested it with SPM and Cocoapods sample projects, and now it successfully fetches the information.

The process to add it in Cocoapods is the same as used by multiple projects. Ref. CocoaPods/CocoaPods#10325 (comment) and to add it in SPM, I have looked at other projects such as Revenuecat https://github.com/RevenueCat/purchases-ios/blob/main/Package.swift.

Issues Related:
#482
#470

Thanks for Amplitude!

Checklist

  • Does your PR title have the correct title format?
  • Does your PR have a breaking change?: no

@sdurban sdurban changed the title fix(PrivacyInfo.xcprivacy) SPM/Cocoapods support for privacy manifest fix(PrivacyInfo) SPM/Cocoapods support for privacy manifest Mar 19, 2024
@sdurban sdurban mentioned this pull request Mar 19, 2024
@wlxo0401
Copy link

This should be accepted.

@crleona
Copy link
Collaborator

crleona commented Mar 22, 2024

Merging this, thanks @sdurban!

@crleona crleona merged commit 92109a8 into amplitude:main Mar 22, 2024
12 checks passed
@wlxo0401
Copy link

wlxo0401 commented Mar 22, 2024

Merging this, thanks @sdurban!

@crleona
Amplitude-Swift also needs it

Copy link

🎉 This PR is included in version 8.18.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@astrokin
Copy link

astrokin commented Apr 8, 2024

Hello @wlxo0401 and @sdurban. Unfortunately, these changes do not effectively address the issue.
Using syntax

<spec>.<platform>.resources

produce a copy into the target bundle (as per official documentation) also for static linking process there is strongly recommend to use different way of attaching resources into the pod should be used.

<spec>.<platform>.resource_bundle

so the spec should looks like this.

...
s.ios.resource_bundles = {
    "#{s.module_name}" => 'Sources/Resources/*.{der}'
    "#{s.module_name}_Privacy" => 'Sources/PrivacyInfo.xcprivacy'
  }
...
s.tvos.resource_bundles = {
    "#{s.module_name}" => 'Sources/Resources/*.{der}'
    "#{s.module_name}_Privacy" => 'Sources/PrivacyInfo.xcprivacy'
  }
...
s.osx.resource_bundles = {
    "#{s.module_name}" => 'Sources/Resources/*.{der}'
    "#{s.module_name}_Privacy" => 'Sources/PrivacyInfo.xcprivacy'
  }
....
s.watchos.resource_bundles = {
    "#{s.module_name}" => 'Sources/Resources/*.{der}'
    "#{s.module_name}_Privacy" => 'Sources/PrivacyInfo.xcprivacy'
  }
...

Current Amplitude-iOS version 8.19.1 for cocoapods linked statically produce a compile error

Multiple commands produce .... PrivacyInfo.xcprivacy .... script phase “[CP] Copy Pods Resources”

if i add my own PrivacyInfo.xcprivacy file into the target. What do you think about it?

@sdurban
Copy link
Contributor Author

sdurban commented Apr 8, 2024

Hello @astrokin,

You're right. There is a problem with cocoapods definition if you use cocoapods without use_frameworks.

If you use use_frameworks!, the pods are integrated as frameworks rather than static libraries. This means the file belongs to the framework and not to your project, thereby avoiding this issue.

Perhaps it would be best to leave the .der files copied and place the privacy file in its own bundle as you suggested.

I'll submit another PR this week to address this issue, @crleona or you can fix It with a PR @astrokin if you want.

@crleona
Copy link
Collaborator

crleona commented Apr 9, 2024

@sdurban @astrokin - I've opened a PR with a fix #493

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants