brew icon indicating copy to clipboard operation
brew copied to clipboard

Add cask install receipts

Open Rylan12 opened this issue 1 year ago • 7 comments

  • [x] Have you followed the guidelines in our Contributing document?
  • [x] Have you checked to ensure there aren't other open Pull Requests for the same change?
  • [x] Have you added an explanation of what your changes do and why you'd like us to include them?
  • [x] Have you written new tests for your changes? Here's an example.
  • [x] Have you successfully run brew style with your changes locally?
  • [x] Have you successfully run brew typecheck with your changes locally?
  • [x] Have you successfully run brew tests with your changes locally?

This PR is my first go at adding a cast install receipt (which I've called Tab to align for formulae) as per https://github.com/Homebrew/brew/issues/17013.

Here is a sample INSTALL_RECEIPT.json that is generated when I run brew install --cask warp:

{
  "homebrew_version": "4.3.6-71-gd665c42-dirty",
  "loaded_from_api": true,
  "installed_as_dependency": false,
  "installed_on_request": true,
  "time": 1719106768,
  "dependencies": {},
  "arch": "arm64",
  "source": {
    "path": "",
    "tap": "homebrew/cask",
    "tap_git_head": "f755fc333ebe7647fa3988e360d47a82120db032",
    "version": "0.2024.06.18.08.02.stable_03"
  },
  "installed_on": {
    "os": "Macintosh",
    "os_version": "macOS 14",
    "cpu_family": "arm_firestorm_icestorm",
    "xcode": "15.4",
    "clt": "15.3.0.0.1.1708646388",
    "preferred_perl": "5.34"
  },
  "artifacts": [
    {
      "app": [
        "Warp.app"
      ]
    },
    {
      "zap": [
        {
          "trash": [
            "~/Library/Application Support/dev.warp.Warp-Stable",
            "~/Library/Logs/warp.log",
            "~/Library/Preferences/dev.warp.Warp-Stable.plist",
            "~/Library/Saved Application State/dev.warp.Warp-Stable.savedState"
          ]
        }
      ]
    }
  ]
}

It doesn't quite work yet (there are some test failures and some other things still) but I wanted to push this up to start getting feedback on the strategy.

Rylan12 avatar Jun 23 '24 01:06 Rylan12

Do you have an example what the tab of a cask with an uninstall DSL looks like?

Idea being we avoid reading the Ruby file entirely when uninstalling, except for flight blocks which the tab should have a boolean or something that indicates those are used.

Bo98 avatar Jun 24 '24 02:06 Bo98

Do you have an example what the tab of a cask with an uninstall DSL looks like?

Here is the tab for slack:

{
  "homebrew_version": "4.3.6-71-ge30aea5-dirty",
  "loaded_from_api": true,
  "installed_as_dependency": false,
  "installed_on_request": true,
  "time": 1719197835,
  "dependencies": {
    "macos": {
      ">=": [
        "10.15"
      ]
    }
  },
  "arch": "arm64",
  "source": {
    "path": "",
    "tap": "homebrew/cask",
    "tap_git_head": "f755fc333ebe7647fa3988e360d47a82120db032",
    "version": "4.39.88"
  },
  "installed_on": {
    "os": "Macintosh",
    "os_version": "macOS 14",
    "cpu_family": "arm_firestorm_icestorm",
    "xcode": "15.4",
    "clt": "15.3.0.0.1.1708646388",
    "preferred_perl": "5.34"
  },
  "artifacts": [
    {
      "uninstall": [
        {
          "quit": "com.tinyspeck.slackmacgap"
        }
      ]
    },
    {
      "app": [
        "Slack.app"
      ]
    },
    {
      "zap": [
        {
          "trash": [
            "~/Library/Application Scripts/com.tinyspeck.slackmacgap",
            "~/Library/Application Support/com.apple.sharedfilelist/com.apple.LSSharedFileList.ApplicationRecentDocuments/com.tinyspeck.slackmacgap.sfl*",
            "~/Library/Application Support/Slack",
            "~/Library/Caches/com.tinyspeck.slackmacgap*",
            "~/Library/Containers/com.tinyspeck.slackmacgap*",
            "~/Library/Cookies/com.tinyspeck.slackmacgap.binarycookies",
            "~/Library/Group Containers/*.com.tinyspeck.slackmacgap",
            "~/Library/Group Containers/*.slack",
            "~/Library/HTTPStorages/com.tinyspeck.slackmacgap*",
            "~/Library/Logs/Slack",
            "~/Library/Preferences/ByHost/com.tinyspeck.slackmacgap.ShipIt.*.plist",
            "~/Library/Preferences/com.tinyspeck.slackmacgap*",
            "~/Library/Saved Application State/com.tinyspeck.slackmacgap.savedState",
            "~/Library/WebKit/com.tinyspeck.slackmacgap"
          ]
        }
      ]
    }
  ]
}

Idea being we avoid reading the Ruby file entirely when uninstalling, except for flight blocks which the tab should have a boolean or something that indicates those are used.

Right now the format I'm saving the artifacts in is the same that is used in the API, so that should be possible (that's what I anticipated happening). Good note on the boolean to indicate about flight blocks, I'll add that.

