ci: Build depends only once for Android build #21541
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
tACK 04254c7 Thanks again @MarcoFalke |
|
Code review ACK faf847f. Thanks for fixing this! It was affecting some of my prs (#17227 (comment)). In the future since the build system is pretty obscure it would be really helpful if commit messages didn't just say what was changing but also gave some hint why changes were being made.
|
|
Are you going to keep the latest "cirrus: temp" commit (04254c7)? |
|
no |
hebasto
reviewed
Mar 28, 2021
added 5 commits
March 28, 2021 20:04
The depends dir can not be overwritten by a FILE_ENV file. Also, a FILE_ENV file might depend on the DEPENDS_DIR value. Thus, set it before reading FILE_ENV. This commit does not change behavior, but is required for later commits. Can be reviewed with --color-moved=dimmed-zebra
Depends is currently built twice for the Android build. For example, the same task building it twice: * https://cirrus-ci.com/task/6673185279049728?logs=ci#L3418 (aarch64-linux-android) * https://cirrus-ci.com/task/6673185279049728?logs=ci#L3422 (x86_64-pc-linux-gnu, 4 lines later)
This does not change behavior, but bumping to Focal now means it doesn't have to be done later when Bionic is no longer used and EOL.
This does not change behavior, but removes unneeded and empty cache instructions for tasks that don't need them.
This cache entry is required for completeness. The file src/Makefile.qt.include needs it in this line: QT_BASE_PATH = $(shell find ../depends/sources/ -maxdepth 1 -type f -regex ".*qtbase.*\.tar.xz") This cache entry is tied to the depends_built_cache cache entry. Either both are present and cached, or neither of them is. Otherwise, the build will fail.
|
Added text to commit bodies |
|
Code review ACK fa52d7d. Only change since last review is adding descriptions and changing new RUN_UNIT_TESTS line from true to false. (I assume that change doesn't do anything because even though prior default was true, it's a cross compiled build and enabling unit tests would have no effect.) I'm still not clear if building depends twice before commit 2/5 "ci: Build depends only once for Android build" (fac577d) was actually causing problems or just a messy and unnecessary thing to do. But maybe it's not known. In any case the new descriptions are clear and very helpful! |
|
Maybe also optimize SDK cache: --- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -33,8 +33,6 @@ global_task_template: &GLOBAL_TASK_TEMPLATE
folder: "/tmp/ccache_dir"
depends_built_cache:
folder: "/tmp/cirrus-ci-build/depends/built"
- depends_sdk_cache:
- folder: "/tmp/cirrus-ci-build/depends/sdk-sources"
ci_script:
- ./ci/test_run_all.sh
@@ -161,6 +159,8 @@ task:
task:
name: 'macOS 10.14 [gui, no tests] [focal]'
+ depends_sdk_cache:
+ folder: "/tmp/cirrus-ci-build/depends/sdk-sources"
<< : *GLOBAL_TASK_TEMPLATE
container:
image: ubuntu:focal
@@ -185,6 +185,8 @@ task:
name: 'ARM64 Android APK [focal]'
depends_sources_cache:
folder: "/tmp/cirrus-ci-build/depends/sources"
+ depends_sdk_cache:
+ folder: "/tmp/cirrus-ci-build/depends/sdk-sources"
<< : *GLOBAL_TASK_TEMPLATE
container:
image: ubuntu:focal? |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently the Android task has several issues:
Fix those issues