ci: Re-introduce `depends_built` cache back in macOS and Android tasks #27062

hebasto commented at 12:24 PM on February 8, 2023: member

This PR brings a depends_built cache back to the "macOS 10.15" and "ARM64 Android APK" CI tasks.

Fixes #27031.

DrahtBot commented at 12:24 PM on February 8, 2023: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	MarcoFalke

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#25797 (build: Add CMake-based build system by hebasto)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot added the label Tests on Feb 8, 2023

hebasto force-pushed on Feb 8, 2023

hebasto renamed this:
~~ci: Use `GLOBAL_TASK_TEMPLATE` in "macOS 10.15" task~~
ci: Re-introduce `depends_built` cache` back in "macOS 10.15" task
on Feb 8, 2023

hebasto renamed this:
~~ci: Re-introduce `depends_built` cache` back in "macOS 10.15" task~~
ci: Re-introduce `depends_built` cache back in "macOS 10.15" task
on Feb 8, 2023

hebasto commented at 12:33 PM on February 8, 2023: member

Updated commit message.

maflcko commented at 12:35 PM on February 8, 2023: member

When was this introduced?

in .cirrus.yml:58 in f6b4bedf39 outdated

  49 | @@ -50,6 +50,9 @@ global_task_template: &GLOBAL_TASK_TEMPLATE
  50 |    depends_built_cache:
  51 |      folder: "depends/built"
  52 |      fingerprint_script: echo $CIRRUS_TASK_NAME $(git rev-list -1 HEAD ./depends)
  53 | +
  54 | +global_task_template: &GLOBAL_TASK_TEMPLATE
  55 | +  << : *CONTAINER_DEPENDS_TEMPLATE
  56 |    << : *MAIN_TEMPLATE
  57 |  
  58 |  macos_native_task_template: &MACOS_NATIVE_TASK_TEMPLATE

maflcko commented at 12:35 PM on February 8, 2023:

unrelated: What is the point of a template that is only used once? We don't have a template for windows either.

hebasto commented at 12:37 PM on February 8, 2023:

There is no point now. The initial idea was to re-use it with two Apple tasks: x86_64 and arm64.

maflcko commented at 10:13 AM on February 9, 2023:

nit: Mind inlining it here?

maflcko commented at 10:13 AM on February 9, 2023:

(in a new commit)

hebasto commented at 10:32 AM on February 9, 2023:

nit: Mind inlining it here?

(in a new commit)

Done.

hebasto commented at 12:54 PM on February 8, 2023: member

When was this introduced?

8e017f32889e9536a436f40c91f34f24bfd10525 :man_facepalming:

hebasto force-pushed on Feb 9, 2023

in .cirrus.yml:315 in ffac7397b6 outdated

 321 |    << : *MAIN_TEMPLATE
 322 | -  container:
 323 | -    image: ubuntu:focal
 324 |    env:
 325 |      MACOS_SDK: "Xcode-12.2-12B45b-extracted-SDK-with-libcxx-headers"
 326 |      << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV

maflcko commented at 12:04 PM on February 9, 2023:

first commit: Isn't the same diff needed by the android task?

hebasto commented at 12:13 PM on February 9, 2023:

Thanks! Added.

maflcko commented at 12:05 PM on February 9, 2023: member

review ACK 8f8e54277d785b287fb062687bd349a36e1b4e9e 🎻

<details><summary>Show signature</summary>

Signature:

</details>

ci: Re-introduce `depends_built` cache back in macOS and Android tasks 8a61527cf6

ci: Inline `MACOS_NATIVE_TASK_TEMPLATE` 73a3b161b7

hebasto force-pushed on Feb 9, 2023

hebasto renamed this:
~~ci: Re-introduce `depends_built` cache back in "macOS 10.15" task~~
ci: Re-introduce `depends_built` cache back in macOS and Android tasks
on Feb 9, 2023

maflcko commented at 6:05 PM on February 9, 2023: member

reACK 73a3b161b7bb29327d0a529235cdc7d0295bd316 🌻

<details><summary>Show signature</summary>

Signature:

</details>

maflcko merged this on Feb 9, 2023

maflcko closed this on Feb 9, 2023

hebasto deleted the branch on Feb 9, 2023

in .cirrus.yml:42 in 73a3b161b7

  38 | @@ -39,7 +39,7 @@ main_template: &MAIN_TEMPLATE
  39 |    ci_script:
  40 |      - ./ci/test_run_all.sh
  41 |  
  42 | -global_task_template: &GLOBAL_TASK_TEMPLATE
  43 | +container_depends_template: &CONTAINER_DEPENDS_TEMPLATE

maflcko commented at 6:41 PM on February 9, 2023:

Any reason to add a new template when the existing can be used? (Untested diff):

diff --git a/.cirrus.yml b/.cirrus.yml
index 232b24117a..9367d50f6b 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -39,7 +39,7 @@ main_template: &MAIN_TEMPLATE
   ci_script:
     - ./ci/test_run_all.sh
 
-container_depends_template: &CONTAINER_DEPENDS_TEMPLATE
+global_task_template: &GLOBAL_TASK_TEMPLATE
   << : *BASE_TEMPLATE
   container:
     # https://cirrus-ci.org/faq/#are-there-any-limits
@@ -50,9 +50,6 @@ container_depends_template: &CONTAINER_DEPENDS_TEMPLATE
   depends_built_cache:
     folder: "depends/built"
     fingerprint_script: echo $CIRRUS_TASK_NAME $(git rev-list -1 HEAD ./depends)
-
-global_task_template: &GLOBAL_TASK_TEMPLATE
-  << : *CONTAINER_DEPENDS_TEMPLATE
   << : *MAIN_TEMPLATE
 
 compute_credits_template: &CREDITS_TEMPLATE
@@ -303,13 +300,12 @@ task:
 
 task:
   name: 'macOS 10.15 [gui, no tests] [focal]'
-  << : *CONTAINER_DEPENDS_TEMPLATE
-  container:
-    image: ubuntu:focal
   macos_sdk_cache:
     folder: "depends/SDKs/$MACOS_SDK"
     fingerprint_key: "$MACOS_SDK"
-  << : *MAIN_TEMPLATE
+  << : *GLOBAL_TASK_TEMPLATE
+  container:
+    image: ubuntu:focal
   env:
     MACOS_SDK: "Xcode-12.2-12B45b-extracted-SDK-with-libcxx-headers"
     << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV
@@ -334,16 +330,15 @@ task:
 
 task:
   name: 'ARM64 Android APK [jammy]'
-  << : *CONTAINER_DEPENDS_TEMPLATE
-  container:
-    image: ubuntu:jammy
   android_sdk_cache:
     folder: "depends/SDKs/android"
     fingerprint_key: "ANDROID_API_LEVEL=28 ANDROID_BUILD_TOOLS_VERSION=28.0.3 ANDROID_NDK_VERSION=23.2.8568313"
   depends_sources_cache:
     folder: "depends/sources"
     fingerprint_script: git rev-list -1 HEAD ./depends
-  << : *MAIN_TEMPLATE
+  << : *GLOBAL_TASK_TEMPLATE
+  container:
+    image: ubuntu:jammy
   env:
     << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV
     FILE_ENV: "./ci/test/00_setup_env_android.sh"

hebasto commented at 6:57 PM on February 9, 2023:

Untested diff

It looks correct and elegant.

sidhujag referenced this in commit aa8cb5e99f on Feb 10, 2023

bitcoin locked this on Feb 9, 2024