-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[TIMOB-25226] iOS: Use the new Xcode build-system by default on Xcode 10, make configurable on older #10181
[TIMOB-25226] iOS: Use the new Xcode build-system by default on Xcode 10, make configurable on older #10181
Changes from 12 commits
b549b2c
e5d7cb8
1905696
81a2d60
a120247
e23641a
7ef53f1
5defc11
862f6b4
fb67906
30f2ebd
6a1a875
81f122d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -110,9 +110,13 @@ function iOSBuilder() { | |
// when true, uses the JavaScriptCore that ships with iOS instead of the original Titanium version | ||
this.useJSCore = true; | ||
|
||
// when true, uses the new build system (Xcode 9+) | ||
this.useNewBuildSystem = true; | ||
|
||
// when false, JavaScript will run on its own thread - the Kroll Thread | ||
this.runOnMainThread = true; | ||
|
||
// when true, uses the AutoLayout engine | ||
this.useAutoLayout = false; | ||
|
||
// populated the first time getDeviceInfo() is called | ||
|
@@ -2384,10 +2388,10 @@ iOSBuilder.prototype.initialize = function initialize() { | |
this.currentBuildManifest.encryptJS = !!this.encryptJS; | ||
this.currentBuildManifest.showErrorController = this.showErrorController; | ||
|
||
// Use native JSCore by default (TIMOB-23136) | ||
// use native JSCore by default (TIMOB-23136) | ||
this.currentBuildManifest.useJSCore = this.useJSCore = !this.debugHost && !this.profilerHost && this.tiapp.ios['use-jscore-framework'] !== false; | ||
|
||
// Remove this check on 7.0.0 | ||
// remove this check on 8.0.0 | ||
if (this.tiapp.ios && (this.tiapp.ios.hasOwnProperty('run-on-main-thread'))) { | ||
this.logger.warn(__('run-on-main-thread no longer set in the <ios> section of the tiapp.xml. Use <property name="run-on-main-thread" type="bool">true</property> instead')); | ||
this.currentBuildManifest.runOnMainThread = this.runOnMainThread = (this.tiapp.ios['run-on-main-thread'] === true); | ||
|
@@ -2396,13 +2400,13 @@ iOSBuilder.prototype.initialize = function initialize() { | |
} | ||
this.currentBuildManifest.useAutoLayout = this.useAutoLayout = this.tiapp.ios && (this.tiapp.ios['use-autolayout'] === true); | ||
|
||
// Deprecate TiJSCore and leave a warning if used anyway | ||
// deprecate TiJSCore and leave a warning if used anyway | ||
if (!this.useJSCore) { | ||
this.logger.warn(__('Titanium 7.0.0 deprecates the legacy JavaScriptCore library in favor of the built-in JavaScriptCore.')); | ||
this.logger.warn(__('The legacy JavaScriptCore library will be removed in Titanium SDK 8.0.0.')); | ||
} | ||
|
||
// Deprecate KrollThread and leave a warning if used anyway | ||
// deprecate KrollThread and leave a warning if used anyway | ||
if (!this.runOnMainThread) { | ||
this.logger.warn(__('Titanium 7.0.0 deprecates the legacy UI-execution on Kroll-Thread in favor of the Main-Thread.')); | ||
this.logger.warn(__('The legacy execution will be removed in Titanium SDK 8.0.0.')); | ||
|
@@ -2443,6 +2447,17 @@ iOSBuilder.prototype.initialize = function initialize() { | |
this.defaultLaunchScreenStoryboard = false; | ||
} | ||
|
||
if (!this.tiapp.ios.hasOwnProperty('use-new-build-system') && appc.version.lt(this.xcodeEnv.version, '10.0.0')) { | ||
// if running on Xcode < 10, do not use the new build system by default | ||
this.useNewBuildSystem = false; | ||
} else if (this.tiapp.ios.hasOwnProperty('use-new-build-system')) { | ||
// if explicitly set via tiapp.xml, go with that one | ||
this.useNewBuildSystem = this.tiapp.ios['use-new-build-system']; | ||
} else { | ||
// if not set and Xcode >= 10, use the new build system | ||
this.useNewBuildSystem = true; | ||
} | ||
|
||
if (this.enableLaunchScreenStoryboard && (fs.existsSync(path.join(this.projectDir, 'platform', 'ios', 'LaunchScreen.storyboard')) || fs.existsSync(path.join(this.projectDir, 'platform', 'iphone', 'LaunchScreen.storyboard')))) { | ||
this.defaultLaunchScreenStoryboard = false; | ||
} | ||
|
@@ -2839,36 +2854,44 @@ iOSBuilder.prototype.checkIfNeedToRecompile = function checkIfNeedToRecompile() | |
|
||
// check if the use JavaScriptCore flag has changed | ||
if (this.useJSCore !== manifest.useJSCore) { | ||
this.logger.info(__('Forcing rebuild: use JSCore flag changed since last build')); | ||
this.logger.info(__('Forcing rebuild: use-jscore-framework flag changed since last build')); | ||
this.logger.info(' ' + __('Was: %s', manifest.useJSCore)); | ||
this.logger.info(' ' + __('Now: %s', this.useJSCore)); | ||
return true; | ||
} | ||
|
||
// check if the use RunOnMainThread flag has changed | ||
if (this.runOnMainThread !== manifest.runOnMainThread) { | ||
this.logger.info(__('Forcing rebuild: use RunOnMainThread flag changed since last build')); | ||
this.logger.info(__('Forcing rebuild: run-on-main-thread flag changed since last build')); | ||
this.logger.info(' ' + __('Was: %s', manifest.runOnMainThread)); | ||
this.logger.info(' ' + __('Now: %s', this.runOnMainThread)); | ||
return true; | ||
} | ||
|
||
// check if the use UserAutoLayout flag has changed | ||
if (this.useAutoLayout !== manifest.useAutoLayout) { | ||
this.logger.info(__('Forcing rebuild: use UserAutoLayout flag changed since last build')); | ||
this.logger.info(__('Forcing rebuild: use-autolayout flag changed since last build')); | ||
this.logger.info(' ' + __('Was: %s', manifest.useAutoLayout)); | ||
this.logger.info(' ' + __('Now: %s', this.useAutoLayout)); | ||
return true; | ||
} | ||
|
||
// check if the use use-app-thinning flag has changed | ||
if (this.useAppThinning !== manifest.useAppThinning) { | ||
this.logger.info(__('Forcing rebuild: use use-app-thinning flag changed since last build')); | ||
this.logger.info(__('Forcing rebuild: use-app-thinning flag changed since last build')); | ||
this.logger.info(' ' + __('Was: %s', manifest.useAppThinning)); | ||
this.logger.info(' ' + __('Now: %s', this.useAppThinning)); | ||
return true; | ||
} | ||
|
||
// check if the use use-new-build-system flag has changed | ||
if (this.useNewBuildSystem !== manifest.useNewBuildSystem) { | ||
this.logger.info(__('Forcing rebuild: use-new-build-system flag changed since last build')); | ||
this.logger.info(' ' + __('Was: %s', manifest.useNewBuildSystem)); | ||
this.logger.info(' ' + __('Now: %s', this.useNewBuildSystem)); | ||
return true; | ||
} | ||
|
||
// check if the showErrorController flag has changed | ||
if (this.showErrorController !== manifest.showErrorController) { | ||
this.logger.info(__('Forcing rebuild: showErrorController flag changed since last build')); | ||
|
@@ -2947,7 +2970,7 @@ iOSBuilder.prototype.createXcodeProject = function createXcodeProject(next) { | |
this.logger.info(__('Creating Xcode project')); | ||
|
||
const appName = this.tiapp.name, | ||
scrubbedAppName = appName.replace(/[-\W]/g, '_'), | ||
scrubbedAppName = this.sanitizedAppName(), | ||
srcFile = path.join(this.platformPath, 'iphone', 'Titanium.xcodeproj', 'project.pbxproj'), | ||
contents = fs.readFileSync(srcFile).toString(), | ||
xcodeProject = xcode.project(path.join(this.buildDir, this.tiapp.name + '.xcodeproj', 'project.pbxproj')), | ||
|
@@ -4409,7 +4432,7 @@ iOSBuilder.prototype.copyTitaniumiOSFiles = function copyTitaniumiOSFiles() { | |
this.logger.info(__('Copying Titanium iOS files')); | ||
|
||
const nameChanged = !this.previousBuildManifest || this.tiapp.name !== this.previousBuildManifest.name, | ||
name = this.tiapp.name.replace(/[-\W]/g, '_'), | ||
name = this.sanitizedAppName(), | ||
extRegExp = /\.(c|cpp|h|m|mm)$/, | ||
|
||
// files to watch for while copying | ||
|
@@ -4689,7 +4712,7 @@ iOSBuilder.prototype.cleanXcodeDerivedData = function cleanXcodeDerivedData(next | |
} | ||
|
||
const exe = this.xcodeEnv.executables.xcodebuild, | ||
args = [ 'clean', '-UseNewBuildSystem=NO' ]; // Use old build system until www.openradar.me/40906897 is fixed | ||
args = [ 'clean', '-scheme', this.sanitizedAppName() ]; | ||
let tries = 0, | ||
lastErr = null, | ||
done = false; | ||
|
@@ -6537,12 +6560,12 @@ iOSBuilder.prototype.invokeXcodeBuild = function invokeXcodeBuild(next) { | |
this.target === 'dist-appstore' || this.target === 'dist-adhoc' ? 'archive' : 'build', | ||
'-target', this.tiapp.name, | ||
'-configuration', this.xcodeTarget, | ||
'-scheme', this.tiapp.name.replace(/[-\W]/g, '_'), | ||
'-scheme', this.sanitizedAppName(), | ||
'-derivedDataPath', path.join(this.buildDir, 'DerivedData'), | ||
'-UseNewBuildSystem=' + this.useNewBuildSystem ? 'YES' : 'NO', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You need parentheses around There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the issue was that it was passed as 2 array args ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Use parentheses. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cough https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals? 🙂 `-UseNewBuildSystem=${x ? 'YES' : 'NO'}` There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sigh. Yeah, template literals. |
||
'OBJROOT=' + path.join(this.buildDir, 'build', 'Intermediates'), | ||
'SHARED_PRECOMPS_DIR=' + path.join(this.buildDir, 'build', 'Intermediates', 'PrecompiledHeaders'), | ||
'SYMROOT=' + path.join(this.buildDir, 'build', 'Products'), | ||
'-UseNewBuildSystem=NO' // Use old build system until www.openradar.me/40906897 is fixed | ||
]; | ||
|
||
if (this.simHandle) { | ||
|
@@ -6577,6 +6600,15 @@ iOSBuilder.prototype.writeBuildManifest = function writeBuildManifest(next) { | |
})(this.currentBuildManifest, next); | ||
}; | ||
|
||
/** | ||
* Returns the sanitized app name to replace invalid characters. | ||
* | ||
* @returns {String} | ||
*/ | ||
iOSBuilder.prototype.sanitizedAppName = function sanitizedAppName() { | ||
return this.tiapp.name.replace(/[-\W]/g, '_'); | ||
}; | ||
|
||
function sha1(value) { | ||
return crypto.createHash('sha1').update(value).digest('hex'); | ||
} | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is just fine, but I would have removed the extra "if" like this: