Hi,

We have CI support for mingw, but don't run the task by default, as it eats up
precious CI credits.  However, for cfbot we are using custom compute resources
(currently donated by google), so we can afford to run the mingw tests. Right
now that'd require cfbot to patch .cirrus.tasks.yml.

While one can manually trigger manual task in one one's own repo, most won't
have the permissions to do so for cfbot.


I propose that we instead run the task automatically if
$REPO_MINGW_TRIGGER_BY_DEFAULT is set, typically in cirrus' per-repository
configuration.

Unfortunately that's somewhat awkward to do in the cirrus-ci yaml
configuration, the set of conditional expressions supported is very
simplistic.

To deal with that, I extended .cirrus.star to compute the required environment
variable. If $REPO_MINGW_TRIGGER_BY_DEFAULT is set, CI_MINGW_TRIGGER_TYPE is
set to 'automatic', if not it's 'manual'.


We've also talked in other threads about adding CI support for
1) windows, building with visual studio
2) linux, with musl libc
3) free/netbsd

That becomes more enticing, if we can enable them by default on cfbot but not
elsewhere. With this change, it'd be easy to add further variables to control
such future tasks.



I also attached a second commit, that makes the "code" dealing with ci-os-only
in .cirrus.tasks.yml simpler. While I think it does nicely simplify
.cirrus.tasks.yml, overall it adds lines, possibly making this not worth it.
I'm somewhat on the fence.


Thoughts?


On the code level, I thought if it'd be good to have a common prefix for all
the automatically set variables. Right now that's CI_, but I'm not at all
wedded to that.


Greetings,

Andres Freund
>From 5d26ecfedbcbc83b4cb6e41a34c1af9a3ab24cdb Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Fri, 12 Apr 2024 15:04:32 -0700
Subject: [PATCH v2 1/2] ci: Allow running mingw tests by default via
 environment variable

---
 .cirrus.yml         | 12 ++++++++++--
 .cirrus.star        | 39 +++++++++++++++++++++++++++++++++++----
 .cirrus.tasks.yml   | 12 ++++++------
 src/tools/ci/README | 12 ++++++++++++
 4 files changed, 63 insertions(+), 12 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index a83129ae46d..f270f61241f 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -10,12 +10,20 @@
 #
 # 1) the contents of this file
 #
-# 2) if defined, the contents of the file referenced by the, repository
+# 2) computed environment variables
+#
+#    Used to enable/disable tasks based on the execution environment. See
+#    .cirrus.star: compute_environment_vars()
+#
+# 3) if defined, the contents of the file referenced by the, repository
 #    level, REPO_CI_CONFIG_GIT_URL variable (see
 #    https://cirrus-ci.org/guide/programming-tasks/#fs for the accepted
 #    format)
 #
-# 3) .cirrus.tasks.yml
+#    This allows running tasks in a different execution environment than the
+#    default, e.g. to have sufficient resources for cfbot.
+#
+# 4) .cirrus.tasks.yml
 #
 # This composition is done by .cirrus.star
 
diff --git a/.cirrus.star b/.cirrus.star
index 36233872d1e..7a164bb00de 100644
--- a/.cirrus.star
+++ b/.cirrus.star
@@ -7,7 +7,7 @@ https://github.com/bazelbuild/starlark/blob/master/spec.md
 See also .cirrus.yml and src/tools/ci/README
 """
 
-load("cirrus", "env", "fs")
+load("cirrus", "env", "fs", "yaml")
 
 
 def main():
@@ -18,19 +18,36 @@ def main():
 
     1) the contents of .cirrus.yml
 
-    2) if defined, the contents of the file referenced by the, repository
+    2) computed environment variables
+
+    3) if defined, the contents of the file referenced by the, repository
        level, REPO_CI_CONFIG_GIT_URL variable (see
        https://cirrus-ci.org/guide/programming-tasks/#fs for the accepted
        format)
 
-    3) .cirrus.tasks.yml
+    4) .cirrus.tasks.yml
     """
 
     output = ""
 
     # 1) is evaluated implicitly
 
