-
Notifications
You must be signed in to change notification settings - Fork 1.4k
chore(ci): fix iOS builds hanging on Github Actions #6854
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
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughKeychain creation flags changed (timeout 0, Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI runner
participant Fastlane as Fastlane (Fastfile)
participant Match as match / Apple provisioning
participant Security as macOS `security` / set-key-partition-list
participant Gym as gym / build step
CI->>Fastlane: start build_experimental / build_official
Fastlane->>Fastlane: create_keychain(timeout:0, unlock:true, lock_when_sleeps:false)
Fastlane->>Security: set-keychain-settings / unlock keychain
Fastlane->>Match: match(type:"appstore", platform:"ios")
Match-->>Fastlane: provisioning profiles & certificates
Fastlane->>Security: security set-key-partition-list / set_key_partion_list
Fastlane->>Gym: gym (build/sign with unlocked keychain)
Gym-->>CI: build artifacts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Gemfile (1)
6-6: Cosmetic reordering with no functional impact.The reordering of version exclusions from
!= 1.15.0, != 1.15.1to!= 1.15.1, != 1.15.0doesn't change the effective constraint—both exclude versions 1.15.0 and 1.15.1.Optionally, consider using ascending order (
!= 1.15.0, != 1.15.1) as it's more conventional for version constraints.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
Gemfile(1 hunks)ios/fastlane/Fastfile(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- ios/fastlane/Fastfile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
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.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
ios/fastlane/Fastfile(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🔇 Additional comments (2)
ios/fastlane/Fastfile (2)
59-62: LGTM! Explicit platform specification improves clarity.Adding
platform: "ios"to thematchandget_provisioning_profilecalls makes the configuration more explicit and prevents potential ambiguity.
88-92: Document why|| trueis needed forbuild_officialbut notbuild_experimental.The
set-key-partition-listcommand on line 92 uses|| trueto suppress failures, while the identical command inbuild_experimental(line 68) does not. This asymmetry suggests either:
- The command can fail in specific scenarios only in the official build, or
- Error suppression should also be applied to
build_experimentalfor consistencyPlease clarify:
- Under what conditions does
set-key-partition-listfail in the official build?- Should
build_experimentalalso use|| truefor resilience?If the command failure is expected and safe to ignore, consider adding a comment explaining why, such as:
# set-key-partition-list may fail if the keychain is already configured; safe to ignore sh "security set-key-partition-list ... || true"
| create_keychain( | ||
| name: ENV["MATCH_KEYCHAIN_NAME"], | ||
| password: ENV["MATCH_KEYCHAIN_PASSWORD"], | ||
| timeout: 1200 | ||
| timeout: 0, | ||
| lock_when_sleeps: false, | ||
| unlock: true | ||
| ) |
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.
Clarify timeout inconsistency between keychain creation and later shell commands.
The keychain is created with timeout: 0 (no automatic lock), but lines 67 and 91 later use security set-keychain-settings -lut 3600 which sets a 3600-second timeout. This creates conflicting timeout configurations.
Consider either:
- Using
timeout: 3600here to match the shell commands, or - Removing the
-lut 3600flag from lines 67 and 91 to maintain the no-timeout behavior
create_keychain(
name: ENV["MATCH_KEYCHAIN_NAME"],
password: ENV["MATCH_KEYCHAIN_PASSWORD"],
- timeout: 0,
+ timeout: 3600,
lock_when_sleeps: false,
unlock: true
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| create_keychain( | |
| name: ENV["MATCH_KEYCHAIN_NAME"], | |
| password: ENV["MATCH_KEYCHAIN_PASSWORD"], | |
| timeout: 1200 | |
| timeout: 0, | |
| lock_when_sleeps: false, | |
| unlock: true | |
| ) | |
| create_keychain( | |
| name: ENV["MATCH_KEYCHAIN_NAME"], | |
| password: ENV["MATCH_KEYCHAIN_PASSWORD"], | |
| timeout: 3600, | |
| lock_when_sleeps: false, | |
| unlock: true | |
| ) |
🤖 Prompt for AI Agents
In ios/fastlane/Fastfile around lines 21 to 27 the keychain is created with
timeout: 0 while later shell commands at lines ~67 and ~91 set a 3600s timeout
via security set-keychain-settings -lut 3600, producing conflicting timeout
behavior; to fix, make the timeout consistent by either changing timeout: 0 to
timeout: 3600 in the create_keychain call to match the shell commands, or remove
the -lut 3600 flags from the later security set-keychain-settings invocations so
the created keychain remains unlocked indefinitely (choose the option that
matches the intended security policy and update both places accordingly).
| # Allow codesign to access keys without prompting | ||
| keychain_path = "~/Library/Keychains/#{ENV['MATCH_KEYCHAIN_NAME']}-db" | ||
| sh "security unlock-keychain -p \"#{ENV['MATCH_KEYCHAIN_PASSWORD']}\" #{keychain_path}" | ||
| sh "security set-keychain-settings -lut 3600 #{keychain_path}" | ||
| sh "security set-key-partition-list -S apple-tool:,apple:,codesign: -s -k \"#{ENV['MATCH_KEYCHAIN_PASSWORD']}\" #{keychain_path}" |
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.
🧩 Analysis chain
🏁 Script executed:
cat -n ios/fastlane/Fastfile | sed -n '60,100p'Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 2382
Add error suppression to line 68 for consistency with build_official.
The set-key-partition-list command on line 68 should include || true to suppress errors, matching the same command on line 92 in build_official. Both lanes perform identical keychain setup, so they should use consistent error handling.
🤖 Prompt for AI Agents
In ios/fastlane/Fastfile around lines 64 to 68, the `security
set-key-partition-list` command on line 68 lacks error suppression and should
match `build_official` by appending `|| true`; update that sh invocation to add
`|| true` so the command's failure doesn't abort the lane, keeping keychain
setup error handling consistent across lanes.
|
iOS Build Available Rocket.Chat Experimental 4.68.0.107898 |
|
iOS Build Available Rocket.Chat Experimental 4.68.0.107901 |
Added options to the keychain creation for locking behavior and unlocking. Introduced key partition list settings for improved code signing management.
Refined the Fastlane setup by specifying the platform for provisioning profiles and match type, ensuring better compatibility and clarity in the configuration.
Bumped several gem versions for improved compatibility and security, including CFPropertyList, activesupport, aws-sdk components, and others. Updated dependencies to their latest stable releases to ensure optimal performance and functionality.
Reordered the exclusion of specific cocoapods versions in the Gemfile and updated several gem versions in the Gemfile.lock for improved compatibility and stability, including activesupport, aws-sdk components, and others. Adjusted Fastlane keychain settings for better command execution.
Updated Fastlane configuration to allow codesigning access without prompting by unlocking the keychain and adjusting keychain settings. Set timeout to 0 for the keychain and added commands to enhance keychain management during the build process.
82609dc to
5008e1a
Compare
Proposed changes
Added options to the keychain creation for locking behavior and unlocking. Introduced key partition list settings for improved code signing management.
Issue(s)
https://rocketchat.atlassian.net/browse/CORE-1570
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.