Re: [PATCH v2 8/9] longest_ancestor_length(): resolve symlinks before comparing paths

[Junio C Hamano <gitster@pobox.com>]

Michael Haggerty <mhagger@alum.mit.edu> 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?