Re: [tor-bugs] #29211 [Core Tor/Tor]: Distribute config.c functionality across more modules

[Tor Bug Tracker & Wiki]

#29211: Distribute config.c functionality across more modules
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  task | Status:  assigned
 Priority:  Medium   |  Milestone:  Tor:
 |  0.4.3.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  network-team-roadmap-2020Q1  |  Actual Points:
Parent ID:   | Points:  23
 Reviewer:   |Sponsor:  Sponsor31-can
-+-
Changes (by gaba):

 * keywords:  network-team-roadmap-august => network-team-roadmap-2020Q1


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #29211 [Core Tor/Tor]: Distribute config.c functionality across more modules

[Tor Bug Tracker & Wiki]

#29211: Distribute config.c functionality across more modules
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  task | Status:  assigned
 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:   | Points:  23
 Reviewer:   |Sponsor:  Sponsor31-can
-+-

Comment (by teor):

 Replying to [comment:9 gk]:
 > Replying to [comment:8 teor]:
 >
 > [snip]
 >
 > > But I would like to hear your ideas at the sponsor 31 meeting on
 Thursday.
 > {{{
 > 10:24 <+GeKo> teor4: so, i tried to find out somethign about the s31
 meeting you
 >   mentioned in comment:8 in #29211
 > 10:24 -zwiebelbot:#tor-project- tor#29211: Distribute config.c
 functionality across
 >   more modules - [assigned] - https://bugs.torproject.org/29211
 > 10:24 <+GeKo> are there notes somewhere?
 > 10:24 <+GeKo> i did not see any log from meetbot
 > 10:24 <+GeKo> or was that a sekrit meeting?
 > 10:25 <+GeKo> i'd be interested in the results as it seems this
 refactoring is
 >   breaking a bunch of things
 > 10:25 <+GeKo> which hurts at least tor browser nightly users
 > }}}
 > [snip]

 We rescheduled the meeting to 2300 UTC Tuesday 3 September.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #29211 [Core Tor/Tor]: Distribute config.c functionality across more modules

[Tor Bug Tracker & Wiki]

#29211: Distribute config.c functionality across more modules
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  task | Status:  assigned
 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:   | Points:  23
 Reviewer:   |Sponsor:  Sponsor31-can
-+-

Comment (by gk):

 Replying to [comment:8 teor]:

 [snip]

 > But I would like to hear your ideas at the sponsor 31 meeting on
 Thursday.
 {{{
 10:24 <+GeKo> teor4: so, i tried to find out somethign about the s31
 meeting you
   mentioned in comment:8 in #29211
 10:24 -zwiebelbot:#tor-project- tor#29211: Distribute config.c
 functionality across
   more modules - [assigned] - https://bugs.torproject.org/29211
 10:24 <+GeKo> are there notes somewhere?
 10:24 <+GeKo> i did not see any log from meetbot
 10:24 <+GeKo> or was that a sekrit meeting?
 10:25 <+GeKo> i'd be interested in the results as it seems this
 refactoring is
   breaking a bunch of things
 10:25 <+GeKo> which hurts at least tor browser nightly users
 }}}
 [snip]

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #29211 [Core Tor/Tor]: Distribute config.c functionality across more modules

[Tor Bug Tracker & Wiki]

#29211: Distribute config.c functionality across more modules
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  task | Status:  assigned
 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:   | Points:  23
 Reviewer:   |Sponsor:  Sponsor31-can
-+-

