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.

Sign up for GitHub

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

Jump to bottom

[TIMOB-25226] iOS: Use the new Xcode build-system by default on Xcode 10, make configurable on older #10181

Merged
merged 13 commits into from
Jul 20, 2018
Merged

[TIMOB-25226] iOS: Use the new Xcode build-system by default on Xcode 10, make configurable on older #10181

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
b549b2c
[TIMOB-25226] Use the new Xcode build-system by default on Xcode 10, …
hansemannn Jul 16, 2018
e5d7cb8
[TIMOB-25226] Commit missing dash
hansemannn Jul 16, 2018
1905696
Update _build.js
hansemannn Jul 16, 2018
81a2d60
Update _build.js
hansemannn Jul 16, 2018
a120247
Update to node-titanium-sdk 0.6.0
sgtcoolguy Jul 17, 2018
e23641a
Merge branch 'master' into TIMOB-25226
hansemannn Jul 18, 2018
7ef53f1
Review comments
hansemannn Jul 18, 2018
5defc11
Merge branch 'master' into TIMOB-25226
hansemannn Jul 19, 2018
862f6b4
Merge branch 'master' into TIMOB-25226
hansemannn Jul 19, 2018
fb67906
Merge branch 'master' into TIMOB-25226
hansemannn Jul 19, 2018
30f2ebd
Update _build.js
hansemannn Jul 19, 2018
6a1a875
Merge branch 'master' into TIMOB-25226
hansemannn Jul 19, 2018
81f122d
Update _build.js
hansemannn Jul 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
61 changes: 48 additions & 13 deletions iphone/cli/commands/_build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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.'));
Expand Down Expand Up @@ -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 explicitely set via tiapp.xml, go with that one
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: explicitely -> explicitly.

this.useNewBuildSystem = this.tiapp.ios['use-new-build-system'];
} else {
// if not set and Xcode >= 10, use the new build system
this.useNewBuildSystem = true;
Copy link
Contributor

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:

if (this.tiapp.ios.hasOwnProperty('use-new-build-system')) {
    this.useNewBuildSystem = this.tiapp.ios['use-new-build-system'];
} else {
    // use the new build system if Xcode >= 10
    this.useNewBuildSystem = appc.version.gte(this.xcodeEnv.version, '10.0.0');
}

}

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;
}
Expand Down Expand Up @@ -2831,36 +2846,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'));
Expand Down Expand Up @@ -2939,7 +2962,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')),
Expand Down Expand Up @@ -4401,7 +4424,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
Expand Down Expand Up @@ -4681,7 +4704,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;
Expand Down Expand Up @@ -6529,14 +6552,17 @@ 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'),
'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.useNewBuildSystem) {
args.push('-UseNewBuildSystem=NO');
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it hurt to be explicit and say:

args.push(`-UseNewBuildSystem=${this.useNewBuildSystem ? 'YES' : 'NO'}`);


if (this.simHandle) {
// when building for the simulator, we need to specify a destination and a scheme (above)
// so that it can compile all targets (phone and watch targets) for the simulator
Expand Down Expand Up @@ -6569,6 +6595,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');
}
Expand Down
6 changes: 3 additions & 3 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
"markdown": "0.5.0",
"moment": "^2.22.2",
"node-appc": "^0.2.47",
"node-titanium-sdk": "^0.5.0",
"node-titanium-sdk": "^0.6.0",
"node-uuid": "1.4.8",
"pngjs": "^3.3.3",
"request": "^2.87.0",
Expand Down