Skip to content

[Tooling] Update Xcode version refs in tooling scripts #8033

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

Merged
merged 2 commits into from
Nov 23, 2022

Conversation

AliSoftware
Copy link
Contributor

@AliSoftware AliSoftware commented Nov 4, 2022

Why?

This updates the references to the minimum Xcode version needed to compile WCiOS to 14.0, as some references in .xcversion and Fastfile were still mentioning ~> 13.4 being the minimum, while the project now requiring Xcode 14 minimum to compile.

Context

While I was trying to run the Unit Tests in my Xcode on my local machine, I encountered some issues because:

  1. I hadn't worked on this repo in a while, so hadn't compiled the code in ages
  2. I hadn't have to use Xcode in a while on that machine either, so my default Xcode (xcode-select -p) was still 13.4.1

As a result, I accidentally tried to run the Unit Tests with Xcode 13.4, which failed to compile — at the very least because of this line which uses a new feature only available since Swift 5.7 — and I had to open the project with Xcode 14 to be able to compile it.

Our CI config for this project is also using Xcode 14 anyway, so this makes sense to keep those aligned.

To Test

  • Check that CI passes, proving that the checks made via xcversion() calls in the Fastfile as working and relying on .xcode-version file as expected
  • Run bundle exec fastlane build_for_testing locally and validate that it passes (i.e. it doesn't complain about Xcode 14 missing).
    • Ideally, it would be nice to test this while you don't have Xcode 14.0 installed on your machine, but do have Xcode 14.1, so that we can prove that the ~> operator works as expected and does not restrict to just 14.0 exact match. (I personally only had Xcode 14.0 on my machine, and have wifi trouble today so not the day I'm going to download 14.1 to test 😅 , but hopefully someone can test on their machine)
  • Temporarily change .xcode-version to something like ~> 14.5, try bundle exec fastlane build_for_testing again, and validate it complains about Xcode 14.5 or later not being found on your machine
    • (the proper test for that would instead be to Zip your Xcode 14 .app and delete that .app so that it couldn't be found on your Mac anymore and retry to verify it complains about not finding Xcode 14 being installed anymore… but that might take a bit long to zip / uninstall Xcode 14 then unzip/reinstall it once done, so…)

In `.xcode-version` and `Fastfile`
@AliSoftware AliSoftware added the category: tooling Anything that involves building & maintaining the project, including scripts, `Fastfile`, etc. label Nov 4, 2022
@AliSoftware AliSoftware added this to the 11.1 milestone Nov 4, 2022
@AliSoftware AliSoftware requested review from oguzkocer and a team November 4, 2022 12:15
@AliSoftware AliSoftware self-assigned this Nov 4, 2022
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 4, 2022

You can test the changes from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr8033-04c81f1 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

And thus also allowing future 14.x versions after 14.0
@@ -0,0 +1 @@
~> 14.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The file we had before was .xcversion, but the file used by Fastlane's xcversion action is supposed to be named .xcode-version (by convention similar to files like .ruby-version and the like), not .xcversion, hence this file rename (in addition to updating its content to ~> 14.0).

@oguzkocer oguzkocer modified the milestones: 11.1, 11.2 Nov 4, 2022
Copy link
Contributor

@mokagio mokagio left a comment

Choose a reason for hiding this comment

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

I'm downloading Xcode 14.1, so in the meantime here's some notes and a preemptive approval.

Comment on lines +14 to 15
# Be sure to also update the `.xcode-version` file when updating the Xcode image/version here
IMAGE_ID: xcode-14
Copy link
Contributor

Choose a reason for hiding this comment

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

Great addition 👍

In the past, I used dynamic pipelines, where instead of having an hardcoded YAML pipeline we'd run a script that generated the YAML at runtime. In that setup, we could read .xcode-version and generate the IMAGE_ID name based on it.

But, dynamic pipelines are harder to read, might have bugs in the generation logic, and we don't change Xcode versions that fast to justify the investment and tradeoff at this point in time.

unless options[:skip_prechecks]
ios_build_prechecks(skip_confirm: options[:skip_confirm], external: true)
ios_build_preflight
xcversion() # Ensure we're using the right version of Xcode, defined in `.xcode-version` file
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick. Have you considered wrapping xcversion() in a function to integrate the comment in the code? Something like check_xcode_version or enforce_xcode_version?

Copy link
Contributor

Choose a reason for hiding this comment

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

Update: I accidentally landed on this document that mentions xcode-install/xcversion has been deprecated and how to update to the new xcodes alternative.

Might be worth looking into before merging, or as an update followup.

Copy link
Contributor

Choose a reason for hiding this comment

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

or as an update followup.

I'd be happy to look into it if your plate's full.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh TIL this was deprecated. Yeah I'd love if you can take a look at migrating the command to xcodes. A bit unfortunate to see that it seems to require the installation of a separate tool tho, unlike xcversion

Copy link
Contributor Author

@AliSoftware AliSoftware Nov 14, 2022

Choose a reason for hiding this comment

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

@mokagio Seems latest fastlane shipped with official new usage of xcodes over xcversion: https://twitter.com/fastlanetools/status/1591561723794030592

@mokagio
Copy link
Contributor

mokagio commented Nov 7, 2022

Tested on a Mac with Xcode 14.1. Out of the box, build_for_testing worked. Once I updated .xcode-version to ~> 14.2, it failed as expected with:

image

👍

@oguzkocer oguzkocer modified the milestones: 11.2, 11.3 Nov 12, 2022
@mokagio mokagio modified the milestones: 11.3, 11.4 Nov 16, 2022
@mokagio
Copy link
Contributor

mokagio commented Nov 16, 2022

FYI, I run into some limitations when trying to use xcodes, which I described in a Fastlane issue fastlane/fastlane#20866. The fix to get the behavior we want is straightforward, but I haven't been able to properly unit tests it yet.

I also have a WIP PR to use xcodes here: #8131.

@AliSoftware
Copy link
Contributor Author

Given we parked in the PR migrating things from xcversion to xcodes for the time being after all, I'm going to merge this one for the time being then.

@AliSoftware AliSoftware merged commit ef43694 into trunk Nov 23, 2022
@AliSoftware AliSoftware deleted the tooling/xcode14-refs branch November 23, 2022 12:02
@mokagio
Copy link
Contributor

mokagio commented Nov 23, 2022

Given we parked in the PR migrating things from xcversion to xcodes for the time being after all, I'm going to merge this.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: tooling Anything that involves building & maintaining the project, including scripts, `Fastfile`, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants