Michael Haggerty <mhagger@xxxxxxxxxxxx> writes: > longest_ancestor_length() relies on a textual comparison of directory > parts to find the part of path that overlaps with one of the paths in > prefix_list. But this doesn't work if any of the prefixes involves a > symbolic link, because the directories will look different even though > they might logically refer to the same directory. So canonicalize the > paths listed in prefix_list using real_path_if_valid() before trying > to find matches. I somehow feel that this is making the logic unnecessarily convoluted by solving the problem at a wrong level. If longest_ancestor_length() takes a single string and a list of candidate string prefixes, conceptually it should be usable for any hierarchy-looking string that uses slashes as hierarchy separator (e.g. refs that may be stored in packed-refs that you cannot expect lstat(2) or readlink(2) to give you any sensible results). The real problem is that the list given from the environment may contain a path that violates that "it suffices to take the longest string-prefix taking slashes into account" assumption in such a generic l-a-l implementation, no? And this patch solves it by making l-a-l _less_ generic and forcing it to be aware of the glitch of its caller (you can either blame clueless user who lies when setting the GIT_CEILING_DIRECTORIES by including paths contaminated with symlinks, or blame the calling setup_git_directory_gently_1() function that does not resolve the symbolic links before calling this function). As l-a-l is only used by the "stop at the ceiling" logic, isn't it a far simpler solution to keep the function work at the string level, and make the caller fix its input, i.e. the value taken from the environment variable, before calling it? That is, grab the value of GIT_CEILING_DIRECTORIES, split it into a list at PATH_SEP (while rejecting any non-absolute paths), normalize the elements of that list by resolving symbolic links, and then pass the cwd[] and the normalized string list to l-a-l? The resulting callsite in setup_git_directory_gently_1() would pass cwd[] and the ceiling_dirs (which now is a string list), all of whose elements would happen to begin with "/" (or dos drive prefix followed by the "root" symbol), but l-a-l can be written in such a way that it does not even require that all the input has to begin at root, which would later make it usable with things that are not paths (references, for example, all of which begin with "refs/" and not "/refs/"). Hrm? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html