Skip to content

cask/artifact/pkg: fix type error#22923

Merged
p-linnane merged 2 commits into
mainfrom
cask-artifact-pkg-fix-type-error
Jul 3, 2026
Merged

cask/artifact/pkg: fix type error#22923
p-linnane merged 2 commits into
mainfrom
cask-artifact-pkg-fix-type-error

Conversation

@samford

@samford samford commented Jul 2, 2026

Copy link
Copy Markdown
Member

  • Have you followed our Contributing guidelines?
  • Have you checked for other open Pull Requests for the same change?
  • Have you explained what your changes do? Performance claims (e.g. "this is faster") must include Hyperfine benchmarks.
  • Have you explained why you'd like these changes included, not just what they do?
  • For bug fixes, have you given step-by-step brew commands to reproduce the bug?
  • Have you written new tests (excluding integration tests)? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) locally?

  • AI was used to generate or assist with generating this PR.

When the HOMEBREW_SORBET_RECURSIVE=1 environment variable is set and a cask using the pkg stanza is installed/upgraded, Sorbet will give a type error (Parameter 'env': Expected type T::Hash[String, T.nilable(T.any(PATH, String, T::Boolean))], got T::Hash[String, User]) related to SystemCommand.run!. I looked for usage of User in cask and saw that Cask::Artifact::Pkg.run_installer uses User objects in the env arg to SystemCommand.run!. This resolves the issue by calling #to_s on User.current, to ensure it's a string.

You can replicate this by setting the HOMEBREW_SORBET_RECURSIVE=1 environment variable and installing/upgrading a cask that uses the pkg stanza (e.g., HOMEBREW_SORBET_RECURSIVE=1 brew install cloudflare-warp).

Copilot AI review requested due to automatic review settings July 2, 2026 18:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a Sorbet type error in the Cask pkg artifact installer path by ensuring the environment variables passed to SystemCommand.run! are typed as strings.

Changes:

  • Convert User.current to a string before populating env: for /usr/sbin/installer.
  • Reuse the converted value across LOGNAME/USER/USERNAME to satisfy SystemCommand’s Sorbet signature.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Library/Homebrew/cask/artifact/pkg.rb Outdated
Comment thread Library/Homebrew/cask/artifact/pkg.rb Outdated
@samford samford force-pushed the cask-artifact-pkg-fix-type-error branch from f9e8d99 to 6ebdb75 Compare July 2, 2026 19:27

@p-linnane p-linnane left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. The remaining gap seems to be in Library/Homebrew/test/cask/artifact/pkg_spec.rb, since the existing expectation can still pass with a User delegator.

How about something like this in the spec?:

current_user = User.current&.to_s

env: {
  "LOGNAME"  => an_instance_of(String).and(eq(current_user)),
  "USER"     => an_instance_of(String).and(eq(current_user)),
  "USERNAME" => an_instance_of(String).and(eq(current_user)),
}

@samford

samford commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

Nice, that seems to do what we're looking for. Added in the latest push 👍

@p-linnane

Copy link
Copy Markdown
Member

This PR should unblock CI here: #22927

samford and others added 2 commits July 3, 2026 10:03
When the `HOMEBREW_SORBET_RECURSIVE=1` environment variable is set
and a cask using the `pkg` stanza is installed/upgraded, Sorbet will
give a type error (`Parameter 'env': Expected type T::Hash[String,
T.nilable(T.any(PATH, String, T::Boolean))], got T::Hash[String,
User]`) related to `SystemCommand.run!`. I looked for usage of `User`
in cask and saw that `Cask::Artifact::Pkg.run_installer` uses `User`
objects in the `env` arg to `SystemCommand.run!`. This resolves the
issue by calling `#to_s` on `User.current`, to ensure it's a string.
Co-authored-by: Patrick Linnane <patrick@linnane.io>
@p-linnane p-linnane force-pushed the cask-artifact-pkg-fix-type-error branch from cbbbe2d to 746fa0a Compare July 3, 2026 17:04
@p-linnane p-linnane enabled auto-merge July 3, 2026 17:04
@p-linnane p-linnane added this pull request to the merge queue Jul 3, 2026
Merged via the queue into main with commit dba262d Jul 3, 2026
41 checks passed
@p-linnane p-linnane deleted the cask-artifact-pkg-fix-type-error branch July 3, 2026 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants