bump-formula|cask-pr: do not allow to bump autobumped packages #16750

iMichka · 2024-02-26T20:43:09Z

Avoids:

Bumping stuff by mistake that will get autobumped anyway
Abusing the system to get "free" commits
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 typecheck with your changes locally?
Have you successfully run brew tests with your changes locally?

Library/Homebrew/dev-cmd/bump-cask-pr.rb

Library/Homebrew/dev-cmd/bump-formula-pr.rb

MikeMcQuaid

Great work @iMichka! Some Pathname and messaging improvements.

Library/Homebrew/dev-cmd/bump-cask-pr.rb

Library/Homebrew/dev-cmd/bump-formula-pr.rb

Library/Homebrew/dev-cmd/bump-cask-pr.rb

Library/Homebrew/dev-cmd/bump-formula-pr.rb

iMichka · 2024-02-27T22:07:27Z

One odd thing (and the reason why used a regex in the first place): cask.name returns an array of uppercase cask names.
I hade to change that to cask.name.first.downcase ...

I had no time to check what is the origin of this array. I got confused because formula.name was just a string as expected.

The current code is working, I'll defer to the reviewers if you want me to check the cask.name.first.downcase thing (I'll double check this tomorrow nevertheless, just had no time to dig through the whole code today).

ZhongRuoyu

Seems we also need to allow @BrewTestBot to bypass this restriction.

Library/Homebrew/dev-cmd/bump-cask-pr.rb

Library/Homebrew/dev-cmd/bump-formula-pr.rb

Library/Homebrew/dev-cmd/bump-cask-pr.rb

MikeMcQuaid

Suggestions look good to me!

iMichka · 2024-02-28T20:55:28Z

This should be good now: thanks for all the suggestions and the help :)

p-linnane

Looks great. Thanks @iMichka!

ZhongRuoyu

@BrewTestBot is also using the bump-formula-pr/bump-cask-pr commands under the hood. Will this prevent it from bumping formulae normally?

apainintheneck · 2024-02-29T03:56:41Z

Looks good to me!

$ brew bump-cask-pr --dry-run --version=4.4.4 tsh
Error: Whoops, the tsh cask has its version update
pull requests automatically opened by BrewTestBot!
We'd still love your contributions, though, so try another one
that's not in the autobump list:
  https://github.com/Homebrew/homebrew-cask/blob/master/.github/autobump.txt

$ brew bump-formula-pr --dry-run --version=4.4.4 agda
Error: Whoops, the agda formula has its version update
pull requests automatically opened by BrewTestBot!
We'd still love your contributions, though, so try another one
that's not in the autobump list:
  https://github.com/Homebrew/homebrew-core/blob/master/.github/autobump.txt

Thanks @iMichka!

apainintheneck · 2024-02-29T04:13:26Z

@BrewTestBot is also using this bump-formula-pr command under the hood. Will this prevent it from bumping formulae normally?

Good question. I looked at brew test bot and it doesn't seem to use bump-cask-pr or bump-formula-pr directly outside of the stale PR workflow.

/u/l/H/L/T/h/homebrew-test-bot (master|✔) $ git grep bump-cask-pr
.github/workflows/triage-issues.yml:      - name: Mark/Close Stale `bump-formula-pr` and `bump-cask-pr` Pull Requests
.github/workflows/triage-issues.yml:          any-of-labels: "bump-formula-pr,bump-cask-pr"
/u/l/H/L/T/h/homebrew-test-bot (master|✔) $ git grep bump-formula-pr
.github/workflows/triage-issues.yml:      - name: Mark/Close Stale `bump-formula-pr` and `bump-cask-pr` Pull Requests
.github/workflows/triage-issues.yml:          any-of-labels: "bump-formula-pr,bump-cask-pr"

I looked at the core repos and it seems we use a separate action to bump packages that calls brew bump directly instead of using brew bump-cask-pr and brew bump-formula-pr.

https://github.com/Homebrew/actions/tree/master/bump-packages

There is a deprecated action for bumping formulae using brew bump-formula-pr but it's not used anymore in core.

https://github.com/Homebrew/actions/tree/master/bump-formulae

ZhongRuoyu · 2024-02-29T04:26:44Z

brew bump --open-pr invokes brew bump-formula-pr/brew bump-cask-pr under the hood, actually:

brew/Library/Homebrew/dev-cmd/bump.rb

Lines 506 to 516 in 154a217

    
           bump_cask_pr_args = [ 
        
             "bump-#{version_info.type}-pr", 
        
             name, 
        
             *version_args, 
        
             "--no-browse", 
        
             "--message=Created by `brew bump`", 
        
           ] 
        
           bump_cask_pr_args << "--no-fork" if args.no_fork? 
        
           system HOMEBREW_BREW_FILE, *bump_cask_pr_args

apainintheneck · 2024-02-29T05:13:21Z

Ah, my bad. I had those backwards. I guess we could allow this only on CI somehow.

Edit: We can probably just use ENV["GITHUB_ACTIONS"] to check if we're on CI.

apainintheneck

We need to find a way to not break CI scripts that run brew bump.

ZhongRuoyu · 2024-02-29T08:26:39Z

We can probably just use ENV["GITHUB_ACTIONS"] to check if we're on CI.

Or maybe HOMEBREW_TEST_BOT_AUTOBUMP, added in #16664.

MikeMcQuaid · 2024-02-29T10:45:40Z

Or maybe HOMEBREW_TEST_BOT_AUTOBUMP, added in #16664.

This feels like best fit to me 👍🏻

MikeMcQuaid

Looks good once there's a workaround for BrewTestBot to not 💥

Library/Homebrew/dev-cmd/bump-cask-pr.rb

This will unblock Homebrew/brew#16750 by providing us with an environment variable that's only on when we're autobumping packages. In all other scenarios, we want people to get an error message if they try to bump one of the packages that gets regularly autobumped on CI.

apainintheneck · 2024-03-01T04:23:35Z

I opened a PR to add the environment variable to the bump-packages action. After that gets merged in and the cleanup mentioned in #16750 (comment) is done, this should be good to go.

Set HOMEBREW_TEST_BOT_AUTOBUMP in bump-packages actions#506

Unblocking other reviewers since we've agreed on how to not break CI.

apainintheneck · 2024-03-01T04:52:52Z

Just to over communicate a bit. Three PRs need to be merged in in a specific order and this one will be the last one to land.

See Homebrew/actions#506 (comment)

Library/Homebrew/dev-cmd/bump-cask-pr.rb

Library/Homebrew/dev-cmd/bump-formula-pr.rb

Library/Homebrew/dev-cmd/bump-cask-pr.rb

Library/Homebrew/dev-cmd/bump-formula-pr.rb

Avoids: - Bumping stuff by mistake that will get autobumped anyway - Abusing the system to get "free" commits

MikeMcQuaid

Looks good to me now, thanks @iMichka!

This is needed for Homebrew/brew#16750. Note that #163023 is no longer relevant as the ability to skip duplicate PR check in `brew bump` has been completely removed in Homebrew/brew#16781.

ZhongRuoyu

Thanks everyone! Let's merge this after Homebrew/homebrew-core#164745 and Homebrew/homebrew-cask#167993.

This is needed for Homebrew/brew#16750.

p-linnane

Awesome work everyone!

iMichka · 2024-03-01T21:49:31Z

Thanks all :)

iMichka mentioned this pull request Feb 26, 2024

workflows: notify on autobump bypass Homebrew/homebrew-core#162592

Closed

6 tasks

apainintheneck reviewed Feb 27, 2024

View reviewed changes

Library/Homebrew/dev-cmd/bump-cask-pr.rb Outdated Show resolved Hide resolved

Library/Homebrew/dev-cmd/bump-formula-pr.rb Outdated Show resolved Hide resolved

Library/Homebrew/dev-cmd/bump-formula-pr.rb Outdated Show resolved Hide resolved

MikeMcQuaid reviewed Feb 27, 2024

View reviewed changes

iMichka force-pushed the autobump branch from a7b18e4 to d08c0a8 Compare February 27, 2024 22:03

ZhongRuoyu reviewed Feb 28, 2024

View reviewed changes

MikeMcQuaid reviewed Feb 28, 2024

View reviewed changes

iMichka force-pushed the autobump branch from d08c0a8 to 07a31b9 Compare February 28, 2024 20:54

iMichka requested review from MikeMcQuaid, ZhongRuoyu and apainintheneck February 28, 2024 20:55

p-linnane approved these changes Feb 28, 2024

View reviewed changes

apainintheneck approved these changes Feb 29, 2024

View reviewed changes

ZhongRuoyu reviewed Feb 29, 2024

View reviewed changes

apainintheneck previously requested changes Feb 29, 2024

View reviewed changes

MikeMcQuaid reviewed Feb 29, 2024

View reviewed changes

iMichka force-pushed the autobump branch from 07a31b9 to 9614b50 Compare February 29, 2024 21:33

iMichka requested review from MikeMcQuaid, apainintheneck, p-linnane and ZhongRuoyu February 29, 2024 21:40

ZhongRuoyu reviewed Feb 29, 2024

View reviewed changes

Library/Homebrew/dev-cmd/bump-cask-pr.rb Outdated Show resolved Hide resolved

apainintheneck mentioned this pull request Mar 1, 2024

Set HOMEBREW_TEST_BOT_AUTOBUMP in bump-packages Homebrew/actions#506

Closed

MikeMcQuaid reviewed Mar 1, 2024

View reviewed changes

