dev-cmd: add bump-cask-pr command #7986

SeekingMeaning · 2020-07-12T22:08:41Z

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?

Usage: brew bump-cask-pr [options] [cask]

Create a pull request to update cask with a new version.

A best effort to determine the SHA-256 will be made if the value is not
supplied by the user.

  -n, --dry-run                    Print what would be done rather than doing
                                   it.
      --write                      Make the expected file modifications
                                   without taking any Git actions.
      --commit                     When passed with --write, generate a new
                                   commit after writing changes to the cask
                                   file.
      --no-audit                   Don't run brew cask audit before opening
                                   the PR.
      --no-style                   Don't run brew cask style --fix before
                                   opening the PR.
      --no-browse                  Print the pull request URL instead of
                                   opening in a browser.
      --no-fork                    Don't try to fork the repository.
      --version                    Specify the new version for the cask.
      --message                    Append message to the default pull
                                   request message.
      --url                        Specify the URL for the new download.
      --sha256                     Specify the SHA-256 checksum of the new
                                   download.
  -f, --force                      Ignore duplicate open PRs.
  -d, --debug                      Display any debugging information.
  -q, --quiet                      Suppress any warnings.
  -v, --verbose                    Make some output more verbose.
  -h, --help                       Show this message.

Old description

Something I worked on today, bump-cask-pr based on bump-formula-pr

Usage: brew bump-cask-pr [options] [cask]

Create a pull request to update cask with a new version.

If a SHA-256 is specified, the version must also be supplied.

If a URL is specified, a best effort to determine the version, SHA-256,
and cask name will be made if not supplied by the user.

        ...
        --version                    Specify the new version for the cask.
        --sha256                     Specify the SHA-256 checksum of the new
                                     download.
        --url                        Specify the URL for the new download.
        ...

Example usage:

% brew bump-cask-pr --url https://github.com/dwarvesf/blurred/releases/download/v1.2.0/Blurred.1.2.0.dmg

==> Downloading https://github.com/dwarvesf/blurred/releases/download/v1.2.0/Blurred.1.2.0.dmg
Already downloaded: ~/Library/Caches/Homebrew/downloads/bf76434825c890a75052082d1b07b5c1735f817ce1ba1003fd64fd23e7ee0b23--Blurred.1.2.0.dmg
Warning: Cannot verify integrity of bf76434825c890a75052082d1b07b5c1735f817ce1ba1003fd64fd23e7ee0b23--Blurred.1.2.0.dmg
A checksum was not provided for this resource.
For your reference the SHA-256 is: 15903ce2484f783c53cbad905ea93a5045c87767e7b89e37300d2200902dff37
==> replace "1.1.0" with "1.2.0"
==> replace "81414e9e9b094039f49453060200eef3e13ac98c8a3e3d61beefd53fa3d0f7fb" with "15903ce2484f783c53cbad905ea93a5045c
audit for blurred: passed
M	Casks/blurred.rb
Switched to a new branch 'blurred-1.2.0'
[blurred-1.2.0 c436328bb9] Update blurred from 1.1.0 to 1.2.0
 1 file changed, 2 insertions(+), 2 deletions(-)
Enumerating objects: 7, done.
Counting objects: 100% (7/7), done.
Delta compression using up to 4 threads
Compressing objects: 100% (4/4), done.
Writing objects: 100% (4/4), 448 bytes | 448.00 KiB/s, done.
Total 4 (delta 3), reused 0 (delta 0)
remote: Resolving deltas: 100% (3/3), completed with 3 local objects.
remote: 
remote: Create a pull request for 'blurred-1.2.0' on GitHub by visiting:
remote:      https://github.com/SeekingMeaning/homebrew-cask/pull/new/blurred-1.2.0
remote: 
To https://github.com/SeekingMeaning/homebrew-cask.git
 * [new branch]            blurred-1.2.0 -> blurred-1.2.0
Branch 'blurred-1.2.0' set up to track remote branch 'blurred-1.2.0' from 'https://github.com/SeekingMeaning/homebrew-cask.git'.

Example pull request created by bump-cask-pr: Homebrew/homebrew-cask#85823

Some things to address:

Should this be a sub-command for cask instead?
Does this need to run brew cask style?
How should this deal with more complex version schemes such as with java?
I probably could extract out some functions that are the same with bump-formula-pr
Other things I probably forgot to mention

