workflows/tests: run tap_syntax on stable brew

[cho-m]

• Have you followed the guidelines for contributing?
• Have you ensured that your commits follow the commit style guide?
• Have you checked that there aren't other open pull requests for the same formula update/change?
• Have you built your formula locally with HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
• Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
• Does your build pass brew audit --strict <formula> (after doing HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

---

[carlocab]

Love this.

[Bo98]

Looking at CI times, matrix.branch should be added to the style cache key and restore key I reckon

[cho-m]

Looking at CI times, matrix.branch should be added to the style cache key and restore key I reckon

Is it just:

-          key: style-cache-${{ github.sha }}
-          restore-keys: style-cache-
+          key: style-cache-${{ matrix.branch }}-${{ github.sha }}
+          restore-keys: style-cache-${{ matrix.branch }}-

[Bo98]

Yes. First build will be slow since it'll be a new cache, but subsequent reruns should be quick again.

[cho-m]

I may need to adjust https://github.com/Homebrew/actions/blob/master/setup-homebrew/main.sh#L146 since we do have tags that aren't linked to master branch.

EDIT: it does look like it got latest so may be okay?

==> Using Homebrew/brew 4.2.14 (test/utils/github_spec: filter further to fix test failure)

vs. master

==> Using Homebrew/brew 4.2.13-117-g999ecf8b54 (Merge pull request #16937 from Homebrew/ported-cmds)

[Bo98]

Yeah 4.2.14 is a special out-of-band release so master being on 4.2.13 is intentional. Such scenarios are rare but we're in one such occurrence.

So that output is correct.

[cho-m]

I was wondering if might miss latest tag in current action since we do git fetch --force origin rather than git fetch --force --tags origin (like in brew - https://github.com/Homebrew/brew/blob/master/Library/Homebrew/cmd/update.sh#L63)

[Bo98]

Oh right, that's probably because it's prefetched in a Docker container:

linuxbrew@770efc9b046a:~/.linuxbrew/Homebrew$ git tag --list --sort="-version:refname" | head -n1
4.2.14

Your concern may however apply to macOS or bare Ubuntu: https://github.com/Bo98/workflow-test/actions/runs/8394589883/job/22992069978

[cho-m]

If there isn't much overhead, I can just add --tags to existing command or just run git_retry -C "$HOMEBREW_REPOSITORY" fetch --force --tags origin right before git tag.

Another option I considered was brew update-reset "$HOMEBREW_REPOSITORY" but didn't get chance to check on full impact of command.

[Bo98]

Adding --tags to the existing command makes sense

[ZhongRuoyu]

Required check names need to be updated in repo settings. (Which also means all PRs need a rebase after this I think?)

Alternatively if we want this to be less disruptive: we can maybe keep "tap_syntax (master)" as "tap_syntax".

[MikeMcQuaid]

Thanks again @cho-m, great idea!

[cho-m]

Going to merge and try it out.

I updated workflow to try to keep existing required tap_syntax with same cache there. Changed variable to boolean stable to avoid specific master branch name (to reduce usage if we switch to main).

Action should work in current state for homebrew-core, but will open PR to grab --tags in case used elsewhere.

---

One scenario that could happen is if we want to introduce a new tap-wide audit/style/syntax that is incompatible between stable and master.

This could lead to brew PR failing on tap syntax of homebrew-core while a corresponding PR on this side would fail on tap_syntax (stable).

We might have to bypass check and make sure to sync the merge of both PRs along with a new tagged release of brew. Or modify corresponding PRs to phase in the changes for master without impacting stable checks.

For bypass, maybe something along lines of:

--- a/.github/workflows/tests.yml
+++ b/.github/workflows/tests.yml
@@ -30,6 +30,10 @@ jobs:
     strategy:
       matrix:
         stable: [false, true]
+        new_syntax: contains(github.event.pull_request.labels.*.name, 'new syntax')
+        exclude:
+          - stable: true
+            new_syntax: true
     name: tap_syntax${{ matrix.stable && ' (stable)' || '' }}
     runs-on: ubuntu-22.04
     container:

[Bo98]

It would probably make sense to disable elements on the stable check. For example, brew style on stable probably doesn't make sense (and is not supported as it's a dev-cmd) but brew readall does.

The label idea won't work as it'll affect every PR after merging.

[cho-m]

The label idea won't work as it'll affect every PR after merging.

Label would require new tag after whatever breaking syntax change is introduced. But this could be some ongoing development work to a new brew release so may not be possible to tag. Could also consider putting these changes behind some sort of flag that are activated upon next version or for developer.

---

If there is a lot of conditional differences, may be easier to split matrix to separate jobs and selectively tune each. If just an extra flag to test-bot and logic is done there, then may be okay to keep current workflow.

[Bo98]

brew style changes are really never breaking syntax changes. Breaking changes should already caught by brew readall - if not then I consider that a bug.

[cho-m]

brew style changes are really never breaking syntax changes. Breaking changes should already caught by brew readall - if not then I consider that a bug.

Makes sense. Going to need changes in test-bot to tune this.

---

This did work for my PR - https://github.com/Homebrew/homebrew-core/actions/runs/8402834719/job/23012649264?pr=166501#step:5:20

• brew style only showed the non-impacting change.
• brew readall and brew audit both pointed to same breaking change.

[MikeMcQuaid]

Agreed that ideally this would run brew readall only.