Library/Homebrew/dev-cmd/bump-cask-pr.rb Outdated Show resolved Hide resolved

Library/Homebrew/dev-cmd/bump-formula-pr.rb Outdated Show resolved Hide resolved

MikeMcQuaid reviewed Mar 1, 2024

View reviewed changes

Library/Homebrew/dev-cmd/bump-cask-pr.rb Show resolved Hide resolved

Library/Homebrew/dev-cmd/bump-formula-pr.rb Show resolved Hide resolved

MikeMcQuaid force-pushed the autobump branch from 062c181 to 0f1a385 Compare March 1, 2024 09:03

bump-formula|cask-pr: do not allow to bump autobumped packages

f96290e

Avoids: - Bumping stuff by mistake that will get autobumped anyway - Abusing the system to get "free" commits

MikeMcQuaid force-pushed the autobump branch from 0f1a385 to f96290e Compare March 1, 2024 09:15

MikeMcQuaid approved these changes Mar 1, 2024

View reviewed changes

This was referenced Mar 1, 2024

autobump: set HOMEBREW_TEST_BOT_AUTOBUMP Homebrew/homebrew-core#164745

Merged

autobump: set HOMEBREW_TEST_BOT_AUTOBUMP Homebrew/homebrew-cask#167993

Merged

ZhongRuoyu approved these changes Mar 1, 2024

View reviewed changes

chenrui333 pushed a commit to Homebrew/homebrew-cask that referenced this pull request Mar 1, 2024

autobump: set HOMEBREW_TEST_BOT_AUTOBUMP

e5bab98

This is needed for Homebrew/brew#16750.

p-linnane approved these changes Mar 1, 2024

View reviewed changes

ZhongRuoyu merged commit 38b671d into Homebrew:master Mar 1, 2024
26 checks passed

p-linnane mentioned this pull request Mar 1, 2024

autobump: additions & removals Homebrew/homebrew-core#164770

Merged

6 tasks

iMichka deleted the autobump branch March 1, 2024 21:48

github-actions bot added the outdated PR was locked due to age label Apr 3, 2024

github-actions bot locked as resolved and limited conversation to collaborators Apr 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bump-formula|cask-pr: do not allow to bump autobumped packages #16750

bump-formula|cask-pr: do not allow to bump autobumped packages #16750

iMichka commented Feb 26, 2024 •

edited

Loading

MikeMcQuaid left a comment

iMichka commented Feb 27, 2024

ZhongRuoyu left a comment

MikeMcQuaid left a comment

iMichka commented Feb 28, 2024

p-linnane left a comment

ZhongRuoyu left a comment •

edited

Loading

apainintheneck commented Feb 29, 2024

apainintheneck commented Feb 29, 2024

ZhongRuoyu commented Feb 29, 2024

apainintheneck commented Feb 29, 2024 •

edited

Loading

apainintheneck left a comment

ZhongRuoyu commented Feb 29, 2024

MikeMcQuaid commented Feb 29, 2024

MikeMcQuaid left a comment

apainintheneck commented Mar 1, 2024

apainintheneck commented Mar 1, 2024

MikeMcQuaid left a comment

ZhongRuoyu left a comment

p-linnane left a comment

iMichka commented Mar 1, 2024

bump-formula|cask-pr: do not allow to bump autobumped packages #16750

bump-formula|cask-pr: do not allow to bump autobumped packages #16750

Conversation

iMichka commented Feb 26, 2024 • edited Loading

MikeMcQuaid left a comment

Choose a reason for hiding this comment

iMichka commented Feb 27, 2024

ZhongRuoyu left a comment

Choose a reason for hiding this comment

MikeMcQuaid left a comment

Choose a reason for hiding this comment

iMichka commented Feb 28, 2024

p-linnane left a comment

Choose a reason for hiding this comment

ZhongRuoyu left a comment • edited Loading

Choose a reason for hiding this comment

apainintheneck commented Feb 29, 2024

apainintheneck commented Feb 29, 2024

ZhongRuoyu commented Feb 29, 2024

apainintheneck commented Feb 29, 2024 • edited Loading

apainintheneck left a comment

Choose a reason for hiding this comment

ZhongRuoyu commented Feb 29, 2024

MikeMcQuaid commented Feb 29, 2024

MikeMcQuaid left a comment

Choose a reason for hiding this comment

apainintheneck commented Mar 1, 2024

apainintheneck commented Mar 1, 2024

MikeMcQuaid left a comment

Choose a reason for hiding this comment

ZhongRuoyu left a comment

Choose a reason for hiding this comment

p-linnane left a comment

Choose a reason for hiding this comment

iMichka commented Mar 1, 2024

iMichka commented Feb 26, 2024 •

edited

Loading

ZhongRuoyu left a comment •

edited

Loading

apainintheneck commented Feb 29, 2024 •

edited

Loading