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

ios build: Take auto updates to Podfile.lock and an Xcode file #1444

Merged
merged 2 commits into from
Mar 27, 2025

Conversation

chrisbobbe
Copy link
Collaborator

I think the Podfile.lock changes are needed because #1426 didn't include the pod step of tools/upgrade.

I suspect the Xcode-file update is due to an Xcode upgrade.

Done by building for iOS, with Xcode 16.2 (16C5032a), then (when I remembered that macOS/Podfile.lock needs updating too) tools/upgrade pod.


cc @rajveermalviya as the author of #1426.

@gnprice
Copy link
Member

gnprice commented Mar 27, 2025

(Ah, I suggested "ios build" as a summary-line prefix because I was thinking of the Xcode config changes as a separate commit. I agree with your first thought that "ios deps" is a good prefix for the Podfile.lock changes.)

@@ -26,6 +26,7 @@
buildConfiguration = "Debug"
selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
customLLDBInitFile = "$(SRCROOT)/Flutter/ephemeral/flutter_lldbinit"
Copy link
Member

Choose a reason for hiding this comment

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

Looking at this line more closely, the content of it sounds like something the flutter tool might put there.

And indeed, git log -G flutter_lldbinit in the Flutter tree finds this from earlier this month, 2025-03-06:
flutter/flutter@eb07c51

So this is a side effect of the Flutter upgrade in #1426.

Copy link
Member

@rajveermalviya rajveermalviya Mar 27, 2025

Choose a reason for hiding this comment

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

Right, I'd only ran those commits in the draft (#1422) only on Android, sorry, forgot to note that in #1426.

(Guess this could've been caught if we had the iOS build in CI (#773), which I think should be good to have especially now, as #1379 introduces some Swift code.)

Copy link
Member

Choose a reason for hiding this comment

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

Cool, makes sense.

And yeah, agreed on both points re #773.

Copy link
Member

Choose a reason for hiding this comment

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

Made a note of that on #773, and bumped it up to M5b (from M6).

@gnprice gnprice mentioned this pull request Mar 27, 2025
Copy link
Member

@rajveermalviya rajveermalviya left a comment

Choose a reason for hiding this comment

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

Tested on iOS Simulator, and don't see any automated changes being made when running on macOS.

Greg found that this is a side effect of the Flutter upgrade in
PR zulip#1426:

zulip#1444 (comment)
> And indeed, `git log -G flutter_lldbinit` in the Flutter tree
> finds this from earlier this month, 2025-03-06:
> flutter/flutter@eb07c5123

Committing it here, after building for iOS, which we forgot to do
when preparing that PR.
These are needed because zulip#1426 didn't include the `pod` step of
`tools/upgrade`.

Done by building for iOS, then (when I remembered that
macOS/Podfile.lock needs updating too) `tools/upgrade pod`.
@chrisbobbe chrisbobbe force-pushed the pr-podfile-xcode-file-auto-updates branch from 0dd9eda to 16e4d88 Compare March 27, 2025 19:07
@chrisbobbe
Copy link
Collaborator Author

Thanks for the reviews! Revision pushed.

@gnprice
Copy link
Member

gnprice commented Mar 27, 2025

Thanks both! Looks good; merging.

@gnprice gnprice merged commit 16e4d88 into zulip:main Mar 27, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants