Should audit check appcast shasums?

[Amorymeltzer]

I don't usually go mucking about in ruby, so this is meant more to open the question.

Is it worth having cask audit also run through any sums in the appcast stanza? Officially, audit is to "verify installability", and the appcast stanza has no impact on that[1]. Still, it's cheap, and if the appcast stanza is to exist this would be valuable in the future.

---

[1]: Not that it matters, but in light of that these checks warn on failure instead of error.

[jawshooah]

I don't think there's much point in verifying the appcast until we actually have a use for it.

Personally, I'm still not entirely clear on exactly how it would be used for upgrades. I know @vitorgalvao and others have talked about it before, but it's not clear to me how we would extract updated information if the appcast itself has to be updated.

[adidalal]

@jawshooah The first thing that comes to mind is to use a changed appcast shasum as the trigger for a Cask version update - it wouldn't permit automatic updates, but it would allow a notice that the Cask does need to be updated. I could see this being used in sort of an "automated" cask-repair script, but again, that would still be somewhat manual (because someone would need to look over the PR).

[vitorgalvao]

Yes, the idea was somewhat in line with what @adityadalal924 said. It could actually be completely automated if we knew the structure of the appcast well, which is why we have types for them. The idea was to understand how each type behaves, and then have rules for them. That explains why so many casks have appcasts, but the feature isn’t working. We needed to collect a lot of real appcasts first to understand how to best tackle the problem.

I agree there isn’t much point in this as of now, but since this shows a warning instead of an error, it could be merged. However, I’d be against it right now, simply because we actively discourage people from updating appcast shasums most of the time (so they don’t waste time with it), and having the system telling them one thing and the maintainers another could get confusing.

[Amorymeltzer]

Yeah, I'm fine with all that @vitorgalvao, figured it was worth taking the temperature of the crew. It's a cheap fix to ensure no issues down the line, but I do agree it's of little value and would send the wrong message to a user getting a warning: audit failed message. A quick scan suggests only 6 casks have issues that would be caught anyway, so it's not exactly pressing.

[adidalal]

We can probably just fix those 6 Casks if you have a list handy (or open up another PR :)

[jawshooah]

Okay, so here's what I understand so far (assuming we don't want brew cask outdated to incur any network overhead):

Cask has...	versioned url	unversioned url
appcast with sha256	Run script to check sha256 against remote appcast. If different, cask needs an update	Run script to check sha256 against remote appcast. If different, ???
appcast without sha256	???	???

Would anyone care to fill in the blanks?

[vitorgalvao]

Simply put, the only case that should exist is the one you already understand completely: appcast with sha256 and versioned url. Every other case should be fixed to match that.

As for the second row (appcast without sha256), it can be completely removed, since it is not meant to exist. The reason it does is that appcast is currently non-functional, so it doesn’t really matter. A bunch of those are github appcasts which were mostly added en masse by me upon discovering the “just append .atom to the releases page” method. We didn’t (and don’t) even support that as an appcast format (since we did not know about it when first working on it), so adding a :sha26 served no purpose.

As for appcast with sha256 and unversioned url, those shouldn’t exist as well. Since an appcast tells you when a new version is available, it also keeps track of them. This means that even if a cask currently uses an unversioned url, we can change them to use a versioned one by checking the appcast. Sometimes we do, and have a familiar but slightly different comment for that. Even if an appcast were to point to unversioned urls itself (unlikely), we would still know when the cask is updated and have a version, thus being able to safely have a version and sha256 in a cask with an unversioned url.

[vitorgalvao]

Seems like we’re all in agreement, so closing this.