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.

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

[plz rebase merge] Cache Sentry Builds to S3 #3392

Merged
merged 2 commits into from
Jul 31, 2021

Conversation

rpadaki
Copy link
Contributor

@rpadaki rpadaki commented Jul 29, 2021

Description
This PR should greatly reduce protocol build time by building Sentry ahead of time! Note that this PR should go after #3388.

Implementation

I first wrote a workflow job to build Sentry/Crashpad on Windows/Linux/macOS. Then, I added Sentry to our protocol cmake while removing the old Sentry pipeline.

Importantly, we have to make sure not to build Sentry tests on Windows, since there's an outstanding issue solved by getsentry/sentry-native#583. Once that fix is in, we can bump our Sentry version to make our CI a little more robust, though I don't think we want to build tests in any case.

Note that Sentry by default builds with -O3, as I manually confirmed in its cmake configuration. I only chose build type Release to get rid of debug symbols.

@github-actions github-actions bot added the 📁 Repo: protocol This PR/Issue modifies /protocol code label Jul 29, 2021
@rpadaki rpadaki force-pushed the roshan/protocol/cache-sentry-builds-to-s3 branch 9 times, most recently from ebd2236 to edf4375 Compare July 30, 2021 03:31
Base automatically changed from roshan/protocol/remove-global-binary-downloads to dev July 30, 2021 04:03
@rpadaki rpadaki force-pushed the roshan/protocol/cache-sentry-builds-to-s3 branch from edf4375 to ce1e32b Compare July 30, 2021 04:05
@rpadaki rpadaki changed the title Cache Sentry Builds to S3 [plz rebase merge] Cache Sentry Builds to S3 Jul 30, 2021
@netlify
Copy link

netlify bot commented Jul 30, 2021

✔️ Deploy Preview for fractal-dev canceled.

🔨 Explore the source changes: af11cb3

🔍 Inspect the deploy log: https://app.netlify.com/sites/fractal-dev/deploys/6104296adf1f50000759fdd9

philippemnoel
philippemnoel previously approved these changes Jul 30, 2021
Copy link
Owner

@philippemnoel philippemnoel left a comment

Choose a reason for hiding this comment

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

thanks I've been meaning to get around to this for a long time

djsavvy
djsavvy previously approved these changes Jul 30, 2021
sardination
sardination previously approved these changes Jul 30, 2021
@rpadaki rpadaki dismissed stale reviews from sardination, djsavvy, and philippemnoel via 8369e5a July 30, 2021 15:46
@rpadaki rpadaki force-pushed the roshan/protocol/cache-sentry-builds-to-s3 branch 2 times, most recently from 8369e5a to af11cb3 Compare July 30, 2021 16:31
@djsavvy djsavvy requested a review from sardination July 30, 2021 17:16
Copy link
Owner

@philippemnoel philippemnoel left a comment

Choose a reason for hiding this comment

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

CI part looks good to me

@philippemnoel philippemnoel merged commit 2fb55a3 into dev Jul 31, 2021
@philippemnoel philippemnoel deleted the roshan/protocol/cache-sentry-builds-to-s3 branch July 31, 2021 03:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📁 Repo: protocol This PR/Issue modifies /protocol code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pre-build sentry-native like we do for SDL and FFmpeg, and check for O3 compiler flag
4 participants