[TIMOB-25226] iOS: Use the new Xcode build-system by default on Xcode 10, make configurable on older #10181
Conversation
…make configurable on older
|
@sgtcoolguy Looks like Jenkins is not happy with Xcode 10, yet. |
|
@hansemannn You'll need to do an: to grab the node-titanium-sdk with support for this flag. |
|
I don't think it's published to npm, yet:
|
iphone/cli/commands/_build.js
Outdated
| ]; | ||
|
|
||
| if (!this.useNewBuildSystem) { | ||
| args.push('-UseNewBuildSystem=NO'); | ||
| } |
There was a problem hiding this comment.
Would it hurt to be explicit and say:
args.push(`-UseNewBuildSystem=${this.useNewBuildSystem ? 'YES' : 'NO'}`);
iphone/cli/commands/_build.js
Outdated
| // 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 |
There was a problem hiding this comment.
Typo: explicitely -> explicitly.
iphone/cli/commands/_build.js
Outdated
| this.useNewBuildSystem = this.tiapp.ios['use-new-build-system']; | ||
| } else { | ||
| // if not set and Xcode >= 10, use the new build system | ||
| this.useNewBuildSystem = true; |
There was a problem hiding this comment.
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');
}
|
@hansemannn The CI build is failing with: |
|
Thanks Chris, silly error. Apparently, |
iphone/cli/commands/_build.js
Outdated
| '-derivedDataPath', path.join(this.buildDir, 'DerivedData'), | ||
| '-UseNewBuildSystem=' + this.useNewBuildSystem ? 'YES' : 'NO', |
There was a problem hiding this comment.
You need parentheses around this.useNewBuildSystem ? 'YES' : 'NO'. The + has higher precedence than ? :. This will likely address the issue @sgtcoolguy was having.
There was a problem hiding this comment.
I think the issue was that it was passed as 2 array args ([-UseNewBuildSystem, YES/NO]) before, which resulted in -UseNewBuildSystem YES instead of -UseNewBuildSystem=YES.
There was a problem hiding this comment.
$ node
> let x = true;
undefined
> '-UseNewBuildSystem=' + x ? 'YES' : 'NO'
'YES'
> x = false;
false
> '-UseNewBuildSystem=' + x ? 'YES' : 'NO'
'YES'
> '-UseNewBuildSystem=' + (x ? 'YES' : 'NO')
'-UseNewBuildSystem=NO'
Use parentheses.
There was a problem hiding this comment.
Cough https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals? 🙂
`-UseNewBuildSystem=${x ? 'YES' : 'NO'}`There was a problem hiding this comment.
Sigh. Yeah, template literals.
Generated by 🚫 dangerJS |
… 10, make configurable on older (#10181) * [TIMOB-25226] Use the new Xcode build-system by default on Xcode 10, make configurable on older * [TIMOB-25226] Commit missing dash * Update _build.js * Update _build.js * Update to node-titanium-sdk 0.6.0 * Review comments * Update _build.js * Update _build.js
JIRA: https://jira.appcelerator.org/browse/TIMOB-25226