Homebrew-cask: `cask-repair` command

[commitay]

A ruby version of cask-repair implemented as command has been mentioned a couple of times, opened this to discuss and see if its something we might want to do.

Benefits of the command approach:

• Always up to date.
• Could use brew GitHub API utils rather than the hub formula.

Possible features that would (probably) be easier to implement in ruby:

• Checking for duplicate PRs.
• Support updates for language casks (firefox, etc.)

Couple of things that would need to be discussed:

• Where it would live. Adding it to the brew repo may not be the best option, another tap (similar to the bundle and services commands) or the homebrew-cask repo may work better.
• Whether it would be a brew command (e.g. brew cask-repair) or a brew cask command (e.g. brew cask repair)

[vitorgalvao]

Possible features that would (probably) be easier to implement in ruby:

Duplicate PRs are a hard nut. Those are as hard to fix in bash as in ruby. Since cask-repair allows manual edits (and that’s an important feature), we can’t just check the PR/branch name and be sure they’re duplicates. There are a few ways we could go about it (checking first for name and on conflict download and check for content), but I don’t think duplicate PRs are a big enough problem.

As for language (and don’t forget url do), those wouldn’t just be easier to implement with direct access to the core, they’d be feasible at all. The reason I don’t add support those in cask-repair is the amount of work would outweigh the benefit.

Couple of things that would need to be discussed

It should live either in homebrew-cask or in the repo of the appcast definitions.

It should be brew cask repair. It’s consistent and intuitive. It was while playing with this that https://github.com/Homebrew/homebrew-cask/issues/28977 was born.

[brianbrownton]

I would be a fan of integrating it.

FWIW, my duplicate PR logic, while rudimentary by some standards, has worked well for me:
• search open PRs for cask name
• not found -> clear to submit PR
• found -> grep version strict match (if exact match found, flag as duplicate PR else clear to submit)

[commitay]

we can’t just check the PR/branch name and be sure they’re duplicates. There are a few ways we could go about it (checking first for name and on conflict download and check for content)

The bump formula command with the GitHub API utils check the PR name/version and warns if there is a PR with the same already open but gives the option of submitting it anyway with --force. I think something similar would be sufficient as we only need it for duplicate version bumps and if the change is unrelated to the open PR it can be submitted anyway.

I don’t think duplicate PRs are a big enough problem.

Yeah, it isn't really a problem, would mostly be so we don't have to close them manually. Might also be useful if we ever get a bot for submitting version bumps so it doesn't try to submit the same PRs.

[commitay]

Mentioned in https://github.com/Homebrew/homebrew-cask/pull/48378, the "duplicate" check should run immediately prior to the PR submission.

[MikeMcQuaid]

A ruby version of cask-repair implemented as command

How is it currently implemented?

[commitay]

https://github.com/vitorgalvao/tiny-scripts/blob/master/cask-repair

https://github.com/vitorgalvao/homebrew-tiny-scripts/blob/master/cask-repair.rb

[vitorgalvao]

https://github.com/vitorgalvao/tiny-scripts/blob/master/cask-repair

Compare that monstrosity to the original, four years ago. Less than five times the size (and did so little). Not sure I should be proud or surprised it still manages to hold itself together and work effectively.

[MikeMcQuaid]

A command sounds better. I wonder what makes brew cask-repair worse than brew cask repair?

[reitermarkus]

I wonder what makes brew cask-repair worse than brew cask repair?

The hyphen. All other commands are sub-commands of brew cask, so why should repair be an exception?

[MikeMcQuaid]

All other commands are sub-commands of brew cask, so why should repair be an exception?

Adding (and fixing, currently) support for brew cask subcommands in the PATH adds additional complexity assuming the implementation doesn't differ otherwise.

Furthermore, brew cask is itself a subcommand of Homebrew so it feels strange to have additional custom subcommands of that with their own command naming scheme just to avoid a -.

[reitermarkus]

Adding (and fixing, currently) support for brew cask subcommands in the PATH adds additional complexity assuming the implementation doesn't differ otherwise.

Implementing a cask-specific command as a brew sub-command adds far more complexity than having first-class support for brew cask sub-commands.

[core-code]

sorry to just chime in. just wanted to mention, from a user-perspective the single thing i would want changed about cask-repair (or a hypothetical "brew cask repair") is the ability to run multiple instances concurrently (which currently results in damaged merge requests). if you are updating 20 casks having to do it in a serial way slows things down considerably, especially with long downloads.

[brianbrownton]

@core-code I am with you on that - a bit off topic for sure - but I have brought it up previously.

[stale[bot]]

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

[brianbrownton]

Has this gone anywhere?

[reitermarkus]

@brianmorton, no, there hasn't been any progress on this.

[vitorgalvao]

I’m fine with letting this close. If this is ever going to be worked on (I’d say about now the chances are a bit over 50%) I’ll probably be the one to do it. And don’t worry, I won’t forget about this without an open issue, I’ve been considering it for longer than this has been open.

[brianbrownton]

@vitorgalvao I also have an open issue (I believe) for my cask-automation project to port cask-repair into python, or port cask-automation into ruby so that I could integrate cask-repair functionality as it would be helpful when doing automated cask updates.

[stale[bot]]

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