[Monotone-devel] Re: nvm.experiment.encapsulation review

[Zack Weinberg]

On Jan 9, 2008 4:38 PM, Markus Schiltknecht <address@hidden> wrote:
> > I do have two big concerns that I think need to get resolved first, though:-
> >
> > Obviously, we need to do something about the hackish exposure of small
> > pieces of app_state through the database and keystore (and workspace?)
> > objects.
[...]
> I did the hackish variant, exactly because I wasn't sure how to deal
> with that.  At the moment, I'd say we should pass down as arguments
> wherever feasible, otherwise use globals or app_state.

There's an argument for continuing to do the hack for things that
can't be feasibly passed in as arguments - especially Lua hooks -
because it constrains the set of such items visible at any particular
part of the code.

There's also an argument for turning the Lua interpreter into a
global, like ui and sanity.  I can, however, see a brave future where
we want to have multiple active sets of hooks within the context of,
say, a netsync daemon serving multiple databases.

Probably it is best to get to a stopping place on the app_state
passdown removals first, and then revisit each hack individually.

> > but is too ambitious
> > for a branch that needs to keep up with mainline the way this one
> > does.  The merge yesterday was No Fun.
>
> Oh dear, yeah. Thank you for bringing this up to speed again. I was
> really surprised to see it compile and pass all tests right away ;-)

Yeah, I was expecting lots more breakage than the one test failure I
actually got.

> > You made some changes in CMD()
> > functions that only make sense if you're trying for a total removal,
[...]
> Well, I remember trying to separate into read-only and writable database
> objects, which is troublesome because the app_state gets initialized
> *before* we know what kind of database connection we need.
>
> However, I'm fine with keeping the app_state until CMD()s for now.

I think a reasonable merge point for this branch might be when the
database, keystore, and workspace objects can be removed from
app_state; your CMD_REQUIRES_*() macros would expand to things like

  database db(app);

(perhaps at that point we get rid of the macros, if only so that the
name of the variable declared is explicit in the code)

I do still want to do the ro/rw_database stuff - contra the other
thread on that, I think *even if* the database itself doesn't
necessarily care, it would be useful for enforcing high-level
constraints, so that we can say for instance "this subset of the
commands is intended to work on a database which the user does not
have write privileges for."  But I think it should wait till after
this branch is merged.

> Ah, yes, that part puzzled me as well.  But policy branches didn't die,
> yet, did they?

They're not dead, but they might be sleeping until the stars are
right.  IIRC we got stuck on very basic design arguments at the last
summit and there's been no forward progress since.

> Thank you very much for taking this on! I'll help you wherever I can, or
> as soon as I get some free cycles. However, I'm currently focusing on
> finishing cvs_import whenever hacking monotone code. And with the
> db-compaction branch, there's yet another unfinished project of mine. :-(

db-compaction is another one of my pet wishes, I'll see if I can find
time to look at it.

I wonder is CVS import actually all that important anymore?  Should we
rather be concentrating on SVN, git, and mercurial compatibility?

zw