Rylan12 avatar Jun 24 '24 03:06 Rylan12

  • Storing the tap that a cask was installed from.
  • Storing the cask version.

In both of these cases: we should store the same for formulae and in the same format for both (to make parsing either easier).

Idea being we avoid reading the Ruby file entirely when uninstalling, except for flight blocks which the tab should have a boolean or something that indicates those are used.

My understanding is also we hope to be able to eventually deprecate these flight blocks so that only the tab is needed for uninstall.

MikeMcQuaid avatar Jun 24 '24 08:06 MikeMcQuaid

My understanding is also we hope to be able to eventually deprecate these flight blocks so that only the tab is needed for uninstall.

Yeah, though key word there is "eventually" and no one's working on it yet so still should have something in the meantime to improve the majority of cases while we work on improving the final few cases that use flight blocks.

Flight block phasing out will likely require multiple distinct enhancements to the uninstall DSL.

Bo98 avatar Jun 24 '24 13:06 Bo98

Yeah, though key word there is "eventually" and no one's working on it yet so still should have something in the meantime to improve the majority of cases while we work on improving the final few cases that use flight blocks.

Agreed: just think it's worth considering with the design that we will hopefully not keep this around indefinitely.

MikeMcQuaid avatar Jun 24 '24 14:06 MikeMcQuaid

I've pushed changes to have casks use the same Tab class as formulae, just with some different methods occasionally. When creating a new tab via an ambiguous method (i.e. when reading from a file and not calling for_formula, for_keg, for_cask, etc), you should specify whether this is a formula or cask tab (formula will be chosen by default if no selection was made). Then, methods like #to_json will intelligently include the correct fields.

I also updated the info and tab commands to support new features here (like marking casks as installed on request or not). Maybe in the future, brew autoremove can be extended to include casks.

Rylan12 avatar Jun 25 '24 18:06 Rylan12

