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): ignore duplicate and unnecessary framework search paths #12146

Merged
merged 2 commits into from Oct 1, 2020

Conversation

janvennemann
Copy link
Contributor

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

This addresses two issues with framework search paths:

We used appc.util.mix to assign default build settings to the Debug and Release build configurations in the Xcode project. This would assign the FRAMEWORK_SEARCH_PATHS by references so they both used the same array which caused duplicate entries. Switching to lodash.merge fixes this since it merges arrays recursively by value.

In #12026 recursive search paths were added to handle the nested structure of XCFrameworks. This causes Xcode to construct huge framework search path arguments during the build since it would expand to every subfolder inside native modules. This article suggests that this is unnecessary since Xcode has a special build phase that automatically scans XCFrameworks and automatically selects the appropriate .framework for the current architecture without consulting FRAMEWORK_SEARCH_PATHS. This PR excludes adding recursive framework search paths to the build settings. I've tested it with ti.map and hyperloop and i was still able to build without issues so it indeed seems that the recursive search paths are not required.

@janvennemann
Copy link
Contributor Author

@sgtcoolguy I think this might also be worth considering for 9.2.1

@build build added this to the 9.3.0 milestone Oct 1, 2020
@build
Copy link
Contributor

build commented Oct 1, 2020

Warnings
⚠️

iphone/cli/hooks/frameworks.js#L67 - iphone/cli/hooks/frameworks.js line 67 – Avoid calling back inside of a promise. (promise/no-callback-in-promise)

⚠️

iphone/cli/hooks/frameworks.js#L67 - iphone/cli/hooks/frameworks.js line 67 – Expected catch() or return (promise/catch-or-return)

⚠️

iphone/cli/hooks/frameworks.js#L71 - iphone/cli/hooks/frameworks.js line 71 – Avoid calling back inside of a promise. (promise/no-callback-in-promise)

Messages
📖

💾 Here's the generated SDK zipfile.

📖 ✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.
📖

✅ All tests are passing
Nice one! All 9913 tests are passing.
(There are 979 skipped tests not included in that total)

New dependencies added: lodash.merge.

lodash.merge

Author: John-David Dalton

Description: The Lodash method `_.merge` exported as a module.

Homepage: https://lodash.com/

Createdabout 7 years ago
Last Updatedabout 1 year ago
LicenseMIT
Maintainers2
Releases37
Keywordslodash-modularized and merge
README

lodash.merge v4.6.2

The Lodash method _.merge exported as a Node.js module.

Installation

Using npm:

$ {sudo -H} npm i -g npm
$ npm i --save lodash.merge

In Node.js:

var merge = require('lodash.merge');

See the documentation or package source for more details.

Generated by 🚫 dangerJS against 7e3b0d0

Copy link
Contributor

@sgtcoolguy sgtcoolguy left a comment

Choose a reason for hiding this comment

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

LGTM

@lokeshchdhry
Copy link
Contributor

FR Passed.

@ssaddique
Copy link
Contributor

FR #2 passed. KitchenSink project builds as normal.

Test environment
SDK Ver: 9.2.1.v20201002104158
OS Ver: 10.15.3
Xcode Ver: Xcode 11.6
Appc NPM: 5.0.0
Appc CLI: 8.1.1
Ti CLI Ver: 5.2.5
Node Ver: 10.17.0
NPM Ver: 6.13.6
Java Ver: 1.8.0_162
Emulator: iPhone 11 Pro (13.6)

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