-
Notifications
You must be signed in to change notification settings - Fork 270
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
base: master
Are you sure you want to change the base?
Conversation
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.
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) |
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.
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 toxcode_build_number
(e.g.15C65
)
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.
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.
Ah good catch regarding the lack of use 👍
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.
Sorry for the delay, but I looked at this now and noticed the following:
xcode_version
with the above change will be equal to15.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 as15C65
.
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
Before this change, the
xcode_version
variable was evaluated to the same value as thexcode_build_number
variable.This change instead assigns the XCode version to the
xcode_version
variable.