[tor-bugs] #31625 [Core Tor/Tor]: config refactoring: fix hierarchy of configuration variable flags

[Tor Bug Tracker & Wiki]

#31625: config refactoring: fix hierarchy of configuration variable flags
-----------------------------------------+---------------------------------
 Reporter:  nickm                        |          Owner:  nickm
     Type:  defect                       |         Status:  needs_review
 Priority:  Medium                       |      Milestone:  Tor:
                                         |  0.4.2.x-final
Component:  Core Tor/Tor                 |        Version:
 Severity:  Normal                       |     Resolution:
 Keywords:  network-team-roadmap-august  |  Actual Points:
Parent ID:  #29211                       |         Points:  .5
 Reviewer:  teor                         |        Sponsor:
-----------------------------------------+---------------------------------

Comment (by teor):

 Replying to [comment:1 nickm]:
 > Here's a proposed design -- what do you think?

 Overall, it looks good.

 I'd like this documentation to turn into module comment(s) in the
 appropriate module(s).

 I have some suggestions about names.
 But naming is hard, so I am happy to go with whatever you decide.

 > == The current situation ==
 >
 > Currently we have two kinds of flags:
 >    * flags for types
 >    * flags for variables.
 >
 > If a flag is set on a type, it applies to every variable of that type.
 > If a flag is set on a variable, it applies only to that variable.

 I hadn't really noticed this, it's helpful to have it explained.

 > The type flags are:
 >
 >    unsettable -- cannot be set directly by name. (LINELIST_V, OBSOLETE)
 >
 >    contained -- addresses part of another type. (LINELIST_S, OBSOLETE)

 Can we renamed "contained" to "derived"?

 I am still confused when I read "contained", because it means a lot of
 different things, particularly when we have a containers module.

 >    cumulative -- setting a variable of this type does not override older
 >       values set to this type. (all LINELIST, LINELIST_V, LINELIST_S)


 In the man page, we say "append", I think that's a clearer name.
 But the grammar of "is_append" also feels weird to me. "is_appended"?
 So I don't have a strong opinion either way.

 > The variable flags are:
 >
 >    invisible -- does not show up on lists of variables, does not get
 written to disk, and is not visible to the controller.
 >
 >    obsolete -- produce a warning on any attempt to set or fetch the
 option. Do not list it as a valid option.

 What does setting or fetching an obsolete option do? Nothing?

 >    nodump -- do not write to disk.  These are mostly testing options.
 >
 > == The refactoring ==
 >
 > The new orthogonal low-level options are:

 In general, I like this set of names.
 But negatives are hard: most of these options start with "NO".

 There's a design conflict here:
 * we want the defaults to be 0x0000
 * we want the option names to be easy to understand

 I think this design's use of "NO" is a good compromise.
 But if you can think of a better one, I would be happy to review it.

 >   * NOSET -- cannot be set by name.
 >
 >   * NOLIST -- does not appear in lists of options.
 >
 >   * NODUMP -- do not dump this option to disk from config_dump() --
 either because it is a testing option, or because it is contained in
 another option.

 derived from another option?

 >   * NOCOPY -- do not try to copy this option in config_dup, because it
 is contained in another option, or has no storage.

 derived from another option?

 >   * NOGET -- cannot be fetched by the controller.
 >
 >   * CUMULATIVE -- remains unchanged.  We might call it NOREPLACE or
 >     NOOVERWRITE if that's clearer?

 I like NOREPLACE for consistency with the other options: it's weird to
 have a single option that is not inverted.

 > With this set of options:
 >
 >   "cumulative" remains CUMULATIVE.

 Might be renamed to "append" / NOREPLACE.

 >   "nodump" remains "NODUMP".
 >
 >   "unsettable" becomes NOSET.
 >
 >   "contained" becomes NODUMP + NOCOPY

 "contained" is renamed to "derived", and becomes NODUMP + NOCOPY

 >   "invisible" becomes NOGET + NOSET + NODUMP + NOLIST
 >
 >   "obsolete" becomes NOGET + NOSET + NODUMP + NOCOPY + NOLIST.
 >
 > == How flags are set ==
 >
 > Flags can be set either on a type or on a variable.  Variable flags are
 or'd with the flags of their type before checking them.

 How do the struct_var_is_* functions change?
 Do we make matching changes in both the typed var and struct var modules?

 I also opened #31637 to check that we have test coverage for:
 * Option, +Option, and /Option,
 * across command-line, torrc, and defaults,
 since we're changing the "cumulative" implementation.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/31625#comment:5>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online