ci: Allow running mingw tests by default via environment variable

  • Jump to comment-1
    Andres Freund<andres@anarazel.de>
    Apr 13, 2024, 2:13 AM UTC
    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
    $REPOMINGWTRIGGERBYDEFAULT 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 $REPOMINGWTRIGGERBYDEFAULT is set, CIMINGWTRIGGER_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
    • Jump to comment-1
      Nazir Bilal Yavuz<byavuz81@gmail.com>
      Apr 25, 2024, 12:03 PM UTC
      Hi,
      Thank you for working on this!
      On Sat, 13 Apr 2024 at 05:12, Andres Freund <andres@anarazel.de> wrote:

      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.
      And I think mingw ends up not running most of the time. +1 to running
      it as default at least on cfbot. Also, this gives people a chance to
      run mingw as default on their personal repositories (which I would
      like to run).
      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
      $REPOMINGWTRIGGERBYDEFAULT 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 $REPOMINGWTRIGGERBYDEFAULT is set, CIMINGWTRIGGER_TYPE is
      set to 'automatic', if not it's 'manual'.
      Changes look good to me. My only complaint could be using only 'true'
      for the REPOMINGWTRIGGERBYDEFAULT, not a possible list of values
      but this is not important.
      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 agree.
      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?
      Although it adds more lines, this makes the .cirrus.tasks.yml file
      more readable and understandable which is more important in my
      opinion.
      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.
      I agree with your thoughts and CI_ prefix.
      I tested both patches and they work as expected.
      --
      Regards,
      Nazir Bilal Yavuz
      Microsoft