Corrected. This originally proposed adding a --config flag. That was wrong: hardcoded startup config paths are a deliberate design decision, to avoid stale and multiple conflicting configs. The recommendation below is the opposite of what I first filed.
What is actually wrong
main.go:143-149:
var cfgFile string
if cfgFile != "" {
viper.SetConfigFile(cfgFile)
} else {
viper.SetConfigFile(tapir.DefaultPopCfgFile)
}
cfgFile is declared and never assigned, and no flag is registered for it. The if branch is unreachable.
The problem is not that the capability is missing — it is that the code advertises a capability that was deliberately not wanted. A reader sees a cfgFile branch and reasonably concludes that pointing pop at another config is supported but the flag is merely missing. That is exactly the conclusion I came to, and I filed the wrong issue off the back of it.
Suggested fix
Delete the dead branch, and leave a comment saying why the paths are fixed:
// Config paths are deliberately fixed. pop must not be pointed at an
// arbitrary config: that invites stale configs and multiple conflicting
// ones on the same host, which is worse than the inconvenience of not
// being able to try one out of tree.
viper.SetConfigFile(tapir.DefaultPopCfgFile)
The comment is the valuable part. Without it this gets "fixed" again by the next person who reads the dead branch as an oversight.
Note for testing
Fixed paths mean an integration rig cannot run pop against fixtures as a bare subprocess. That is a constraint to design around, not a reason to change this: a disposable CI runner can populate /etc/dnstapir freely, and a developer machine can run pop in a container with the fixture directory mounted there. Neither requires pop to change.
Corrected. This originally proposed adding a
--configflag. That was wrong: hardcoded startup config paths are a deliberate design decision, to avoid stale and multiple conflicting configs. The recommendation below is the opposite of what I first filed.What is actually wrong
main.go:143-149:cfgFileis declared and never assigned, and no flag is registered for it. Theifbranch is unreachable.The problem is not that the capability is missing — it is that the code advertises a capability that was deliberately not wanted. A reader sees a
cfgFilebranch and reasonably concludes that pointing pop at another config is supported but the flag is merely missing. That is exactly the conclusion I came to, and I filed the wrong issue off the back of it.Suggested fix
Delete the dead branch, and leave a comment saying why the paths are fixed:
The comment is the valuable part. Without it this gets "fixed" again by the next person who reads the dead branch as an oversight.
Note for testing
Fixed paths mean an integration rig cannot run pop against fixtures as a bare subprocess. That is a constraint to design around, not a reason to change this: a disposable CI runner can populate
/etc/dnstapirfreely, and a developer machine can run pop in a container with the fixture directory mounted there. Neither requires pop to change.