(Slightly off-topic: I also played with converting livecheck to check for new versions of casks instead of formulae, but that's something to discuss at another time, especially with Samford migrating homebrew-livecheck into brew and homebrew-core)

miccal · 2020-07-13T10:03:52Z

Note that there already exists a tool for this called cask-repair.

Ping @vitorgalvao (the author and maintainer of cask-repair).

vitorgalvao · 2020-07-13T10:31:57Z

Thank you for your work and submission. At the moment it does feel like duplicate effort due to cask-repair, which is already used by most contributors. That said, I have no quarrel with this—quite the opposite—seeing as it’s integrated and written in Ruby.

I do think it’s missing (judging by the description) a very important feature: auto-setting the sha256. One of the major selling points of cask-repair and why so many people enjoy using it is that you can just give it a version and everything else is done for you. If this still requires one to set the shasum, that’s a major disadvantage that’ll prevent it from getting more usage.

I also feel the automatic version detection may be overkill and will fail a considerable amount of times. In all the years of cask-repair’s existence, no one has ever missed the feature. All casks use interpolation in the url when available and they seldom change. Unless you’re auto-including the interpolation, passing a versioned string to --url will produce a wrong result. In addition, we use a common pattern of true_version,version_in_url (note the comma), which this won’t detect. Reducing code by scrapping the feature might be a win.

Finally, yes, I do think this should run style, specifically style --fix. Also yes to extracting functions from the formula equivalent.

Good work so far. I’ve commented on the feature-set but will leave the code review to someone else, as I’m not as familiar with the workings of bump-formula-pr.

vitorgalvao · 2020-07-14T15:58:16Z

Also forgot to ask: how does this handle casks with multiple version, sha256, url? E.g. casks with if MacOS.version or casks with language.

MikeMcQuaid · 2020-07-20T07:46:57Z

Should this be a sub-command for cask instead?

I don't think it needs to be. I think ideally most cask subcommands would migrate to not being so.

I probably could extract out some functions that are the same with bump-formula-pr

That'd be great 👍

stale · 2020-08-16T08:10:26Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

vitorgalvao · 2020-08-19T12:23:30Z

@SeekingMeaning I’d welcome this becoming a reality and replacing the current tool for most users. Would you still like to pursue the requested changes to bring this up to par with the most common use of cask-repair, or are you no longer able to work on this?

SeekingMeaning · 2020-08-19T17:04:24Z

Would you still like to pursue the requested changes to bring this up to par with the most common use of cask-repair[?]

Yeah definitely, I've been doing quite a bit of refactoring since initially opening this PR. (This was actually the spark that led me to adding support for specifying --version alone for brew bump-formula-pr!)

Once we get #8368 merged, I think everything will be mostly ready for me to dive back into working on this

SeekingMeaning · 2020-09-05T00:31:32Z

I do think it’s missing (judging by the description) a very important feature: auto-setting the sha256.

Now available as of the latest commit

I also feel the automatic version detection may be overkill and will fail a considerable amount of times.

Removed — thank you for the feedback, I wasn't sure if it was necessary

Finally, yes, I do think this should run style, specifically style --fix. Also yes to extracting functions from the formula equivalent.

Done, and done (It took a bit longer than expected)

how does this handle casks with multiple version, sha256, url? E.g. casks with if MacOS.version

It replaces the version for the currently installed version of macOS

or casks with language.

~~It only replaces the sha256 for the current language; I'll look into supporting all languages~~
Added basic support in commit 33ffdfe

Here's the new process for determining the download url and checksum:

Require --version argument
Read old cask contents into string
Replace old version in string with new version
Replace old url if new url is provided
Load string as cask and get the new download url from this cask
Download the file
Get sha256 checksum
Replace version, url, and sha256 in actual cask

MikeMcQuaid

LGTM when cask folks are happy and there's a _spec that checks the help parsing (like all other commands).

MikeMcQuaid · 2020-09-08T07:12:43Z

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

+      min_named 1
+      max_named 1


Suggested change

min_named 1

max_named 1

named 1

vitorgalvao · 2020-09-08T11:23:37Z

LGTM when cask folks are happy

I’m happy! Just did a short test run and was quite pleased with the results.

claui · 2020-09-08T16:45:06Z

Nice job @SeekingMeaning!

SeekingMeaning · 2020-09-08T16:45:44Z

Thanks!

vitorgalvao · 2020-09-11T01:43:18Z

Thank you for your work, @SeekingMeaning.

I’m now in the process of fitting this into the documentation, and I’ve found a bug, it cannot upgrade casks with version :latest:

$ brew bump-cask-pr --version '1.0.0' figma
Error: /usr/local/Homebrew/Library/Homebrew/cask/cask_loader.rb:34: unexpected fraction part after numeric literal
  version :1.0.0
           ^~~
/usr/local/Homebrew/Library/Homebrew/cask/cask_loader.rb:34: syntax error, unexpected tFLOAT, expecting tSTRING_CONTENT or tSTRING_DBEG or tSTRING_DVAR or tSTRING_END
  version :1.0.0
           ^~~~~
Please report this issue:
  https://docs.brew.sh/Troubleshooting
/usr/local/Homebrew/Library/Homebrew/cask/cask_loader.rb:33:in `instance_eval'
/usr/local/Homebrew/Library/Homebrew/cask/cask_loader.rb:33:in `load'
/usr/local/Homebrew/Library/Homebrew/cask/cask_loader.rb:194:in `load'
/usr/local/Homebrew/Library/Homebrew/dev-cmd/bump-cask-pr.rb:120:in `bump_cask_pr'
/usr/local/Homebrew/Library/Homebrew/brew.rb:119:in `<main>'

core-code · 2020-09-18T08:37:52Z

thanks a lot for working on this!

one limitation of 'cask-repair' is that only instance can be run concurrently. i understand why this is the case but sending a large number of updates at once is a bit cumbersome when doing it sequencially. is parallel operation something that bump-cask-pr supports?

SeekingMeaning · 2020-09-18T16:35:36Z

is parallel operation something that bump-cask-pr supports?

No, unfortunately

SeekingMeaning force-pushed the bump-cask-pr branch from e8b69c7 to 36bd889 Compare July 12, 2020 23:58

vitorgalvao added the cask label Jul 14, 2020

This was referenced Aug 12, 2020

utils/github: add check_for_duplicate_pull_requests #8316

Merged

utils/github: delete redundant GitHub. and GitHub:: #8334

Closed

stale bot added the stale label Aug 16, 2020

stale bot removed the stale label Aug 19, 2020

SeekingMeaning mentioned this pull request Sep 4, 2020

utils/github: add create_bump_pr #8603

Merged

6 tasks

SeekingMeaning added the in progress label Sep 4, 2020

SeekingMeaning mentioned this pull request Sep 4, 2020

utils: add tar #8611

Merged

6 tasks

SeekingMeaning force-pushed the bump-cask-pr branch 3 times, most recently from db1e309 to cf2bb55 Compare September 5, 2020 00:11

MikeMcQuaid approved these changes Sep 8, 2020

View reviewed changes

SeekingMeaning added 2 commits September 8, 2020 08:26

utils/git: add origin_branch

be92ef2

dev-cmd: add bump-cask-pr

0a4fbf7

SeekingMeaning force-pushed the bump-cask-pr branch from 33ffdfe to 0a4fbf7 Compare September 8, 2020 15:26

SeekingMeaning marked this pull request as ready for review September 8, 2020 16:40

SeekingMeaning merged commit 6d1de3a into Homebrew:master Sep 8, 2020

SeekingMeaning deleted the bump-cask-pr branch September 8, 2020 16:40

vitorgalvao mentioned this pull request Sep 11, 2020

brew bump-cask-pr above cask-repair Homebrew/homebrew-cask#89077

Merged

SeekingMeaning mentioned this pull request Sep 11, 2020

bump-cask-pr: add support for :latest #8688

Merged

6 tasks

BrewTestBot added the outdated label Dec 1, 2020

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GitHub Sponsors

dev-cmd: add bump-cask-pr command #7986

dev-cmd: add bump-cask-pr command #7986

SeekingMeaning commented Jul 12, 2020 •

edited

Loading

miccal commented Jul 13, 2020

vitorgalvao commented Jul 13, 2020

vitorgalvao commented Jul 14, 2020

MikeMcQuaid commented Jul 20, 2020

stale bot commented Aug 16, 2020

vitorgalvao commented Aug 19, 2020

SeekingMeaning commented Aug 19, 2020 •

edited

Loading

SeekingMeaning commented Sep 5, 2020 •

edited

Loading

MikeMcQuaid left a comment

MikeMcQuaid Sep 8, 2020

vitorgalvao commented Sep 8, 2020

claui commented Sep 8, 2020

SeekingMeaning commented Sep 8, 2020

vitorgalvao commented Sep 11, 2020

core-code commented Sep 18, 2020 •

edited

Loading

SeekingMeaning commented Sep 18, 2020

dev-cmd: add bump-cask-pr command #7986

dev-cmd: add bump-cask-pr command #7986

Conversation

SeekingMeaning commented Jul 12, 2020 • edited Loading

miccal commented Jul 13, 2020

vitorgalvao commented Jul 13, 2020

vitorgalvao commented Jul 14, 2020

MikeMcQuaid commented Jul 20, 2020

stale bot commented Aug 16, 2020

vitorgalvao commented Aug 19, 2020

SeekingMeaning commented Aug 19, 2020 • edited Loading

SeekingMeaning commented Sep 5, 2020 • edited Loading

MikeMcQuaid left a comment

Choose a reason for hiding this comment

MikeMcQuaid Sep 8, 2020

Choose a reason for hiding this comment

vitorgalvao commented Sep 8, 2020

claui commented Sep 8, 2020

SeekingMeaning commented Sep 8, 2020

vitorgalvao commented Sep 11, 2020

core-code commented Sep 18, 2020 • edited Loading

SeekingMeaning commented Sep 18, 2020

SeekingMeaning commented Jul 12, 2020 •

edited

Loading

SeekingMeaning commented Aug 19, 2020 •

edited

Loading

SeekingMeaning commented Sep 5, 2020 •

edited

Loading

core-code commented Sep 18, 2020 •

edited

Loading