[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Separated meta directories

From: Kumaran Santhanam <kumaran_at_tigris.org>
Date: 2003-08-30 19:08:49 CEST

Hi Karl -

> > Attached is the promised patch that attempts to provide the
> > separated meta directory functionality. Please be advised
> > that this is a first cut, so it should not be used on
> > production data.
>
> As Ben said, a log message would help greatly; see HACKING for
> more. Your high-level overview was very helpful, however,
> thanks.

Sorry - I didn't put that together yet because the feature isn't
quite complete. I just thought I'd send a preliminary out on the
list for those folks who expressed interest in trying it. Also,
it's good to get some feedback before I get too deep into it.

> What happens when two people share a working copy?

This was part of some points brought up a few days ago.
Basically, the conclusion was that separated and embedded meta
data encompass two sets of functionality that overlap quite a
bit, but still have some unique qualities.

Particularly, shared working copies are really in the domain of
embedded meta data. On the other hand, pure working copies are
in the domain of separated meta data. Most of the functionality
can be achieved with one or the other, though. It's like a Venn
diagram with two circles and a large overlapping region. In any
case, this is why I advocated keeping both as an option for the
user, though we can debate later which one should be default.

I'm not entirely clear on why someone would share a working copy.
Many SCM systems don't even allow this. Maybe you can provide an
example where sharing a working copy is the only recourse?

> On the one hand, branches are supposed to be "cheap". On the
> other hand, there's inevitably some overhead to having one: the
> commit mails go out to everyone (or alternatively, we spend
> valuable time making them go somewhere else); discussion about
> branch commits occupies mental space on the dev list, etc.
> Therefore, we do exercise some judgement about making branches.
> For one thing, it'd be nice to see a patch that conforms to the
> HACKING guidelines, and passes at least some meaningful portion
> of the regression tests (you don't say whether this does yet).

Fair enough. I'll go ahead and fix these issues, and get the
regression tests in order. As I mentioned before, a couple of
pieces are not yet implemented, so I'm sure it won't pass the
regression tests yet. However, I'll test all of that before
writing up a formal log file for submission.

> going to pursue consensus on it myself, that's up to you
> <grin>. I know coding tasks are not interchangeable: just
> because you're motivated to work on separate metadata areas
> doesn't necessarily mean you're motivated to fix bugs... but I
> can still wish, right? :-)

This feature is important to me, so I needed to get it done
first. However, once it's done, I'm happy to lend a hand to fix
a few bugs. One thing at a time, though. ;-)

> +/** Cached meta data index. */
> +static apr_hash_t *cachedIndex = NULL;
> +static const char *cachedConfigDir = NULL;
>
> If it's truly safe for these to be global (which is rare!), the
> comment should explain why. Subversion needs to stay thread-safe.
> Based on later code, it doesn't look to me like these globals are
> thread-safe.
>
> When there's a hash, you need to document the types and meanings of
> the keys and the values.
>
> In other words, when another programmer reads the variable
> declarations, they should come away knowing what the variables mean
> and how they are used.

The purpose of this variable is to create a cache so that the
index file doesn't need to be re-read every time the function is
called. In this architecture, how do I go about creating a
thread-safe persistent variable?

> > + if (ctx->meta == svn_meta_separate)
> > + SVN_ERR (svn_wc_create_adm_root (path, pool));
> > SVN_ERR (svn_wc_ensure_adm (path, URL, revnum, pool));
> > -
> > +
>
> Urk.
>
> The fact that the meta cache is even *known* outside libsvn_wc is a
> bad sign... The libsvn_client code should not be aware of how
> libsvn_wc manages itself (we may violate this in a few places
> currently, but if so, it's a trend we want to shrink, not grow!).

I'll push this decision down to libsvn_wc instead, so that the
call will be: svn_wc_create_adm_root (ctx->meta, path, pool)
instead. Or should it be: svn_wc_create_adm_root (ctx, path, pool)?

I will sync with your other comments that I haven't repeated
here. For the most part, they are quick syntactic fixes.

Cheers,
Kumaran

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Aug 30 19:09:47 2003

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.