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

[Josh Triplett <josh@joshtriplett.org>]

On Thu, Jun 02, 2011 at 03:44:33PM -0700, Junio C Hamano wrote:
> 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