InsightSoftwareConsortium / ITKModuleTemplate

A template to start an ITK Module
https://itk.org/ITKSoftwareGuide/html/Book1/ITKSoftwareGuide-Book1ch9.html#x50-1430009
Apache License 2.0
12 stars 16 forks source link

Migrate build-test-package GitHub Action configuration into a composite action #131

Closed thewtex closed 5 months ago

thewtex commented 1 year ago

Migrate the .github/workflows/build-test-package.yml configuration into the relatively recently added composite GitHub Action.

A repository has been created for this action.

This will prevent the need to copy the yaml configuration to remote modules, enable remote modules to always get the latest version of the script if they use master of the action or used a specific tagged version.

tbirdso commented 1 year ago

Hi @thewtex , are we trying to support modules with extra dependencies, such as ITKUltrasound? Or should modules with any extra dependencies be expected to continue to fork the build-test-package.yml configuration?

thewtex commented 1 year ago

@tbirdso let's start with no extra dependencies, then see if we can add extra dependency support if possible! :rocket: :moon: :stars:

tbirdso commented 1 year ago

Hi @thewtex , per discussion in https://github.com/InsightSoftwareConsortium/ITKRemoteModuleBuildTestPackageAction/pull/1 my understanding is that 1) Github requires one repository per composite action and 2) a composite action runs in sequence and does not support multiple jobs.

What are your thoughts on this path forward:

dzenanz commented 1 year ago

Since we have many remote modules, maybe it makes sense to have 3 test actions (one per platform) in addition to the 1 package action which you propose. That will make maintenance easier and simpler.

blowekamp commented 1 year ago

There seems to be some limitations with composite actions that make me wonder if they are the best option. I have created reusable step in Azure, but not GHA. My initial thoughts would be to create reusable steps that can be better customized by the package. But there is the question of how much customization is needed with the module.

1) Github requires one repository per composite action

In this documentation I see the following example:

name: Call a reusable workflow

on:
  pull_request:
    branches:
      - main

jobs:
  call-workflow:
    uses: octo-org/example-repo/.github/workflows/workflow-A.yml@v1

  call-workflow-passing-data:
    uses: octo-org/example-repo/.github/workflows/workflow-B.yml@main
    with:
      username: mona
    secrets:
      token: ${{ secrets.TOKEN }}

Isn't that multiple workflows in on repo?

tbirdso commented 1 year ago

Hi @blowekamp, thanks for the feedback.

Isn't that multiple workflows in on repo?

Github Actions defines both "composite actions" and "reusable workflows", the former is what we've implemented so far and has the requirement of one per repo. I've found conflicting discussion over the advantages of each one, but the reusable workflow documentation you've pointed to does show multiple reusable workflows in one repo. I've seen hints but no clear answers on whether reusable workflows can be consumed between different organizations' repos.

I will test out adding a reusable workflow in ITKRemoteModuleBuildTestPackageAction to determine whether that will meet our needs without creating a separate composite action for Python builds.

EDIT: Access across organizations looks promising: https://docs.github.com/en/actions/using-workflows/reusing-workflows#access-to-reusable-workflows

tbirdso commented 1 year ago

Roadmap to closing this issue:

  1. Review and merge reusable pipelines for ITK remote module CI: https://github.com/InsightSoftwareConsortium/ITKRemoteModuleBuildTestPackageAction/pull/3
  2. Tag ITKRemoteModuleBuildTestPackageAction repo
  3. Replace ITKModuleTemplate cookiecutter CI workflow implementation with a reference to reusable pipelines
  4. Replace CI workflow implementations in most ITK remote module repos to reference the reusable workflows. Not currently applicable for remote modules that require custom CMake arguments to build or have dependencies other than ITK that are not fetched as part of the module build command (i.e. superbuilt).
dzenanz commented 5 months ago

@thewtex I think we can close this issue now?

thewtex commented 5 months ago

@thewtex I think we can close this issue now?

Yes! Thanks to everyone who worked together on this, and major kudos to @tbirdso for making the fantastic new action happen! :clap: :champagne: