Item-repository allocation/locking refactor

[Ivan Shapovalov]

On 10 January 2013 02:28:41 Milian Wolff wrote:
> On Wednesday 09 January 2013 22:13:53 Milian Wolff wrote:
> > On Wednesday 19 December 2012 17:43:29 Milian Wolff wrote:
> > > On Wednesday 19 December 2012 17:33:56 Ivan Shapovalov wrote:
> > > > On 06 December 2012 23:44:10 Milian Wolff wrote:
> > > > > On Wednesday 05 December 2012 10:11:13 Ivan Shapovalov wrote:
> > > > > > Hi!
> > > > > > 
> > > > > > Following Milian's suggestion on
> > > > > > https://git.reviewboard.kde.org/r/106945/,
> > > > > > I'm putting the work on refactoring the "disk storage allocation"
> > > > > > part
> > > > > > of
> > > > > > the item-repositories into a public repo instead of a
> > > > > > review-request.
> > > > > > 
> > > > > > The refactoring is just about
> > > > > > - getting rid of locking in the item-repository directories
> > > > > > (because
> > > > > > we
> > > > > > have rather good per-session locks);
> > > > > > - getting rid of numbered subdirectories inside of per-session
> > > > > > cache
> > > > > > directories (because nobody shall use and nobody actually uses
> > > > > > multiple
> > > > > > ItemRepositoryRegistries);
> > > > > > - changing the cache path once again
> > > > > > 
> > > > > >   from "~/.cache/kdevduchain/$session"
> > > > > >   to "~/.cache/kdevduchain/$appname-$session";
> > > > > > 
> > > > > > - making ItemRepositoryRegistry a true singleton;
> > > > > > - finally making the cache directories deleted when a session is
> > > > > > deleted
> > > > > > from disk.
> > > > > > 
> > > > > > The real reason behind all this was the last point.
> > > > > > 
> > > > > > The branch is called "itemrepository" and is on
> > > > > > "git://github.com/intelfx/kdevplatform.git".
> > > > > 
> > > > > Cool!
> > > > > 
> > > > > Some points I spotted so far - please don't be overwhelmed by
> > > > > that...
> > > > > But
> > > > > I
> > > > > rather want it to merge a nicely designed refactoring which should
> > > > > make
> > > > > this stuff future proof.
> > > > 
> > > > Please, review the next iteration. It's on the same branch, rebased
> > > > against
> > > > current master (non-fast-forward, of course).
> > > 
> > > <snip>
> > > 
> > > Wow, you seem to be able to read minds. I was about to ping you about
> > > the
> > > status here. Awesome work, I'll try to review it as soon as possible.
> > 
> > Reviewing it now and fixing it up and making it ready for merging. You btw
> > introduced a cyclic dependency (language required shell and vice versa).
> > I'm breaking that up now and cleaning up the session controller/lock code
> > while at it.
> 
> OK, I just landed my big refactoring patch. Ivan, please take a look. I
> pushed your branch + my fixes to the origin/itemrepository branch.
> 
> The tl;dr; for those that don't want to read the long commit message: I
> introduced a proper RAII SessionLock which is now passed around to ensure
> that destructive operations on the session folders (either in ~/.kde or in
> ~/.cache/kdevduchain) are only ever done when the proper session lock is
> held.
> 
> I'd welcome it if someone else could take a look at that stuff and then I'd
> like to merge this beast into master. The big advantage is that now the
> duchain caches for our various unit tests don't pile up and clog up your
> disk. Finally!
> 
> Cheers and many thanks Ivan for looking into this!

Ah. Didn't see this letter before replying in the review.
Disregard that reply then. This is the second time I introduced a cyclic dep, 
eh?..
/me needs to make clang to notify me about these cases. And your solution 
seems pretty beautiful, btw. The only thing is I still don't understand why 
there shall be an assert on KLockFile acquisition.

And - thanks for fixing and reviewing!
- Ivan