refactor: fix brew-bump.sh script and automate homebrew releases

[jsjoeio]

This fixes the brew-bump.sh script by using the correct logic. It also moves a git clone line from the script to a step in the homebrew job.

Testing Plan

1. Fork coder/code-server
2. hard-code VERSION to 4.1.0-4970-6afc87b46dfa70adc0afe164a89662aac1a0b0cd (otherwise it will fail) and disable npm job
3. Push up this change to a new branch
4. Add HOMEBREW_GITHUB_API_TOKEN as secret to fork (using my own token).
5. Manually dispatch action
6. See that it opens a PR upstream in homebrew-core

✅ tested this with @cdr-oss account on Coder workspace
✅ tested with @cdrci account and it worked: Homebrew/homebrew-core#97235 (oops, should have just used the --dry-run option)

Other Notes

🔑 Updated the HOMEBREW_GITHUB_API_TOKEN under secrets with a PAT generated by @deansheather from the @cdrci account which has the scopes repo and workflow.

Fixes #4950

[github-actions]

✨ code-server docs for PR #4996 is ready! It will be updated on every commit.

• Host: https://coder.com/docs/code-server/a8c77a1
• Last deploy status: success
• Commit: 1ec1cdc
• Workflow status: https://github.com/coder/code-server/actions/runs/2019296605

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (main@60ebf2f). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #4996   +/-   ##
=======================================
  Coverage        ?   71.58%           
=======================================
  Files           ?       29           
  Lines           ?     1675           
  Branches        ?      373           
=======================================
  Hits            ?     1199           
  Misses          ?      405           
  Partials        ?       71           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 60ebf2f...1ec1cdc. Read the comment docs.

[jsjoeio]

Notes

Ran out of time with asher but stopped at the step where we push latest changes to fork

[jsjoeio]

question: With this change we can no longer distinguish between a failure and the PR already existing but I am not sure this matters.

Agreed. The original intent of that block was to prevent duplicate PRs since yarn wouldn't do that for us.

I think the question to ask is if we re-run this script and the version is already pushed do we want the job to be marked red or green?

My thinking is that is should be marked as green. Reason? Because running this job again and having the version already in upstream isn't a failure and shouldn't be marked as red. It should only be red if it fails to push a version (meaning that version never gets to Homebrew upstream).

In a world with more options, I would do this:

• failed to publish version? red.
• published version? green.
• skipped since version already published? yellow. (netural)

Red should be used to signal failures.

But to counter myself...we should match the error code of brew bump-formula-pr in which this case it would be 1 or red. This is the output:

➜  ~ brew bump-formula-pr --version="4.0.1" code-server --no-browse --no-audit  
Error: These pull requests may be duplicates:
code-server 4.0.1 https://github.com/Homebrew/homebrew-core/pull/92582
Duplicate PRs should not be opened. Use --force to override this error

Hmmm going back to the other side I feel like this should be a warning, not an error. I don't know. As you can tell, I can understand both sides. I can go either way.

[code-asher]

Agreed. The original intent of that block was to prevent duplicate PRs since yarn wouldn't do that for us.

A little different than that I think, it just checks whether brew-formula-pr aborted because the PR was a duplicate so we could exit with 0 instead of 1 (to be green instead of red). The prevention itself was always delegated to bump-formula-pr.

Hmmm going back to the other side I feel like this should be a warning, not an error. I don't know. As you can tell, I can understand both sides. I can go either way.

Pretty much the same thoughts going through my head.

I think...ultimately if the action goes red we have to take a look at it. If we see it already uploaded then we have nothing to do, in which case we just wasted our time. So maybe green would be the least frustrating. I definitely agree if there was some kind of warning state that would be ideal. Or if we could somehow make use of skipped steps. But that would require we check beforehand whether the PR was already submitted in a previous step and if so skip the actual publish step.

[jsjoeio]

I think...ultimately if the action goes red we have to take a look at it. If we see it already uploaded then we have nothing to do, in which case we just wasted our time. So maybe green would be the least frustrating. I definitely agree if there was some kind of warning state that would be ideal. .

I agree, I think for now, I'll add back the check to do that.

Or if we could somehow make use of skipped steps. But that would require we check beforehand whether the PR was already submitted in a previous step and if so skip the actual publish step

Yeah, I think for now, it's okay but we could revisit this if we think we need that.

[jsjoeio]

FWIW, I did look at the Homebrew docs. brew info --json --formula code-server returns JSON like this:

(omitted some info)

  {
    "name": "code-server",
    "full_name": "code-server",
    "tap": "homebrew/core",
    "desc": "Access VS Code through the browser",
    "license": "MIT",
    "homepage": "https://github.com/cdr/code-server",
    "versions": {
      "stable": "4.1.0",
      "head": null,
      "bottle": true
  }

We could get the latest published version but we'd have to check for a PR somehow. Seems like a lot of work for not a lot of benefit so I think we'll go with make green if PR already open.

[jsjoeio]

@code-asher ready for another review!

[jsjoeio]

Blocked by #5006