RFC: workingdir

Greg Ward greg-hg at gerg.ca
Mon Jun 7 08:46:34 CDT 2010


On Sun, Jun 6, 2010 at 8:08 AM, Dirkjan Ochtman <dirkjan at ochtman.nl> wrote:
> My idea is to have a higher-level class representing the working
> directory. This could be a layer in between repo and dirstate, so that
> we have repo.wdir.dirstate. Ideally, the workingdir class would not
> know about the repo, though that's really the tricky part, and maybe
> an object cycle isn't so bad in this case (if it *is* allowed to know
> about the repo, that opens up more stuff we can put in there).

That sounds like a very attractive refactoring.  localrepository is a
bit of a beast.  One might even accuse it of getting close to being a
God Class.  ;-)

> Anyway, I think this is a bunch of stuff from localrepo we could put
> in there, and their dependencies:
>
> filterpats
> _datafilters
> wopener         root
> wjoin           root
> dirstate        opener, ui, root
> getcwd          dirstate
> pathto          dirstate
> wfile           wopener
> _link           wjoin
> _filter         filterpats, ui, _datafilters, root, self
> adddatafilter   _datafilters
> wread           _link, _filter, wjoin, wopener
> wwrite          _filter, wjoin, wopener
> wwritedata      _filter
> wlock           dirstate, _wlockref, origroot, join
> forget          dirstate, wlock, ui
> remove          dirstate, wlock, ui
> undelete        manifest, changelog, dirstate, wlock, ui, wwrite
> copy            wjoin, ui, wlock, dirstate

Seems reasonable.

> As you can see, this is mostly very consistent, and could make a nice
> unit. undelete may a bit harder, but it seems that it's only used for
> qrename in versioned repositories, so maybe we can just get rid of it.
>
> I was also interested (from a conceptual level) in putting the
> following from localrepo into workingdir, but that might be harder:
>
> commit          __getitem__, root, wlock, status, dirstate, commitctx,
> pathto, ui, hook
> status          __getitem__, root, getcwd, ui, dirstate, wlock
>
> These really need to have a repository around.

Nah, I don't think those should be moved.  commit() is fundamentally
repo-related: since it writes to the repo, it really belongs in
localrepository.  No question.  status() is interesting because it's a
very obvious connection between the working dir and the repo.  But
consider:

  * status is the "what if?" version of commit, which argues that it
belongs next to commit
  * status can report on existing changesets, with no reference to the
working dir at all, which
    argues that it does not belong in workingdir

That convinces me that status() should stay in localrepository.

> There is some API in the hg module that I think is very old that I
> think would be a good fit for the workingdir class conceptually:
>
> _showstats      repo.ui
> update          merge.update, wlock, workingctx, dirstate
> clean           merge.update
> merge           merge.update
> revert          merge.update
>
> But it turns out that merge.update() and everything that it calls
> really needs a repo (it calls repo.hook, copies.copies,
> subrepo.submerge), too, so maybe there's not much point. Actually
> revert is a one-liner with two users, so it might not hurt to just get
> rid of the function entirely. There's also hg.verify() which is just
> verify.verify(repo) and also has two users, so that's also something
> we can likely clean up. merge, update, clean have 5, 6 and 14 users
> respectively.

Hmmm.  One thing at a time.  This is going to cause pain for extension
authors.  (It's not that adapting any single extension will be that
hard, but I bet many many extensions will break.)  I think the painful
changesets should have some space between them so we can adjust to one
change at a time.  I don't know if there should be so much space
between them that Mercurial 1.6 fits in between: i.e. should we factor
out workingdir in 1.6 and then cleanup hg.py in 1.7?  Worth
considering.

Greg


More information about the Mercurial-devel mailing list