Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries

Ramkumar Ramachandra [off-list ref] writes:

Junio C Hamano wrote:

quoted

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

Actually, you introduced one naming regression:
has_symlink_leading_path() is a good name for what the function does,
as opposed to die_if_path_outside_our_tree(), which is misleading.
What about die_if_path_contains_links() to encapsulate gitlinks and
symlinks?

The cases we know that "$d/f" (where $d is a path that is one or
more levels, e.g. "dir", "d/i", or "d/i/r") is bad are when

 - "$d" is a symlink, because what you could add to the index is "$d"
   and nothing underneath it; or

 - "$d" is a directory that is the top level of the working tree
   that is controled by "$d/.git", because what you could add to the
   index is "$d" and nothing underneath it.

If "$d" were added to our index, the former will make 120000 entry
and the latter will make 160000 entry.  But the user may not want to
add $d ever to our project, so in that case, neither will give us a
symlink or a gitlink.

We should find a word that makes it clear that "this path is beyond
something we _could_ add".  I do not think "link" is a good word for
it.  It shares the same mistake that led to the original misnomer,
i.e. "the case we happened to notice was when we have symlink so
let's name it with 'symlink' somewhere in it."

quoted

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.

I'm not too sure about this.  It means that I can have symlinks to
files in various parts of my worktree, but not to directories.

It does not mean that.  It is valid to do

	ln -s myetc /etc
        git add myetc

It is NOT valid to do

	git add myetc/passwd

One can have symlinks to anywhere all one wants.  We track symlinks.

It is the same for the top-level of the working tree of a separate
project, be it a submodule or not.  It is valid to do

	mkdir foo && (cd foo && git init && >file)
        git add foo

It is NOT valid to do

	git add foo/file

Junio C Hamano wrote:

One can have symlinks to anywhere all one wants.  We track symlinks.
[...]

Yes, I know.  We store symlinks as blobs containing one line, the path
to the file, without a trailing newline.  And we have a mode for it to
distinguish it from regular files.

What I meant is:

    echo "baz" >newfile
    cd foo/bar/quux
    ln -s ../../../newfile
    cd ../../..                    # Back to toplevel
    git add foo/bar/quux/newfile

This is allowed.  While:

    cd foo/bar/quux
    echo "baz" >newfile
    cd ../../..                    # Back to toplevel
    ln -s foo/bar/quux
    git add quux/newfile

is disallowed.  Then again, if we were to replace the last line with:

    cd quux
    git add newfile

and it works.

Notice that both symlinks are pointing to paths inside out repository,
and the only difference is that the second example attempts to add a
path with a symlink as the non-final component.  The path is not
pointing "outside" our repository, as the function name would
indicate.

Anyway, it's just a minor detail that would be nice to fix in the
future.  Nothing urgent.