Skip to content

Bind options to environment variables, not just document them #76

Description

@Mpdreamz

What exists today

IArghRootBuilder.DocumentEnvironmentVariables(CliEnvVar[], CliConfigFile[]) lets a CLI declare that
environment variables exist, and they flow into help and into __schema as CliEnvVarSchema(Name, Description, Required, DefaultValue).

That is documentation. Nothing connects a CliEnvVar to an option, so the value still has to be read by
hand inside the handler, and everything that follows from "a value can come from two places" is the
caller's problem:

// what every option ends up looking like
var root = Environment.GetEnvironmentVariable("MYAPP_ROOT");
// ...100 lines later, in the parse loop
case "--root" when i + 1 < args.Length: root = args[++i]; break;

The declared CliEnvVar and the hand-written GetEnvironmentVariable are two statements of the same fact
in two places, and only one of them is what actually runs. Help can be right while behaviour is wrong.

The ask

Let an option say where else its value may come from, and have the generator do the lookup.

/// <summary>Where the data lives.</summary>
/// <param name="root">Directory to keep state in.</param>
public static int Serve([Env("MYAPP_ROOT")] string? root = null, [Env("MYAPP_URLS")] string urls = "http://127.0.0.1:8080")

with a root-level prefix so a CLI with fifty options does not repeat itself:

app.UseEnvironmentPrefix("MYAPP_");   // --requests-per-second -> MYAPP_REQUESTS_PER_SECOND

[Env] then appears only where the derived name is wrong, and [Env(None)] (or similar) opts one out.

Precedence, exactly

flag > environment > C# default value, fixed rather than configurable. A configurable chain gives two
CLIs two behaviours, which is the divergence the framework exists to remove. Leave one slot for later:
flag > environment > config file > default, with the file above the default because a file somebody
wrote is more specific than a fallback compiled into the binary.

Presence, not nullability, is what makes this work. argh owns the parse, so it knows whether --root
appeared in argv regardless of the parameter's type or default. Worth saying explicitly in the docs,
because the tempting implementation — compare the bound value against the default and treat "equal" as
absent — is wrong the moment somebody passes a value that happens to equal the default.

Cases that need deciding rather than discovering:

  • Collections replace, never append. If the environment contributed entries that a flag then added to,
    an environment-provided entry could never be removed from the command line, and precedence is a lie.
    The environment supplies the whole list or none of it, split on CollectionSyntaxAttribute.Separator —
    that concept already exists and a second one for the environment would be one too many.
  • Same parser, same validation. An environment value goes through IArgumentParser<T> and the
    DataAnnotations attributes the flag goes through, and fails the same way. This is where hand-rolled
    versions rot first: a strict parse on the flag path and a forgiving TryParse on the environment path.
  • Errors name the source. MYAPP_TIMEOUT=abc should fail as "MYAPP_TIMEOUT is not a duration", not
    "--timeout is not a duration", or the reader goes looking at a command line that never mentioned it.
  • Empty means unset — probably. MYAPP_ROOT= turns up constantly in compose files and CI matrices,
    and an empty string is nearly never the intended value for a path or a URL, so treating empty as absent
    is the safer default with an opt-out for options where empty is meaningful. Some tooling goes the other
    way, so this is the one I would put to a vote rather than assert.

Booleans need three states

This is the case no CLI author can fix on their own, and it silently breaks the precedence above.

A presence flag is bool verbose = false, where absence and false are the same token — there is no
--no-verbose to write. So once MYAPP_VERBOSE=1 is set, the command line cannot turn it off: "flag
beats environment" becomes unenforceable for exactly the type where an operator most wants the override,
because the container set it and they want it off for one run.

Three states are needed — said true, said false, said nothing — which is bool?:

  • bool? opts into the tri-state. argh generates --verbose and --no-verbose; null means nobody
    spoke, so the environment applies, and if that is unset too the handler sees null and decides for
    itself. Precedence then works in both directions.
  • Plain bool keeps today's presence semantics, but binding one to the environment deserves a
    build-time diagnostic: this variable can never be overridden from the command line; use bool?.
    This is a source generator, so it can say so at compile time, which matters because the failure is
    invisible at runtime — the CLI just quietly ignores the operator.
  • Truthy set, and errors rather than silence. 1/true/yes/on and 0/false/no/off, case-insensitive;
    anything else is an error. MYAPP_VERBOSE=nope quietly meaning "off" is the usual form of this bug.
  • Negated flags need a convention (--no-<name>), a way to override the generated name, and a
    collision check against options that already exist.

