Uploaded image for project: 'Apache Cordova'
  1. Apache Cordova
  2. CB-8084

Push Notifications code included by default

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.5.0
    • Fix Version/s: 3.8.0
    • Component/s: cordova-ios
    • Labels:
      None

      Description

      When creating a new Cordova app using the CLI, it adds code to AppDelegate.m for Push Notifications. This causes Apple to issue a warning when you submit the app, if your provisioning profile doesn't have Push Notifications enabled:

      Missing Push Notification Entitlement - Your app appears to include API used to register with the Apple Push Notification service, but the app signature's entitlements do not include the "aps-environment" entitlement. If your app uses the Apple Push Notification service, make sure your App ID is enabled for Push Notification in the Provisioning Portal, and resubmit after signing your app with a Distribution provisioning profile that includes the "aps-environment" entitlement. See "Provisioning and Development" in the Local and Push Notification Programming Guide for more information. If your app does not use the Apple Push Notification service, no action is required. You may remove the API from future submissions to stop this warning. If you use a third-party framework, you may need to contact the developer for information on removing the API.

        Issue Links

          Activity

          Hide
          cojomojo Cody Balos added a comment -

          That was my point exactly, this is not a cordova issue, its an Apple issue.

          Show
          cojomojo Cody Balos added a comment - That was my point exactly, this is not a cordova issue, its an Apple issue.
          Hide
          shazron Shazron Abdullah added a comment -

          I verified like I mentioned that it does not occur in the binary (so it is compliant, and fixes this specific issue), not in an App Store submission. Not sure what else I can do for this issue regarding an external issue with Apple's static analyzer though.

          Show
          shazron Shazron Abdullah added a comment - I verified like I mentioned that it does not occur in the binary (so it is compliant, and fixes this specific issue), not in an App Store submission. Not sure what else I can do for this issue regarding an external issue with Apple's static analyzer though.
          Hide
          cojomojo Cody Balos added a comment - - edited

          When you say 100% verified, does this mean you have you submitted an app to app store to verify as well? Because I don't think this is a matter of there still being push notification symbols in the binary (it is a bug with Apple's analyzer most likely). I was applying the macro using a build hook, and it successfully was preventing the warning from Apple until about a 3 weeks ago (right around when people started reporting that they were receiving the warning for all builds). In addition, it is worth noting the surge in non Cordova developers reporting the issue, thus all signs point to this being a bug with Apple's analyzer.

          Show
          cojomojo Cody Balos added a comment - - edited When you say 100% verified, does this mean you have you submitted an app to app store to verify as well? Because I don't think this is a matter of there still being push notification symbols in the binary (it is a bug with Apple's analyzer most likely). I was applying the macro using a build hook, and it successfully was preventing the warning from Apple until about a 3 weeks ago (right around when people started reporting that they were receiving the warning for all builds). In addition, it is worth noting the surge in non Cordova developers reporting the issue, thus all signs point to this being a bug with Apple's analyzer.
          Hide
          shazron Shazron Abdullah added a comment -

          Looks like some of you are not applying the macro properly. I've 100% verified by going into the symbol files that if the macro is properly set, you will not get any of the push notif symbols in your binary.

          Show
          shazron Shazron Abdullah added a comment - Looks like some of you are not applying the macro properly. I've 100% verified by going into the symbol files that if the macro is properly set, you will not get any of the push notif symbols in your binary.
          Hide
          sdobrev Stefan Dobrev added a comment -

          It looks like Apple has recently updated their static analyzer and are now giving this warning all the time. More information can be found here: https://forums.developer.apple.com/thread/15011

          Show
          sdobrev Stefan Dobrev added a comment - It looks like Apple has recently updated their static analyzer and are now giving this warning all the time. More information can be found here: https://forums.developer.apple.com/thread/15011
          Hide
          mcassidygamma Martin Cassidy added a comment -

          I too have just upgraded to 5.1.1 and have received this email again.

          Show
          mcassidygamma Martin Cassidy added a comment - I too have just upgraded to 5.1.1 and have received this email again.
          Hide
          gabriel.zea Gabriel Zea added a comment -

          I just uploaded an app to iTunes Connect and received again the warning about the Missing Push Notification Entitlement. I'm using cordova 5.1.1

          Show
          gabriel.zea Gabriel Zea added a comment - I just uploaded an app to iTunes Connect and received again the warning about the Missing Push Notification Entitlement. I'm using cordova 5.1.1
          Hide
          shazron Shazron Abdullah added a comment -

          Read the comments starting from Feb 20/2015.
          You'll have to add that macro in your build.xcconfig. This has been disabled in cordova-ios 4.0.x, it has not been disabled in 3.8/3.9 because of backward compatibility reasons.

          Show
          shazron Shazron Abdullah added a comment - Read the comments starting from Feb 20/2015. You'll have to add that macro in your build.xcconfig. This has been disabled in cordova-ios 4.0.x, it has not been disabled in 3.8/3.9 because of backward compatibility reasons.
          Hide
          Filipe Freitas Filipe added a comment -

          I also got cordova iOS platform version 3.8.0 and the issue is still there.

          Show
          Filipe Freitas Filipe added a comment - I also got cordova iOS platform version 3.8.0 and the issue is still there.
          Hide
          vinbtechdf Vinoth Kumar added a comment -

          I updated Cordova to 3.8.0 but still i am getting this notification when I submit the app to the app store.

          Do I need to do anything?

          Show
          vinbtechdf Vinoth Kumar added a comment - I updated Cordova to 3.8.0 but still i am getting this notification when I submit the app to the app store. Do I need to do anything?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user shazron commented on the pull request:

          https://github.com/apache/cordova-ios/pull/128#issuecomment-86250795

          I don't, but you can probably modify this to do what you want: http://stackoverflow.com/a/26563642/219684

          Show
          githubbot ASF GitHub Bot added a comment - Github user shazron commented on the pull request: https://github.com/apache/cordova-ios/pull/128#issuecomment-86250795 I don't, but you can probably modify this to do what you want: http://stackoverflow.com/a/26563642/219684
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user boboldehampsink commented on the pull request:

          https://github.com/apache/cordova-ios/pull/128#issuecomment-82383984

          Do you have an example for a build hook that implements

          `GCC_PREPROCESSOR_DEFINITIONS = $(inherited) DISABLE_PUSH_NOTIFICATIONS=1`

          Show
          githubbot ASF GitHub Bot added a comment - Github user boboldehampsink commented on the pull request: https://github.com/apache/cordova-ios/pull/128#issuecomment-82383984 Do you have an example for a build hook that implements `GCC_PREPROCESSOR_DEFINITIONS = $(inherited) DISABLE_PUSH_NOTIFICATIONS=1`
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user shazron commented on the pull request:

          https://github.com/apache/cordova-ios/pull/128#issuecomment-75339637

          Note that the fix enables it to be disabled using a build.xcconfig preprocessor macro definition (which can be done through a project/plugin hook). In 3.8.0 it still behaves like 3.7.0 by default for backwards compatibility reasons (in 4.0.0, it will be disabled)

          Show
          githubbot ASF GitHub Bot added a comment - Github user shazron commented on the pull request: https://github.com/apache/cordova-ios/pull/128#issuecomment-75339637 Note that the fix enables it to be disabled using a build.xcconfig preprocessor macro definition (which can be done through a project/plugin hook). In 3.8.0 it still behaves like 3.7.0 by default for backwards compatibility reasons (in 4.0.0, it will be disabled)
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Mitko-Kerezov closed the pull request at:

          https://github.com/apache/cordova-ios/pull/128

          Show
          githubbot ASF GitHub Bot added a comment - Github user Mitko-Kerezov closed the pull request at: https://github.com/apache/cordova-ios/pull/128
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Mitko-Kerezov commented on the pull request:

          https://github.com/apache/cordova-ios/pull/128#issuecomment-75227834

          Issue was fixed with https://github.com/apache/cordova-ios/commit/8dcc9dd626c523e6e217dd7e8ce43d0af849fa3a
          So I'm closing this now

          Show
          githubbot ASF GitHub Bot added a comment - Github user Mitko-Kerezov commented on the pull request: https://github.com/apache/cordova-ios/pull/128#issuecomment-75227834 Issue was fixed with https://github.com/apache/cordova-ios/commit/8dcc9dd626c523e6e217dd7e8ce43d0af849fa3a So I'm closing this now
          Hide
          shazron Shazron Abdullah added a comment -

          See issue links. Depends on CB-8254 for a script change (xcconfig use when building for emulator)

          Show
          shazron Shazron Abdullah added a comment - See issue links. Depends on CB-8254 for a script change (xcconfig use when building for emulator)
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 8dcc9dd626c523e6e217dd7e8ce43d0af849fa3a in cordova-ios's branch refs/heads/master from Shazron Abdullah
          [ https://git-wip-us.apache.org/repos/asf?p=cordova-ios.git;h=8dcc9dd ]

          CB-8084 - Allow for a way to disable push notification delegate methods (through xcconfig). Style fixup using uncrustify.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 8dcc9dd626c523e6e217dd7e8ce43d0af849fa3a in cordova-ios's branch refs/heads/master from Shazron Abdullah [ https://git-wip-us.apache.org/repos/asf?p=cordova-ios.git;h=8dcc9dd ] CB-8084 - Allow for a way to disable push notification delegate methods (through xcconfig). Style fixup using uncrustify.
          Hide
          shazron Shazron Abdullah added a comment -

          I've tested it, and the approach I gave above works well (#ifndef and build.xcconfig pre-processor macro). Verified by checking for existence of the delegate symbols using the nm tool.

          I'll add the macro for cordova-ios 3.8.0, will leave the plugin for later since that is not critical for a platform release.

          Show
          shazron Shazron Abdullah added a comment - I've tested it, and the approach I gave above works well (#ifndef and build.xcconfig pre-processor macro). Verified by checking for existence of the delegate symbols using the nm tool. I'll add the macro for cordova-ios 3.8.0, will leave the plugin for later since that is not critical for a platform release.
          Hide
          shazron Shazron Abdullah added a comment -

          I've created CB-8473 (see links section) with dependent links (3 issues in all) regarding what we are doing for this in cordova-ios 4.0

          Show
          shazron Shazron Abdullah added a comment - I've created CB-8473 (see links section) with dependent links (3 issues in all) regarding what we are doing for this in cordova-ios 4.0
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user shazron commented on the pull request:

          https://github.com/apache/cordova-ios/pull/128#issuecomment-74192861

          See the latest comment in https://issues.apache.org/jira/browse/CB-8084 and please continue the discussion there, thanks.

          Show
          githubbot ASF GitHub Bot added a comment - Github user shazron commented on the pull request: https://github.com/apache/cordova-ios/pull/128#issuecomment-74192861 See the latest comment in https://issues.apache.org/jira/browse/CB-8084 and please continue the discussion there, thanks.
          Hide
          shazron Shazron Abdullah added a comment - - edited

          I'm reluctant to put this code in since it will be ripped out later in 4.0. The 3.8.0 release is long overdue and since it doesn't really affect upload (warning for now) I'd rather leave it be for now.

          What I can do is wrap the remote/local notification code in the AppDelegate with:

          #ifndef DISABLE_PUSH_NOTIFICATIONS
          // delegate code here 
          #endif
          

          This means by default the delegate functions are enabled, like always (backwards compatible). Through a build hook (which can be installed through a plugin, or just copied into the project), the code that was in the pull request can be run where it adds the:

          GCC_PREPROCESSOR_DEFINITIONS = $(inherited) DISABLE_PUSH_NOTIFICATIONS=1
          

          ...in the platforms/ios/cordova/build.xcconfig file.

          Show
          shazron Shazron Abdullah added a comment - - edited I'm reluctant to put this code in since it will be ripped out later in 4.0. The 3.8.0 release is long overdue and since it doesn't really affect upload (warning for now) I'd rather leave it be for now. What I can do is wrap the remote/local notification code in the AppDelegate with: #ifndef DISABLE_PUSH_NOTIFICATIONS // delegate code here #endif This means by default the delegate functions are enabled, like always (backwards compatible). Through a build hook (which can be installed through a plugin, or just copied into the project), the code that was in the pull request can be run where it adds the: GCC_PREPROCESSOR_DEFINITIONS = $(inherited) DISABLE_PUSH_NOTIFICATIONS=1 ...in the platforms/ios/cordova/build.xcconfig file.
          Hide
          sdobrev Stefan Dobrev added a comment -

          Actually it does not prevent an App Store upload - it just generates a warning on their side and you got an email with the warning. We had a couple of customers complaining about this on our side and it also looks like a common issue in PhoneGap Build (http://community.phonegap.com/nitobi/searches?query=+Missing+Push+Notification+Entitlement&x=0&y=0&style=topics)

          Shazron Abdullah You are right that the elegant solution with the swizzling code will be a breaking change. What do you think about merging it like this in the next 3.X release and having it done right in the 4.0 line?

          Show
          sdobrev Stefan Dobrev added a comment - Actually it does not prevent an App Store upload - it just generates a warning on their side and you got an email with the warning. We had a couple of customers complaining about this on our side and it also looks like a common issue in PhoneGap Build ( http://community.phonegap.com/nitobi/searches?query=+Missing+Push+Notification+Entitlement&x=0&y=0&style=topics ) Shazron Abdullah You are right that the elegant solution with the swizzling code will be a breaking change. What do you think about merging it like this in the next 3.X release and having it done right in the 4.0 line?
          Hide
          shazron Shazron Abdullah added a comment -

          If this prevents upload to the App Store, we certainly should remove it. I'm not sure the impact of removing it, we definitely have to come up with a plugin alternative before doing so (perhaps a plugin in cordova-plugins). Related: CB-5601

          This fix seems to be more the domain of plugins, and not part of the core. The script can be part of a post-install hook of a plugin. It's not as simple as that however, if we want to be backwards compatible with previous versions.

          So I'm for removing those app delegate lines – put the swizzling code in a cordova-plugins repo plugin that we support, and provide instructions on how to install it for those that want to opt to use this functionality. Therefore cordova-ios in the future will not have this "built-in" by default, thus solving the issue.

          Show
          shazron Shazron Abdullah added a comment - If this prevents upload to the App Store, we certainly should remove it. I'm not sure the impact of removing it, we definitely have to come up with a plugin alternative before doing so (perhaps a plugin in cordova-plugins). Related: CB-5601 This fix seems to be more the domain of plugins, and not part of the core. The script can be part of a post-install hook of a plugin. It's not as simple as that however, if we want to be backwards compatible with previous versions. So I'm for removing those app delegate lines – put the swizzling code in a cordova-plugins repo plugin that we support, and provide instructions on how to install it for those that want to opt to use this functionality. Therefore cordova-ios in the future will not have this "built-in" by default, thus solving the issue.
          Hide
          agrieve Andrew Grieve added a comment -

          Shazron Abdullah - This certainly is a problem that's bitten me when uploading an iOS cordova app to the store. WDYT about this approach?

          I was thinking that we should rather just empty out the default application delegate template and tell plugin authors to use swizzling to add what they want. Might be a good 4.0 change?

          Show
          agrieve Andrew Grieve added a comment - Shazron Abdullah - This certainly is a problem that's bitten me when uploading an iOS cordova app to the store. WDYT about this approach? I was thinking that we should rather just empty out the default application delegate template and tell plugin authors to use swizzling to add what they want. Might be a good 4.0 change?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Mitko-Kerezov commented on the pull request:

          https://github.com/apache/cordova-ios/pull/128#issuecomment-73229125

          Thanks, @ligaz - comments addressed.
          Hey @shazron or @agrieve could someone please take a look at this

          Show
          githubbot ASF GitHub Bot added a comment - Github user Mitko-Kerezov commented on the pull request: https://github.com/apache/cordova-ios/pull/128#issuecomment-73229125 Thanks, @ligaz - comments addressed. Hey @shazron or @agrieve could someone please take a look at this
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ligaz commented on a diff in the pull request:

          https://github.com/apache/cordova-ios/pull/128#discussion_r24230979

          — Diff: bin/templates/scripts/cordova/lib/push-notifications.js —
          @@ -0,0 +1,82 @@
          +(function () {
          + var fs = require('fs'),
          + path = require('path'),
          + exec = require('child_process').exec,
          + PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE = 'PushNotificationsEnabled.h',
          + PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE_CONTENT = '#define PUSH_NOTIFICATIONS_ENABLED ',
          + EXPANDED_PROVISIONING_PROFILE = process.env.EXPANDED_PROVISIONING_PROFILE,
          + PATH_TO_MOBILE_PROVISIONS = path.join(process.env.HOME, 'Library', 'MobileDevice', 'Provisioning Profiles'),
          + PROVISIONING_PROFILE_REQUIRED = process.env.PROVISIONING_PROFILE_REQUIRED,
          + PROVISIONING_PROFILE_UUID_REGEX = new RegExp('<key>UUID<\/key>\\s\\S*<string>' + EXPANDED_PROVISIONING_PROFILE + '<\/string>');
          +
          + function logErrorIfExists(error) {
          + if (error !== null)

          { + console.error('ERROR: ' + error); + }

          + }
          +
          + function sanitizeMobileProvision(mobileProvision) {
          + if (mobileProvision.indexOf('.mobileprovision') === -1)

          { + mobileProvision += '.mobileprovision'; + }

          +
          + return mobileProvision;
          + }
          +
          + function createPushNotificationsEnabledHeaderFile(pushPluginsEnabled)

          { + var cordovaEnablePluginHeaderFileLocation = path.join(__dirname, '..', '..', process.env.PROJECT_NAME, 'Classes', PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE); + fs.writeFileSync(cordovaEnablePluginHeaderFileLocation, PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE_CONTENT + pushPluginsEnabled); + }

          +
          + function createPushNotificationsEnabledHeaderFileWithMobileProvision(mobileProvision) {
          + var pathToProvisioningProfile = path.join(PATH_TO_MOBILE_PROVISIONS, sanitizeMobileProvision(mobileProvision));
          +
          + exec("security cms -D -i \"" + pathToProvisioningProfile + "\"", function (error, stdout, stderr)

          { + logErrorIfExists(error); + var pushPluginsEnabled = stdout.indexOf("<key>aps-environment</key>") > -1; + createPushNotificationsEnabledHeaderFile(pushPluginsEnabled); + }

          );
          + }
          +
          + function isprovisionUUIDSuitable(provisionContents)

          { + return PROVISIONING_PROFILE_UUID_REGEX.test(provisionContents); + }

          +
          + function findValidMobileProvision(provisions, successCallback, errorCallback) {
          — End diff –

          If you want to follow the node convention the errorCallback should be passed before the success one.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ligaz commented on a diff in the pull request: https://github.com/apache/cordova-ios/pull/128#discussion_r24230979 — Diff: bin/templates/scripts/cordova/lib/push-notifications.js — @@ -0,0 +1,82 @@ +(function () { + var fs = require('fs'), + path = require('path'), + exec = require('child_process').exec, + PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE = 'PushNotificationsEnabled.h', + PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE_CONTENT = '#define PUSH_NOTIFICATIONS_ENABLED ', + EXPANDED_PROVISIONING_PROFILE = process.env.EXPANDED_PROVISIONING_PROFILE, + PATH_TO_MOBILE_PROVISIONS = path.join(process.env.HOME, 'Library', 'MobileDevice', 'Provisioning Profiles'), + PROVISIONING_PROFILE_REQUIRED = process.env.PROVISIONING_PROFILE_REQUIRED, + PROVISIONING_PROFILE_UUID_REGEX = new RegExp('<key>UUID<\/key> \\s\\S *<string>' + EXPANDED_PROVISIONING_PROFILE + '<\/string>'); + + function logErrorIfExists(error) { + if (error !== null) { + console.error('ERROR: ' + error); + } + } + + function sanitizeMobileProvision(mobileProvision) { + if (mobileProvision.indexOf('.mobileprovision') === -1) { + mobileProvision += '.mobileprovision'; + } + + return mobileProvision; + } + + function createPushNotificationsEnabledHeaderFile(pushPluginsEnabled) { + var cordovaEnablePluginHeaderFileLocation = path.join(__dirname, '..', '..', process.env.PROJECT_NAME, 'Classes', PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE); + fs.writeFileSync(cordovaEnablePluginHeaderFileLocation, PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE_CONTENT + pushPluginsEnabled); + } + + function createPushNotificationsEnabledHeaderFileWithMobileProvision(mobileProvision) { + var pathToProvisioningProfile = path.join(PATH_TO_MOBILE_PROVISIONS, sanitizeMobileProvision(mobileProvision)); + + exec("security cms -D -i \"" + pathToProvisioningProfile + "\"", function (error, stdout, stderr) { + logErrorIfExists(error); + var pushPluginsEnabled = stdout.indexOf("<key>aps-environment</key>") > -1; + createPushNotificationsEnabledHeaderFile(pushPluginsEnabled); + } ); + } + + function isprovisionUUIDSuitable(provisionContents) { + return PROVISIONING_PROFILE_UUID_REGEX.test(provisionContents); + } + + function findValidMobileProvision(provisions, successCallback, errorCallback) { — End diff – If you want to follow the node convention the errorCallback should be passed before the success one.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ligaz commented on a diff in the pull request:

          https://github.com/apache/cordova-ios/pull/128#discussion_r24230911

          — Diff: bin/templates/scripts/cordova/lib/push-notifications.js —
          @@ -0,0 +1,82 @@
          +(function () {
          + var fs = require('fs'),
          + path = require('path'),
          + exec = require('child_process').exec,
          + PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE = 'PushNotificationsEnabled.h',
          + PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE_CONTENT = '#define PUSH_NOTIFICATIONS_ENABLED ',
          + EXPANDED_PROVISIONING_PROFILE = process.env.EXPANDED_PROVISIONING_PROFILE,
          + PATH_TO_MOBILE_PROVISIONS = path.join(process.env.HOME, 'Library', 'MobileDevice', 'Provisioning Profiles'),
          + PROVISIONING_PROFILE_REQUIRED = process.env.PROVISIONING_PROFILE_REQUIRED,
          + PROVISIONING_PROFILE_UUID_REGEX = new RegExp('<key>UUID<\/key>\\s\\S*<string>' + EXPANDED_PROVISIONING_PROFILE + '<\/string>');
          +
          + function logErrorIfExists(error) {
          + if (error !== null)

          { + console.error('ERROR: ' + error); + }

          + }
          +
          + function sanitizeMobileProvision(mobileProvision) {
          + if (mobileProvision.indexOf('.mobileprovision') === -1)

          { + mobileProvision += '.mobileprovision'; + }

          +
          + return mobileProvision;
          + }
          +
          + function createPushNotificationsEnabledHeaderFile(pushPluginsEnabled)

          { + var cordovaEnablePluginHeaderFileLocation = path.join(__dirname, '..', '..', process.env.PROJECT_NAME, 'Classes', PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE); + fs.writeFileSync(cordovaEnablePluginHeaderFileLocation, PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE_CONTENT + pushPluginsEnabled); + }

          +
          + function createPushNotificationsEnabledHeaderFileWithMobileProvision(mobileProvision) {
          + var pathToProvisioningProfile = path.join(PATH_TO_MOBILE_PROVISIONS, sanitizeMobileProvision(mobileProvision));
          +
          + exec("security cms -D -i \"" + pathToProvisioningProfile + "\"", function (error, stdout, stderr)

          { + logErrorIfExists(error); + var pushPluginsEnabled = stdout.indexOf("<key>aps-environment</key>") > -1; + createPushNotificationsEnabledHeaderFile(pushPluginsEnabled); + }

          );
          + }
          +
          + function isprovisionUUIDSuitable(provisionContents) {
          — End diff –

          Maybe the `p` should be capitalized here.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ligaz commented on a diff in the pull request: https://github.com/apache/cordova-ios/pull/128#discussion_r24230911 — Diff: bin/templates/scripts/cordova/lib/push-notifications.js — @@ -0,0 +1,82 @@ +(function () { + var fs = require('fs'), + path = require('path'), + exec = require('child_process').exec, + PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE = 'PushNotificationsEnabled.h', + PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE_CONTENT = '#define PUSH_NOTIFICATIONS_ENABLED ', + EXPANDED_PROVISIONING_PROFILE = process.env.EXPANDED_PROVISIONING_PROFILE, + PATH_TO_MOBILE_PROVISIONS = path.join(process.env.HOME, 'Library', 'MobileDevice', 'Provisioning Profiles'), + PROVISIONING_PROFILE_REQUIRED = process.env.PROVISIONING_PROFILE_REQUIRED, + PROVISIONING_PROFILE_UUID_REGEX = new RegExp('<key>UUID<\/key> \\s\\S *<string>' + EXPANDED_PROVISIONING_PROFILE + '<\/string>'); + + function logErrorIfExists(error) { + if (error !== null) { + console.error('ERROR: ' + error); + } + } + + function sanitizeMobileProvision(mobileProvision) { + if (mobileProvision.indexOf('.mobileprovision') === -1) { + mobileProvision += '.mobileprovision'; + } + + return mobileProvision; + } + + function createPushNotificationsEnabledHeaderFile(pushPluginsEnabled) { + var cordovaEnablePluginHeaderFileLocation = path.join(__dirname, '..', '..', process.env.PROJECT_NAME, 'Classes', PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE); + fs.writeFileSync(cordovaEnablePluginHeaderFileLocation, PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE_CONTENT + pushPluginsEnabled); + } + + function createPushNotificationsEnabledHeaderFileWithMobileProvision(mobileProvision) { + var pathToProvisioningProfile = path.join(PATH_TO_MOBILE_PROVISIONS, sanitizeMobileProvision(mobileProvision)); + + exec("security cms -D -i \"" + pathToProvisioningProfile + "\"", function (error, stdout, stderr) { + logErrorIfExists(error); + var pushPluginsEnabled = stdout.indexOf("<key>aps-environment</key>") > -1; + createPushNotificationsEnabledHeaderFile(pushPluginsEnabled); + } ); + } + + function isprovisionUUIDSuitable(provisionContents) { — End diff – Maybe the `p` should be capitalized here.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ligaz commented on a diff in the pull request:

          https://github.com/apache/cordova-ios/pull/128#discussion_r24230887

          — Diff: bin/templates/scripts/cordova/lib/push-notifications.js —
          @@ -0,0 +1,82 @@
          +(function () {
          + var fs = require('fs'),
          + path = require('path'),
          + exec = require('child_process').exec,
          + PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE = 'PushNotificationsEnabled.h',
          + PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE_CONTENT = '#define PUSH_NOTIFICATIONS_ENABLED ',
          + EXPANDED_PROVISIONING_PROFILE = process.env.EXPANDED_PROVISIONING_PROFILE,
          + PATH_TO_MOBILE_PROVISIONS = path.join(process.env.HOME, 'Library', 'MobileDevice', 'Provisioning Profiles'),
          + PROVISIONING_PROFILE_REQUIRED = process.env.PROVISIONING_PROFILE_REQUIRED,
          + PROVISIONING_PROFILE_UUID_REGEX = new RegExp('<key>UUID<\/key>\\s\\S*<string>' + EXPANDED_PROVISIONING_PROFILE + '<\/string>');
          +
          + function logErrorIfExists(error) {
          + if (error !== null)

          { + console.error('ERROR: ' + error); + }

          + }
          +
          + function sanitizeMobileProvision(mobileProvision) {
          + if (mobileProvision.indexOf('.mobileprovision') === -1)

          { + mobileProvision += '.mobileprovision'; + }

          +
          + return mobileProvision;
          + }
          +
          + function createPushNotificationsEnabledHeaderFile(pushPluginsEnabled)

          { + var cordovaEnablePluginHeaderFileLocation = path.join(__dirname, '..', '..', process.env.PROJECT_NAME, 'Classes', PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE); + fs.writeFileSync(cordovaEnablePluginHeaderFileLocation, PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE_CONTENT + pushPluginsEnabled); + }

          +
          + function createPushNotificationsEnabledHeaderFileWithMobileProvision(mobileProvision) {
          + var pathToProvisioningProfile = path.join(PATH_TO_MOBILE_PROVISIONS, sanitizeMobileProvision(mobileProvision));
          +
          + exec("security cms -D -i \"" + pathToProvisioningProfile + "\"", function (error, stdout, stderr) {
          + logErrorIfExists(error);
          + var pushPluginsEnabled = stdout.indexOf("<key>aps-environment</key>") > -1;
          — End diff –

          The variable name should be something like `hasPushNotificationsEntitlement` :smile:

          Show
          githubbot ASF GitHub Bot added a comment - Github user ligaz commented on a diff in the pull request: https://github.com/apache/cordova-ios/pull/128#discussion_r24230887 — Diff: bin/templates/scripts/cordova/lib/push-notifications.js — @@ -0,0 +1,82 @@ +(function () { + var fs = require('fs'), + path = require('path'), + exec = require('child_process').exec, + PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE = 'PushNotificationsEnabled.h', + PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE_CONTENT = '#define PUSH_NOTIFICATIONS_ENABLED ', + EXPANDED_PROVISIONING_PROFILE = process.env.EXPANDED_PROVISIONING_PROFILE, + PATH_TO_MOBILE_PROVISIONS = path.join(process.env.HOME, 'Library', 'MobileDevice', 'Provisioning Profiles'), + PROVISIONING_PROFILE_REQUIRED = process.env.PROVISIONING_PROFILE_REQUIRED, + PROVISIONING_PROFILE_UUID_REGEX = new RegExp('<key>UUID<\/key> \\s\\S *<string>' + EXPANDED_PROVISIONING_PROFILE + '<\/string>'); + + function logErrorIfExists(error) { + if (error !== null) { + console.error('ERROR: ' + error); + } + } + + function sanitizeMobileProvision(mobileProvision) { + if (mobileProvision.indexOf('.mobileprovision') === -1) { + mobileProvision += '.mobileprovision'; + } + + return mobileProvision; + } + + function createPushNotificationsEnabledHeaderFile(pushPluginsEnabled) { + var cordovaEnablePluginHeaderFileLocation = path.join(__dirname, '..', '..', process.env.PROJECT_NAME, 'Classes', PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE); + fs.writeFileSync(cordovaEnablePluginHeaderFileLocation, PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE_CONTENT + pushPluginsEnabled); + } + + function createPushNotificationsEnabledHeaderFileWithMobileProvision(mobileProvision) { + var pathToProvisioningProfile = path.join(PATH_TO_MOBILE_PROVISIONS, sanitizeMobileProvision(mobileProvision)); + + exec("security cms -D -i \"" + pathToProvisioningProfile + "\"", function (error, stdout, stderr) { + logErrorIfExists(error); + var pushPluginsEnabled = stdout.indexOf("<key>aps-environment</key>") > -1; — End diff – The variable name should be something like `hasPushNotificationsEntitlement` :smile:
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ligaz commented on a diff in the pull request:

          https://github.com/apache/cordova-ios/pull/128#discussion_r24230792

          — Diff: bin/templates/scripts/cordova/lib/push-notifications.js —
          @@ -0,0 +1,82 @@
          +(function () {
          + var fs = require('fs'),
          + path = require('path'),
          + exec = require('child_process').exec,
          + PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE = 'PushNotificationsEnabled.h',
          + PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE_CONTENT = '#define PUSH_NOTIFICATIONS_ENABLED ',
          + EXPANDED_PROVISIONING_PROFILE = process.env.EXPANDED_PROVISIONING_PROFILE,
          + PATH_TO_MOBILE_PROVISIONS = path.join(process.env.HOME, 'Library', 'MobileDevice', 'Provisioning Profiles'),
          + PROVISIONING_PROFILE_REQUIRED = process.env.PROVISIONING_PROFILE_REQUIRED,
          + PROVISIONING_PROFILE_UUID_REGEX = new RegExp('<key>UUID<\/key>\\s\\S*<string>' + EXPANDED_PROVISIONING_PROFILE + '<\/string>');
          +
          + function logErrorIfExists(error) {
          + if (error !== null)

          { + console.error('ERROR: ' + error); + }

          + }
          +
          + function sanitizeMobileProvision(mobileProvision) {
          + if (mobileProvision.indexOf('.mobileprovision') === -1)

          { + mobileProvision += '.mobileprovision'; + }

          +
          + return mobileProvision;
          + }
          +
          + function createPushNotificationsEnabledHeaderFile(pushPluginsEnabled) {
          + var cordovaEnablePluginHeaderFileLocation = path.join(__dirname, '..', '..', process.env.PROJECT_NAME, 'Classes', PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE);
          — End diff –

          The dirname of the project file should be available as an environment variable.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ligaz commented on a diff in the pull request: https://github.com/apache/cordova-ios/pull/128#discussion_r24230792 — Diff: bin/templates/scripts/cordova/lib/push-notifications.js — @@ -0,0 +1,82 @@ +(function () { + var fs = require('fs'), + path = require('path'), + exec = require('child_process').exec, + PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE = 'PushNotificationsEnabled.h', + PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE_CONTENT = '#define PUSH_NOTIFICATIONS_ENABLED ', + EXPANDED_PROVISIONING_PROFILE = process.env.EXPANDED_PROVISIONING_PROFILE, + PATH_TO_MOBILE_PROVISIONS = path.join(process.env.HOME, 'Library', 'MobileDevice', 'Provisioning Profiles'), + PROVISIONING_PROFILE_REQUIRED = process.env.PROVISIONING_PROFILE_REQUIRED, + PROVISIONING_PROFILE_UUID_REGEX = new RegExp('<key>UUID<\/key> \\s\\S *<string>' + EXPANDED_PROVISIONING_PROFILE + '<\/string>'); + + function logErrorIfExists(error) { + if (error !== null) { + console.error('ERROR: ' + error); + } + } + + function sanitizeMobileProvision(mobileProvision) { + if (mobileProvision.indexOf('.mobileprovision') === -1) { + mobileProvision += '.mobileprovision'; + } + + return mobileProvision; + } + + function createPushNotificationsEnabledHeaderFile(pushPluginsEnabled) { + var cordovaEnablePluginHeaderFileLocation = path.join(__dirname, '..', '..', process.env.PROJECT_NAME, 'Classes', PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE); — End diff – The dirname of the project file should be available as an environment variable.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ligaz commented on a diff in the pull request:

          https://github.com/apache/cordova-ios/pull/128#discussion_r24230690

          — Diff: bin/templates/scripts/cordova/lib/push-notifications.js —
          @@ -0,0 +1,82 @@
          +(function () {
          + var fs = require('fs'),
          + path = require('path'),
          + exec = require('child_process').exec,
          + PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE = 'PushNotificationsEnabled.h',
          + PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE_CONTENT = '#define PUSH_NOTIFICATIONS_ENABLED ',
          + EXPANDED_PROVISIONING_PROFILE = process.env.EXPANDED_PROVISIONING_PROFILE,
          + PATH_TO_MOBILE_PROVISIONS = path.join(process.env.HOME, 'Library', 'MobileDevice', 'Provisioning Profiles'),
          + PROVISIONING_PROFILE_REQUIRED = process.env.PROVISIONING_PROFILE_REQUIRED,
          + PROVISIONING_PROFILE_UUID_REGEX = new RegExp('<key>UUID<\/key>\\s\\S*<string>' + EXPANDED_PROVISIONING_PROFILE + '<\/string>');
          +
          + function logErrorIfExists(error) {
          + if (error !== null) {
          — End diff –

          You can use just `if(error)` here.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ligaz commented on a diff in the pull request: https://github.com/apache/cordova-ios/pull/128#discussion_r24230690 — Diff: bin/templates/scripts/cordova/lib/push-notifications.js — @@ -0,0 +1,82 @@ +(function () { + var fs = require('fs'), + path = require('path'), + exec = require('child_process').exec, + PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE = 'PushNotificationsEnabled.h', + PUSH_NOTIFICATIONS_ENABLED_HEADER_FILE_CONTENT = '#define PUSH_NOTIFICATIONS_ENABLED ', + EXPANDED_PROVISIONING_PROFILE = process.env.EXPANDED_PROVISIONING_PROFILE, + PATH_TO_MOBILE_PROVISIONS = path.join(process.env.HOME, 'Library', 'MobileDevice', 'Provisioning Profiles'), + PROVISIONING_PROFILE_REQUIRED = process.env.PROVISIONING_PROFILE_REQUIRED, + PROVISIONING_PROFILE_UUID_REGEX = new RegExp('<key>UUID<\/key> \\s\\S *<string>' + EXPANDED_PROVISIONING_PROFILE + '<\/string>'); + + function logErrorIfExists(error) { + if (error !== null) { — End diff – You can use just `if(error)` here.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user Mitko-Kerezov opened a pull request:

          https://github.com/apache/cordova-ios/pull/128

          CB-8084 conform AppDelegate.m with provisioning profile

          Hide the two methods in AppDelegate.m for push notifications
          when provisioning profile doesn't include the push notifications service
          Related to https://issues.apache.org/jira/browse/CB-8084

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/Icenium/cordova-ios kerezov/push-notifications-fix

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/cordova-ios/pull/128.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #128


          commit d0a16ac3916ba775a0267206f676a5102f6bd9f4
          Author: Dimitar Kerezov <dimitar.kerezov@telerik.com>
          Date: 2015-01-27T14:29:04Z

          CB-8084 conform AppDelegate.m with provisioning profile

          Hide the two methods in AppDelegate.m for push notifications
          when provisioning profile doesn't include the push notifications service
          Related to https://issues.apache.org/jira/browse/CB-8084


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user Mitko-Kerezov opened a pull request: https://github.com/apache/cordova-ios/pull/128 CB-8084 conform AppDelegate.m with provisioning profile Hide the two methods in AppDelegate.m for push notifications when provisioning profile doesn't include the push notifications service Related to https://issues.apache.org/jira/browse/CB-8084 You can merge this pull request into a Git repository by running: $ git pull https://github.com/Icenium/cordova-ios kerezov/push-notifications-fix Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-ios/pull/128.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #128 commit d0a16ac3916ba775a0267206f676a5102f6bd9f4 Author: Dimitar Kerezov <dimitar.kerezov@telerik.com> Date: 2015-01-27T14:29:04Z CB-8084 conform AppDelegate.m with provisioning profile Hide the two methods in AppDelegate.m for push notifications when provisioning profile doesn't include the push notifications service Related to https://issues.apache.org/jira/browse/CB-8084
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Mitko-Kerezov closed the pull request at:

          https://github.com/apache/cordova-ios/pull/127

          Show
          githubbot ASF GitHub Bot added a comment - Github user Mitko-Kerezov closed the pull request at: https://github.com/apache/cordova-ios/pull/127
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user Mitko-Kerezov opened a pull request:

          https://github.com/apache/cordova-ios/pull/127

          CB-8084 conform AppDelegate.m with provisioning profile

          Hide the two methods in AppDelegate.m for push notifications
          when provisioning profile doesn't include the push notifications service
          Related to https://issues.apache.org/jira/browse/CB-8084

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/Icenium/cordova-ios kerezov/push-notifications-fix

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/cordova-ios/pull/127.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #127


          commit b5936489cd2239fc0df15ab24751cea9f9959c31
          Author: Dimitar Kerezov <dimitar.kerezov@telerik.com>
          Date: 2015-01-27T14:29:04Z

          CB-8084 conform AppDelegate.m with provisioning profile

          Hide the two methods in AppDelegate.m for push notifications
          when provisioning profile doesn't include the push notifications service
          Related to https://issues.apache.org/jira/browse/CB-8084


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user Mitko-Kerezov opened a pull request: https://github.com/apache/cordova-ios/pull/127 CB-8084 conform AppDelegate.m with provisioning profile Hide the two methods in AppDelegate.m for push notifications when provisioning profile doesn't include the push notifications service Related to https://issues.apache.org/jira/browse/CB-8084 You can merge this pull request into a Git repository by running: $ git pull https://github.com/Icenium/cordova-ios kerezov/push-notifications-fix Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-ios/pull/127.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #127 commit b5936489cd2239fc0df15ab24751cea9f9959c31 Author: Dimitar Kerezov <dimitar.kerezov@telerik.com> Date: 2015-01-27T14:29:04Z CB-8084 conform AppDelegate.m with provisioning profile Hide the two methods in AppDelegate.m for push notifications when provisioning profile doesn't include the push notifications service Related to https://issues.apache.org/jira/browse/CB-8084

            People

            • Assignee:
              shazron Shazron Abdullah
              Reporter:
              jweber Jacob Weber
            • Votes:
              2 Vote for this issue
              Watchers:
              12 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development