Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends Homebrew’s Formula#std_cargo_args helper to support Cargo install feature selection and binary selection, aiming to reduce duplicated per-formula argument construction and standardize cargo install usage.
Changes:
- Extend
std_cargo_argswith newfeatures:andbin:keyword parameters. - Add logic to emit
--all-features,--no-default-features,--features=..., and--bin=...based on the provided options.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
7ce1912 to
a696669
Compare
There was a problem hiding this comment.
Seems ok given we have --root and --path already but the point of std_*_args isn't really to be an arg wrapper but rather be something that provides a base set of arguments that all formulae unconditionally use. Seems like this could very large very quickly if we're going to start adding conditional arguments? I'm not completely sold that *std_cargo_args(no_default_features: true) is better than *std_cargo_args, "--no-default-features".
Though there are of course exceptions: std_go_args's ldflags is an exception since you must pass multiple ldflags as a single argument (-ldflags=-a -ldflags=-b doesn't work) and we wanted to retain the ability to prepend default ldflags (e.g. to switch linkers) like we do for other languages - something that would require updating many formulae if we wanted to do later and didn't already have ldflags suppport.
|
@Bo98 If so, I’m happy to keep only the My original intent was to add support in std_*_args for arguments that are relatively common across many formulae. I see As you pointed out, options like |
a696669 to
99b0dbb
Compare
brew lgtm(style, typechecking and tests) with your changes locally?Add
featuresandbinoptions tostd_cargo_argsforcargo install.Currently, around 60 files define
featuresand 6 files definebinseparately. By moving them intostd_cargo_args, we can make more consistentcargo installusage.