ts::ArgParser lets a command or option declare an environment variable:
command1.add_option("--switch", "-s", "switch description", "ENV_VAR", 1, "default", "lookup_key");
The parser reads the variable and stores it, but nothing can ever act on it, so every
declaration in the tree is decorative. Three separate gaps combine to make the feature
unusable.
The env var cannot fill an absent option
getenv() is only called from inside the branch that matches the option on the command
line. If the flag is not typed, the option stays absent and env() is empty, so the
variable cannot supply a parameter the user did not already type:
option absent: present=no value='' env=''
The env var cannot fill a missing value
For a fixed-arity option, omitting the value is a usage error raised before the
environment is consulted:
$ TS_PROBE_ENV=/tmp/from_env.yaml probe config --tags
Error: 1 argument(s) expected by tags
exit=64
So for a fixed-arity option there is no reachable state in which the variable can matter:
with a value it is redundant, without one the parser exits 64. The only shape where it
could contribute is an option whose value is optional, and AT_MOST_ONE_ARG_N was only
added recently (#13570).
Nothing reads what is captured
When the option is matched with a value, the plumbing does work — ArgumentData::env()
returns the variable in every shape (global option, subcommand option, fixed arity,
AT_MOST_ONE_ARG_N). But .env() has no caller in the tree outside
src/tscore/unit_tests/test_ArgParser.cc.
Current declarations:
| Declaration |
Variable |
Effect today |
traffic_ctl config get/set --cold |
TS_RECORD_YAML |
none |
traffic_ctl --tags |
TS_DEBUG_TAGS |
none |
traffic_ctl --run-root, traffic_layout --run-root |
TS_RUNROOT |
works, but only because src/tscore/runroot.cc calls getenv itself |
TS_RECORD_YAML is the clearest symptom. traffic_ctl config get -c with no file name is
supposed to be the case the variable serves, and it is ignored in favour of the compiled-in
config directory:
$ TS_RECORD_YAML=/tmp/coldtest/records.yaml traffic_ctl config get proxy.config.diags.debug.enabled -c
config get error: bad file: /usr/local/etc/trafficserver/records.yaml # exit 2
FileConfigCommand reads only .value() and hands the empty string to fix_filename(),
which falls back to sysconfdir/records.yaml.
Neither TS_RECORD_YAML nor TS_DEBUG_TAGS is documented in doc/, so no user-facing
promise is being broken. doc/developer-guide/internal-libraries/ArgParser.en.rst does
document the parameter without noting that the caller must retrieve it by hand.
Proposed behaviour
Resolve the variable into the option's values during parsing, so callers need no special
handling and every declaration means something:
- Precedence is typed value, then environment variable, then declared default.
- An option that declares a variable accepts a missing value: rather than exiting 64, it
takes the value from the environment when set, and reports the usage error only when the
environment is empty too.
ArgumentData::env() keeps returning the raw variable for compatibility.
That makes traffic_ctl config get -c honour TS_RECORD_YAML and traffic_ctl --tags
honour TS_DEBUG_TAGS, with no change to either call site.
Open question
Should a variable also activate an option that was never typed, as many CLIs do? It is
convenient but surprising here: exporting TS_RECORD_YAML would silently turn every
traffic_ctl config get into a file read rather than an RPC call. The proposal above
deliberately requires the flag, and this issue is the place to settle it.
Acceptance criteria
- With the flag typed and no value, the environment variable supplies it, across fixed
arity, AT_MOST_ONE_ARG_N, and the multi-value arities.
- A typed value always wins over the environment variable, which always wins over the
declared default.
- Usage errors are unchanged when no variable is declared, or when it is declared and unset.
- Unit tests in
test_ArgParser.cc covering the precedence chain, and end-to-end coverage
for traffic_ctl config get -c under TS_RECORD_YAML.
ArgParser.en.rst states the resolution rules; the traffic_ctl appendix documents both
variables.
ts::ArgParserlets a command or option declare an environment variable:The parser reads the variable and stores it, but nothing can ever act on it, so every
declaration in the tree is decorative. Three separate gaps combine to make the feature
unusable.
The env var cannot fill an absent option
getenv()is only called from inside the branch that matches the option on the commandline. If the flag is not typed, the option stays absent and
env()is empty, so thevariable cannot supply a parameter the user did not already type:
The env var cannot fill a missing value
For a fixed-arity option, omitting the value is a usage error raised before the
environment is consulted:
So for a fixed-arity option there is no reachable state in which the variable can matter:
with a value it is redundant, without one the parser exits 64. The only shape where it
could contribute is an option whose value is optional, and
AT_MOST_ONE_ARG_Nwas onlyadded recently (#13570).
Nothing reads what is captured
When the option is matched with a value, the plumbing does work —
ArgumentData::env()returns the variable in every shape (global option, subcommand option, fixed arity,
AT_MOST_ONE_ARG_N). But.env()has no caller in the tree outsidesrc/tscore/unit_tests/test_ArgParser.cc.Current declarations:
traffic_ctl config get/set --coldTS_RECORD_YAMLtraffic_ctl --tagsTS_DEBUG_TAGStraffic_ctl --run-root,traffic_layout --run-rootTS_RUNROOTsrc/tscore/runroot.cccallsgetenvitselfTS_RECORD_YAMLis the clearest symptom.traffic_ctl config get -cwith no file name issupposed to be the case the variable serves, and it is ignored in favour of the compiled-in
config directory:
FileConfigCommandreads only.value()and hands the empty string tofix_filename(),which falls back to
sysconfdir/records.yaml.Neither
TS_RECORD_YAMLnorTS_DEBUG_TAGSis documented indoc/, so no user-facingpromise is being broken.
doc/developer-guide/internal-libraries/ArgParser.en.rstdoesdocument the parameter without noting that the caller must retrieve it by hand.
Proposed behaviour
Resolve the variable into the option's values during parsing, so callers need no special
handling and every declaration means something:
takes the value from the environment when set, and reports the usage error only when the
environment is empty too.
ArgumentData::env()keeps returning the raw variable for compatibility.That makes
traffic_ctl config get -chonourTS_RECORD_YAMLandtraffic_ctl --tagshonour
TS_DEBUG_TAGS, with no change to either call site.Open question
Should a variable also activate an option that was never typed, as many CLIs do? It is
convenient but surprising here: exporting
TS_RECORD_YAMLwould silently turn everytraffic_ctl config getinto a file read rather than an RPC call. The proposal abovedeliberately requires the flag, and this issue is the place to settle it.
Acceptance criteria
arity,
AT_MOST_ONE_ARG_N, and the multi-value arities.declared default.
test_ArgParser.cccovering the precedence chain, and end-to-end coveragefor
traffic_ctl config get -cunderTS_RECORD_YAML.ArgParser.en.rststates the resolution rules; thetraffic_ctlappendix documents bothvariables.