+
     # Add 2)
+    additional_env = compute_environment_vars()
+    env_fmt = """
+###
+# Computed environment variables start here
+###
+{0}
+###
+# Computed environment variables end here
+###
+"""
+    output += env_fmt.format(yaml.dumps({'env': additional_env}))
+
+
+    # Add 3)
     repo_config_url = env.get("REPO_CI_CONFIG_GIT_URL")
     if repo_config_url != None:
         print("loading additional configuration from \"{}\"".format(repo_config_url))
@@ -38,12 +55,26 @@ def main():
     else:
         output += "\n# REPO_CI_CONFIG_URL was not set\n"
 
-    # Add 3)
+
+    # Add 4)
     output += config_from(".cirrus.tasks.yml")
 
+
     return output
 
 
+def compute_environment_vars():
+    cenv = {}
+
+    # See definition of mingw task in .cirrus.tasks.yml
+    if env.get("REPO_MINGW_TRIGGER_BY_DEFAULT") in ['true', '1', 'yes']:
+        cenv['CI_MINGW_TRIGGER_TYPE'] = 'automatic'
+    else:
+        cenv['CI_MINGW_TRIGGER_TYPE'] = 'manual'
+
+    return cenv
+
+
 def config_from(config_src):
     """return contents of config file `config_src`, surrounded by markers
     indicating start / end of the included file
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index a2388cd5036..3446d9d1ba4 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -583,13 +583,13 @@ task:
   << : *WINDOWS_ENVIRONMENT_BASE
   name: Windows - Server 2019, MinGW64 - Meson
 
-  # due to resource constraints we don't run this task by default for now
-  trigger_type: manual
-  # worth using only_if despite being manual, otherwise this task will show up
-  # when e.g. ci-os-only: linux is used.
-  only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*mingw.*'
-  # otherwise it'll be sorted before other tasks
+  # Due to resource constraints we don't run this task by default, unless the
+  # the environment variable REPO_MINGW_TRIGGER_BY_DEFAULT is true. This
+  # computation is done in .cirrus.star:compute_environment_vars()
+  trigger_type: $CI_MINGW_TRIGGER_TYPE
+
   depends_on: SanityCheck
+  only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*mingw.*'
 
   env:
     TEST_JOBS: 4 # higher concurrency causes occasional failures
diff --git a/src/tools/ci/README b/src/tools/ci/README
index 30ddd200c96..d39017b4eb7 100644
--- a/src/tools/ci/README
+++ b/src/tools/ci/README
@@ -82,3 +82,15 @@ defined in .cirrus.yml, by redefining the relevant yaml anchors.
 Custom compute resources can be provided using
 - https://cirrus-ci.org/guide/supported-computing-services/
 - https://cirrus-ci.org/guide/persistent-workers/
+
+
+Enabling manual tasks by default
+================================
+
+Some tasks are not run automatically by default, to avoid using up CI credits
+too quickly. This can be changed on the repository level, e.g. when custom
+compute resources are configured.
+
+The following repository level environment variables are recognized:
+- REPO_MINGW_TRIGGER_BY_DEFAULT - if set to true, the mingw test is run
+  automatically
-- 
2.44.0.279.g3bd955d269

>From b4278f5b5f013dbe4eb1cec0e996f95bd2bfc6e4 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Fri, 12 Apr 2024 18:18:34 -0700
Subject: [PATCH v2 2/2] ci: simplify ci-os-only handling

---
 .cirrus.star      | 28 +++++++++++++++++++++++++++-
 .cirrus.tasks.yml | 17 ++++++++---------
 2 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/.cirrus.star b/.cirrus.star
index 7a164bb00de..685be056e3c 100644
--- a/.cirrus.star
+++ b/.cirrus.star
@@ -7,7 +7,7 @@ https://github.com/bazelbuild/starlark/blob/master/spec.md
 See also .cirrus.yml and src/tools/ci/README
 """
 
-load("cirrus", "env", "fs", "yaml")
+load("cirrus", "env", "fs", "re", "yaml")
 
 
 def main():
@@ -66,6 +66,32 @@ def main():
 def compute_environment_vars():
     cenv = {}
 
+    # Parse "ci-os-only:" tag in commit message and set
+    # CI_{$OS}_ENABLED variable for each OS
+
+    # We want to disable SanityCheck if testing just a specific OS. This
+    # shortens push-wait-for-ci cycle time a bit when debugging operating
+    # system specific failures. Just treating it as an OS in that case
+    # suffices.
+
+    operating_systems = ['freebsd', 'linux', 'macos', 'windows', 'mingw',
+      'sanitycheck', 'compilerwarnings']
+    commit_message = env.get('CIRRUS_CHANGE_MESSAGE')
+    match_re = r"(^|.*\n)ci-os-only: ([^\n]+)($|\n.*)"
+
+    # re.match() returns an array with a tuple of (matched-string, match_1, ...)
+    m = re.match(match_re, commit_message)
+    if m and len(m) > 0:
+        os_only = m[0][2]
+        os_only_list = re.split(r'[, ]+', os_only)
+    else:
+        os_only_list = operating_systems
+
+    for os in operating_systems:
+        os_enabled = os in os_only_list
+        cenv['CI_{0}_ENABLED'.format(os.upper())] = os_enabled
+
+
     # See definition of mingw task in .cirrus.tasks.yml
     if env.get("REPO_MINGW_TRIGGER_BY_DEFAULT") in ['true', '1', 'yes']:
         cenv['CI_MINGW_TRIGGER_TYPE'] = 'automatic'
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index 3446d9d1ba4..aa3d90f5e0d 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -59,7 +59,7 @@ task:
   # push-wait-for-ci cycle time a bit when debugging operating system specific
   # failures. Uses skip instead of only_if, as cirrus otherwise warns about
   # only_if conditions not matching.
-  skip: $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:.*'
+  skip: $CI_SANITYCHECK_ENABLED == false
 
   env:
     CPUS: 4
@@ -141,7 +141,7 @@ task:
   <<: *freebsd_task_template
 
   depends_on: SanityCheck
-  only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*freebsd.*'
+  only_if: $CI_FREEBSD_ENABLED
 
   sysinfo_script: |
     id
@@ -279,7 +279,7 @@ task:
   <<: *linux_task_template
 
   depends_on: SanityCheck
-  only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*linux.*'
+  only_if: $CI_LINUX_ENABLED
 
   ccache_cache:
     folder: ${CCACHE_DIR}
@@ -436,7 +436,7 @@ task:
   <<: *macos_task_template
 
   depends_on: SanityCheck
-  only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*(macos|darwin|osx).*'
+  only_if: $CI_MACOS_ENABLED
 
   sysinfo_script: |
     id
@@ -548,7 +548,7 @@ task:
   <<: *windows_task_template
 
   depends_on: SanityCheck
-  only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*windows.*'
+  only_if: $CI_WINDOWS_ENABLED
 
   setup_additional_packages_script: |
     REM choco install -y --no-progress ...
@@ -589,7 +589,7 @@ task:
   trigger_type: $CI_MINGW_TRIGGER_TYPE
 
   depends_on: SanityCheck
-  only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*mingw.*'
+  only_if: $CI_MINGW_ENABLED
 
   env:
     TEST_JOBS: 4 # higher concurrency causes occasional failures
@@ -644,10 +644,9 @@ task:
 
   # To limit unnecessary work only run this once the SanityCheck
   # succeeds. This is particularly important for this task as we intentionally
-  # use always: to continue after failures. Task that did not run count as a
-  # success, so we need to recheck SanityChecks's condition here ...
+  # use always: to continue after failures.
   depends_on: SanityCheck
-  only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*'
+  only_if: $CI_COMPILERWARNINGS_ENABLED
 
   env:
     CPUS: 4
-- 
2.44.0.279.g3bd955d269

Reply via email to