cmd/bundle: don't raise UsageError when using environment variables.
#21398
Conversation
Otherwise you can't really keep them set globally.
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where setting HOMEBREW_BUNDLE_CHECK or HOMEBREW_BUNDLE_NO_SECRETS environment variables globally would prevent using brew bundle with incompatible subcommands. The change allows these environment variables to bypass the usage validation that would otherwise raise errors.
Changes:
- Modified validation logic for
--checkflag to skip validation whenHOMEBREW_BUNDLE_CHECKis set via environment variable - Modified validation logic for
--no-secretsflag to skip validation whenHOMEBREW_BUNDLE_NO_SECRETSis set via environment variable
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| end | ||
|
|
||
| if args.check? && BUNDLE_EXEC_COMMANDS.exclude?(subcommand) | ||
| if args.check? && !ENV["HOMEBREW_BUNDLE_NO_CHECK"] && BUNDLE_EXEC_COMMANDS.exclude?(subcommand) |
There was a problem hiding this comment.
The environment variable check does not follow Homebrew's standard pattern for boolean environment variables. The expression !ENV["HOMEBREW_BUNDLE_NO_CHECK"] will evaluate to false (thus skipping validation) for any value set in the variable, including falsy values like "false", "no", "off", or "0".
According to Homebrew's EnvConfig pattern (Library/Homebrew/env_config.rb lines 591-596), boolean environment variables should check: env_value.present? && FALSY_VALUES.exclude?(env_value.downcase) where FALSY_VALUES includes "false", "no", "off", "nil", and "0".
Consider implementing this check consistently with other Homebrew boolean environment variables to avoid unexpected behavior when users set the variable to "0" or "false".
| end | ||
|
|
||
| if args.no_secrets? && BUNDLE_EXEC_COMMANDS.exclude?(subcommand) | ||
| if args.no_secrets? && !ENV["HOMEBREW_BUNDLE_NO_SECRETS"] && BUNDLE_EXEC_COMMANDS.exclude?(subcommand) |
There was a problem hiding this comment.
The environment variable check does not follow Homebrew's standard pattern for boolean environment variables. The expression !ENV["HOMEBREW_BUNDLE_NO_SECRETS"] will evaluate to false (thus skipping validation) for any value set in the variable, including falsy values like "false", "no", "off", or "0".
According to Homebrew's EnvConfig pattern (Library/Homebrew/env_config.rb lines 591-596), boolean environment variables should check: env_value.present? && FALSY_VALUES.exclude?(env_value.downcase) where FALSY_VALUES includes "false", "no", "off", "nil", and "0".
Consider implementing this check consistently with other Homebrew boolean environment variables to avoid unexpected behavior when users set the variable to "0" or "false".
Otherwise you can't really keep them set globally.