search

ReactiveX / RxSwift

Reactive Programming in Swift
MIT License
24.32k stars 4.17k forks source link

Add Privacy Manifest #2572

Closed wlxo0401 closed 5 months ago

wlxo0401 commented 8 months ago

  1. Added xcprivacy

    • 'mach_absolute_time' exists in folder 'Tests'
    • System boot time APIs - 35F9.1
    • One each in the RxSwift, RxCocoa, and RxRelay folders.
    • Remove If Not Required

  2. Append privacy file to CocoaPods bundle

    • Added to each podspec of RxSwift, RxRelay, and RxCocoa.
    • I applied privacyinfo as resource_bundles.

  3. SPM Package needs to be modified as well. I didn't do it because I didn't know exactly.

  4. I added it by referring to other libraries. I'm sorry if the method is wrong.

  5. This has not been tested. I need additional comments and validation from others.

This resolves https://github.com/ReactiveX/RxSwift/issues/2567

tommyming commented 8 months ago

Discussion Reference - #2567

@wlxo0401 would suggest to mark this PR as draft, if it is not completely ready.

wlxo0401 commented 8 months ago

@tommyming

I posted it hoping others would continue in one part. Is this also a draft?

Should I just delete PR?

I'm sorry that I'm bothering you because I'm not good at many things.

tommyming commented 8 months ago

It's ok @wlxo0401, asking questions is always a good thing :)

To set a PR to draft, you may check on this Article

It is in Korean already so no worries!

If the repo does not allow you to convert to Draft PR, just add "DRAFT" to the description, then I think it should be good enough.

wlxo0401 commented 8 months ago

@tommyming

Thank you, I changed this article to draft!!

tommyming commented 8 months ago

@wlxo0401 Please allow me to access the forked repo, or else I cannot push changes there. If you want to know how to add to SPM, please let me know, thanks.

wlxo0401 commented 8 months ago

@tommyming

I've invited you to join the collaborators.

Is this right?

tommyming commented 8 months ago

@wlxo0401 should be it, please change the PR to review-ready, many thanks. @freak4pc Please feel free to review when you have time, thanks a lot.

wlxo0401 commented 8 months ago

@tommyming

I changed the title and changed the state of PR to 'review-ready'.

Thank you so much for allowing me to participate in this PR process. It was a good experience for me.

tommyming commented 8 months ago

@wlxo0401 Please add the privacy manifest issue to this pull request: https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue

wlxo0401 commented 8 months ago

@wlxo0401 Please add the privacy manifest issue to this pull request: https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue

Thank you, and thanks to you, I found out about the new features.

Check it out.

wlxo0401 commented 8 months ago

Is it okay to do the resources path of Package.swift as it is now?

Do not have to enter an additional folder path??

For example, as below. RxSwift/PrivacyInfo.xcprivacy RxRelay/PrivacyInfo.xcprivacy

I can't read the package code well yet, so it might be a misunderstanding.

nabs-m commented 8 months ago

Is it okay to do the resources path of Package.swift as it is now?

Do not have to enter an additional folder path??

For example, as below. RxSwift/PrivacyInfo.xcprivacy RxRelay/PrivacyInfo.xcprivacy

I can't read the package code well yet, so it might be a misunderstanding.

@wlxo0401 I haven't tried it myself with SPM, but I saw this in another open source library. Hopefully that helps?

tommyming commented 8 months ago

From my understanding, the privacy file will be loaded based on the path provided to SPM. As on the file structure, the privacy manifest is on the outermost layer, so suppose we don't need to add any filePath prefixes.

Feel free to correct me if I'm wrong.

wlxo0401 commented 7 months ago

I didn't set a target for PrivacyInfo, so I added a setting.

I don't know if this is necessary, but please review it.

ghost commented 7 months ago
  1. Added xcprivacy

  • 'mach_absolute_time' exists in folder 'Tests'

  • System boot time APIs - 35F9.1

  • One each in the RxSwift, RxCocoa, and RxRelay folders.

  • Remove If Not Required

  1. Append privacy file to CocoaPods bundle

  • Added to each podspec of RxSwift, RxRelay, and RxCocoa.

  • I applied privacyinfo as resource_bundles.

  1. SPM Package needs to be modified as well. I didn't do it because I didn't know exactly.

  2. I added it by referring to other libraries. I'm sorry if the method is wrong.

  3. This has not been tested. I need additional comments and validation from others.

This resolves https://github.com/ReactiveX/RxSwift/issues/2567

