-
Notifications
You must be signed in to change notification settings - Fork 202
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
ci: use alias latest and minimum to represent xcode cmd tool version #3465
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,62 +5,99 @@ inputs: | |
required: true | ||
type: string | ||
xcode_version: | ||
description: 'The version of Xcode. Valid values are 14.3 and 15.0' | ||
required: true | ||
description: "The version of Xcode. Available aliases are 'latest' and 'minimum'" | ||
default: 'latest' | ||
type: string | ||
destination: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this new When is it used or needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it is used to define the exact destiation. In case of |
||
description: "The destination associated with the given platform and Xcode version" | ||
default: '' | ||
type: string | ||
|
||
outputs: | ||
destination: | ||
description: "The destination associated with the given platform and Xcode version" | ||
value: ${{ steps.platform.outputs.destination }} | ||
value: ${{ steps.get-destination.outputs.destination }} | ||
sdk: | ||
description: "The SDK associated with the given platform" | ||
value: ${{ steps.platform.outputs.sdk }} | ||
value: ${{ steps.get-sdk.outputs.sdk }} | ||
xcode-version: | ||
description: "The Xcode version to build with" | ||
value: ${{ steps.get-xcode-version.outputs.xcode-version }} | ||
|
||
runs: | ||
using: "composite" | ||
steps: | ||
- id: platform | ||
- name: Validate platform | ||
run: | | ||
PLATFORM=${{ inputs.platform }} | ||
case $PLATFORM in | ||
INPUT_PLATFORM=${{ inputs.platform }} | ||
case $INPUT_PLATFORM in | ||
iOS|tvOS|watchOS|macOS) ;; | ||
*) echo "Unsupported platform: $PLATFORM"; exit 1 ;; | ||
*) echo "Unsupported platform: $INPUT_PLATFORM"; exit 1 ;; | ||
esac | ||
shell: bash | ||
|
||
- id: get-xcode-version | ||
run: | | ||
LATEST_XCODE_VERSION=14.3.1 | ||
MINIMUM_XCODE_VERSION=14.0.1 | ||
|
||
XCODE_VERSION=${{ inputs.xcode_version }} | ||
case $XCODE_VERSION in | ||
14.0.1|14.3|15.0) ;; | ||
*) echo "Unsupported Xcode version: $XCODE_VERSION"; exit 1 ;; | ||
INPUT_XCODE_VERSION=${{ inputs.xcode_version }} | ||
|
||
case $INPUT_XCODE_VERSION in | ||
latest) | ||
XCODE_VERSION=$LATEST_XCODE_VERSION ;; | ||
minimum) | ||
XCODE_VERSION=$MINIMUM_XCODE_VERSION ;; | ||
*) | ||
XCODE_VERSION=$INPUT_XCODE_VERSION ;; | ||
esac | ||
echo "xcode-version=$XCODE_VERSION" >> $GITHUB_OUTPUT | ||
|
||
shell: bash | ||
|
||
- id: get-destination | ||
run: | | ||
INPUT_PLATFORM=${{ inputs.platform }} | ||
INPUT_DESTINATION='${{ inputs.destination }}' | ||
INPUT_XCODE_VERSION=${{ inputs.xcode_version }} | ||
|
||
DESTINATION_MAPPING='{ | ||
"14.0.1": { | ||
"minimum": { | ||
"iOS": "platform=iOS Simulator,name=iPhone 14,OS=16.0", | ||
"tvOS": "platform=tvOS Simulator,name=Apple TV 4K (2nd generation),OS=16.0", | ||
"watchOS": "platform=watchOS Simulator,name=Apple Watch Series 8 (45mm),OS=9.0", | ||
"macOS": "platform=macOS,arch=x86_64" | ||
}, | ||
"14.3": { | ||
"latest": { | ||
"iOS": "platform=iOS Simulator,name=iPhone 14,OS=16.4", | ||
"tvOS": "platform=tvOS Simulator,name=Apple TV 4K (3rd generation),OS=16.4", | ||
"watchOS": "platform=watchOS Simulator,name=Apple Watch Series 8 (45mm),OS=9.4", | ||
"macOS": "platform=macOS,arch=x86_64" | ||
}, | ||
"15.0": { | ||
"iOS": "platform=iOS Simulator,name=iPhone 15,OS=latest", | ||
"tvOS": "platform=tvOS Simulator,name=Apple TV 4K (3rd generation),OS=latest", | ||
"watchOS": "platform=watchOS Simulator,name=Apple Watch Series 9 (45mm),OS=latest", | ||
"macOS": "platform=macOS,arch=x86_64" | ||
} | ||
}' | ||
|
||
if [ -z "$INPUT_DESTINATION" ]; then | ||
DESTINATION=$(echo $DESTINATION_MAPPING | jq -r ".\"$INPUT_XCODE_VERSION\".$INPUT_PLATFORM") | ||
else | ||
DESTINATION=$INPUT_DESTINATION | ||
fi | ||
|
||
if [ -z "$DESTINATION" ]; then | ||
echo "No available destination to build for" | ||
exit 1 | ||
fi | ||
echo "destination=$DESTINATION" >> $GITHUB_OUTPUT | ||
shell: bash | ||
|
||
- id: get-sdk | ||
run: | | ||
INPUT_PLATFORM=${{ inputs.platform }} | ||
SDK_MAPPING='{ | ||
"iOS": "iphonesimulator", | ||
"tvOS": "appletvsimulator", | ||
"watchOS": "watchsimulator", | ||
"macOS": "macosx" | ||
}' | ||
|
||
echo "destination=$(echo $DESTINATION_MAPPING | jq -r ."\"$XCODE_VERSION\"".$PLATFORM)" >> $GITHUB_OUTPUT | ||
echo "sdk=$(echo $SDK_MAPPING | jq -r .$PLATFORM)" >> $GITHUB_OUTPUT | ||
echo "sdk=$(echo $SDK_MAPPING | jq -r .$INPUT_PLATFORM)" >> $GITHUB_OUTPUT | ||
shell: bash | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,7 +39,8 @@ jobs: | |
scheme: ${{ matrix.platform == 'watchOS' && 'AWSCloudWatchLoggingPluginIntegrationTestsWatch' || 'AWSCloudWatchLoggingPluginIntegrationTests' }} | ||
platform: ${{ matrix.platform }} | ||
project_path: ./AmplifyPlugins/Logging/Tests/AWSCloudWatchLoggingPluginHostApp | ||
xcode_version: ${{ matrix.platform == 'watchOS' && '15.0' || '14.3' }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These tests used to run on Xcode 15.0 for watchOS, but now they will use 14.3. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good catch, will update it. |
||
resource_subfolder: logging | ||
xcode_version: ${{ matrix.platform == 'watchOS' && '15.0' || 'latest' }} | ||
destination: ${{ matrix.platform == 'watchOS' && 'platform=watchOS Simulator,name=Apple Watch Series 8 (45mm),OS=10.2' || '' }} | ||
timeout-minutes: 60 | ||
secrets: inherit |
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.
Why not just add the new aliases but still support passing an explicit version?
Because now
"latest
" is not really the latest (i.e. 15.0), so it's kinda misleading to call it like that.If we have
latest = 15.0
set, but we explicitly pass14.3
in our tests invocations, it's clear we need to investigate why we cannot upgrade.So basically what I'm saying is keeping
14.0.1
,14.3
and15.0
as allowed values, but also allowinglatest
(which we consider to be15.0
) andminimum
(which we consider to be14.0.1
).Then you can just use
latest
in tests that support it, but set14.3
in the ones that don't.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.
It is possible to pass a specific version. The alias available is only
latest
andminimum
. You can still pass15.1
asxcode_version
input. Also, you will need to specify thedestination
for it.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.
Aah I see, thanks for clarifying that :)
If setting an explicit destination is indeed an "edge" case (i.e. when testing new beta versions, or for a reduced number of tests), then that's fine. But if we have to write the whole
destination
in many files, then this action becomes irrelevant and exactly what it tried to replace :Ptl;dr: this is fine as long as we don't have to set an explicit xcode version in more than 1 workflow, otherwise we should add the mapped destination for that xcode version and save ourselves one long and messy input.
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.
yeah, if that's the case, we might need to consider adding another alias
beta
and the destination mapping in the script.