tap-new: enable bottles publishing #8762

dawidd6 · 2020-09-18T11:05:22Z

We now support bottles publishing to GitHub Releases.
Add a workflow for that and modify tests.yml a bit, so users can have bottles in their taps too and be happier.

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same change?
Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes? Here's an example.
Have you successfully run brew style with your changes locally?
Have you successfully run brew tests with your changes locally?

Library/Homebrew/dev-cmd/tap-new.rb

dawidd6 · 2020-09-18T11:10:41Z

Also, is it intentional we don't do git init in new tap directory?

MikeMcQuaid

This is SO COOL 😍

Library/Homebrew/dev-cmd/tap-new.rb

chdiza · 2020-09-19T20:06:35Z

Also, is it intentional we don't do git init in new tap directory?

Taps don't have to be git repos.

jonchang · 2020-09-20T03:55:59Z

Library/Homebrew/dev-cmd/tap-new.rb

+
+            - name: Pull bottles
+              env:
+                HOMEBREW_GITHUB_API_TOKEN: ${{secrets.HOMEBREW_GITHUB_API_TOKEN}}


Can we use the default GITHUB_TOKEN here? Means less setup for tap maintainers.

Or at least something clever like using it unless it's a PR from a fork?

jonchang · 2020-09-20T03:56:15Z

Library/Homebrew/dev-cmd/tap-new.rb

+                token: ${{secrets.HOMEBREW_GITHUB_API_TOKEN}}
+
+            - name: Delete branch
+              uses: peter-evans/close-pull@v1


I'd prefer if this was something in the Homebrew org.

Probably simple enough for https://github.com/actions/github-script.

jonchang · 2020-09-20T03:57:39Z

Library/Homebrew/dev-cmd/tap-new.rb

+      When a pull request making changes to a formula (or formulae) becomes green
+      (all checks passed), then you can publish the built bottles.
+      To do so, run:
+        brew pr-publish --tap=#{tap} <pull_request_number>
+      or trigger the workflow via GitHub UI in Actions tab.


I think this text ought to go in the README, rather than a one-time thing that's printed to the terminal.

Good idea or, better still, be something that's linked to in our documentation so we can update it.

MikeMcQuaid

😻

Library/Homebrew/dev-cmd/tap-new.rb

MikeMcQuaid · 2020-09-21T07:57:36Z

Library/Homebrew/dev-cmd/tap-new.rb

+
+            - name: Pull bottles
+              env:
+                HOMEBREW_GITHUB_API_TOKEN: ${{secrets.HOMEBREW_GITHUB_API_TOKEN}}


Or at least something clever like using it unless it's a PR from a fork?

Library/Homebrew/dev-cmd/tap-new.rb

MikeMcQuaid · 2020-09-21T07:57:58Z

Library/Homebrew/dev-cmd/tap-new.rb

+                token: ${{secrets.HOMEBREW_GITHUB_API_TOKEN}}
+
+            - name: Delete branch
+              uses: peter-evans/close-pull@v1


Library/Homebrew/dev-cmd/tap-new.rb

MikeMcQuaid · 2020-09-21T07:59:25Z

Library/Homebrew/dev-cmd/tap-new.rb

+      When a pull request making changes to a formula (or formulae) becomes green
+      (all checks passed), then you can publish the built bottles.
+      To do so, run:
+        brew pr-publish --tap=#{tap} <pull_request_number>
+      or trigger the workflow via GitHub UI in Actions tab.


Good idea or, better still, be something that's linked to in our documentation so we can update it.

MikeMcQuaid · 2020-09-21T08:00:12Z

Also, is it intentional we don't do git init in new tap directory?

Taps don't have to be git repos.

This is true but I think, by default at least, brew tap-new should setup a git repository given we're adding files that should be committed and pushed to GitHub.

maxim-belkin · 2020-09-21T11:02:28Z

Library/Homebrew/dev-cmd/tap-new.rb

              uses: actions/cache@v1
              with:
                path: ${{ steps.set-up-homebrew.outputs.gems-path }}
                key: ${{ runner.os }}-rubygems-${{ steps.set-up-homebrew.outputs.gems-hash }}
                restore-keys: ${{ runner.os }}-rubygems-

            - name: Install Homebrew Bundler RubyGems
-              if: steps.cache.outputs.cache-hit != 'true'
              run: brew install-bundler-gems


Does something like this (👇 ) achieve the desired behavior?

Suggested change

run: brew install-bundler-gems

if: github.repository_owner == 'Homebrew' || steps.cache.outputs.cache-hit != 'true'

run: brew install-bundler-gems

dawidd6 · 2020-10-02T11:38:36Z

Changes since almost 2 weeks:

added --no-git option
initialize the git repo in tap directory by default
skip installing bundler gems step if cache hit
trigger publish workflow on pull request ok label
delete branch via git push if the pull request is not from forked repo
shorter notice after tap creation

MikeMcQuaid

Some very minor suggestions here but I'm happy for this to ship whenever you're happy with it and it's tested. Great work @dawidd6!

MikeMcQuaid · 2020-10-02T11:40:14Z

Library/Homebrew/dev-cmd/tap-new.rb

+            - labeled
+      jobs:
+        pr-pull:
+          if: contains(github.event.pull_request.labels.*.name, 'ok')


Suggested change

if: contains(github.event.pull_request.labels.*.name, 'ok')

if: contains(github.event.pull_request.labels.*.name, 'homebrew-publish-bottles')

or something? I'm pretty open to anything but something more descriptive and/or homebrew-specific might be nice.

Imma name it as pr-pull by default, it matches the workflow name and is short enough.

MikeMcQuaid · 2020-10-02T11:41:02Z

Library/Homebrew/dev-cmd/tap-new.rb

