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

Change to use PROVISIONING_PROFILE_SPECIFIER for manual signing #823

Closed
wants to merge 1 commit into from

Conversation

chriscant
Copy link
Contributor

For manual signing, when a build is started (using the new build system) platform/ios/cordova/lib/build.js creates file build-extras.xcconfig in platforms/ios/cordova. This incorrectly has
PROVISIONING_PROFILE = profile name
when it needs to have
PROVISIONING_PROFILE_SPECIFIER = profile name

Platforms affected

Cordova ios 5.1.1

Motivation and Context

If using manual signing then the provisioning profile not recognised
#536 (comment)

Description

When manually signing the file build-extras.xcconfig is created to specify extra parameters to send to xcodebuild as part of bin/templates/scripts/cordova/build-debug.xcconfig or build-release.xcconfig.
The provisioning profile name is specified currently for PROVISIONING_PROFILE.
To work, it has to be given for PROVISIONING_PROFILE_SPECIFIER.

Testing

I've run the changed code locally in Xcode 11.4 in Catalina and in Xcode 11.3 in Mojave in a Travis build - see Travis build script here which changes the code using sed.
https://github.com/Freegle/iznik-nuxt/blob/app-ios/.travis.yml

Locally I've tested the code with a profile name and a profile UUID.

The branch passes Travis testing:
https://travis-ci.com/github/chriscant/cordova-ios/builds/159430800

The ios6.0.0-dev master builds fails in cordova-plugin-wkwebview-engine for me on my test environment. Within my branch it fails for the same reason.

As far as I can tell there is no test coverage for PROVISIONING_PROFILE.

Checklist

  • [X ] I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • If this Pull Request resolves an issue, I linked to the issue in the text above
  • I've updated the documentation if necessary

Copy link
Member

@dpogue dpogue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks good to me. Ideally I'd like to hear from some people who were having issues with manual signing, to hear if this solves their problems.

@pepenemo
Copy link

pepenemo commented Apr 23, 2020

@chriscant and @dpogue, I got an issue here: #536 (comment) for details

@erisu
Copy link
Member

erisu commented May 19, 2020

@chriscant I want to confirm,

What do you mean by

This incorrectly has
PROVISIONING_PROFILE = profile name
when it needs to have
PROVISIONING_PROFILE_SPECIFIER = profile name

Because PROVISIONING_PROFILE is not the profile name. When configuring for manual signing in build.json, the provisioningProfile property contains the provisioning profile's UUID and not its name. If you are writing the name in this property, that is incorrect.

When I read the changes from this PR, I feel that you might be changing what the value of the provisioningProfile is suppose to be, from what is in the documentation.

What I read from the changes is PROVISIONING_PROFILE_SPECIFIER = UUID when the PROVISIONING_PROFILE_SPECIFIER is the profile name.

Here is the documentation on manual signing:
https://cordova.apache.org/docs/en/latest/guide/platforms/ios/index.html#using-buildjson

If you want to add PROVISIONING_PROFILE_SPECIFIER as an option, for supplying the name, a new build.json property should also be introduced to support this. Then in the code you can write conditional to set either PROVISIONING_PROFILE_SPECIFIER, PROVISIONING_PROFILE, or both if both is provided.

Copy link
Member

@erisu erisu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Waiting for response in regards to the above comment.

@chriscant
Copy link
Contributor Author

I cannot find any official documentation for PROVISIONING_PROFILE_SPECIFIER so it's hard to be sure what XCode needs. There are appears to be some (old) advice on forums indicating that both PROVISIONING_PROFILE and PROVISIONING_PROFILE_SPECIFIER are needed. However I got it to work by changing from PROVISIONING_PROFILE to PROVISIONING_PROFILE_SPECIFIER.

There is then the question as to whether a profile name or profile UUID should be used. My code works with a profile name. I suspect that a UUID will work as well. I'll try to check that soon, but am quite busy so it might be a few days.

I don't think the build.json needs changing as the manual signing code is picking up buildOpts.provisioningProfile. However the docs may need to be updated to say that either a UUID or profile name works.

@chriscant
Copy link
Contributor Author

I ran a test using my change and referring to a profile by UUID and it worked:
https://travis-ci.com/github/Freegle/iznik-nuxt/builds/169686564

The Travis build copies several mobileprovision files into place in ~/Library/MobileDevice/Provisioning\ Profiles

This command was run and it succeeded at line 6941:

- cordova build --debug --device --codeSignIdentity="iPhone Distribution" --developmentTeam="$iosDeveloper" --packageType="ad-hoc" --automaticProvisioning="false" --provisioningProfile="3539d51d-56be-4241-a9b6-b3c0c1a97848"

@erisu
Copy link
Member

erisu commented Mar 6, 2024

@chriscant Would you mind rebasing the PR against the latest main branch?

Previously, Apple lacked documentation that defined the difference between PROVISIONING_PROFILE_SPECIFIER and PROVISIONING_PROFILE...

Based on comments and the limited available information, it was my understanding that PROVISIONING_PROFILE_SPECIFIER was solely for profile names, while PROVISIONING_PROFILE was for UUIDs.

Given that Cordova documentation strictly focused on UUIDs, the changes seemed like they might have broken manual signing with UUIDs, considering the scarce information available.

Since I've been re-reviewing PRs, I conducted another search today to check if Apple has finally documented anything about PROVISIONING_PROFILE_SPECIFIER. The following information was retrieved from the Apple Developer Docs:

Setting name: PROVISIONING_PROFILE_SPECIFIER

Must contain a profile name (or UUID). A missing or invalid profile will cause a build error. Use in conjunction with [DEVELOPMENT_TEAM] to fully specify the provisioning profile.

It's now clearly documented that it can accept both UUIDs and profile names.

If we can get this rebased, I am ready to merge in the changes.

@erisu
Copy link
Member

erisu commented Mar 6, 2024

@chriscant I went ahead and created a new branch, cherry-picked your commit changes, resolved the conflicts, and merged in the PR.

PR: #1405

The commit of the PR still gives you credit for the work.

Apologies for the delay of getting this merged in.

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

Successfully merging this pull request may close these issues.

None yet

4 participants