Apologies for closing @Rylan12, my bad :(

miccal avatar Jun 26 '24 11:06 miccal

Good note on the boolean to indicate about flight blocks, I'll add that.

Do you have an example of what an install receipt looks like now with this change?

Bo98 avatar Jul 03 '24 03:07 Bo98

Tangential: It looks like we don't currently save any info about whether a formula was installed from the API. Would that be useful debugging info to have at times?

The loaded_from_api entry is saved for formulae already. It's set to true when the formula was loaded from the API as opposed to the formula file. Same as is implemented here for casks

Rylan12 avatar Jul 03 '24 04:07 Rylan12

Do you have an example of what an install receipt looks like now with this change?

Here is a cask installed with the API:

{
  "homebrew_version": "4.3.8-28-g53c4dc6",
  "loaded_from_api": true,
  "caskfile_only": false,
  "installed_as_dependency": false,
  "installed_on_request": true,
  "time": 1719979566,
  "source_modified_time": 0,
  "runtime_dependencies": {},
  "source": {
    "path": "",
    "tap": "homebrew/cask",
    "tap_git_head": "82a1e9a2152326048fed872457d6dd307e206855",
    "version": "0.2024.06.25.08.02.stable_01"
  },
  "arch": "arm64",
  "uninstall_artifacts": [
    {
      "app": [
        "Warp.app"
      ]
    }
  ],
  "built_on": {
    "os": "Macintosh",
    "os_version": "macOS 14",
    "cpu_family": "arm_firestorm_icestorm",
    "xcode": "15.4",
    "clt": "15.3.0.0.1.1708646388",
    "preferred_perl": "5.34"
  }
}

Here is the same one installed without the API:

{
  "homebrew_version": "4.3.8-28-g53c4dc6",
  "loaded_from_api": false,
  "caskfile_only": false,
  "installed_as_dependency": false,
  "installed_on_request": true,
  "time": 1719979733,
  "source_modified_time": 0,
  "runtime_dependencies": {},
  "source": {
    "path": "/opt/homebrew/Library/Taps/homebrew/homebrew-cask/Casks/w/warp.rb",
    "tap": "homebrew/cask",
    "tap_git_head": "82a1e9a2152326048fed872457d6dd307e206855",
    "version": "0.2024.06.25.08.02.stable_01"
  },
  "arch": "arm64",
  "uninstall_artifacts": [
    {
      "app": [
        "Warp.app"
      ]
    }
  ],
  "built_on": {
    "os": "Macintosh",
    "os_version": "macOS 14",
    "cpu_family": "arm_firestorm_icestorm",
    "xcode": "15.4",
    "clt": "15.3.0.0.1.1708646388",
    "preferred_perl": "5.34"
  }
}

Rylan12 avatar Jul 03 '24 04:07 Rylan12

Sorry, I meant one with an uninstall_pre/postflight like adobe-creative-cloud

Bo98 avatar Jul 03 '24 04:07 Bo98

Sorry, I meant one with an uninstall_pre/postflight like adobe-creative-cloud

Oops, here it is. Note that caskfile_only is true. The pre/post flight blocks do show up in the artifact lists (just as null), but we could generate it using artifacts_list compact: true to ignore those blocks if we want.

{
  "homebrew_version": "4.3.8-28-g53c4dc6",
  "loaded_from_api": false,
  "caskfile_only": true,
  "installed_as_dependency": false,
  "installed_on_request": true,
  "time": 1719980650,
  "source_modified_time": 0,
  "runtime_dependencies": {},
  "source": {
    "path": "/Users/rylanpolster/Library/Caches/Homebrew/api-source/Homebrew/homebrew-cask/0cad5fc067c6d3048b6221fd001bda1b5df4deaa/Cask/adobe-creative-cloud.rb",
    "tap": "homebrew/cask",
    "tap_git_head": "82a1e9a2152326048fed872457d6dd307e206855",
    "version": "6.2.0.554"
  },
  "arch": "arm64",
  "uninstall_artifacts": [
    {
      "uninstall_preflight": null
    },
    {
      "uninstall": [
        {
          "early_script": {
            "executable": "/usr/bin/pluginkit",
            "args": [
              "-r",
              "/Applications/Utilities/Adobe Sync/CoreSync/Core Sync.app/Contents/PlugIns/ACCFinderSync.appex"
            ],
            "must_succeed": false,
            "print_stderr": false
          },
          "launchctl": [
            "Adobe_Genuine_Software_Integrity_Service",
            "com.adobe.acc.installer",
            "com.adobe.acc.installer.v2",
            "com.adobe.AdobeCreativeCloud",
            "com.adobe.ccxprocess"
          ],
          "quit": "com.adobe.acc.AdobeCreativeCloud",
          "signal": [
            "QUIT",
            "com.adobe.accmac"
          ],
          "script": {
            "executable": "/usr/bin/pkill",
            "args": [
              "Adobe Desktop Service",
              "AdobeIPCBroker",
              "AdobeCRDaemon"
            ],
            "must_succeed": false
          },
          "delete": [
            "/Applications/Adobe Creative Cloud/*Adobe Creative Cloud",
            "/Applications/Adobe Creative Cloud/.Uninstall*",
            "/Applications/Adobe Creative Cloud/Icon?",
            "/Applications/Utilities/Adobe Application Manager",
            "/Applications/Utilities/Adobe Creative Cloud*",
            "/Applications/Utilities/Adobe Installers/.Uninstall*",
            "/Applications/Utilities/Adobe Installers/Uninstall Adobe Creative Cloud",
            "/Applications/Utilities/Adobe Sync",
            "/Library/Internet Plug-Ins/AdobeAAMDetect.plugin",
            "/Library/LaunchDaemons/com.adobe.agsservice.plist"
          ],
          "rmdir": [
            "/Applications/Adobe Creative Cloud",
            "/Applications/Utilities/Adobe Installers",
            "/Library/*/Adobe",
            "/Library/Application Support/Adobe",
            "/Library/Application Support/Adobe{/CEP{/extensions,},}",
            "/Library/Logs/Adobe"
          ]
        }
      ]
    },
    {
      "uninstall_postflight": null
    }
  ],
  "built_on": {
    "os": "Macintosh",
    "os_version": "macOS 14",
    "cpu_family": "arm_firestorm_icestorm",
    "xcode": "15.4",
    "clt": "15.3.0.0.1.1708646388",
    "preferred_perl": "5.34"
  }
}

Rylan12 avatar Jul 03 '24 04:07 Rylan12

Tangential: It looks like we don't currently save any info about whether a formula was installed from the API. Would that be useful debugging info to have at times?

The loaded_from_api entry is saved for formulae already. It's set to true when the formula was loaded from the API as opposed to the formula file. Same as is implemented here for casks

Ah, I got confused. My bad.

apainintheneck avatar Jul 03 '24 05:07 apainintheneck

The pre/post flight blocks do show up in the artifact lists (just as null), but we could generate it using artifacts_list compact: true to ignore those blocks if we want.

This is fine. We needed something to to determine whether flight blocks are used on uninstall so this works. We'll use this later to only load the Ruby file when needed on uninstall.

caskfile_only also checks non-uninstall flight blocks which wouldn't be useful for our case.

Bo98 avatar Jul 03 '24 12:07 Bo98

caskfile_only also checks non-uninstall flight blocks which wouldn't be useful for our case.

Good point. I updated this in the latest version.

Also, I realized that zap artifacts were being ignored since those use zap_phase and not uninstall_phase, so I fixed that too.

Rylan12 avatar Jul 04 '24 06:07 Rylan12

I pushed changes adding some more shared items to the generic items list, but I left off tabfile, runtime_dependencies, and source.path because they all have different formula/cask calls that are used to generate them. With all of those, I can still add them to the generic method as nil if preferred.

Rylan12 avatar Jul 04 '24 16:07 Rylan12

higher threshold than a typical PR here just because we want to avoid writing bad tabs and cleaning them up (as happened with formulae).

Agreed, thanks!

Rylan12 avatar Jul 09 '24 19:07 Rylan12

I've just pushed some more changes because I realized that the runtime dependencies in cask tabs did not include any recursive dependencies, only the dependencies that were directly declared. This should be fixed now (with some shared AbstractTab logic) and I've added a test.

With that, the runtime_dependencies entry in a cask tab looks something like this:

  "runtime_dependencies": {
    "cask": [
      {
        "full_name": "foo-cask",
        "version": "1.2.3",
        "declared_directly": true
      },
      {
        "full_name": "bar-cask",
        "version": "4.5.6",
        "declared_directly": false
      }
    ],
    "formula": [
      {
        "full_name": "foo-formula",
        "version": "1.2.3",
        "revision": 0,
        "pkg_version": "1.2.3",
        "declared_directly": true
      },
      {
        "full_name": "bar-formula",
        "version": "4.5.6",
        "revision": 0,
        "pkg_version": "4.5.6",
        "declared_directly": false
      }
    ],
    "macos": {
      ">=": [
        "10.14"
      ]
    },
    "arch": [
      {
        "type": "arm",
        "bits": 64
      }
    ]
  },

Should the macos and arch entries live somewhere else instead of underneath runtime_dependencies? Or, should we even include them at all? We don't include such information in formula tabs (although maybe we should?)

Rylan12 avatar Jul 09 '24 19:07 Rylan12

The approach to getting the recursive deps for each cask looks good to me.

I'm with you when you say that the arch and macos info maybe shouldn't be included in the runtime dependencies section. We already have arch at the top-level of the install receipt and os version is already included in the built_on section. It's not obvious to me that either of the arch or os dependencies are needed in the install receipt.

apainintheneck avatar Jul 10 '24 04:07 apainintheneck

Looks great, thanks @Rylan12! Let's merge this tomorrow when we're all together in case there's any issues.

MikeMcQuaid avatar Jul 13 '24 00:07 MikeMcQuaid