Re: [PATCH 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks

2012-09-27 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes:

 This series fixes longest_ancestor_length() so that it works even if
 prefix_list contains entries that involve symlinks.  The basic goal of
 the series is to call real_path() on each of the entries so that a
 textual comparison of the potential prefix to the front of path
 correctly decides whether the path is located inside of the entry.
 But along the way some other things had to be changed:

 * real_path() die()s if the path passed to it is invalid, whereas it
   is allowed for GIT_CEILING_DIRECTORIES to contain invalid paths.  So
   create a new function real_path_if_valid() that returns NULL for
   invalid paths.

 * Changing longest_ancestor_length() to call real_path_if_valid()
   would make the former very difficult to test (because the tests
   would depend on the contents of the whole filesystem).  Therefore,
   rewrite longest_ancestor_length() in terms of functions
   string_list_split(), string_list_longest_prefix(), and
   real_path_if_valid() which are tested individually.

 The net results of these changes are that:

 1. t1504 used to have to canonicalize TRASH_DIRECTORY to make itself
work even if the --root directory contains symlinks.  This
canonicalization is no longer necessary (and has been removed).

 2. t4035, which used to fail if the --root directory contained
symlinks, now works correctly in that situation.

 After this change, all tests pass if the --root directory does *not*
 contain symlinks, but t9903 still fails if the --root directory
 contains symlinks.  I haven't analyzed the cause of t9903's failure,
 but it does not appear to be related to the GIT_CEILING_DIRECTORIES
 feature.

I haven't read the actual patches yet, but the all of the above
sounds sensible.

 On the mailing list I suggested *purposely* inserting symlinks into
 the trash directory.* paths to test symlink handling more
 systematically.  This patch series does *NOT* make that change.

And that may be a sensible follow-up step once the dust settles.

Thanks.
--
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


[PATCH 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks

2012-09-26 Thread Michael Haggerty
This series fixes longest_ancestor_length() so that it works even if
prefix_list contains entries that involve symlinks.  The basic goal of
the series is to call real_path() on each of the entries so that a
textual comparison of the potential prefix to the front of path
correctly decides whether the path is located inside of the entry.
But along the way some other things had to be changed:

* real_path() die()s if the path passed to it is invalid, whereas it
  is allowed for GIT_CEILING_DIRECTORIES to contain invalid paths.  So
  create a new function real_path_if_valid() that returns NULL for
  invalid paths.

* Changing longest_ancestor_length() to call real_path_if_valid()
  would make the former very difficult to test (because the tests
  would depend on the contents of the whole filesystem).  Therefore,
  rewrite longest_ancestor_length() in terms of functions
  string_list_split(), string_list_longest_prefix(), and
  real_path_if_valid() which are tested individually.

The net results of these changes are that:

1. t1504 used to have to canonicalize TRASH_DIRECTORY to make itself
   work even if the --root directory contains symlinks.  This
   canonicalization is no longer necessary (and has been removed).

2. t4035, which used to fail if the --root directory contained
   symlinks, now works correctly in that situation.

After this change, all tests pass if the --root directory does *not*
contain symlinks, but t9903 still fails if the --root directory
contains symlinks.  I haven't analyzed the cause of t9903's failure,
but it does not appear to be related to the GIT_CEILING_DIRECTORIES
feature.

On the mailing list I suggested *purposely* inserting symlinks into
the trash directory.* paths to test symlink handling more
systematically.  This patch series does *NOT* make that change.

Michael Haggerty (8):
  Introduce new static function real_path_internal()
  Introduce new function real_path_if_valid()
  longest_ancestor_length(): use string_list_split()
  longest_ancestor_length(): explicitly filter list before loop
  longest_ancestor_length(): always add a slash to the end of prefixes
  longest_ancestor_length(): use string_list_longest_prefix()
  longest_ancestor_length(): resolve symlinks before comparing paths
  t1504: stop resolving symlinks in GIT_CEILING_DIRECTORIES

 abspath.c   | 98 ++---
 cache.h |  1 +
 path.c  | 54 ---
 t/t0060-path-utils.sh   | 64 
 t/t1504-ceiling-dirs.sh | 67 +
 5 files changed, 144 insertions(+), 140 deletions(-)

-- 
1.7.11.3

--
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