On Mon, Dec 30, 2013 at 11:10:31AM -0800, Junio C Hamano wrote: > > So I think the sanest thing is probably: > > > > 1. Teach "fetch" to expand recursively in a single process, and then > > tell sub-processes (via a new command-line option) not to expand > > any further. > > > > 2. Teach "fetch" to detect cycles (probably just by a simple depth > > counter). > > I suspect that the expansion code will just accumulate remotes found > into a string-list (as part of 4. below), so deduping would be > fairly easily done without a depth counter.
I don't think that will work (at least not naively). The end-product of step 1, and the string_list that is de-duped in step 4, is a list of the concrete remotes. The cycles occur between groups, which are not mentioned in the final list. You can keep a separate list of the groups we visit, of course, but we do not otherwise need it. One thing that does make such a list easier is that we do not need to care about order. E.g., in config like this: [remotes] a = c b = c c = d e you can mark "c" as seen after visiting it via "a". It is not technically a cycle, but since we would want to suppress duplicates anyway, we can be overly broad. > > 3. Teach the group-reading code to detect groups more robustly, so > > that a single-item group like "remotes.foo=bar" correctly recurses > > to "bar". > > A single-item remote group is somewhat annoying, but expanding it > only at some places while ignoring it at other places is even more > annoying, so this sounds like a right thing to do. The only configuration that I think would be negatively affected is something like: [remote] foo = foo [remote "foo"] url = ... that silently works now, but would become broken (because we would complain about the cycle). I think that's OK; that config is clearly stupid and broken. If it were "remote.foo = foo bar", trying to expand the concrete "foo" and "bar", that might make some sense, but then it is already broken in the current code (that is the example that started the discussion). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html