It generalises past booleans: a nullable parameter is how a handler is told nobody chose. int? retries = null distinguishes "nobody set retries" from "somebody set it to the number that was already the
default" — which is a different question from presence, because presence is knowledge the parser has and
nullability is what carries that knowledge into your code.

Global and namespace options

The options that most want environment binding are the deployment-level ones — root, urls, log level —
and those are exactly the ones that are not parameters on a leaf command. So:

  • [Env] has to be legal on properties, not only on method parameters: on an [AsParameters] class
    and on whatever declares a global or injected option. If it only reaches parameters, the options that
    need this most are the ones that cannot have it. A command marked [NoOptionsInjection] should not
    inherit those bindings either.
  • AsParameters.Prefix is the natural source of a derived variable prefix — [AsParameters("store")]
    giving MYAPP_STORE_* — and it must be overridable independently. The flag prefix and the variable
    prefix are different vocabularies with different obligations: renaming a flag is a CLI change that can
    be deprecated, while silently renaming MYAPP_STORE_ROOT breaks every deployment at once, with no
    warning and no fallback.
  • A derived name must not depend on the command path. If a global --root became MYAPP_CLUSTER_ROOT
    when invoked under a cluster namespace and MYAPP_ROOT at the root, one setting would have several
    names depending on how it was invoked, and a container that sets it once would have it honoured only
    sometimes. Namespace segments should therefore not contribute to derived names; prefixes come from the
    root and optionally from [AsParameters], both lexical and both fixed at compile time, with explicit
    [Env("…")] for anything that does not fit.
  • Collisions are a build error. Two options binding one variable, or a derived name landing on an
    explicit one — much likelier once globals and per-command options coexist, and a generator can simply
    say so.

What it should do to the output surfaces

  • Help: the variable beside the option it feeds, e.g. --root <dir> ... [env: MYAPP_ROOT]. Today
    documented variables are a separate list a reader has to join up by eye.
  • __schema: CliEnvVarSchema already has the shape; what it lacks is the link. Either an
    Option field on it or an Env field on the option's own node, so an agent reading the schema can tell
    that two names are one setting rather than two.
  • And the root list should become a projection. DocumentEnvironmentVariables lives only on
    IArghRootBuilder, so today the environment is modelled as one flat global list, while bindings would be
    declared per option at three levels. If the list stays hand-maintained beside the bindings, this issue's
    own complaint — two statements of one fact, only one of which executes — reappears inside argh. Deriving
    the documentation from the bindings is most of the value.

Secondary, and separable

  • Redaction. A [Secret]-style marker so a token arriving from the environment is never echoed in
    help, errors, __schema or a dry-run rendering. Adjacent to RequiresAuthAttribute already existing.
  • Seeing what won. Something like __env or --show-config listing each resolved option, its value
    and where it came from (flag, environment, default). For a service with fifty settings this is the most
    useful operational output a CLI can have, and it is nearly free once resolution happens in one place.
  • Testability. An injectable reader (Func<string, string?> on the builder, defaulting to
    Environment.GetEnvironmentVariable) so a test can drive precedence without mutating process state,
    which is global and hostile to parallel test runs.
  • Config files later. CliConfigFileSchema is already in the schema beside the environment. Modelling
    resolution as an ordered chain of sources rather than as "the environment, specially" lets a file source
    join the same chain later without a second design.

Shape of the CLI this comes from

A single-binary service with roughly fifty options, every one with an environment equivalent because it is
deployed in containers where flags are awkward and the environment is the native idiom. Today that is ~40
GetEnvironmentVariable calls at the top of Program.cs, a switch over the flags below them, and a
hand-maintained help string listing both — three places to change to add one setting, with no compiler
anywhere in the loop. The parser and the help text are what argh already removes; the environment half is
the reason the hand-written file cannot be deleted.

Happy to try a patch if the direction is agreeable. The questions I would want settled first are the
derived-name convention (--requests-per-second → REQUESTS_PER_SECOND vs REQUESTSPERSECOND), whether
empty means unset, and whether bool? is acceptable as the opt-in to negated flags or whether those
should be generated for every bool.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions