publish-commit-bottles: use public action

[woodruffw]

• 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>?

---

Adds the 'attestations' permission, which will be required in the future.

Also adds a duplicate "last ditch" step that will (hopefully) reduce the number of hard failures we see here, requiring manual rollback of the corresponding package upload.

CCing some GH folks to double-check my changes here: @bdehamer @phillmv

[woodruffw]

I think this is the right way to continue if this step fails. The other approach is to have the last ditch step use if: failure(), which AFAIK means it'll run if any step fails, not just the last one.

[carlocab]

I feel like it would be nicer if actions/attest-build-provenance had some sort of retry logic built-in, but this approach suits for now, I think. (Assuming actions/attest-build-provenance is idempotent, which I suppose it should be.)

[woodruffw]

Yeah, agreed. I think the team working on this at GH has been tackling it though internal retries, so I suspect it'll get better over time.

[chenrui333]

there is no side effect of ignoring the action step failure?

[woodruffw]

I don't believe so -- if it fails it should fall through to the "last ditch" version, which will then hard-fail the action if it also fails. But maybe I missed something?

[woodruffw]

actionlint is failing because it doesn't know the attestations permission, since it's too new 🙃

Edit: rhysd/actionlint#421

Edit 2: I see @bdehamer landed this with rhysd/actionlint@1f0efe1, it's just not in a release yet.

[carlocab]

I feel like it would be nicer if actions/attest-build-provenance had some sort of retry logic built-in, but this approach suits for now, I think. (Assuming actions/attest-build-provenance is idempotent, which I suppose it should be.)

[carlocab]

Edit 2: I see @bdehamer landed this with rhysd/actionlint@1f0efe1, it's just not in a release yet.

We could probably apply that patch to the actionlint formula (which is what's used in the workflow)

[woodruffw]

We could probably apply that patch to the actionlint formula (which is what's used in the workflow)

SGTM! I can do a follow-up PR for that.

[carlocab]

This approach is an improvement, but I feel like the correct thing to do would be to avoid uploading the bottles to GitHub Packages if the build provenance step fails.

This way we can simply re-run publish workflows when needed.

[woodruffw]

This approach is an improvement, but I feel like the correct thing to do would be to avoid uploading the bottles to GitHub Packages if the build provenance step fails.

Yes, agreed. I think the only current blocker for that is splitting the pr-pull step into two steps. I'll look into how brew pr-pull works and see if I can't adapt things.

[carlocab]

Yes, agreed. I think the only current blocker for that is splitting the pr-pull step into two steps. I'll look into how brew pr-pull works and see if I can't adapt things.

brew pr-pull just calls brew pr-upload to do the upload step: https://github.com/Homebrew/brew/blob/020d9947b0a195c9f91b264a0ebf8dc87d6b14d3/Library/Homebrew/dev-cmd/pr-pull.rb#L158-L171

So I think you can just add --no-upload to the pr-pull call and then add a pr-upload step after the build provenance generation step.

[MikeMcQuaid]

Looks good, happy to merge once CI is 🟢 and you're around for a bit to babysit/debug master issues. Thanks @woodruffw!

[carlocab]

#171161 should fix the actionlint failure.

[woodruffw]

Looks good, happy to merge once CI is 🟢 and you're around for a bit to babysit/debug master issues. Thanks @woodruffw!

I'm able to watch this all day today, so whenever is convenient for you and @carlocab as well!

Edit: looks like the actionlint bottle hasn't propagated quite yet, so that step is still failing.

[carlocab]

I think we should be ready to give this a try after these changes!

[carlocab]

Let's try this.

[ZhongRuoyu]

It seems that this approach does not work for all bottles because the bottle merge step happens after provenance generation.

HOMEBREW_VERIFY_ATTESTATIONS=1 brew install -d docker-completion, for example, fails. It can be seen from the publish job logs that attestations were created for the individual bottles before the merge (e.g., docker-completion--26.1.3.sonoma.bottle.tar.gz), so there is no attestation for docker-completion--26.1.3.all.bottle.tar.gz.

[woodruffw]

It seems that this approach does not work for all bottles because the bottle merge step happens after provenance generation.

HOMEBREW_VERIFY_ATTESTATIONS=1 brew install -d docker-completion, for example, fails. It can be seen from the publish job logs that attestations were created for the individual bottles before the merge (e.g., docker-completion--26.1.3.sonoma.bottle.tar.gz), so there is no attestation for docker-completion--26.1.3.all.bottle.tar.gz.

Huh, I didn't even know about all bottles -- this shows how out of date I am 🙂

I'll look into a fix.