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(ios): fix extension app group and entitlements support #10813

Merged
merged 5 commits into from Apr 15, 2019
Merged

fix(ios): fix extension app group and entitlements support #10813

merged 5 commits into from Apr 15, 2019

Conversation

cb1kenobi
Copy link
Contributor

JIRA: https://jira.appcelerator.org/browse/TIMOB-26948

Fixes a typo where it was enabling the app group by setting true instead of 1.

Also correctly reference the entitlements file, specifically when the entitlements is referenced by the CODE_SIGN_ENTITLEMENTS build setting that is a relative path from the extension project's root.

@build build added this to the 8.1.0 milestone Mar 28, 2019
@build
Copy link
Contributor

build commented Mar 28, 2019

Warnings
⚠️

iphone/cli/commands/_build.js#L6436 - iphone/cli/commands/_build.js line 6436 – 'out' is defined but never used. (no-unused-vars)

Messages
📖

💾 Here's the generated SDK zipfile.

📖

✅ All tests are passing
Nice one! All 3647 tests are passing.

Generated by 🚫 dangerJS against 38e0ad7

extBuildSettings.CODE_SIGN_ENTITLEMENTS = '"' + path.join(ext.relPath, targetName, entFile) + '"';
targetInfo.entitlementsFile = path.join(this.buildDir, ext.relPath, targetName, entFile);
} else {
delete extBuildSettings.CODE_SIGN_ENTITLEMENTS;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should spit out a warning here that we couldn't resolve the configured entitlements file instead of just silently deleting the CODE_SIGN_ENTITLEMENTS setting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what I originally thought too, but I realized that not every extension target has entitlements. For example, File Provider UI targets do not have entitlements. So, having a warning leads to false positives and that would be confusing.

Copy link
Contributor

Choose a reason for hiding this comment

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

True, but wouldn't CODE_SIGN_ENTITLEMENTS then be empty anyway and we could check for that? Like an extra parameter if we should print a warning or not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, so in that specific case it should be fine to print a warning, shouldn't it? If CODE_SIGN_ENTITLEMENTS is explicitly set, warn if we can't properly resolve it. If it's not set, e.g. for the File Provider UI extension, that code path is ignored anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a warning. Should be good to go now.

@janvennemann janvennemann changed the title fix(ios): Fixed extension app group and entitlements support. fix(ios): fix extension app group and entitlements support Mar 29, 2019
@keerthi1032
Copy link
Contributor

FR Passed. App group showed and enabled correctly and entitlements files listed correctly.
Name = Mac OS X
Version = 10.13.6
Node.js
Node.js Version = 8.9.1
npm Version = 5.5.1
Titanium CLI
CLI Version = 5.1.1
Studio =5.1.2.201903111843
Titanium SDK
SDK Version = local 8.0.1 sdk and 8.1 master sdk
Cli =7.0.10

@keerthi1032
Copy link
Contributor

Danger is failing and not able to merge. can someone please resolve it

@sgtcoolguy sgtcoolguy merged commit 4b599eb into tidev:master Apr 15, 2019
@cb1kenobi cb1kenobi deleted the TIMOB-26948 branch June 7, 2019 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants