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): xcode project change detection causing unnecessary rebuilds #12755

Merged
merged 5 commits into from
Aug 5, 2021

Conversation

janvennemann
Copy link
Contributor

@janvennemann janvennemann commented Apr 28, 2021

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

Summary:
This fixes two issues where our build was unnecessarily doing full Xcode re-builds...

  • Always integrate frameworks into into the Xcode project in the same order. Otherwise the Xcode project file is different from the last run. Switched from iterating a map to a sorted array to ensure order. Also fixed an issue with loading previous state data which caused the framework hook to always scan for available frameworks inside the project.
  • Trim whitespaces when comparing generated .entitlements with existing ones from Xcode extensions before overwriting. Otherwise they may differ just by a trailing newline for example and always cause a full re-build.

Test:

  1. Build kitchensink-v2 for iOS.
  2. Clear the log.
  3. Build the project for iOS again without making any changes.
  4. Verify that you do NOT see the following in the log. (This is the fix.)
Forcing rebuild: Xcode project has changed since last build

@hansemannn
Copy link
Collaborator

Really cool fix, it works very well! Hope this can be cherry picked into GA

@build build added this to the 10.1.0 milestone Apr 28, 2021
@build
Copy link
Contributor

build commented Apr 28, 2021

Fails
🚫 Tests have failed, see below for more information.
Messages
📖 ✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.
📖 ❌ 4 tests have failed There are 4 tests failing and 982 skipped out of 17544 total tests.
📖

💾 Here's the generated SDK zipfile.

📖 👍 Hey!, You deleted more code than you added. That's awesome!

Tests:

ClassnameNameTimeError
android.emulator.Titanium.Network.HTTPClientresponseXML (5.0.2)0.213
Error: expected true to be false
at Assertion.fail (/node_modules/should/cjs/should.js:275:13)
      at Assertion.value (/node_modules/should/cjs/should.js:356:9)
      at HTTPClient.xhr.onload (/ti.network.httpclient.test.js:28:40)
android.emulator.Titanium.UI.View"after all" hook for "rgba fallback" (5.0.2)20.293
Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (app.js)
android.emulator.Titanium.UI.View"after each" hook for "getOrCreateView() should always return a View" (5.0.2)10.288
Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (app.js)
ios.macos.Titanium.Media.properties.audioPlaying is a Boolean (10.15.5)27.762
Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (app.js)
run@file:///Users/build/jenkins/workspace/ium-sdk_titanium_mobile_PR-12755/tmp/mocha/build/iphone/build/Products/Debug-maccatalyst/mocha.app/Contents/Resources/ti.main.js:9258:22
processImmediateQueue@file:///Users/build/jenkins/workspace/ium-sdk_titanium_mobile_PR-12755/tmp/mocha/build/iphone/build/Products/Debug-maccatalyst/mocha.app/Contents/Resources/ti.main.js:9321:18
drainQueues@file:///Users/build/jenkins/workspace/ium-sdk_titanium_mobile_PR-12755/tmp/mocha/build/iphone/build/Products/Debug-maccatalyst/mocha.app/Contents/Resources/ti.main.js:9298:52

Generated by 🚫 dangerJS against 6400902

@hansemannn
Copy link
Collaborator

@jquick-axway May it be possible to include this in the 10.0.1 release? It improves the developer experience for larger projects massively.

Copy link
Contributor

@jquick-axway jquick-axway left a comment

Choose a reason for hiding this comment

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

CR: Pass

@jquick-axway jquick-axway added no tests backport 10_2_X when applied, PRs with this label will get an auto-generated backport to 10_2_X branch on merge and removed needs jira 🚨 labels Jul 13, 2021
Copy link
Collaborator

@ewanharris ewanharris left a comment

Choose a reason for hiding this comment

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

FR Pass. No longer seeing unnecessary rebuilds on kitchensink

@ewanharris ewanharris merged commit 1007398 into tidev:master Aug 5, 2021
@build build removed the backport 10_2_X when applied, PRs with this label will get an auto-generated backport to 10_2_X branch on merge label Aug 5, 2021
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.

5 participants