plugins icon indicating copy to clipboard operation
plugins copied to clipboard

[in_app_purchase] Add macOS support

Open alihassan143 opened this issue 2 years ago • 34 comments

macos os support added also the example app also added

  • [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • [x] I read the Tree Hygiene wiki page, which explains my responsibilities.
  • [x] I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • [x] I signed the CLA.
  • [x] The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • [x] I listed at least one issue that this PR fixes in the description above.
  • [x] I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • [x] I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • [x] I updated/added relevant documentation (doc comments with ///).
  • [x] I added new tests to check the change I am making, or this PR is test-exempt.
  • [ ] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

alihassan143 avatar May 28 '22 10:05 alihassan143

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

flutter-dashboard[bot] avatar May 28 '22 10:05 flutter-dashboard[bot]

Thanks for the submission! Since this currently doesn't pass any of the CI tests, and the checklist is missing critical elements like tests, I'm going to mark this as a draft. Please also read https://github.com/flutter/flutter/issues/84391#issuecomment-970474366 as it looks like you are duplicating code, which isn't an approach we would land.

Once this PR is complete, please mark it as ready for review and we'll take a look. Thanks!

stuartmorgan avatar May 28 '22 14:05 stuartmorgan

This still has a number of high-level issues that need to be addressed before we can even start reviewing this:

  • As mentioned above:

    Please also read https://github.com/flutter/flutter/issues/84391#issuecomment-970474366 as it looks like you are duplicating code, which isn't an approach we would land.

    This doesn't appear to have been resolved; the diff is showing duplication of all of the files. They need to be symlinked from a shared location.

  • The plugin does not build. See failures in macos-build_all_plugins and macos-platform_tests. It needs to build and pass tests.

  • You have a lot of unrelated changes. You've made pubspec changes to packages with no changes, for reasons that aren't clear, and add a lot of files to the iOS and Android examples. All the changes that aren't related to this PR need to be reverted.

  • You need to follow the process described here so that CI can build the changes. You should also not be disabling publishing of in_app_purchase; there's no step in this process that would require that, and it disables important checks.

Some other issues as well:

  • This doesn't appear to have been run; see failures in the format task. The license check there is also failing; you'll need to add the license to all new files.

  • You've checked these boxes, but haven't actually done the steps. As I said in your previous PR, please don't check boxes when you haven't done the steps. We have this checklist for a reason; these steps are part of our development process.

stuartmorgan avatar May 30 '22 16:05 stuartmorgan

thank you for clearing the confusion i will change the things and resend for review again

alihassan143 avatar May 30 '22 16:05 alihassan143

@stuartmorgan i am running tests for the macos error: "Runner" has entitlements that require signing with a development certificate. Enable development signing in the Signing & Capabilities editor. (in target 'Runner' from project 'Runner') showing this error but in test is it required to sign in with capabilities

alihassan143 avatar May 31 '22 09:05 alihassan143

I'm not sure where you are seeing that error, but the errors in CI are from the build not being set up correctly in the first place. For instance:

Errno::ENOENT - No such file or directory @ rb_check_realpath_internal - /private/var/folders/2f/dq81klt17l3g6yg3_nf0t2gh0000gn/T/cirrus-ci-build/packages/in_app_purchase/in_app_purchase_storekit/example/macos/Flutter/ephemeral/.symlinks/plugins/in_app_purchase_storekit/macos/Classes/FIAObjectTranslator.h

stuartmorgan avatar May 31 '22 13:05 stuartmorgan

@stuartmorgan i tried the ci test for ios also but they showing the same error also

alihassan143 avatar May 31 '22 15:05 alihassan143

I don't see that error anywhere in the CI output for iOS, so again I'm not sure where you are seeing this error.

stuartmorgan avatar May 31 '22 15:05 stuartmorgan

@stuartmorgan can you try to run again the ci test i am getting this error for ios also in native tests

alihassan143 avatar May 31 '22 15:05 alihassan143

@stuartmorgan can you try to run again the ci test i am getting this error for ios also in native tests

I'm not sure what you are asking for here; CI tests are run automatically every time you push changes. Please link to the specific line in the Cirrus logs that you are referring to, because I've looked at the logs and don't see the errors you are describing.

stuartmorgan avatar May 31 '22 15:05 stuartmorgan

I'm not sure what you are asking for here; CI tests are run automatically every time you push changes. Please link to the specific line in the Cirrus logs that you are referring to, because I've looked at the logs and don't see the errors you are describing.

you can run the test locally and i am running flutter ci test locally that mentioned in documentation

alihassan143 avatar May 31 '22 15:05 alihassan143

@stuartmorgan can you try to run again the ci test i am getting this error for ios also in native tests

I'm not sure what you are asking for here; CI tests are run automatically every time you push changes. Please link to the specific line in the Cirrus logs that you are referring to, because I've looked at the logs and don't see the errors you are describing.

@stuartmorgan also in the automatic ci test getting the same error

alihassan143 avatar May 31 '22 16:05 alihassan143

@stuartmorgan also in the automatic ci test getting the same error

Again, I'm not seeing that, so please provide the specific link (by opening the Cirrus log view, clicking the relevant line, and then copying the resulting address).

stuartmorgan avatar May 31 '22 17:05 stuartmorgan

@stuartmorgan For more information, see https://blog.cocoapods.org and the CHANGELOG for this version at https://github.com/CocoaPods/CocoaPods/releases/tag/1.11.3

  CDN: trunk Relative path: CocoaPods-version.yml exists! Returning local because checking is only performed in repo update
[!] CocoaPods could not find compatible versions for pod "in_app_purchase_storekit":
  In Podfile:
    in_app_purchase_storekit (from `Flutter/ephemeral/.symlinks/plugins/in_app_purchase_storekit/macos`)

Specs satisfying the `in_app_purchase_storekit (from `Flutter/ephemeral/.symlinks/plugins/in_app_purchase_storekit/macos`)` dependency were found, but they required a higher minimum deployment target.
can you help how to resolve this error we need macos 10.15 for most of the in app purchase apis

alihassan143 avatar May 31 '22 17:05 alihassan143

That error is completely different than the error you quoted earlier and said that you were seeing in the CI. It's very hard to help when you are providing conflicting information.

You can resolve that error by changing the minimum deployment target of the targets in the Runner project and the Podfile.

stuartmorgan avatar May 31 '22 18:05 stuartmorgan

already done this set minimum target in podfile 10.15

alihassan143 avatar May 31 '22 18:05 alihassan143

That error is completely different than the error you quoted earlier and said that you were seeing in the CI. It's very hard to help when you are providing conflicting information.

You can resolve that error by changing the minimum deployment target of the targets in the Runner project and the Podfile.

sorry i was wrong the error is showing only in local tests i don't know why

alihassan143 avatar May 31 '22 18:05 alihassan143

I see; that error is specific to build_all_plugins. Please understand that I have a lot of other demands on my time, including many other PRs to review, so the amount of time that I can devote to reviewing this PR is limited. When I ask for a link to the specific error line (and provide instructions for doing so), please actually do that instead of just quoting part of an error and expecting me to sift through the many CI failures this PR currently has every time in order to find the one you are talking about.

build_all_plugins uses a generated app, so will require some tooling adjustments to handle this correctly. Once you have fixed all of the other failures I can address build_all_plugins.

stuartmorgan avatar May 31 '22 18:05 stuartmorgan

I see; that error is specific to build_all_plugins. Please understand that I have a lot of other demands on my time, including many other PRs to review, so the amount of time that I can devote to reviewing this PR is limited. When I ask for a link to the specific error line (and provide instructions for doing so), please actually do that instead of just quoting part of an error and expecting me to sift through the many CI failures this PR currently has every time in order to find the one you are talking about.

build_all_plugins uses a generated app, so will require some tooling adjustments to handle this correctly. Once you have fixed all of the other failures I can address build_all_plugins.

ok thanks i will do all other things done

alihassan143 avatar May 31 '22 19:05 alihassan143

@stuartmorgan
darwin-lint_podspecs getting error in drain lints can you help me to fix this( https://github.com/flutter/plugins/pull/5854/checks?check_run_id=6684439956) #format error I tried the commands provided in the documentations that format the code but getting the same issue (https://github.com/flutter/plugins/pull/5854/checks?check_run_id=6684439927) macOS build failed pod is not installing (https://github.com/flutter/plugins/pull/5854/checks?check_run_id=6684439966) kindly help me is these error i am not getting any solution how to solve them

macOS tests did not even starting because of some cocopods issue(

https://github.com/flutter/plugins/pull/5854/checks?check_run_id=6684439946)

macOS test for stable ( https://github.com/flutter/plugins/pull/5854/checks?check_run_id=6684439932) Publishable issue( https://github.com/flutter/plugins/pull/5854/checks?check_run_id=6684439933) macOS build failed for all plugin( https://github.com/flutter/plugins/pull/5854/checks?check_run_id=6684439951)

Submit queue issue ( https://api.github.com/repos/flutter/plugins/check-suites/6737496588/check-runs)

alihassan143 avatar Jun 01 '22 09:06 alihassan143

@stuartmorgan all other tests are passing

alihassan143 avatar Jun 01 '22 09:06 alihassan143

@stuartmorgan i ran the unit tests and all unit tests are passing

alihassan143 avatar Jun 01 '22 09:06 alihassan143

@alihassan143 I can help answer specific questions, but I can't investigate and fix every issue from scratch; this is a low-priority feature, so the amount of time we can spend on a PR for it is, as I said above, limited.

As a starting point for your investigation: you appear to have used absoluted paths for your symlinks, which means this PR won't work on any machine except yours.

stuartmorgan avatar Jun 01 '22 11:06 stuartmorgan

@alihassan143 I can help answer specific questions, but I can't investigate and fix every issue from scratch; this is a low-priority feature, so the amount of time we can spend on a PR for it is, as I said above, limited.

As a starting point for your investigation: you appear to have used absoluted paths for your symlinks, which means this PR won't work on any machine except yours.

oh i see thanks i will add path that is accessible by every machine

alihassan143 avatar Jun 01 '22 11:06 alihassan143

@stuartmorgan getting error in ci Select a development team in the Signing & Capabilities editor for build https://github.com/flutter/plugins/pull/5854/checks?check_run_id=6750599811

alihassan143 avatar Jun 06 '22 05:06 alihassan143

@stuartmorgan getting error in ci Select a development team in the Signing & Capabilities editor for build https://github.com/flutter/plugins/pull/5854/checks?check_run_id=6750599811

The line right before that says:

error: Signing for "Runner" requires a development team.

Our CI isn't set up to sign applications. Why have you enabled signing?

Also, note that the implementation package itself still doesn't even build, which seems like a more fundamental issue: https://cirrus-ci.com/task/6046547497451520?logs=build#L64

stuartmorgan avatar Jun 06 '22 10:06 stuartmorgan

i didn't enable the signing option it setup to none like other

alihassan143 avatar Jun 06 '22 10:06 alihassan143

@stuartmorgan getting error in ci Select a development team in the Signing & Capabilities editor for build https://github.com/flutter/plugins/pull/5854/checks?check_run_id=6750599811

The line right before that says:

error: Signing for "Runner" requires a development team.

Our CI isn't set up to sign applications. Why have you enabled signing?

Also, note that the implementation package itself still doesn't even build, which seems like a more fundamental issue: https://cirrus-ci.com/task/6046547497451520?logs=build#L64

when i am trying this code in real device and real project it working fine without any errors

alihassan143 avatar Jun 06 '22 10:06 alihassan143

@stuartmorgan this feature to enable in app purchase for macos is very important i don't know why flutter team is not woking on it please if you help me to find some issue that will enable pr to be merged

alihassan143 avatar Jun 06 '22 10:06 alihassan143

i didn't enable the signing option it setup to none like other

Every other example application in our repository builds successfully, so something about the signing setup of the one you've added is different. I suggest comparing settings with another, working example project.

when i am trying this code in real device and real project it working fine without any errors

I'm not sure what you mean by "real device", but the example application in the implementation package is a real project, and it doesn't compile.

Working in customized local testing on your machine is a good start, but a PR needs to work everywhere--especially our CI.

i don't know why flutter team is not woking on it

Because there is significant amount of higher-priority work at the moment.

please if you help me to find some issue that will enable pr to be merged

I have pointed out a number of issues; fully debugging and resolving all of the issues, however, is beyond the scope of how much time we can devote to this right now.

stuartmorgan avatar Jun 06 '22 15:06 stuartmorgan