Skip to content

Don't compact service_args in FormulaStruct#serialize#22002

Merged
MikeMcQuaid merged 1 commit intomainfrom
compact-blank-skip-service-args
Apr 13, 2026
Merged

Don't compact service_args in FormulaStruct#serialize#22002
MikeMcQuaid merged 1 commit intomainfrom
compact-blank-skip-service-args

Conversation

@Rylan12
Copy link
Copy Markdown
Member

@Rylan12 Rylan12 commented Apr 13, 2026

Fixes the following issue:

$ brew info nginx
==> nginx ✘: stable 1.29.8 (bottled), HEAD
HTTP(S) server and reverse proxy, and IMAP/POP3 proxy server
https://nginx.org/
Not installed
From: https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/n/nginx.rb
License: BSD-2-Clause
==> Dependencies
Required (2): openssl@3 ✔, pcre2 ✔
==> Options
--HEAD
 Install HEAD version
Defining service run_type with argument immediate
Defining service working_dir with argument /opt/homebrew
Defining service keep_alive with argument
Error: Parameter 'value': Expected type T.any(T::Boolean, T::Hash[Symbol, T.untyped]), got type NilClass
Caller: /opt/homebrew/Library/Homebrew/formulary.rb:315
Definition: /opt/homebrew/Library/Homebrew/service.rb:169 (Homebrew::Service#keep_alive)
...

Since service_args are defined by the service DSL and only include methods explicitly called, we shouldn't compact away falsey values. For example:

keep_alive successful_exit: false

Internally, this sets @keep_alive = { successful_exit: false } which has a different effect from @keep_alive = nil which is what you get if you don't call keep_alive at all.

However, the current FormulaStruct serialization filters away all falsey values, reducing this to keep_alive nil which causes a type error. In reality, we need the falsey DSL values to be preserved.

The values passed to service_args are computed by Homebrew::Service.from_hash which only includes DSLs that were explicitly used, so this doesn't add any unnecessary information to the serialized files.

Please merge at the same time or after Homebrew/formulae.brew.sh#2173


  • 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 (excluding integration tests) for your changes? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) with your changes locally?

  • AI was used to generate or assist with generating this PR. Please specify below how you used AI to help you, and what steps you have taken to manually verify the changes.

Copilot AI review requested due to automatic review settings April 13, 2026 02:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts FormulaStruct#serialize to preserve falsy values inside service_args when generating the formula API hash, preventing invalid service DSL reconstruction (e.g., keep_alive successful_exit: false being compacted into keep_alive nil and triggering Sorbet runtime type errors during brew info/API formula loading).

Changes:

  • Preserve service_args across Utils.deep_compact_blank so false (and other “blank” values per ActiveSupport) are not dropped from service DSL arguments.
  • Return the compacted hash explicitly after restoring service_args.
  • Remove now-unneeded service_args arg-pair formatting in FormulaStruct.deserialize.

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

@Rylan12
Copy link
Copy Markdown
Member Author

Rylan12 commented Apr 13, 2026

Note: technically this isn't backward compatible, and in between when this is merged and formulae.brew.sh is regenerated, there is a very small number of formulae1 that will not be installable. Since I think only me and Mike have this set, I'll just avoid this by merging when you're asleep 😅 (or you can merge it while I'm asleep). As long as it merges with or after Homebrew/formulae.brew.sh#2173, the broken period will be a max of 15 minutes.

Footnotes

  1. 34 formulae that set either keep_alive successful_exit: false or keep_alive always: false in service do. It will be a type error, and temporarily unsetting HOMEBREW_REALLY_USE_INTERNAL_API will fix it.

Copy link
Copy Markdown
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Thanks!

@MikeMcQuaid MikeMcQuaid added this pull request to the merge queue Apr 13, 2026
Merged via the queue into main with commit 52431bd Apr 13, 2026
42 checks passed
@MikeMcQuaid MikeMcQuaid deleted the compact-blank-skip-service-args branch April 13, 2026 16:05
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