Streamlining Updates

[adidalal]

Some thoughts and a tentative roadmap towards minimizing both the time and effort to keep Casks up to date in homebrew-cask

Currently: Cask updates are submitted as PRs by (our awesome) contributors. A new PR must be made for each version, and then merged into caskroom/homebrew-cask

We have some pieces (ie. appcast URLs and cask-repair to push towards a better process, but there's nothing streamlined yet.

---

Roadmap (updated based on @vitorgalvao's comments below):

• Update cask-repair to also update the appcast stanza’s :sha256.
• Instead of trying to be “smart” and parse different types of appcasts, remove :format altogether.
• Add :sha256 to all appcasts
• Make :sha256 in appcasts a requirement.
• Update cask-repair to also submit a PR automatically. ¹
• Add cask-repair to CONTRIBUTING.md (documentation)

Build a script/bot, that will:

- [ ] Auto-merge PRs only if said PR:
- Consists of a single commit to a single cask.
- Modifies the cask (i.e. not valid for creating casks).
- Modifies exactly (no more, no less) the stanzas: version, sha256, and :sha256 on the appcast if there is one.
- Passes all tests.² (#15908 (comment))

• Iterate through the casks with an appcast . Updates would clearly show up as having a changed :sha256.
• Upon seeing appcast changes, open a new issue in the form of a Cask Request. Includes:
  • The token of the cask (with a link).
  • The contents the appcast.
  • A link to the appcast (in case the 50 lines aren’t enough).

---

¹ At which point cask-repair could be used to submit the PR, set it and forget it. Perhaps it should also be updated (would likely have to use a third-party tool as well, like how prfixmaster uses ghi) to auto-submit the PR, instead of the current behaviour where you still need to go to the actual page and click to submit it.

² Since @jawshooah made Travis confirm shasums, we know that also means the download is valid.

[vitorgalvao]

I like the sentiment a lot. I’d change the solution a bit, though, since I perceive a hole as it stands. I’ll explain what I mean so we can discuss it and add/replace/remove from your top post, which should (as we usually do) contain the final decisions at any given point.

Basically, I’d change where (part of) the automation is. Instead of making simple PRs automatically, merge them automatically.

Instead of trying to be "smart" and parse different types of appcasts, go back to having just the stanza, with a required shasum. The type could be left in for future work, but it's not currently being used anywhere.

I’m a big fan of “stopping to try to be smart” for this, as #13201 should illustrate. Being smart before having things that work is what caused a lot of the problems in the first place. I’d go as far as removing that second sentence and removing types altogether, though. However, for the current proposal that wouldn’t work.

The problem is that for us to be able to make an automatic PR, we need to know what the new version is. To know that, we need to parse the appcast, and to do that we need to know how it’s structured, hence the types. However, not every type can be trusted to be consistent. For example, the same type of appcast might be different between apps. For example, github appcasts that contain source releases and different OSs in one app, but not in another. Also, there are non-standard appcasts that will add even more overhead. It might not be possible then, to make this work at all without a lot of workarounds for each app, so we should scrape types altogether.

But now, this also makes it impossible for cask-repair to do that job, because it needs you to input an actual version, as it cannot guess it. So my proposal is to update your proposal as follows:

(updated, see first post)

---

As for brew-cask-upgrade, just went through it and it’s hacky: It basically just checks /opt/homebrew-cask/Caskroom (so right there it’ll fail if you don’t use our default location) and checks the directory names of apps (since we append the version). It works, but isn’t robust enough to be used officially. In addition, since it relies on the directories in the Caskroom having versions, it’ll completely break after #13201.

[vitorgalvao]

Thanks to @Amorymeltzer, the first item, “update cask-repair to also update the appcast stanza’s :sha256” is now done.

[adidalal]

Updated the main post with the revised checklist.

[vitorgalvao]

At which point cask-repair could be used to submit the PR, set it and forget it. Perhaps it should also be updated (would likely have to use a third-party tool as well, like how prfixmaster uses ghi) to auto-submit the PR, instead of the current behaviour where you still need to go to the actual page and click to submit it.

Just updated cask-repair, and it does this now. At the end of the run, it’ll auto-submit the PR.

[adidalal]

@vitorgalvao Tested the updated script with #15992 and #15993, seems to work well.

I'd probably put auto-merging PRs as fairly low-prioirity, as it's already so low friction with fastmerge.

The next big step is to mass-add appcast shasums to Casks without one. AFAIK no Cask hosted on Github has one yet.

[vitorgalvao]

Tested the updated script with #15992 and #15993, seems to work well.

And I see they also worked to update the appcast’s :sha256, which is also good news.

The next big step is to mass-add appcast shasums to Casks without one.

Agreed.

[vitorgalvao]

“Add :sha256 to all appcasts” is now a check on every repo.

[zmwangx]

If cask-repair is stable enough, maybe it could be added to developer/bin instead of semi-officially living in the wild with a bunch of unrelated scripts?

[vitorgalvao]

@zmwangx From #16134 (comment):

I have a few scripts that are regularly used by some maintainers (and even users) here, but I keep them in my own repo for a few reasons. Some of them aren’t homebrew-cask specific and can easily benefit other projects and the one that is specific to us (cask-repair) isn’t really targeted for development, and can be used by users. I also very much like to keep them organised and even have formulas for them all all my others “tiny-scripts”. I never gave it any thought, really, but perhaps (at least cask-repair) could make sense here.

It is indeed stable enough. It has been relatively stable ever since its release, and problems are usually caught early (sometimes by @adityadalal924).

That said, I have no quarrel whatsoever with unofficial (or semi-official) projects (and like that there are many other scripts and gems made by users). I also enjoy having cask-repair always on hand (i.e. in my PATH), accessible from a standard location (/usr/local/bin), and especially enjoy having a formula for it.

Like I quoted, it’s not out of the question at all, but making it so will lose (some of) those advantages. The fact that it depends on other formula is a big one, even. Though I check to see if they are installed anyway (in case you installed cask-repair manually), having the formula do it for you is so much more convenient. In the end, stangely and in addition to those reasons, it seems to me that the more frequently a script is used, the stronger the case for it to be outside the repo. It is indeed not a developer script (users use it, and may do so frequently), in contrast to those in /developer/bin which are used occasionally and only when you’re solving a difficult problem (as opposed to “oh, an app update, let me run cask-repair”).

[adidalal]

I am in favor of keeping it in tiny scripts, and I'll look into adding a comment or two about it into contributing.md also

[zmwangx]

@vitorgalvao What you said makes sense, but there are two problems:

1. It's hard to keep it updated. One has to clone your whole repo and manually pull changes; Overlooked the part about having a formula, sorry!
2. The discoverability is too low, especially if you consider it a user script. It's only mentioned all the way down in maintaining.md — it's hard to imagine users reading maintaining.md, and I wasn't aware of it until being told.

[vitorgalvao]

The discoverability is too low, especially if you consider it a user script.

That is being addressed by @adityadalal924 in #16511. CONTRIBUTING is really the best place to mention it, anyway. Contributors who just want to bump a version won’t look inside /developer (and why should they), so that doesn’t exactly help discoverability; mentioning it in the documentation does.

[zmwangx]

Sure.

[vitorgalvao]

Removed the idea about the auto-merging bot. Thinking about it last night, I realised it could be potentially dangerous, since we allow pretty much anything in version. Picture this: an app that uses a cloudfront or amazonaws URL and has #{version} pretty early on its path. A malicious user could set a version that would change the actual path (using /s) and point to something hosted by them. With human review (that doesn’t actually take that long for these simple submissions), such a change to version would pop out.

[adidalal]

#16529 should help make checking simple anyway - Once Travis lights up green and one of the maintainers gives a quick once-over to the PR, it should be okay to merge (which we generally are pretty quick about, anyway).

The "auto-update" script is still a reasonable idea, and would go well with cask-repair

[vitorgalvao]

The "auto-update" script is still a reasonable idea

I still intend to work on it after I finish updating all appcasts.

and would go well with cask-repair

Have you cask-repair --help lately (do it after an update, as I recently rename some options)? I’ve added some options that should indeed make it more enjoyable, namely giving version and url directly on the command line, without being prompted for it.

I’m also thinking of adding a --blind-submit option so you don’t even have to y afterwards, but am still thinking if it is sensible. I’ll probably add a time delay so you can cancel if you notice a mistake.

[adidalal]

#18237 finishes up this initiative. Closing in favor of that PR.