On Thu, Jun 27, 2019 at 3:47 PM Johannes Schindelin <Johannes.Schindelin@xxxxxx> wrote: > > Hi Matheus, > > On Thu, 27 Jun 2019, Matheus Tavares Bernardino wrote: > > > On Wed, Jun 26, 2019 at 3:04 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > > > > > Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes: > > > > > > > Hi Matheus, > > > > > > > > On Tue, 18 Jun 2019, Matheus Tavares wrote: > > > > > > > >>[...] > > > >> +/* > > > >> + * Look for a recursive symlink at iter->base.path pointing to any directory on > > > >> + * the previous stack levels. If it is found, return 1. If not, return 0. > > > >> + */ > > > >> +static int find_recursive_symlinks(struct dir_iterator_int *iter) > > > >> +{ > > > >> + int i; > > > >> + > > > >> + if (!(iter->flags & DIR_ITERATOR_FOLLOW_SYMLINKS) || > > > >> + !S_ISDIR(iter->base.st.st_mode)) > > > >> + return 0; > > > >> > > > >> + for (i = 0; i < iter->levels_nr; ++i) > > > >> + if (iter->base.st.st_ino == iter->levels[i].ino) > > > > > > > > This does not work on Windows. [[ Windows port does not have > > > > usable st_ino field ]]] > > > > > > And if you cross mountpoint, st_ino alone does not guarantee > > > uniqueness; you'd need to combine it with st_dev, I would think, > > > even on POSIX systems. > > > > Ok, thanks for letting me know. I'm trying to think of another > > approach to test for recursive symlinks that does not rely on inode: > > Given any symlink, we could get its real_path() and compare it with > > the path of the directory current being iterated. If the first is a > > prefix of the second, than we mark it as a recursive symlink. > > > > What do you think of this idea? > > I think this would be pretty expensive. Too expensive. Hmm, yes unfortunately :( > A better method might be to rely on st_ino/st_dev when we can, and just > not bother looking for recursive symlinks when we cannot, What if we fallback on the path prefix strategy when st_ino is not available? I mean, if we don't look for recursive symlinks, they would be iterated over and over until we get an ELOOP error. So I think using real_path() should be less expensive in this case. (But just as a fallback to st_ino, off course) > like I did in > https://github.com/git-for-windows/git/commit/979b00ccf44ec31cff4686e24adf27474923c33a Nice! At dir-iterator.h the documentation says that recursive symlinks will be ignored. If we don't implement any fallback, should we add that this is not available on Windows, perhaps? > Ciao, > Johannes