Re: Difficulty adding a symbolic link, part 3

2013-08-01 Thread Dale R. Worley
 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

2013-08-01 Thread Dale R. Worley
 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

2013-08-01 Thread Duy Nguyen
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

2013-07-31 Thread Dale R. Worley
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

2013-07-31 Thread Duy Nguyen
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

2013-06-24 Thread Dale R. Worley
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

2013-06-14 Thread Dale R. Worley
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