Run CI tests in all repos

[myk002]

It looks like we could get tests running in the scripts and df-structures repos by copying a significant chunk of https://github.com/DFHack/dfhack/blob/develop/.github/workflows/build.yml to https://github.com/DFHack/scripts/blob/master/.github/workflows/build.yml and https://github.com/DFHack/df-structures/blob/master/.github/workflows/build.yml

Is there a better way to do this?

Edit: reusable workflows are a thing: https://docs.github.com/en/actions/using-workflows/reusing-workflows

We could create a .githib repo (named so we can also use it for starter workflows if we so choose) with reusable workflows in it that can be called by all repositories. Then repos can be migrated to use the common logic one by one.

[lethosor]

I was planning on taking a stab at this once I've gotten my dfhack-clone script into a good state.

From https://docs.github.com/en/actions/using-workflows/reusing-workflows#calling-a-reusable-workflow, it looks like reusable workflows could go in any repo, so I might put it in dfhack-ci too.

(I'm not a huge fan of GitHub's "all-or-nothing" approach to reusing workflows, but I think it'll work for our case as long as we can parameterize what we need.)

[myk002]

putting it in dfhack-ci is a good idea -- I don't think we'll ever really need starter workflows.

[myk002]

There is one case that's been bugging me. Some tests that use scripts are in the dfhack repo, so if those scripts are changed in the scripts repo, the test action may fail until a corresponding PR is merged in the dfhack repo.

Would it be possible to put configuration in a PR description to indicate another PR should be pulled instead of HEAD? E.g. for a dfhack PR, if the description contains "CI_SCRIPTS_PR=659" (or BUILD_ACTION_SCRIPTS_PR if we want to reserve "CI" for the buildmaster) then the build action would pull PR 659 instead of HEAD for scripts. We'd still need to be careful and merge the linked PRs at the same time, but at least we could avoid spurious test failures.

[lethosor]

Yep! I think that's exactly what my dfhack-clone script is designed to do. Just needs a little more work to connect its PR-parsing logic to its submodule-pulling logic, and then I'll make a PR for it.

[myk002]

another idea (that you may have already addressed) -- will dfhack-clone automatically merge latest HEAD before running tests? It would help reviewers (us) greatly if we can be sure everything will work after clicking the 'merge' button without having to manually merge and test locally!

[lethosor]

If I'm reading https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request correctly, it sounds like the on: pull_request trigger for Github Actions already does that:

Note that GITHUB_SHA for this event is the last merge commit of the pull request merge branch. If you want to get the commit ID for the last commit to the head branch of the pull request, use github.event.pull_request.head.sha instead.

so I might just be able to pass $GITHUB_SHA directly to dfhack-clone and have it work.

If you meant for other respositories (other than the one where the PR was submitted), I believe refs/pull/<number>/merge is an autogenerated ref that refers to a hypothetical merge... and that should work in for any PR that doesn't have a merge conflict, regardless of repo. (I had planned to fetch /head instead of /merge without thinking too much about it, so thank you for the input!)

[myk002]

For that most recent comment, I was referring to the repo that the PR was submitted against (and I'm glad that is handled automatically), but it's just as valuable to fetch /merge from other repos when a specific PR is requested. Your plan LGTM