Add warning message to postinstall

[hyuraku]

• 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?

---

add A warning or some sort of message should be shown when installing a formula with a post_install block or a cask with a {pre,post}flight block for #13794

[Rylan12]

Currently, cask flight blocks probably do work as expected since we're loading directly from the source. Once we start loading from the API, we'll probably need to add something here but for now, there's no need.

For formulae, I don't think this is the proper fix. First of all, we only need to show an error message if HOMEBREW_INSTALL_FROM_API is set and the formula the user is trying to postinstall was installed using the API (i.e. it's a formula from homebrew/core). Also, the fix should go in either FormulaInstaller#post_install or Formula#run_post_install since brew postinstall isn't directly called during the regular formula install step.

Finally, the error should be clearer and specific to HOMEBREW_INSTALL_FROM_API. I'd recommend something like "When HOMEBREW_INSTALL_FROM_API is set, the postinstall may be outdated. If you need to run the latest version, install without HOMEBREW_INSTALL_FROM_API." (although we'll probably need to workshop that because it's a bit wordy)

[MikeMcQuaid]

@Rylan12 Personally: I don't think we need a warning here at all. I'm not convinced post_install changes often enough without rebottling to warrant this sort of edge case spamming everyone else.

[Rylan12]

@MikeMcQuaid I think you're right in most cases. To make this even more sensitive: here's an idea. What if we store the hash of either the postinstall source or the postinstall method object (I assume that's something that can be done) in the JSON API. That way, it would be updated every hour and would always represent the latest postinstall code. When a user runs the postinstall (whether through a regular install or manually), we can compare what's going to be run with the hash from the API. If they don't match, that's an indication that there is a new postinstall block available.

Thoughts?

[MikeMcQuaid]

When a user runs the postinstall (whether through a regular install or manually), we can compare what's going to be run with the hash from the API. If they don't match, that's an indication that there is a new postinstall block available.

Thoughts?

@Rylan12 Makes sense to me! If we're going to store a hash here, though, we may want to consider also storing the actual post_install source and consider using that when the hash differs (assuming the version/revision/etc. are the same).

[Rylan12]

We could, but that means we're potentially executing code online which is exactly what we're trying to avoid. So if we do that, I think it should not be done automatically

[MikeMcQuaid]

@Rylan12 I think if we want this to be the default path in the nearish future we need to brainstorm a fix that works here 100% of the time. Perhaps that is requiring a revision bump or at least new bottles if a post_install changes?

[Rylan12]

@Rylan12 I think if we want this to be the default path in the nearish future we need to brainstorm a fix that works here 100% of the time. Perhaps that is requiring a revision bump or at least new bottles if a post_install changes?

I think this is a better option, as long as we believe it can be maintained. A revision bump probably would be easier to write an audit for, since I don't think there are any checks we run to make sure bottles are made, but has the downside of forcing users to upgrade when they may not have to

[MikeMcQuaid]

I think this is a better option, as long as we believe it can be maintained.

Yeh, I think we'll need an audit to enforce it.

A revision bump probably would be easier to write an audit for, since I don't think there are any checks we run to make sure bottles are made, but has the downside of forcing users to upgrade when they may not have to

Yeh, unsure how to force the bottle requirement but I think it'd be better. We want to avoid doing so many unnecessary revision bumps so I fear a little making this mandatory. CC @Homebrew/brew for thoughts on approach here.

[Rylan12]

We could probably set up @BrewTestBot to request changes to PRs that modify the postinstall blocks so that they cannot be merged regularly. And then update the various merge workflows to have @BrewTestBot remove their requested changes. That way new bottles are always made. Unless there are cases where pr-automerge does not make new bottles—guess I haven't really played with that in a while so I don't really remember.

[MikeMcQuaid]

@Rylan12 Yeh, that sounds like a good idea 👍🏻

[Rylan12]

@hyuraku I'm going to close this out because it sounds like we'd like to address the issue in a different way. Thanks for the PR, though!

I don't have the bandwidth right now to work on this but if you or someone else would like to I'll happily review a PR!

I also will update the HOMEBREW_INSTALL_FROM_API tracking issue with a link to this for future reference