fix(ios): ignore the mac build if it failed and exclude sim arm64 if thirdparty .framework is there #12093

vijaysingh-axway · 2020-09-16T22:54:25Z

https://jira.appcelerator.org/browse/TIMOB-28138

Silently ignore the mac build, if it failed for any reason( e.g included third party framework does not support mac). Log a warning.
If in module there is any third party .framework file, exclude arm 64 from simulator build and give a warning. .xcframework is only way to support simulator arm64.
Build fb module using PR Mac support ti.facebook#128. It is building but obviously no mac-target support as Facebook framework do not have mac support.
It will ensure backward compatibility of module build.

build · 2020-09-16T22:59:07Z

	Fails
🚫	Tests have failed, see below for more information.

	Warnings
⚠️	This PR has milestone set to 9.2.0, but the version defined in package.json is 9.3.0 Please either: - Update the milestone on the PR - Update the version in package.json - Hold the PR to be merged later after a release and version bump on this branch

	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.
📖	❌ 1 tests have failed There are 1 tests failing and 981 skipped out of 10942 total tests.

Tests:

Classname

Name

Time

Error

ios.ipad.Titanium.UI.iOS.CollisionBehavior.example

works (14.0)

15

Error: timeout of 15000ms exceeded
file:///Users/build/Library/Developer/CoreSimulator/Devices/083EB293-6DD6-4119-A4D5-72351034E69F/data/Containers/Bundle/Application/99616149-032C-4FA2-91B8-10F70A0E8307/mocha.app/ti-mocha.js:4326:27

Generated by 🚫 dangerJS against d862d43

sgtcoolguy

suggestions aren't required, but I think would clean the code up a little/make it simpler.

iphone/cli/commands/_buildModule.js

sgtcoolguy · 2020-09-17T14:18:55Z

iphone/cli/commands/_buildModule.js

+			// Assumption is that simulator arm64  architecture can only be supported via .xcframework.
+			const frameworkPattern = /([^/]+)\.framework$/;
+			let frameworksPath = path.join(this.projectDir, 'platform');
+			let legacyFrameworks = new Set();


same, use const

iphone/cli/commands/_buildModule.js

sgtcoolguy · 2020-09-17T14:23:31Z

iphone/cli/commands/_buildModule.js

@@ -447,7 +447,14 @@ iOSModuleBuilder.prototype.buildModule = function buildModule(next) {
 					this.logger.error('[' + type + '] ' + line);
 				}, this);
 				this.logger.log();
