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

add github actions workflow for build and test on linux #973

Merged
merged 7 commits into from
Apr 23, 2020
Merged

add github actions workflow for build and test on linux #973

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
baac1e3
add github actions workflow for build and test on linux
toffaletti Mar 30, 2020
e521fd3
always build protobuf
toffaletti Apr 4, 2020
a997c4b
add git commit hash to cache key
toffaletti Apr 4, 2020
8662ac2
add cache fallback restore-key temporarily
toffaletti Apr 4, 2020
b0e6e4e
fixes for caching and checkout
toffaletti Apr 4, 2020
76e9a7e
add swift 4.2.4, 5.2.1, and protobuf current master commit hash
toffaletti Apr 4, 2020
7433747
include swift image platform in cache key
toffaletti Apr 4, 2020
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
Prev Previous commit
include swift image platform in cache key
  • Loading branch information
@toffaletti
toffaletti committed Apr 4, 2020
commit 743374730bf3a1999f75cc224827bd7f965d49ec
9 changes: 8 additions & 1 deletion .github/workflows/build.yml
View file
Original file line number Diff line number Diff line change
@@ -14,6 +14,13 @@ jobs:
swift: ["5.2.1-bionic", "5.1.5-bionic", "4.2.4"]
# master as of 4/4/2020: cf601047ebf87cf7f443753ded41132eb689cb10
protobuf_gitref: ["v3.11.4", "cf601047ebf87cf7f443753ded41132eb689cb10"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So we just update that one hash when we want to test a newer version of master? Works for me.

Yes, though this was a temporary workaround until I got an answer here: actions/checkout#209
With that I will be able to put "master" in this list and make the caching safe.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it worth landing that fix now while @tbkka works on getting the hook set up?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, I'll work on it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Issue 993 is a newer failure on updated conformance tests, so it would be nice to get our master validation update to float with those commits.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@toffaletti how do we check if the caching of things is working? Looking at https://github.com/apple/swift-protobuf/pull/994/checks it seems like it is building protobuf, I would have thought it would be cached from when @tbkka set things up and did a test PR to ensure they built?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’ll take a look.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree it seems like the cache should have had a cache hit, in
https://github.com/apple/swift-protobuf/runs/664761605?check_suite_focus=true I see

Cache not found for input keys: Linux-ubuntu-18.04-protobuf-cf601047ebf87cf7f443753ded41132eb689cb10.

then later in Post Cache:

Cache saved successfully

And in the other parallel builds:

Running JavaScript Action with default external tool: node12
[warning]Cache already exists. Scope: refs/pull/991/merge, Key: Linux-ubuntu-18.04-protobuf-cf601047ebf87cf7f443753ded41132eb689cb10, Version: (null)

But then 3 days later in your PR a cache miss for the same key:

https://github.com/apple/swift-protobuf/runs/674773193?check_suite_focus=true

Cache not found for input keys: Linux-ubuntu-18.04-protobuf-cf601047ebf87cf7f443753ded41132eb689cb10.

However, I do see at least one cache hit:

https://github.com/apple/swift-protobuf/runs/675011233?check_suite_focus=true

Cache Size: ~182 MB (190677067 B)
/bin/tar -xz -f /__w/_temp/90437206-2a9e-46a5-9320-7515799d57c5/cache.tgz -C /__w/swift-protobuf/swift-protobuf/protobuf
Cache restored from key: Linux-ubuntu-18.04-protobuf-6935eae45c99926a000ecbef0be20dfd3d159e71

It seems like the cache isn't reliable. I will open an issue on https://github.com/actions/cache and update here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actions/cache#310 - will see if they can investigate this better than I'm able to. Meanwhile I'll work on the improved caching for floating on master.

include:
- swift: "5.2.1-bionic"
platform: "ubuntu-18.04"
- swift: "5.1.5-bionic"
platform: "ubuntu-18.04"
- swift: "4.2.4"
platform: "ubuntu-16.04"
container:
image: swift:${{ matrix.swift }}
steps:
@@ -33,7 +40,7 @@ jobs:
with:
path: protobuf
# NOTE: for refs that can float like 'master' the cache might be out of date!
key: ${{ runner.os }}-protobuf-${{ matrix.protobuf_gitref }}
key: ${{ runner.os }}-${{ matrix.platform}}-protobuf-${{ matrix.protobuf_gitref }}
- name: Update and install dependencies
if: steps.cache-protobuf.outputs.cache-hit != 'true'
# dependencies from https://github.com/protocolbuffers/protobuf/blob/master/src/README.md