+
+      When a pull request making changes to a formula (or formulae) becomes green
+      (all checks passed), then you can publish the built bottles.
+      To do so, label your PR as 'ok' and the workflow will be triggered.


Suggested change

To do so, label your PR as 'ok' and the workflow will be triggered.

To do so, label your PR as #{label} and the workflow will be triggered.

and could share the variable above?

Even better, it could be an option with default value.

Yeh, good idea 👍🏻

Although, for a counterpoint: I tend to think it's not worth adding options until someone requests them?

I request it 😄

to GitHub Releases

dawidd6 · 2020-10-02T11:54:31Z

I encountered this error, while testing the publishing workflow:

Error: undefined method `split' for nil:NilClass
Please report this issue:
  https://docs.brew.sh/Troubleshooting
/home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/dev-cmd/pr-pull.rb:385:in `block in pr_pull'
/home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/dev-cmd/pr-pull.rb:378:in `each'
/home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/dev-cmd/pr-pull.rb:378:in `pr_pull'
/home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/brew.rb:119:in `<main>'
Error: Process completed with exit code 1.

04687ae#r42912894

dawidd6 · 2020-10-03T17:17:51Z

It's working:
https://github.com/dawidd6/homebrew-test-tap-new/pull/1

dawidd6 commented Sep 18, 2020

View reviewed changes

Library/Homebrew/dev-cmd/tap-new.rb Outdated Show resolved Hide resolved

dawidd6 commented Sep 18, 2020

View reviewed changes

Library/Homebrew/dev-cmd/tap-new.rb Show resolved Hide resolved

dawidd6 requested a review from MikeMcQuaid September 18, 2020 11:08

MikeMcQuaid reviewed Sep 18, 2020

View reviewed changes

Library/Homebrew/dev-cmd/tap-new.rb Show resolved Hide resolved

Library/Homebrew/dev-cmd/tap-new.rb Outdated Show resolved Hide resolved

Library/Homebrew/dev-cmd/tap-new.rb Outdated Show resolved Hide resolved

jonchang reviewed Sep 20, 2020

View reviewed changes

MikeMcQuaid reviewed Sep 21, 2020

View reviewed changes

maxim-belkin reviewed Sep 21, 2020

View reviewed changes

dawidd6 force-pushed the tap-new-gh-rel branch 3 times, most recently from 4cf6d15 to daabae9 Compare October 2, 2020 11:18

MikeMcQuaid approved these changes Oct 2, 2020

View reviewed changes

dawidd6 force-pushed the tap-new-gh-rel branch from daabae9 to a2e6bef Compare October 2, 2020 11:42

tap-new: enable bottles publishing

843dffe

to GitHub Releases

dawidd6 force-pushed the tap-new-gh-rel branch from a2e6bef to 843dffe Compare October 2, 2020 11:49

MikeMcQuaid approved these changes Oct 2, 2020

View reviewed changes

jonchang mentioned this pull request Oct 3, 2020

utils/git: set HEAD from remote if needed #8839

Closed

7 tasks

dawidd6 merged commit 9ec618f into Homebrew:master Oct 3, 2020

dawidd6 deleted the tap-new-gh-rel branch October 3, 2020 17:18

BrewTestBot added the outdated label Dec 3, 2020

Homebrew locked as resolved and limited conversation to collaborators Dec 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GitHub Sponsors

tap-new: enable bottles publishing #8762

tap-new: enable bottles publishing #8762

dawidd6 commented Sep 18, 2020

dawidd6 commented Sep 18, 2020

MikeMcQuaid left a comment

chdiza commented Sep 19, 2020

jonchang Sep 20, 2020

MikeMcQuaid Sep 21, 2020

jonchang Sep 20, 2020

MikeMcQuaid Sep 21, 2020

reitermarkus Sep 22, 2020

jonchang Sep 20, 2020

MikeMcQuaid Sep 21, 2020

MikeMcQuaid left a comment

MikeMcQuaid Sep 21, 2020

MikeMcQuaid Sep 21, 2020

MikeMcQuaid Sep 21, 2020

MikeMcQuaid commented Sep 21, 2020

maxim-belkin Sep 21, 2020

dawidd6 commented Oct 2, 2020

MikeMcQuaid left a comment

MikeMcQuaid Oct 2, 2020

dawidd6 Oct 2, 2020

MikeMcQuaid Oct 2, 2020

dawidd6 Oct 2, 2020

MikeMcQuaid Oct 2, 2020

MikeMcQuaid Oct 2, 2020

dawidd6 Oct 2, 2020

dawidd6 commented Oct 2, 2020 •

edited

Loading

dawidd6 commented Oct 3, 2020

	run: brew install-bundler-gems
	if: github.repository_owner == 'Homebrew' \|\| steps.cache.outputs.cache-hit != 'true'
	run: brew install-bundler-gems

	if: contains(github.event.pull_request.labels.*.name, 'ok')
	if: contains(github.event.pull_request.labels.*.name, 'homebrew-publish-bottles')

	To do so, label your PR as 'ok' and the workflow will be triggered.
	To do so, label your PR as #{label} and the workflow will be triggered.

tap-new: enable bottles publishing #8762

tap-new: enable bottles publishing #8762

Conversation

dawidd6 commented Sep 18, 2020

dawidd6 commented Sep 18, 2020

MikeMcQuaid left a comment

Choose a reason for hiding this comment

chdiza commented Sep 19, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeMcQuaid left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeMcQuaid commented Sep 21, 2020

Choose a reason for hiding this comment

dawidd6 commented Oct 2, 2020

MikeMcQuaid left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dawidd6 commented Oct 2, 2020 • edited Loading

dawidd6 commented Oct 3, 2020

dawidd6 commented Oct 2, 2020 •

edited

Loading