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.

Sign up for GitHub

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

Jump to bottom

fix(docs): update Xcode Version Selection example #2451

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

fix(docs): update Xcode Version Selection example #2451

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/common_info.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ bazelrc_lines=()

if [[ $OSTYPE == darwin* ]]; then
xcode_path=$(xcode-select -p)
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)
Comment on lines -509 to 510
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.

@BalestraPatrick

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


bazelrc_lines+=("startup --host_jvm_args=-Xdock:name=$xcode_path")
Expand Down