-				process.exit(1);
+				if (type === 'xcode-macos') {


Feels like this should be handled by the caller (code at bottom of the file that launches these xcodebuilds). I'm fine with the change here since that code would get messy if we just bubbled up the exit code and had to do error handling there as it stands. But I think long-term we should move towards async/await in place of callbacks and do the handling there (so just bubble up the code as return value here, don't call process.exit here, etc and then it's a matter of try/catch down there)

sgtcoolguy · 2020-09-17T20:19:28Z

As per @hansemannn 's comments in the linked ticket, we may just want to use the Xcode build setting as the "flag" to determine if the module is intended to target macOS. We've basically been forcing SUPPORTS_MACCATALYST to YES. It may make more sense to just let them toggle it on/off themselves. Maybe we can default it to YES in our module template for new modules going forward?

Or are we worried that module developers just won't toggle it on, even though it may literally require no other work for them to get macOS support included?

Perhaps some combination of the above and checking the flag - i.e. try to build for macOS, and bubble up an error/build failure if they have the flag on , treat it as a "warning" and continue if they don't have it on? cc @janvennemann

vijaysingh-axway · 2020-09-17T22:50:59Z

Regarding Han's solution- The modules which are not generating frameworks (Objc based modules), do not have option to opt out from mac-target build in Xcode project. This option is available for frameworks only. I think his PR will not work for all modules.
Regarding exclusion of target or arch - In Xcode 12, Apple has given EXCLUDED_ARCHS for excluding the architectures from build. Can we add new keys in manifest file to exclude architectures?
e.g.
exclude_mac_archs: arm64, x86_64, all
exclude_device_archs: arm64, armv7, all
exclude_sim_archs: arm64, i386, x86_64, all

all = do not build for particular target .
Default will be build for all architectures and targets.

Or will it be more complex ? cc @sgtcoolguy @janvennemann .

hansemannn · 2020-09-18T07:38:07Z

Maybe just a mac: true flag in the manifest? Defaulting to true, easy to parse by the module script and highly backwards compatible.

hansemannn · 2020-09-18T07:54:29Z

@vijaysingh-axway I tried your update and it still doesn't work:

[ERROR] There is discrepancy between the architectures specified in module manifest and compiled binary.
[ERROR] Architectures in manifest: armv7,arm64,i386,x86_64
[ERROR] Compiled binary architectures:
[ERROR] Please update manifest to match module binary architectures.

Did you try with Ti.WidgetKit?

janvennemann · 2020-09-18T08:58:23Z

Just using the setting from the Xcode project would be ideal of course. But as Vijay already pointed out this is not possible in ObjC modules since the project is still setup to build static libraries and not framework packages, so Xcode doesn't offer the same options as for Swift based native modules.

Maybe we can use the mac: true flag as suggested by Hans as an option that works for both, and fallback to check the Xcode project setting for modern Swift based modules. That way ObjC modules can still control it, but Swift module devs don't need to hassle with the manifest file, especially since there already is an equivalent option in Xcode available.

vijaysingh-axway · 2020-09-18T19:55:48Z

@vijaysingh-axway I tried your update and it still doesn't work:

[ERROR] There is discrepancy between the architectures specified in module manifest and compiled binary.
[ERROR] Architectures in manifest: armv7,arm64,i386,x86_64
[ERROR] Compiled binary architectures:
[ERROR] Please update manifest to match module binary architectures.

Did you try with Ti.WidgetKit?

@hansemannn It is failing because the architectures mentioned in manifest file is not same as in created XCFramework. Reason is, in _buildModule.js we are using hardcoded way to read architecture. Jan Vennemann is working on parsing of Info.plist available inside .xcframework as part of PR #12092 . After that we can update it to use architectures from there.

It reminds me, should we fail the module build if manifest architectures do not match the architectures available in xcframework? Any how arm64 is in device and simulator both and x86_64 is in mac and simulator both. We can not verify exact architectures with current state of manifest.
I think in verifyBuildArch we should verify if it is xcframework and log the supported targets and corresponding architectures in module and proceed to package module. cc @janvennemann

hansemannn · 2020-09-18T20:15:29Z

@vijaysingh-axway Thanks for the insight! So there is no workaround to compile the module? It's blocking our next update right now. Amy advice to work around it would help already!

You can comment this line in your sdk (generated with this PR) and it should be good.

hansemannn · 2020-09-19T08:27:33Z

@vijaysingh-axway With the latest change, the module compiles successfully already! The app build then fails with the following error:

Error: ENOENT: no such file or directory, open '/Users/user/Documents/dev/apps/myapp/modules/iphone/ti.widgetkit/1.0.0/TiWidgetkit.xcframework/ios-i386_x86_64-simulator/TiWidgetkit.framework/TiWidgetkit'
    at Object.openSync (fs.js:462:3)
    at Object.readFileSync (fs.js:364:35)
    at iOSBuilder.<anonymous> (/Users/user/Library/Application Support/Titanium/mobilesdk/osx/9.2.0.v20200915121722/iphone/cli/commands/_build.js:2252:54)
    at Array.forEach (<anonymous>)
    at iOSBuilder.<anonymous> (/Users/user/Library/Application Support/Titanium/mobilesdk/osx/9.2.0.v20200915121722/iphone/cli/commands/_build.js:2176:20)
    at iOSBuilder.<anonymous> (/Users/user/Library/Application Support/Titanium/mobilesdk/osx/9.2.0.v20200915121722/node_modules/node-titanium-sdk/lib/builder.js:387:3)
    at Object.callback (/Users/user/Library/Application Support/Titanium/mobilesdk/osx/9.2.0.v20200915121722/node_modules/node-appc/lib/timodule.js:349:52)
    at /Users/user/Library/Application Support/Titanium/mobilesdk/osx/9.2.0.v20200915121722/node_modules/node-appc/lib/timodule.js:415:52 {
  errno: -2,
  syscall: 'open',
  code: 'ENOENT',
  path: '/Users/user/Documents/dev/apps/myapp/modules/iphone/ti.widgetkit/1.0.0/TiWidgetkit.xcframework/ios-i386_x86_64-simulator/TiWidgetkit.framework/TiWidgetkit'
}

I suppose that will be fixed by the changes from Jan then.

EDIT: Workaround: Add the following to the two arch-checks inside _build.js:

if (!fs.existsSync(path.join(module.libFile, archDir))) {
    archDir = 'ios-arm64_x86_64-simulator';
}

if (!fs.existsSync(path.join(module.libFile, archDir))) {
    this.logger.error(__('Cannot find matching architecture!!'));
    process.exit(1);
}

EDIT 2: But on device builds, it logs the following warning:

[TRACE] ld: warning: ignoring file /Users/user/Documents/dev/apps/myapp/build/iphone/build/Products/Debug-iphoneos/TiWidgetkit.framework/TiWidgetkit, building for iOS-armv7 but attempting to link with file built for iOS-arm64

vijaysingh-axway · 2020-09-21T22:54:34Z

@hansemannn It should work now.

hansemannn · 2020-09-22T11:46:16Z

@vijaysingh-axway The module build works, but the containing app cannot use the module in Simulator:

Couldn't find module: ti.widgetkit for architecture: x86_64

And same for device:

Couldn't find module: ti.widgetkit for architecture: arm64

sgtcoolguy · 2020-09-22T14:19:27Z

iphone/cli/commands/_buildModule.js

@@ -894,8 +917,6 @@ iOSModuleBuilder.prototype.runModule = function runModule(next) {

 			// 5. unzip module to the tmp dir. Use native binary on macOS, as AdmZip doesn't support symlinks used in mac catalyst frameworks
 			const proc = spawn('unzip', [ '-o', this.moduleZipPath, '-d', tmpProjectDir ]);
-			proc.stdout.on('data', data => this.logger.info(data.toString().trimEnd()));


without these lines, the unzip never finished for me. We have to "consume" the stdout/stderr for it to finish.

sgtcoolguy · 2020-09-22T14:20:22Z

iphone/cli/commands/_buildModule.js

+	if (this.isMacOSEnabled) {
+		lib = findLib('macosx');
+		if (lib instanceof Error) {
+			this.logger.warn(__('The module is missing 64-bit support of macos. Ignoring mac target for this module...'));


The warning is misleading. It's missing macOS support entirely, not 64-bit support.

Also, they've "turned on" Mac support, so shouldn't this be an Error, not a warning log and move on?

sgtcoolguy · 2020-09-22T14:23:04Z

iphone/cli/commands/_buildModule.js

-					// 3. Create a build for the maccatalyst
-					xcodebuildHook(xcBuild, xcodeBuildArgumentsForTarget('macosx'), opts, 'xcode-macos', next);
+					// 3. Create a build for the mac-catalyst if enabled
+					if (this.isMacOSEnabled) {


Kind of a dumb edge-case, but if they have no indication of Mac support in either a positive or negative value, what should we do? I think this will assume no macOS support, so will not try to build for it.

But if they haven't set SUPPORTS_MACCATALYST in Xcode and they have no mac key/value pair in the manifest, then technically they haven't indicated one way or the other. Would we try to build for Mac and just be ok with it failing in that case?

sgtcoolguy · 2020-09-22T15:00:52Z

@vijaysingh-axway I'm testing locally with WidgetKit and Facebook and cleaning up the commits. I'll force push and merge (if build is successful) afterwards.

- based on if all libraries support arm64 sim. If not, exclude arm64 at build time - decide mac on basis of manifest (if available) or xcode target's SUPPORTS_MACCATALYST value - read info.plist to decide the xcframework archs

sgtcoolguy · 2020-09-22T15:26:18Z

OK, I've force-pushed a rebased set of commits that worked for me locally for both @hansemannn WidgetKit module and our Facebook Mac support PR. Note that on Hans' module I had to edit the manifest to remove armv7 and i386 from the architectures (as it does not build for them).

For Facebook, I basically just had to point to the locally built SDK and set the manifest's mac: false

build · 2020-09-22T15:42:25Z

iphone/cli/commands/_buildModule.js

@@ -107,7 +138,9 @@ iOSModuleBuilder.prototype.run = function run(logger, config, cli, finished) {
 		'compileJS',
 		'buildModule',
 		'createUniversalBinary',
-		'verifyBuildArch',
+		function (next) {
+			this.verifyBuildArch().then(next, next);


⚠️ iphone/cli/commands/_buildModule.js line 142 – Avoid calling back inside of a promise. (promise/no-callback-in-promise)

⚠️ iphone/cli/commands/_buildModule.js line 142 – Expected catch() or return (promise/catch-or-return)

hansemannn · 2020-09-22T16:40:18Z

@sgtcoolguy I wonder what's wrong with my config. I pulled the latest 9.2.0 build, updated the architectures and $(TITANIUM_SDK) path, but still get (a now different) error:

[TRACE] [xcode-macos] error: While building for macOS, no library for this platform was found in '~/Library/Application Support/Titanium/mobilesdk/osx/9.2.0.v20200922084018/iphone/Frameworks/TitaniumKit.xcframework'. (in target 'TiWidgetkit' from project 'titanium-widget-kit')

It should not even attempt to pick up macOS because it's deactivated on both manifest and Xcode. This was my last commit to align with the latest changes.

EDIT: 9.2.0.v20200922084018 may be too old already, because it does not contain your changes I think. Waiting for the new build then.

hansemannn · 2020-09-22T21:11:06Z

Update: It seems like the latest 9.2.0 build is still not available, but building from source finally fixes the issue. Thanks everyone!

hansemannn · 2020-09-30T20:40:53Z

Are there plans to finally fix all iOS 14 module issues and therefore make Titanium compatible with iOS 14+ modules? I filed this ticket to track it, but it may have been overseen so far :)

vijaysingh-axway requested review from sgtcoolguy and ewanharris September 16, 2020 22:55

build added this to the 9.3.0 milestone Sep 16, 2020

sgtcoolguy approved these changes Sep 17, 2020

View reviewed changes

sgtcoolguy added backport 9_2_X bug cli 📟 in-qe-testing 🕵 ios labels Sep 17, 2020

sgtcoolguy modified the milestones: 9.3.0, 9.2.0 Sep 17, 2020

sgtcoolguy removed the in-qe-testing 🕵 label Sep 17, 2020

build added the in-qe-testing 🕵 label Sep 17, 2020

vijaysingh-axway added work in progress 🚧 and removed in-qe-testing 🕵 labels Sep 19, 2020

build added the in-qe-testing 🕵 label Sep 19, 2020

build added the merge conflicts 🚨 label Sep 21, 2020

vijaysingh-axway removed the work in progress 🚧 label Sep 21, 2020

build removed the merge conflicts 🚨 label Sep 21, 2020

vijaysingh-axway mentioned this pull request Sep 22, 2020

feat(ios): support AppTrackingTransparency for iOS 14 tidev/ti.admob#100

Merged

sgtcoolguy reviewed Sep 22, 2020

View reviewed changes

sgtcoolguy self-assigned this Sep 22, 2020

fix(ios): optionally build macos and/or ios arm64 sim

d862d43

- based on if all libraries support arm64 sim. If not, exclude arm64 at build time - decide mac on basis of manifest (if available) or xcode target's SUPPORTS_MACCATALYST value - read info.plist to decide the xcframework archs

sgtcoolguy force-pushed the TIMOB-28138 branch from 6fd53da to d862d43 Compare September 22, 2020 15:24

build reviewed Sep 22, 2020

View reviewed changes

sgtcoolguy merged commit 3971dc1 into tidev:master Sep 22, 2020

build mentioned this pull request Sep 22, 2020

[Backport 9_2_X] fix(ios): ignore the mac build if it failed and exclude sim arm64 if thirdparty .framework is there #12115

Merged

build removed the backport 9_2_X label Sep 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(ios): ignore the mac build if it failed and exclude sim arm64 if thirdparty .framework is there #12093

fix(ios): ignore the mac build if it failed and exclude sim arm64 if thirdparty .framework is there #12093

vijaysingh-axway commented Sep 16, 2020 •

edited

build commented Sep 16, 2020 •

edited

sgtcoolguy left a comment

sgtcoolguy Sep 17, 2020

sgtcoolguy Sep 17, 2020

sgtcoolguy commented Sep 17, 2020 •

edited

vijaysingh-axway commented Sep 17, 2020 •

edited

hansemannn commented Sep 18, 2020

hansemannn commented Sep 18, 2020

janvennemann commented Sep 18, 2020 •

edited

vijaysingh-axway commented Sep 18, 2020 •

edited

hansemannn commented Sep 18, 2020 •

edited by vijaysingh-axway

hansemannn commented Sep 19, 2020 •

edited

vijaysingh-axway commented Sep 21, 2020

hansemannn commented Sep 22, 2020 •

edited

sgtcoolguy Sep 22, 2020

sgtcoolguy Sep 22, 2020

sgtcoolguy Sep 22, 2020

sgtcoolguy Sep 22, 2020

sgtcoolguy commented Sep 22, 2020

sgtcoolguy commented Sep 22, 2020

build Sep 22, 2020

hansemannn commented Sep 22, 2020 •

edited

hansemannn commented Sep 22, 2020

hansemannn commented Sep 30, 2020

fix(ios): ignore the mac build if it failed and exclude sim arm64 if thirdparty .framework is there #12093

fix(ios): ignore the mac build if it failed and exclude sim arm64 if thirdparty .framework is there #12093

Conversation

vijaysingh-axway commented Sep 16, 2020 • edited

build commented Sep 16, 2020 • edited

Tests:

sgtcoolguy left a comment

Choose a reason for hiding this comment

sgtcoolguy Sep 17, 2020

Choose a reason for hiding this comment

sgtcoolguy Sep 17, 2020

Choose a reason for hiding this comment

sgtcoolguy commented Sep 17, 2020 • edited

vijaysingh-axway commented Sep 17, 2020 • edited

hansemannn commented Sep 18, 2020

hansemannn commented Sep 18, 2020

janvennemann commented Sep 18, 2020 • edited

vijaysingh-axway commented Sep 18, 2020 • edited

hansemannn commented Sep 18, 2020 • edited by vijaysingh-axway

hansemannn commented Sep 19, 2020 • edited

vijaysingh-axway commented Sep 21, 2020

hansemannn commented Sep 22, 2020 • edited

sgtcoolguy Sep 22, 2020

Choose a reason for hiding this comment

sgtcoolguy Sep 22, 2020

Choose a reason for hiding this comment

sgtcoolguy Sep 22, 2020

Choose a reason for hiding this comment

sgtcoolguy Sep 22, 2020

Choose a reason for hiding this comment

sgtcoolguy commented Sep 22, 2020

sgtcoolguy commented Sep 22, 2020

build Sep 22, 2020

Choose a reason for hiding this comment

hansemannn commented Sep 22, 2020 • edited

hansemannn commented Sep 22, 2020

hansemannn commented Sep 30, 2020

vijaysingh-axway commented Sep 16, 2020 •

edited

build commented Sep 16, 2020 •

edited

sgtcoolguy commented Sep 17, 2020 •

edited

vijaysingh-axway commented Sep 17, 2020 •

edited

janvennemann commented Sep 18, 2020 •

edited

vijaysingh-axway commented Sep 18, 2020 •

edited

hansemannn commented Sep 18, 2020 •

edited by vijaysingh-axway

hansemannn commented Sep 19, 2020 •

edited

hansemannn commented Sep 22, 2020 •

edited

hansemannn commented Sep 22, 2020 •

edited