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

Test Windows runtime on presubmit, merge build/test runtime jobs. #11032

Merged
merged 2 commits into from
Nov 7, 2022
Merged

Test Windows runtime on presubmit, merge build/test runtime jobs. #11032

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
03ec4bd
Test Windows runtime on presubmit, merge build/test runtime jobs.
ScottTodd Nov 4, 2022
546a7e3
Merge remote-tracking branch 'upstream/main' into windows-ci-runtime-…
ScottTodd Nov 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 21 additions & 48 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,16 +252,12 @@ jobs:
################################## Subsets ###################################
# Jobs that build some subset of IREE
##############################################################################
build_runtime:
build_test_runtime:
needs: setup
if: needs.setup.outputs.should-run == 'true'
runs-on: ubuntu-20.04
env:
BUILD_DIR: build-runtime
outputs:
# Pass through the build directory as output so it's available to
# dependent jobs.
build-dir: ${{ env.BUILD_DIR }}
steps:
- name: "Checking out repository"
uses: actions/checkout@7884fcad6b5d53d10323aee724dc68d8b9096a2e # v2
Expand All @@ -273,57 +269,35 @@ jobs:
gcr.io/iree-oss/base@sha256:5d43683c6b50aebe1fca6c85f2012f3b0fa153bf4dd268e8767b619b1891423a \
./build_tools/cmake/build_runtime.sh \
"${BUILD_DIR}"
# Using a tar archive is necessary to preserve file permissions. See
# https://github.com/actions/upload-artifact#maintaining-file-permissions-and-case-sensitive-files
# The upload action already does its own gzip compression, so no need to
# do our own.
- name: "Create build dir archive"
run: tar -cf ${BUILD_DIR}.tar ${BUILD_DIR}
- uses: actions/upload-artifact@3cea5372237819ed00197afe530f5a7ea3e805c8 # v3.1.0
with:
name: "${{ env.BUILD_DIR }}.tar"
path: "${{ env.BUILD_DIR }}.tar"
- name: "Testing runtime"
run: |
./build_tools/github_actions/docker_run.sh \
--env IREE_VULKAN_DISABLE=1 \
gcr.io/iree-oss/base@sha256:5d43683c6b50aebe1fca6c85f2012f3b0fa153bf4dd268e8767b619b1891423a \
./build_tools/cmake/ctest_all.sh \
"${BUILD_DIR}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe make this a single docker run call as in our other build&test workflows

Copy link
Member Author

Choose a reason for hiding this comment

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

I like having build and test in separate steps, so we get a nice timing breakdown in the action summary:

image

Duplicating the docker command in each step does repeat many characters though. Can we put our Docker image in uses? https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#example-using-a-docker-hub-action Or set a container? https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idcontainer

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not just characters repeating: it's also tearing down and relaunching the container (although I'm not sure how long that takes, in practice. Apparently less than 3s). I've steered away from GitHub actions specific ways of using Docker containers because they're more tied to the CI and therefore harder to reproduce locally or migrate to other CI systems. That includes translating all the docker run options into the GitHub Actions syntax. The script for doing this does additional setup (like creating directories to mount), that would have to be sharded out elsewhere. Overall, it just seems like a false simplification. One option we have would be to start the container running in one step and then use docker exec to run various commands in it.

Regardless, what you've got here is fine for now, though I do still have a slight preference for doing it in a single step to match how other jobs work and avoid launching a container multiple times (although that doesn't seem to have meaningful latency overhead). If you wanted to follow-up by splitting all of them, then that would be fine too :-P


build_runtime_windows:
build_test_runtime_windows:
needs: setup
if: needs.setup.outputs.should-run == 'true' && needs.setup.outputs.ci-stage == 'postsubmit'
runs-on: windows-2022
if: needs.setup.outputs.should-run == 'true'
runs-on: managed-windows-cpu
defaults:
run:
shell: bash
env:
BUILD_DIR: build-runtime
BUILD_DIR: build-runtime-windows
IREE_VULKAN_DISABLE: 1
steps:
- name: "Checking out repository"
uses: actions/checkout@7884fcad6b5d53d10323aee724dc68d8b9096a2e # v2
- name: "Checking out runtime submodules"
run: bash ./build_tools/scripts/git/update_runtime_submodules.sh
run: ./build_tools/scripts/git/update_runtime_submodules.sh
- name: "Configuring MSVC"
uses: ilammy/msvc-dev-cmd@7315a94840631165970262a99c72cfb48a65d25d # v1.12.0
- name: "Building runtime"
run: bash ./build_tools/cmake/build_runtime.sh "${BUILD_DIR}"

test_runtime:
needs: [setup, build_runtime]
if: needs.setup.outputs.should-run == 'true'
runs-on: ubuntu-20.04
env:
BUILD_DIR: ${{ needs.build_runtime.outputs.build-dir }}
steps:
- name: "Checking out repository"
uses: actions/checkout@7884fcad6b5d53d10323aee724dc68d8b9096a2e # v2
with:
submodules: true
- name: "Downloading runtime build dir archive"
uses: actions/download-artifact@fb598a63ae348fa914e94cd0ff38f362e927b741 # v3.0.0
with:
name: "${{ env.BUILD_DIR }}.tar"
- name: "Extracting runtime build dir archive"
run: tar -xf ${BUILD_DIR}.tar
run: ./build_tools/cmake/build_runtime.sh "${BUILD_DIR}"
- name: "Testing runtime"
run: |
./build_tools/github_actions/docker_run.sh \
--env IREE_VULKAN_DISABLE=1 \
gcr.io/iree-oss/base@sha256:5d43683c6b50aebe1fca6c85f2012f3b0fa153bf4dd268e8767b619b1891423a \
./build_tools/cmake/ctest_all.sh \
"${BUILD_DIR}"
run: ./build_tools/cmake/ctest_all.sh "${BUILD_DIR}"

################################# Tensorflow #################################
# Jobs that build the IREE-Tensorflow integrations
Expand Down Expand Up @@ -773,9 +747,8 @@ jobs:
- test_gpu

# Subsets
- build_runtime
- build_runtime_windows
- test_runtime
- build_test_runtime
- build_test_runtime_windows

# Tensorflow
- build_tf_integrations
Expand Down