On Mon, Oct 23, 2017 at 7:12 AM, Heiko Voigt <hvoigt@xxxxxxxxxx> wrote: > On Thu, Oct 19, 2017 at 11:11:09AM -0700, Stefan Beller wrote: >> Currently when fetching we collect the names of submodules to be fetched >> in a list. As we also want to support fetching 'gitlinks, that happen to >> have a repo checked out at the right place', we'll just pretend that these >> are submodules. We do that by assuming their path is their name. This in >> turn can yield collisions between the name-namespace and the >> path-namespace. (See the previous test for a demonstration.) >> >> This patch rewrites the code such that we treat the 'real submodule' case >> differently from the 'gitlink, but ok' case. This introduces a bit >> of code duplication, but gets rid of the confusing mapping between names >> and paths. >> >> The test is incomplete as the long term vision is not achieved yet. >> (which would be fetching both the renamed submodule as well as >> the gitlink thing, putting them in place via e.g. git-pull) >> >> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx> >> --- >> >> Heiko, >> Junio, >> >> I assumed the code would ease up a lot more, but now I am undecided if >> I want to keep arguing as the code is not stopping to be ugly. :) > > So we are basically coming to the same conclusion? :) My previous > fallback approach basically did the same but with the old architecture > (without parallel fetch, ...) and was already ugly. It depends on the conclusion you drew. ;) Here is my conclusion: * It would really be nice to have this fallback separated out. * However for the current state the ugliness of such code trumps the more maintainable, long term oriented thing with path/names not clashing. I could not spend more time polishing these patches, so I could not ask you to do it either -> I think your patches are fine as is for inclusion -> We may have #leftoverbits here to clear up the confusion around path/names, as well as making the code more pleasant to read. > With the fallback on submodule default names approach we can keep most > of the old functionality and keep the code that handles that minimal. > > Since there is only a small (IMO quite unlikely) cornercase that could > break peoples expectations I would like to have a look whether anyone > even notices the behavioral change on next or master. If there are > complaints we can still extend and add the two lists. That sounds good to me. > >> The idea is to treat submodule and gitlinks separately, with submodules >> supporting renames, and gitlinks as a historic artefact. >> >> Sorry for the noise about code ugliness. > > Why sorry? For me it is actually interesting to see you basically coming > to the same conclusions. I thought I might come off awkwardly criticizing code for ugliness without having a better alternative to show. Thanks for working on this, Stefan > > Cheers Heiko
