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(docs): update Xcode Version Selection example #2451

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sitaktif
Copy link

Before this change, the xcode_version variable was evaluated to the same value as the xcode_build_number variable.

This change instead assigns the XCode version to the xcode_version variable.

Before this change, the `xcode_version` variable was evaluated to the
same value as the `xcode_build_number` variable.

This change instead assigns the XCode version to the `xcode_version`
variable.
Comment on lines -509 to 510
xcode_version=$(xcodebuild -version | tail -1 | cut -d " " -f3)
xcode_version=$(/usr/bin/xcodebuild -version 2>/dev/null | head -1 | cut -d " " -f2)
xcode_build_number=$(/usr/bin/xcodebuild -version 2>/dev/null | tail -1 | cut -d " " -f3)
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually looks like xcode_build_number is unused in this snippet and from what I can tell we likely want to update it such that:

  • remove usage of xcode_version (e.g. 15.1)
  • set XCODE_VERSION equal to xcode_build_number (e.g. 15C65)

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Ah good catch regarding the lack of use 👍

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the delay, but I looked at this now and noticed the following:

  • xcode_version with the above change will be equal to 15.1. This isn't great because if you're using a beta version of Xcode (and possibly have multiple versions installed) it won't be deterministic. For someone it could pick beta 1 and for someone else it could pick beta 2.
  • xcode_build_number instead gives the unique build number for an Xcode version (which changes between betas of the same version as well), such as 15C65.

I think in this script we likely want to replace the usage of $xcode_version with $xcode_build_number instead (and of course remove xcode_version=(...) since it'll then be unused). I see that rules_xcodeproj (cc: @brentleyjones) also does something similar (where XCODE_PRODUCT_BUILD_VERSION looks like 15C65): https://github.com/MobileNativeFoundation/rules_xcodeproj/blob/9a6ec001d7ac96bb8474d95b0ecab909db9dab06/xcodeproj/internal/templates/bazel_build.sh#L86-L95

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.

None yet

3 participants