Re: [PATCHv4 2/4] Add infrastructure for ref namespaces

From: Josh Triplett <josh@joshtriplett.org>
Date: Jun 2, 2011, 23:36 UTC
Message-ID: <20110602233619.GA1931@leaf>
In-Reply-To: <7vfwnrdfam.fsf@alter.siamese.dyndns.org>

On Thu, Jun 02, 2011 at 03:44:33PM -0700, Junio C Hamano wrote:

Show 14 quoted lines

> Jamey Sharp <jamey@minilop.net> writes:
> 
> > Note that namespaces which include a / will expand to a hierarchy of
> > namespaces; for example, GIT_NAMESPACE=foo/bar will store refs under
> > refs/namespaces/foo/refs/namespaces/bar/.  This makes GIT_NAMESPACE
> > behave hierarchically, and avoids ambiguity with namespaces such as
> > foo/refs/heads.
> 
> Sorry, but I fail to see what problem you are trying to solve here.  I am
> not suggesting that it would be better to do things in a way different
> from what your patch does, but what problem will you have if you stored
> the branch head for baz in refs/namespaces/foo/bar/refs/heads/baz given
> the namespace foo/bar, and how does it solve that problem to store it
> instead at refs/namespaces/foo/refs/namespaces/bar/refs/heads/baz?

Two reasons. First, if you use GIT_NAMESPACE=foo (which puts its refs under refs/namespaces/foo/refs/{heads,tags}), and also used GIT_NAMESPACE=foo/refs/heads, that would put its refs under refs/namespaces/foo/refs/heads/refs/{heads,tags}, which would make them potentially conflict with foo's references. So, for instance, you could end up with directory/file conflicts in the refs directory. Using hierarchies avoids any possible conflicts and corner cases there.

Second, by making the namespaces hierarchical, we provide a kind of composability, similar to that suggested by the analogy to chroots. With the way we've constructed them, cloning a repo with GIT_NAMESPACE=foo/bar has the same effect as cloning a repo with GIT_NAMESPACE=foo and cloning from that repo with GIT_NAMESPACE=bar.

> > +int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
> 
> Just a naming and interface preference, but I would have called this
> for-each-ref-in-namespace, perhaps giving the namespace as a parameter.

for_each_ref_in and other variants already exist for that purpose; for_each_namespaced_ref exists to automatically uses GIT_NAMESPACE. Happy to rename it if you have another preference, but I don't think it makes sense to support passing in arbitrary namespaces when the callers only use it to access the currently requested namespace. If some situation arises in later code that needs to handle arbitrary namespaces, it seems easy enough to provide a more generalized function at that point, but doing so now would just make the existing callers more complex by forcing them to do the call to get_git_namespace() rather than allowing for_each_namespaced_ref to do it.

As far as naming, though, we have no preference whatsoever about the color of the bikeshed. :)

- Josh Triplett