Re: Difficulty adding a symbolic link, part 3
From: Duy Nguyen pclo...@gmail.com With the above change, the test suite runs with zero failures, so it doesn't affect any common Git usage. It means the test suite is incomplete. As you can see, the commit introducing this change does not come with a test case to catch people changing this. Who should be blamed for omitting the test? Can someone give me advice on what this code *should* do? It does as the function name says: given cwd, a prefix (i.e. a relative path with no .. components) and a path relative to cwd+prefix, convert 'path' to something relative to cwd. In the simplest case, prepending the prefix to 'path' is enough. cwd is also get_git_work_tree(). But as you can see, the exact behavior that the function is intended to exhibit regarding symlinks is not clear from the function name; there should have been a real explanation in the comment above the function. Dale -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Difficulty adding a symbolic link, part 3
From: Duy Nguyen pclo...@gmail.com Can someone give me advice on what this code *should* do? It does as the function name says: given cwd, a prefix (i.e. a relative path with no .. components) and a path relative to cwd+prefix, convert 'path' to something relative to cwd. In the simplest case, prepending the prefix to 'path' is enough. cwd is also get_git_work_tree(). I agree with you that this code should not resolve anything in the components after 'cwd', after rebasing the path to 'cwd' (not just the final component). Not sure how to do it correctly though because we do need to resolve symlinks before cwd. Maybe a new variant of real_path that stops at 'cwd'? We may also have problems with resolve symlinks before cwd when 'path' is relative, as normalize_path_copy() does not resolve symlinks. We just found out emacs has this bug [1] but did not realize we also have one :-P. Thanks for the detailed information. It seems to me that the minimum needed change is that the handling of relative and absolute paths should be made consistent. [1] http://thread.gmane.org/gmane.comp.version-control.git/231268 That problem isn't so much a matter of not resolving symlinks but rather the question of what .. means. In the case shown in that message, the current directory *is* {topdir}/z/b, but it was entered with cd {topdir}/b -- and the shell records the latter as the value of $PWD. (Actually, the bash shell can handle symbolic-linked directories *either* way, depending on whether it is given the -P option.) When Emacs is given the file name ../a/file, does the .. mean to go up one directory *textually* in the pathspec based on $PWD {topdir}/b/../a/file - {topdir}/a/file (which does not exist) or according to the directory linkage on the disk (where the .. entry in the current directory goes to the directory named {topdir}/z, which does contain a file a/file. It looks like Emacs uses the first method. The decision of which implementation to use depends on the semantics you *want*. As I noted, bash allows the user to choose *either* interpretation, which shows that both semantics are considered valid (by some people). Dale -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Difficulty adding a symbolic link, part 3
On Thu, Aug 1, 2013 at 10:00 PM, Dale R. Worley wor...@alum.mit.edu wrote: From: Duy Nguyen pclo...@gmail.com Can someone give me advice on what this code *should* do? It does as the function name says: given cwd, a prefix (i.e. a relative path with no .. components) and a path relative to cwd+prefix, convert 'path' to something relative to cwd. In the simplest case, prepending the prefix to 'path' is enough. cwd is also get_git_work_tree(). I agree with you that this code should not resolve anything in the components after 'cwd', after rebasing the path to 'cwd' (not just the final component). Not sure how to do it correctly though because we do need to resolve symlinks before cwd. Maybe a new variant of real_path that stops at 'cwd'? We may also have problems with resolve symlinks before cwd when 'path' is relative, as normalize_path_copy() does not resolve symlinks. We just found out emacs has this bug [1] but did not realize we also have one :-P. Thanks for the detailed information. It seems to me that the minimum needed change is that the handling of relative and absolute paths should be made consistent. Agreed. [1] http://thread.gmane.org/gmane.comp.version-control.git/231268 That problem isn't so much a matter of not resolving symlinks but rather the question of what .. means. In the case shown in that message, the current directory *is* {topdir}/z/b, but it was entered with cd {topdir}/b -- and the shell records the latter as the value of $PWD. (Actually, the bash shell can handle symbolic-linked directories *either* way, depending on whether it is given the -P option.) When Emacs is given the file name ../a/file, does the .. mean to go up one directory *textually* in the pathspec based on $PWD {topdir}/b/../a/file - {topdir}/a/file (which does not exist) or according to the directory linkage on the disk (where the .. entry in the current directory goes to the directory named {topdir}/z, which does contain a file a/file. It looks like Emacs uses the first method. The decision of which implementation to use depends on the semantics you *want*. As I noted, bash allows the user to choose *either* interpretation, which shows that both semantics are considered valid (by some people). We pass paths around, the semantics must be one that others expect, not what we want. For one, the kernel seems to resolve symlinks first, then ... We use chdir() and getcwd() provided by the kernel. We need to agree with it on .. semantics. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Difficulty adding a symbolic link, part 3
I've run into a problem (with Git 1.8.3.3) where I cannot add a symbolic link (as such) to the repository *if* its path is given absolutely; instead Git adds the file the symbolic link points to. (If I give the path relatively, Git does what I expect, that is, adds the symbolic link.) I've written a test script that shows the problem and included it below. I would not expect *how* a path is presented to Git to change how Git processes the path. In the test case, I would expect /bin/awk and ../../bin/awk to produce the same effect when used as arguments to git add. What is going on in the code is this: In git add, all paths are normalized by the function prefix_path_gently() in abspath.c. That function removes symbolic links from the pathspec *only if* it is absolute, as shown in the first few lines of the function: static char *prefix_path_gently(const char *prefix, int len, const char *path) { const char *orig = path; char *sanitized; if (is_absolute_path(orig)) { -const char *temp = real_path(path); +const char *temp = absolute_path(path); sanitized = xmalloc(len + strlen(temp) + 1); strcpy(sanitized, temp); } else { real_path() is specified to remove symbolic links. As shown, I've replaced real_path() with absolute_path(), based on the comment at the top of real_path(): /* * Return the real path (i.e., absolute path, with symlinks resolved * and extra slashes removed) equivalent to the specified path. (If * you want an absolute path but don't mind links, use * absolute_path().) The return value is a pointer to a static * buffer. * If I replace real_path() with absolute_path() as shown, the problem I am testing for disappears. With the above change, the test suite runs with zero failures, so it doesn't affect any common Git usage. But I don't know enough about the internal architecture of Git to know that my change is correct in all cases. I'm almost certain that the normalization process for pathspecs should *not* normalize a final component that is a symbolic link. But I would expect it would be desirable to normalize non-final components that are symbolic links. On the other hand, that might not matter. Can someone give me advice on what this code *should* do? I believe I can prepare a proper test for the test suite for this, so once I know what the code change should be, I can prepare a proper patch for it. Dale -- Here's a test case for adding a symbolic link. This test exploits the fact that on my system, /bin/awk is a symbolic link to gawk. As you can see, the behavior of Git differs if the link's path is given to git add as an absolute path or a relative path. Here is the test script: -- #! /bin/bash # Illustrates a problem with applying git add to a symbolic link. set -x # To be run from a directory one step below the root directory. E.g., # /tmp. # On this system, /bin/awk is a symbolic link to gawk, which # means /tmp/gawk. # Show the Git version. git version # Make a test directory and cd to it. DIR=temp.$$ mkdir $DIR cd $DIR # Create a Git repository. git init # Set the worktree to be / git config core.worktree / # Create an empty commit. git commit --allow-empty -m Empty. # Add the symbolic link using its absolute name. ABSOLUTE=/bin/awk ls -l $ABSOLUTE git add $ABSOLUTE # Notice that the target of the link is added, not the link itself. git status -uno # Reset the index. git reset # Add the symbolic link using its relative name. # Remember that we are two directory levels below the root directory now. RELATIVE=../..$ABSOLUTE ls -l $RELATIVE git add $RELATIVE # Notice that now the link itself is added. git status -uno -- Here is sample output of the script: -- + git version git version 1.8.3.3.756.g07a2553.dirty + DIR=temp.22366 + mkdir temp.22366 + cd temp.22366 + git init Initialized empty Git repository in /git-add-link/temp.22366/.git/ + git config core.worktree / + git commit --allow-empty -m Empty. [master (root-commit) fb232e5] Empty. + ABSOLUTE=/bin/awk + ls -l /bin/awk lrwxrwxrwx. 1 root root 4 Nov 2 2012 /bin/awk - gawk + git add /bin/awk + git status -uno # On branch master # Changes to be committed: # (use git reset HEAD file... to unstage) # # new file: ../../bin/gawk # # Untracked files not listed (use -u option to show untracked files) + git reset + RELATIVE=../../bin/awk + ls -l ../../bin/awk lrwxrwxrwx. 1 root root 4 Nov 2 2012 ../../bin/awk - gawk + git add ../../bin/awk + git status -uno # On branch master # Changes to be committed: # (use git reset HEAD file... to unstage) # # new file: ../../bin/awk # # Untracked files not
Re: Difficulty adding a symbolic link, part 3
On Thu, Aug 1, 2013 at 3:29 AM, Dale R. Worley wor...@alum.mit.edu wrote: I've run into a problem (with Git 1.8.3.3) where I cannot add a symbolic link (as such) to the repository *if* its path is given absolutely; instead Git adds the file the symbolic link points to. (If I give the path relatively, Git does what I expect, that is, adds the symbolic link.) I've written a test script that shows the problem and included it below. I would not expect *how* a path is presented to Git to change how Git processes the path. In the test case, I would expect /bin/awk and ../../bin/awk to produce the same effect when used as arguments to git add. What is going on in the code is this: In git add, all paths are normalized by the function prefix_path_gently() in abspath.c. That function removes symbolic links from the pathspec *only if* it is absolute, as shown in the first few lines of the function: static char *prefix_path_gently(const char *prefix, int len, const char *path) { const char *orig = path; char *sanitized; if (is_absolute_path(orig)) { -const char *temp = real_path(path); +const char *temp = absolute_path(path); sanitized = xmalloc(len + strlen(temp) + 1); strcpy(sanitized, temp); } else { real_path() is specified to remove symbolic links. As shown, I've replaced real_path() with absolute_path(), based on the comment at the top of real_path(): /* * Return the real path (i.e., absolute path, with symlinks resolved * and extra slashes removed) equivalent to the specified path. (If * you want an absolute path but don't mind links, use * absolute_path().) The return value is a pointer to a static * buffer. * If I replace real_path() with absolute_path() as shown, the problem I am testing for disappears. I think it also reverts 18e051a (setup: translate symlinks in filename when using absolute paths - 2010-12-27). real_path() (or make_absolute_path() back then) is added to resolve symlinks, at least ones leading to the work tree, not ones inside the work tree, if I understand the commit message correctly. With the above change, the test suite runs with zero failures, so it doesn't affect any common Git usage. It means the test suite is incomplete. As you can see, the commit introducing this change does not come with a test case to catch people changing this. But I don't know enough about the internal architecture of Git to know that my change is correct in all cases. I'm almost certain that the normalization process for pathspecs should *not* normalize a final component that is a symbolic link. But I would expect it would be desirable to normalize non-final components that are symbolic links. On the other hand, that might not matter. Can someone give me advice on what this code *should* do? It does as the function name says: given cwd, a prefix (i.e. a relative path with no .. components) and a path relative to cwd+prefix, convert 'path' to something relative to cwd. In the simplest case, prepending the prefix to 'path' is enough. cwd is also get_git_work_tree(). I agree with you that this code should not resolve anything in the components after 'cwd', after rebasing the path to 'cwd' (not just the final component). Not sure how to do it correctly though because we do need to resolve symlinks before cwd. Maybe a new variant of real_path that stops at 'cwd'? We may also have problems with resolve symlinks before cwd when 'path' is relative, as normalize_path_copy() does not resolve symlinks. We just found out emacs has this bug [1] but did not realize we also have one :-P. [1] http://thread.gmane.org/gmane.comp.version-control.git/231268 I believe I can prepare a proper test for the test suite for this, so once I know what the code change should be, I can prepare a proper patch for it. Dale -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Difficulty adding a symbolic link, part 2
Here's a slightly simpler test case for adding a symbolic link. This test exploits the fact that on my system, /bin/awk is a symbolic link to gawk. As you can see, the behavior of Git differs if the link's path is given to git add as an absolute path or a relative path. Here is the test script: -- #! /bin/bash # Illustrates a problem with applying git add to a symbolic link. set -x # To be run from a directory one step below the root directory. E.g., # /git-add-link. # Exploits the fact that /bin/awk is a symbolic link to gawk. # Show the Git version. git version # Make a test directory and cd to it. DIR=temp.$$ mkdir $DIR cd $DIR # Create a Git repository. git init # Set the worktree to be / git config core.worktree / # Create an empty commit. git commit --allow-empty -m Empty. # Add the symbolic link using its absolute name. ABSOLUTE=/bin/awk ls -l $ABSOLUTE git add $ABSOLUTE # Notice that the target of the link is added, not the link itself. git status -uno # Reset the index. git reset # Add the symbolic link using its relative name. # Remember that we are two directory levels below the root directory now. RELATIVE=../..$ABSOLUTE ls -l $RELATIVE git add $RELATIVE # Notice that now the link itself is added. git status -uno -- Here is the output of the script: -- + git version git version 1.7.7.6 + DIR=temp.22366 + mkdir temp.22366 + cd temp.22366 + git init Initialized empty Git repository in /git-add-link/temp.22366/.git/ + git config core.worktree / + git commit --allow-empty -m Empty. [master (root-commit) fb232e5] Empty. + ABSOLUTE=/bin/awk + ls -l /bin/awk lrwxrwxrwx. 1 root root 4 Nov 2 2012 /bin/awk - gawk + git add /bin/awk + git status -uno # On branch master # Changes to be committed: # (use git reset HEAD file... to unstage) # # new file: ../../bin/gawk # # Untracked files not listed (use -u option to show untracked files) + git reset + RELATIVE=../../bin/awk + ls -l ../../bin/awk lrwxrwxrwx. 1 root root 4 Nov 2 2012 ../../bin/awk - gawk + git add ../../bin/awk + git status -uno # On branch master # Changes to be committed: # (use git reset HEAD file... to unstage) # # new file: ../../bin/awk # # Untracked files not listed (use -u option to show untracked files) -- I can't see any principle of operation of Git that would cause git add /bin/awk and git add ../../bin/awk to be handled differently. Dale -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Difficulty adding a symbolic link
I'm having a problem with git add in version 1.7.7.6. The situation is that I have a repository that is contained in a second-level directory, a sub-sub-directory of /. The core.worktree of the repository is /, so the working directory is the entire file tree. I want this repository to track selected files, which I add with git add. The difficulty is that git add seems to not add specific files, depending on how they are specified as arguments to git add. In particular, /dev/dvd (which is a symbolic link) can be added with git add ../../dev/dvd but not with git add /dev/dvd. On the other hand, /etc/hosts (an ordinary file) can be added with either git add /etc/hosts or git add ../../etc/hosts. To demonstrate the problem, I've written a script. A typical execution of the script is as follows. The lines at the left margin start with $, and are the lines of the script. The lines indented 4 spaces start with +, and are the simple commands as they are executed by the shell, showing the values substituted for the shell variables. The lines indented 8 spaces are the output of the various commands. Someone suggested that the problem may be triggered by the fact that /dev is in a different filesystem than / and /etc. I added a third section to the script by creating a symbolic link from /etc/hosts.link to /etc/hosts, which is thus in the same filesystem as / and the repository. Git handles it as expected. Any help with this would be appreciated. Dale $ set -x $ $ # Show git version. $ git version + git version git version 1.7.7.6 $ $ # To be run in a directory that is contained in /. $ pwd + pwd /git-add-issue $ $ # Make a test directory. $ DIR=temp.$$ + DIR=temp.10714 $ mkdir $DIR + mkdir temp.10714 $ cd $DIR + cd temp.10714 ++ pwd $ DIR_ABSOLUTE=$( pwd ) + DIR_ABSOLUTE=/git-add-issue/temp.10714 $ $ # Create a new repository. $ git init + git init Initialized empty Git repository in /git-add-issue/temp.10714/.git/ $ # Set the worktree to be / $ git config core.worktree / + git config core.worktree / $ # Create empty commit. $ git commit --allow-empty -m Empty. + git commit --allow-empty -m Empty. [master (root-commit) 62d86cb] Empty. $ $ # Show empty status $ git status -uno + git status -uno # On branch master nothing to commit (use -u to show untracked files) $ $ # First test: /dev/dvd, which is a symbolic link. $ $ # Try to add /dev/dvd $ ABSOLUTE_NAME=/dev/dvd + ABSOLUTE_NAME=/dev/dvd $ ll $ABSOLUTE_NAME + ls -al /dev/dvd lrwxrwxrwx. 1 root root 9 Jun 12 22:23 /dev/dvd - /dev/dvd1 $ git add $ABSOLUTE_NAME + git add /dev/dvd $ git status -uno + git status -uno # On branch master nothing to commit (use -u to show untracked files) $ git reset + git reset $ $ # Try with alternative name ../../dev/dvd $ RELATIVE_NAME=../../dev/dvd + RELATIVE_NAME=../../dev/dvd $ ll $RELATIVE_NAME + ls -al ../../dev/dvd lrwxrwxrwx. 1 root root 9 Jun 12 22:23 ../../dev/dvd - /dev/dvd1 $ git add $RELATIVE_NAME + git add ../../dev/dvd $ git status -uno + git status -uno # On branch master # Changes to be committed: # (use git reset HEAD file... to unstage) # # new file: ../../dev/dvd # # Untracked files not listed (use -u option to show untracked files) $ git reset + git reset $ $ # Second test: /etc/hosts, which is an ordinary file. $ $ # Try to add /etc/hosts $ ABSOLUTE_NAME=/etc/hosts + ABSOLUTE_NAME=/etc/hosts $ ll $ABSOLUTE_NAME + ls -al /etc/hosts -rw-r--r--. 1 root root 222 Nov 4 2012 /etc/hosts $ git add $ABSOLUTE_NAME + git add /etc/hosts $ git status -uno + git status -uno # On branch master # Changes to be committed: # (use git reset HEAD file... to unstage) # # new file: ../../etc/hosts # # Untracked files not listed (use -u option to show untracked files) $ git reset + git reset $ $ # Try with alternative name ../../etc/hosts $ RELATIVE_NAME=../../etc/hosts + RELATIVE_NAME=../../etc/hosts $ ll $RELATIVE_NAME + ls -al ../../etc/hosts -rw-r--r--. 1 root root 222 Nov 4 2012 ../../etc/hosts $ git add $RELATIVE_NAME + git add ../../etc/hosts $ git status -uno + git status -uno # On branch master # Changes to be committed: # (use git reset HEAD file... to unstage) # # new file: ../../etc/hosts # # Untracked files not listed (use -u option to show untracked files) $ git reset + git reset $ $ # Third test: /etc/hosts.link, which is a link to /etc/hosts $ $ # Try to add /etc/hosts.link $ ABSOLUTE_NAME=/etc/hosts.link + ABSOLUTE_NAME=/etc/hosts.link $ ll $ABSOLUTE_NAME + ls -al /etc/hosts.link lrwxrwxrwx. 1 root root 5 Jun 14 12:04 /etc/hosts.link - hosts $ git add