tommyming commented 7 months ago

@freak4pc May I know if there are any schedule for this PR? Thanks

aiKrice commented 6 months ago

Just an up here as a gentle reminder

wlxo0401 commented 6 months ago

Hey, first of all thank you for leading this and taking this.

Unfortunately, I think this is actually incorrect.

The API is used in the tests for RxSwift itself, and not any sources that are embedded for any consumers that depend on any fo the libraries.mach_absolute_time

This repo was only flagged because it is a very popular OSS repo and some bot probably scanned this API in the GitHub repo, but there is nothing to report from a privacy perspective.

Also, we'll need this PR to support SPM since it's the modern and popular way to consume libraries. Here's an example of how to achieve this: https://github.com/firebase/firebase-ios-sdk/blob/fe09d61a539e11fdbe24f269bba10144b6145fe2/Package.swift#L202

Once you make the changes please ping me directly in this PR and I'll take care of a release.

Thanks!

@freak4pc As you told me, I pushed a new Comit.

Can I ask you to check if 'Target MemberShip' is also done properly??

I invited you to the Forked Repository just in case.

I would appreciate it if other people could check based on the latest content and feel free to give their opinions.

tommyming commented 6 months ago

Hey, first of all thank you for leading this and taking this.

Unfortunately, I think this is actually incorrect.

The mach_absolute_time API is used in the tests for RxSwift itself, and not any sources that are embedded for any consumers that depend on any fo the libraries.

This repo was only flagged because it is a very popular OSS repo and some bot probably scanned this API in the GitHub repo, but there is nothing to report from a privacy perspective.

Also, we'll need this PR to support SPM since it's the modern and popular way to consume libraries. Here's an example of how to achieve this: https://github.com/firebase/firebase-ios-sdk/blob/fe09d61a539e11fdbe24f269bba10144b6145fe2/Package.swift#L202

Once you make the changes please ping me directly in this PR and I'll take care of a release.

Thanks!

@freak4pc thanks for the reply.

I agree that the API is used in tests only, which won't cause any privacy issues mentioned by Apple.

For the SPM implementation, suppose my commit has done it. Please feel free to review the Packaged.swift file, in which I committed changes.

1 thing I don't understand is some libs use .copy, while some use .process for the loading resource. Not sure which one is correct.

Please feel free to tag me about SPM privacy manifest issues related to this PR. Feel free to provide any opinion on this.

tommyming commented 6 months ago

Hey, first of all thank you for leading this and taking this. Unfortunately, I think this is actually incorrect. The API is used in the tests for RxSwift itself, and not any sources that are embedded for any consumers that depend on any fo the libraries.mach_absolute_time This repo was only flagged because it is a very popular OSS repo and some bot probably scanned this API in the GitHub repo, but there is nothing to report from a privacy perspective. Also, we'll need this PR to support SPM since it's the modern and popular way to consume libraries. Here's an example of how to achieve this: https://github.com/firebase/firebase-ios-sdk/blob/fe09d61a539e11fdbe24f269bba10144b6145fe2/Package.swift#L202 Once you make the changes please ping me directly in this PR and I'll take care of a release. Thanks!

As you told me, I pushed a new Comit.

Can I ask you to check if 'Target MemberShip' is also done properly??

I invited you to the Forked Repository just in case.

I would appreciate it if other people could check based on the latest content and feel free to give their opinions.

@wlxo0401 I think that should work fine, if we didnt use the API for privacy use.

nabs-m commented 6 months ago

Just want to +1 the comment around not having to report an API if it's only in tests, as it won't show up in the symbol lookup table that Apple will likely use to scan release apps. We did the same with a library we develop.

tommyming commented 6 months ago

Update: I have updated the SPM resource processing rules to .process instead of .copy, following firebase-ios-sdk practices.

@freak4pc please review when you are free, thanks.

linhaosunny commented 5 months ago

2. CocoaPods

I 'm not use SPM, When will you support CocoaPods? When will you release the new version?

freak4pc commented 5 months ago

I'm closing this for now, let's continue the discussion on #2567 - From my current perspective RxSwift will not need to add a privacy manifest.

linhaosunny commented 5 months ago

I'm closing this for now, let's continue the discussion on #2567 - From my current perspective RxSwift will not need to add a privacy manifest.

@freak4pc so that's all?

andschdk commented 5 months ago

@linhaosunny https://github.com/ReactiveX/RxSwift/issues/2567#issuecomment-2008689007