Re: [Monotone-devel] RFC: Restricting app_state to commands

[Markus Schiltknecht]

Hi,

Zack Weinberg wrote:
> So okay, we have to do it to the database, why generalize to
> everything else?  For clarity, "everything else" is the lua hooks, the
> options, the workspace, the ssh-agent connection, the project, and the
> keystore.  For the workspace, keystore, and agent, a similar argument
> to the database applies: we would like to be explicit about which
> operations use them and which do not, and it may also make sense to be
> explicit about whether the operation may write to the object (note in
> particular that we have *no* keystore locking right now).  For the
> rest of it, I have two rationales: (1) it will enable further
> reductions in which headers are exposed in which source files
> (currently any number of places need to read app_state.hh just so they
> can get at the one or two subcomponents of the structure that they
> need - and it's common for this to be just the lua hooks, or just one
> or two options), (2) it will be easier to implement a general policy
> of "never pass an app_state reference" than to pull individual
> sub-objects out of the app_state one at a time.
>
> Thoughts?

Hm.. I like the idea of ripping out some stuff from the app_state, but 
getting rid of app_state completely... I don't know. Of course the 
single functions often only need one or two things from app_state, in a 
complete calling path, thing sum up quickly.

Have a look at keys.cc:make_signature() for example, the function itself 
already needs all of the following:

        keys.cc:make_signature()
           needs: app.opts
                  app.keys
                  app.agent
                  app.signers
                  app.verifiers
                  app.lua

And unfortunately it's not one of the top level functions, no, it's 
quite a low-level one. One calling path to it is, for example:

project_t::put_revision_in_branch()
  -> cert_revision_in_branch()
    -> put_simple_revision_cert()
      -> make_simple_cert()
        -> calculate_cert()
          -> make_signature()

So it's called from project_t and the functions in between certainly 
need the database. The only thing we do not need in that path is the 
workspace.


Anyway, I would also really like to get a better separation. I have 
started with (partly) ripping out the database from app_state and 
passing only that where possible.

Probably we can combine app.signers, app.verifiers and the key_store?

Maybe combine certain options and lua hooks with the appropriate objects 
(workspace, database, key_store and project), where they are used?


> file              .ii      .s
...
> total          -73655 -167042

How do you count that stuff? Is there a script? Or a makefile target or 
something?

Regards

Markus