Comment (by teor):

 Hi Nick,

 Here are my initial thoughts on the branches I reviewed.
 I want to think about them a bit more, and review and revise them before
 the sponsor 31 meeting on Thursday.
 I'd like a response at that meeting, rather than on this ticket.
 But I wanted to give you time to think through them before the meeting.

 The goal of sponsor 31 is to improve code quality.
 But I worry that this pull request series has not improved our code
 quality (yet!), team processes, or coding habits.

 I wanted more:
 * collaboration on the design,
 * summary and detailed design documentation,
 * opportunities to write code for large refactors myself,
 * function documentation and callback documentation,
 * code comments around tricky code (or less tricky code),
 * automated code transform scripts (sed or coccinelle),
 * unit tests,
 * tests for typical options used by Tor Browser, and possibly other
 significant uses of tor, (unit tests, chutney, or stem),
 * working stem tests in CI (rather than stem CI having allow failures due
 to #29437).

 I wanted less:
 * CI failures,
 * regressions (#31495, #31527),
 * technical debt,
 * missing changes files,
 * pull request merge conflicts / target branch mistakes (which make
 reviews difficult),
 * pull requests in the review backlog,
 * dependencies between commits and pull requests (which make fixes
 difficult),
 * exceptions to the normal review rules and review processes,
 * pressure to complete reviews by a deadline.

 Overall it seems that this process has been difficult for all of us.
 I would like us to pause for a while, and work through the tensions that
 this refactor has brought up.

 I have some ideas about the root causes here.

 But I would like to hear your ideas at the sponsor 31 meeting on Thursday.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #29211 [Core Tor/Tor]: Distribute config.c functionality across more modules

[Tor Bug Tracker & Wiki]

#29211: Distribute config.c functionality across more modules
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  task | Status:  assigned
 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:   | Points:  23
 Reviewer:   |Sponsor:  Sponsor31-can
-+-

Comment (by teor):

 I am collecting a list of tasks for future branches as child tickets of
 this ticket. They all start with the prefix "config refactor:".

 These tasks should not block reviews for any code that is already in
 commits or branches.

 But they should block reviews for any new commits.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #29211 [Core Tor/Tor]: Distribute config.c functionality across more modules

[Tor Bug Tracker & Wiki]

#29211: Distribute config.c functionality across more modules
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  task | Status:  assigned
 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:   | Points:  23
 Reviewer:   |Sponsor:  Sponsor31-can
-+-
Changes (by gaba):

 * cc: gaba (added)


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #29211 [Core Tor/Tor]: Distribute config.c functionality across more modules

[Tor Bug Tracker & Wiki]

#29211: Distribute config.c functionality across more modules
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  task | Status:  assigned
 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:   | Points:  23
 Reviewer:   |Sponsor:  Sponsor31-can
-+-
Changes (by gaba):

 * keywords:   => network-team-roadmap-august
 * cc: nickm, teor (added)
 * points:  15 => 23


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #29211 [Core Tor/Tor]: Distribute config.c functionality across more modules

[Tor Bug Tracker & Wiki]

#29211: Distribute config.c functionality across more modules
--+
 Reporter:  nickm |  Owner:  nickm
 Type:  task  | Status:  assigned
 Priority:  Medium|  Milestone:  Tor: 0.4.2.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:  15
 Reviewer:|Sponsor:  Sponsor31-can
--+

Comment (by nickm):

 Here is my current plan for the refactoring.

 '''High-level goals'''

 The end goal is that every module should define, via a hook in the
 subsystem API, which variables it uses.  The config_var_t structure is a
 natural way to note this, since it maps a variable to a struct field, but
 it has some problems:
   * Its implementation is a huge pile of switch statements and special
 cases
   * It combines multiple responsibilities in one blob of code
   * It is difficult to extend
   * It describes a field in a _single_ structure, and is difficult to
 aggregate into our target implementation, wherein every module has its own
 configuration structure.
   * Its current implementation depends on every type in the code that can
 be used as a configuration variable.  Since routerset_t is one such (high
 level) type, and since we may expect other high-level types to exist in
 the future, we need a better way to make this system extensible.

 '''Code dependencies'''

 src/lib/conf will be code that is used by modules that need to define
 configurable items.

 src/lib/confmgt/ will be generic code for manipulating configuration
 variables in objects.  It will handle reading and writing from strings to
 struct members, and finding the appropriate struct members to read and
 write, and so on.  It will only be used by higher level modules that need
 to do data-driven access to configuration.  Configurable modules will just
 need lib/conf and lib/susbsys.

 src/app/config/ will do the work of collecting all the configurable
 objects, reloading the configuration when appropriate, and notifying
 different subsystems when the configuration has changed.

 '''Branches so far'''

 In #30893, I tried to get to 100% test coverage on confparse.c, since
 that's the module that will be most affected by this refactoring.  I
 missed a couple of spots, but those will be fixed as part of the #30864
 branch to avoid conflicts.  This branch is now merged.

 The next branch, #30864, extracts the lowest level of introspection code
 from confparse.c: the code to manipulate typed values via a pointer and a
 type definition.  This functionality is in the typed_var module, and it's
 not meant for direct use: it's a very low level api.

 The next branch, #30914, wraps the typed_var API inside a struct_var API,
 which instead of referring to a pointer to typed data, refers to a typed
 member within a struct.  It extracts this API from confparse.c, and cleans
 it up a lot.  Once again, it makes confparse.c simpler.

 The next branch, #30935, refactors config_var_t a bit, and moves it to
 lib/conf where it belongs.  It is full of miscellaneous refactorings on
 the confparse.c code and its users.  Notably, it removes all code in
 confparse.c or config.c that does "special-case" inspection of a
 variable's type, and it removes the need to update tiny little macro
 definitions all over the code.  It also removes special-case handling of
 "magic" variable names like those that start with __ and ___.  Finally --
 and necessarily for my sanity -- it refactors our horrible old handling of
 TestingTorNetwork.

 '''Next steps'''

 The next major branch here will refactor config_fmt_t so that instead of
 describing a mapping from variable names to a single struct, it describes
 an extensible mapping from variable names to many structs.  There will
 need to be a corresponding "group of structs" object that confparse.c and
 config.c use in place of their current or_options_t manipulation.  That
 will be done in #30866.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #29211 [Core Tor/Tor]: Distribute config.c functionality across more modules

[Tor Bug Tracker & Wiki]

#29211: Distribute config.c functionality across more modules
--+
 Reporter:  nickm |  Owner:  nickm
 Type:  task  | Status:  assigned
 Priority:  Medium|  Milestone:  Tor: 0.4.2.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:  15
 Reviewer:|Sponsor:  Sponsor31-can
--+
Changes (by gaba):

 * status:  new => assigned
 * owner:  (none) => nickm


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs