Junio C Hamano <gitster@xxxxxxxxx> writes:
>> I looked at the output from "grep has_symlink_leading_path" and also
>> for "die_if_path_beyond"; all of these places are checking "I have
>> this multi-level path; I want to know if the path does not (should
>> not) be part of the current project", I think. Certainly the one in
>> the "update-index" is about the same operation as "git add" you are
>> patching.
>>
>> Isn't it a better approach to _rename_ the existing function not to
>> single out "symlink"-ness of the path first ? A symlink in the
>> middle of such a multi-level path that leads to a place outside the
>> project is _not_ the only way to step out of our project boundary. A
>> directory in the middle of a multi-level path that is the top-level
>> of the working tree of a foreign project is another way to step out
>> of our project boundary. Perhaps
>>
>> die_if_path_outside_our_project()
>> path_outside_our_project()
>>
>> And then update the implementation of path_outside_our_project(),
>> which only took "symlink in the middle" into account so far, and
>> teach it that such a "top-level of the working tree of a foreign
>> project" is also stepping out of our project?
>>
>> That way, you do not have to settle on fixing the bug only in "git
>> add" and keep the bug in "git update-index", I think.
>>
>> I think the hit in builtin/apply.c deals with the same "beyond
>> symlink is outside our project" check and can be updated like so. I
>> didn't look at the ones in diff-lib.c and dir.c so you may want to
>> double check on what they use it for.
>
> The first step (renaming and adjusting comments) would look like
> this.
Actually, there is another function "check_leading_path()" you may
want also adjust.
/*
* Return zero if path 'name' has a leading symlink component or
* if some leading path component does not exists.
*
* Return -1 if leading path exists and is a directory.
*
* Return path length if leading path exists and is neither a
* directory nor a symlink.
*/
int check_leading_path(const char *name, int len)
{
return threaded_check_leading_path(&default_cache, name, len);
}
I think what the callers of this function care about is if the name
is a path that should not be added to our index (i.e. points
"outside the repository"). If you had a symlink d that points at e
when our project does have a subdirectory e with file f,
check_leading_path("d/f")
wants to say "bad", even though the real file pointed at, i.e. "e/f"
is inside our working tree, so "outside our working tree" is not
quite correct in the strict sense (this applies equally to
has_symlink_leading_path), but I think we should treat the case
where "d" (and "d/f") belongs to the working tree of a repository
for a separate project, that is embedded in our working tree the
same way.
--
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