-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Darwin CI: use macos-13 runners and other tweaks #32748
Changes from all commits
0bbc582
0d41c56
fecc7af
f5cd6cc
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 |
---|---|---|
|
@@ -12,49 +12,66 @@ inputs: | |
outputs: | ||
cache-hit: | ||
description: "Bootstrap environment was restored from cache" | ||
value: ${{ fromJSON(steps.bootstrap-cache.outputs.outputs).cache-hit }} # retry returns all outputs in `outputs` | ||
value: ${{ fromJSON(steps.restore.outputs.outputs).cache-hit }} # dynamic action returns all outputs in `outputs` | ||
|
||
runs: | ||
using: "composite" | ||
steps: | ||
- name: Calculate bootstrap cache key | ||
id: bootstrap-cache-key | ||
- name: Determine bootstrap cache configuration | ||
id: prepare | ||
shell: bash | ||
run: | | ||
# Calculate bootstrap cache key | ||
# Determine bootstrap cache configuration | ||
# In addition to the various setup files, the work directory matters as well, | ||
# because the bootstrapped Pigweed environment contains absolute paths. | ||
echo "Calculating bootstrap cache key for '$PWD'" | ||
FILES_HASH="${{ hashFiles('scripts/setup/*', 'third_party/pigweed/**') }}" | ||
FINAL_HASH="$(echo "$PWD:$FILES_HASH" | shasum -a 256 | cut -d' ' -f1)" | ||
echo "Calculated bootstrap cache key for '$PWD': $FINAL_HASH" | ||
echo "hash=$FINAL_HASH" >> "$GITHUB_OUTPUT" | ||
echo key="${RUNNER_OS}-${RUNNER_ARCH}-${{ inputs.platform }}-${FINAL_HASH}" | tee -a "$GITHUB_OUTPUT" | ||
|
||
- uses: Wandalen/[email protected] | ||
name: Bootstrap from cache | ||
id: bootstrap-cache | ||
# Split caches across backends | ||
case "$RUNNER_OS" in | ||
macOS) echo backend=actions;; | ||
*) echo backend=buildjet;; | ||
esac | tee -a "$GITHUB_OUTPUT" | ||
|
||
- name: Bootstrap from cache | ||
id: restore | ||
uses: ./.github/actions/dynamic | ||
continue-on-error: true | ||
with: | ||
action: buildjet/cache@v4 | ||
attempt_limit: 3 | ||
attempt_delay: 2000 | ||
action: ${{ steps.prepare.outputs.backend }}/cache/restore@v4 | ||
with: | | ||
key: ${{ runner.os }}-${{ runner.arch }}-${{ inputs.platform }}-${{ steps.bootstrap-cache-key.outputs.hash}} | ||
key: ${{ steps.prepare.outputs.key }} | ||
path: | | ||
.environment | ||
build_overrides/pigweed_environment.gni | ||
.environment | ||
build_overrides/pigweed_environment.gni | ||
|
||
- name: Run bootstrap | ||
if: fromJSON(steps.bootstrap-cache.outputs.outputs).cache-hit != 'true' | ||
if: ${{ fromJSON(steps.restore.outputs.outputs).cache-hit != 'true' }} | ||
env: | ||
PW_NO_CIPD_CACHE_DIR: Y | ||
PW_NO_CIPD_CACHE_DIR: 1 | ||
PW_ENVSETUP_NO_BANNER: 1 | ||
shell: bash | ||
run: source scripts/bootstrap.sh -p all,${{ inputs.platform }} | ||
|
||
- name: Uploading bootstrap logs | ||
- name: Save bootstrap cache | ||
uses: ./.github/actions/dynamic | ||
if: ${{ fromJSON(steps.restore.outputs.outputs).cache-hit != 'true' }} | ||
continue-on-error: true | ||
with: | ||
action: ${{ steps.prepare.outputs.backend }}/cache/save@v4 | ||
with: | | ||
key: ${{ steps.prepare.outputs.key }} | ||
path: | | ||
.environment | ||
build_overrides/pigweed_environment.gni | ||
|
||
- name: Upload bootstrap logs | ||
uses: actions/upload-artifact@v4 | ||
if: always() && !env.ACT && fromJSON(steps.bootstrap-cache.outputs.outputs).cache-hit != 'true' | ||
if: ${{ always() && !env.ACT && fromJSON(steps.restore.outputs.outputs).cache-hit != 'true' }} | ||
with: | ||
name: ${{ inputs.bootstrap-log-name }} | ||
path: | | ||
.environment/gn_out/.ninja_log | ||
.environment/pigweed-venv/*.log | ||
.environment/gn_out/.ninja_log | ||
.environment/pigweed-venv/*.log |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
name: Dynamic | ||
description: Dynamically resolves another GitHub action | ||
inputs: | ||
action: | ||
description: Action reference | ||
required: true | ||
with: | ||
description: Action inputs as multi-line YAML string | ||
required: false | ||
default: "" | ||
outputs: | ||
outputs: | ||
description: JSON object of outputs from the action | ||
value: ${{ steps.run.outputs.outputs }} | ||
runs: | ||
using: composite | ||
steps: | ||
- name: Instantiate | ||
shell: bash | ||
run: | | ||
# Dynamically invoke ${{ inputs.action }} | ||
with='${{ inputs.with }}' | ||
[[ -z "$with" ]] || with="$(echo ' with:'; sed -e 's/^/ /' <<<"$with")" | ||
mkdir -p ./.tmp/dynamic-action-instance | ||
cat <<END >./.tmp/dynamic-action-instance/action.yaml | ||
runs: | ||
using: composite | ||
steps: | ||
- id: run | ||
uses: ${{ inputs.action }} | ||
$with | ||
outputs: | ||
outputs: | ||
value: $(echo '$'){{ toJSON(steps.run.outputs) }} | ||
END | ||
- name: Run | ||
id: run | ||
uses: ./.tmp/dynamic-action-instance |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,7 +31,7 @@ jobs: | |
framework: | ||
name: Build framework | ||
if: github.actor != 'restyled-io[bot]' | ||
runs-on: macos-latest | ||
runs-on: macos-13 | ||
strategy: | ||
matrix: | ||
options: # We don't need a full matrix | ||
|
@@ -48,30 +48,28 @@ jobs: | |
steps: | ||
- name: Checkout | ||
uses: actions/checkout@v4 | ||
- name: Setup Environment | ||
run: brew install [email protected] | ||
- name: Checkout submodules & Bootstrap | ||
uses: ./.github/actions/checkout-submodules-and-bootstrap | ||
with: | ||
platform: darwin | ||
bootstrap-log-name: bootstrap-logs-framework-${{ matrix.options.flavor }} | ||
- name: Block zap-cli from being used | ||
env: | ||
PW_ENVSETUP_NO_BANNER: 1 | ||
run: | | ||
# Framework builds are NOT expected to require zap-cli | ||
scripts/run_in_build_env.sh 'D=$(dirname $(which zap-cli)) && mv $D/zap-cli $D/zap-cli.moved' | ||
scripts/run_in_build_env.sh 'rm -- "$(which zap-cli)"' | ||
andy31415 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# run_in_build_env.sh is used to ensure PATH is set to something that would otherwise find zap-cli | ||
scripts/run_in_build_env.sh '(zap-cli --version && exit 1) || exit 0' | ||
- name: Build | ||
working-directory: src/darwin/Framework | ||
run: xcodebuild -target "Matter" ${{ matrix.options.arguments }} | ||
# Now restore zap-cli, so that bootstrap caching does not cache the state when it's been renamed. | ||
- name: Make zap-cli work again | ||
run: scripts/run_in_build_env.sh 'D=$(dirname $(which zap-cli.moved)) && mv $D/zap-cli.moved $D/zap-cli' | ||
|
||
tests: | ||
name: Run framework tests | ||
if: github.actor != 'restyled-io[bot]' | ||
runs-on: macos-latest | ||
needs: [ framework ] # serialize to avoid running to many parallel macos runners | ||
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. "too many" 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. Does this really help things? My mental model is that each job that runs in a separate runner has some fixed overhead (checkout, bootstrap) plus whatever work it actually needs to do. Serializing things but still using separate runners does not remove any of the fixed overhead, right? So it increases latency to completion for a single PR, probably does not decrease latency to completion when multiple PRs need jobs run in parallel. Might reduce latency to error detection when multiple PRs are run in parallel. Again, if there is a benefit here that I am missing, maybe the comments could more clearly talk about the tradeoffs and why we are making this one? 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. This is somewhat experimental... will see if this makes the overall queueing behavior worse or better. |
||
runs-on: macos-13 | ||
strategy: | ||
matrix: | ||
options: # We don't need a full matrix | ||
|
@@ -85,8 +83,6 @@ jobs: | |
steps: | ||
- name: Checkout | ||
uses: actions/checkout@v4 | ||
- name: Setup Environment | ||
run: brew install [email protected] | ||
- name: Checkout submodules & Bootstrap | ||
uses: ./.github/actions/checkout-submodules-and-bootstrap | ||
with: | ||
|
@@ -120,6 +116,7 @@ jobs: | |
OTHER_CFLAGS='${inherited} -Werror -Wconversion' CHIP_IS_BLE=NO GCC_PREPROCESSOR_DEFINITIONS='${inherited} MTR_NO_AVAILABILITY=1 ${{ matrix.options.defines }}' \ | ||
> >(tee /tmp/darwin/framework-tests/darwin-tests.log) 2> >(tee /tmp/darwin/framework-tests/darwin-tests-err.log >&2) | ||
- name: Collect crash logs | ||
if: failure() && !env.ACT | ||
run: | | ||
mkdir -p /tmp/darwin/framework-tests | ||
find ~/Library/Developer/Xcode/DerivedData /Library/Logs/DiagnosticReports -name '*.ips' -print0 | xargs -0 -J % cp % /tmp/darwin/framework-tests | ||
|
@@ -134,24 +131,15 @@ jobs: | |
tv-casting-bridge: | ||
name: Build TV Casting Bridge example | ||
if: github.actor != 'restyled-io[bot]' | ||
runs-on: macos-latest | ||
needs: [ framework ] # serialize to avoid running to many parallel macos runners | ||
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. "too many" |
||
runs-on: macos-13 | ||
steps: | ||
- name: Checkout | ||
uses: actions/checkout@v4 | ||
- name: Setup Environment | ||
run: brew install [email protected] | ||
- name: Checkout submodules & Bootstrap | ||
uses: ./.github/actions/checkout-submodules-and-bootstrap | ||
with: | ||
platform: darwin | ||
- name: Build | ||
working-directory: examples/tv-casting-app/darwin/MatterTvCastingBridge | ||
run: xcodebuild -target "MatterTvCastingBridge" -sdk iphoneos | ||
|
||
darwin: | ||
name: Build Darwin # Matches the previous monolithic build that's marked "required" for PRs | ||
needs: [ framework, tests ] | ||
runs-on: macos-latest | ||
steps: | ||
- name: Done | ||
run: 'true' # nothing to do |
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.
This could use comments explaining why we are doing that. Because I don't understand why we're doing that.