# 
# 
# patch "ChangeLog"
#  from [a0c2396f81ab99368d25e12d44d331b9ddc2ddfb]
#    to [482740ac1c38ea409af30122be4b06921e5878f3]
# 
# patch "file_io.cc"
#  from [2128664fe9acc07e084954a0bfa9a2f023282dd0]
#    to [40eb975296b733095cccdf78c7a94093e8c36444]
# 
# patch "tests/ls_unknown_in_subdir/__driver__.lua"
#  from [d1cd25cd2101a2f373ab5c8e3280f98980267c5f]
#    to [2e2e2cd17c2ff3b8aec9167e588ede917e239203]
# 
============================================================
--- ChangeLog	a0c2396f81ab99368d25e12d44d331b9ddc2ddfb
+++ ChangeLog	482740ac1c38ea409af30122be4b06921e5878f3
@@ -1,3 +1,13 @@
+2007-01-21  Derek Scherger  <address@hidden>
+
+	* (read_directory): use 'entry.' instead of 'di->' for consistency
+	(walk_tree_recursive): use read_directory instead of a
+	directory_iterator directly to reuse some code we already have and
+	to separate the files and dirs phases of the walk; also move the
+	check for bookkeeping paths to the directory phase of the walk
+	* tests/ls_unknown_in_subdir/__driver__.lua: fix syntax problem
+	hidden by xfail
+
 2007-01-21  Zack Weinberg  <address@hidden>
 
 	* schema_migration.hh (mtn_creator_code): Define the creator code
============================================================
--- file_io.cc	2128664fe9acc07e084954a0bfa9a2f023282dd0
+++ file_io.cc	40eb975296b733095cccdf78c7a94093e8c36444
@@ -337,8 +337,8 @@ void read_directory(any_path const & pat
     {
       fs::path entry = *di;
       if (!fs::exists(entry)
-          || di->string() == "."
-          || di->string() == "..")
+          || entry.string() == "."
+          || entry.string() == "..")
         continue;
 
       // FIXME: BUG: this screws up charsets (assumes blindly that the fs is
@@ -463,62 +463,55 @@ walk_tree_recursive(fs::path const & abs
                     fs::path const & relative,
                     tree_walker & walker)
 {
-  std::vector<std::pair<fs::path, fs::path> > dirs;
-  fs::directory_iterator ei;
-  for(fs::directory_iterator di(absolute);
-      di != ei; ++di)
-    {
-      fs::path entry = *di;
-      // the fs::native is necessary here, or it will bomb out on any paths
-      // that look at it funny.  (E.g., rcs files with "," in the name.)
-      fs::path rel_entry = relative / fs::path(entry.leaf(), fs::native);
-      rel_entry.normalize();
+  system_path root(absolute.string());
+  vector<utf8> files, dirs;
 
-      if (bookkeeping_path::is_bookkeeping_path(rel_entry.string()))
-        {
-          L(FL("ignoring book keeping entry %s") % rel_entry.string());
-          continue;
-        }
+  read_directory(root, files, dirs);
 
-      if (!fs::exists(entry)
-          || entry.string() == "."
-          || entry.string() == "..")
-        {
-          // ignore
-          continue;
-        }
-      else
-        {
-          if (fs::is_directory(entry))
-            dirs.push_back(std::make_pair(entry, rel_entry));
-          else
-            {
-              file_path p;
-              if (!try_file_pathize(rel_entry, p))
-                continue;
-              walker.visit_file(p);
-            }
-        }
-    }
   // At this point, the directory iterator has gone out of scope, and its
   // memory released.  This is important, because it can allocate rather a
   // bit of memory (especially on ReiserFS, see [1]; opendir uses the
   // filesystem's blocksize as a clue how much memory to allocate).  We used
-  // to recurse into subdirectories directly in the loop above; this left
+  // to recurse into subdirectories directly in the loop below; this left
   // the memory describing _this_ directory pinned on the heap.  Then our
   // recursive call itself made another recursive call, etc., causing a huge
   // spike in peak memory.  By splitting the loop in half, we avoid this
-  // problem.
+  // problem. By using read_directory instead of a directory_iterator above
+  // we hopefully make this all a bit more clear.
   // 
   // [1] http://lkml.org/lkml/2006/2/24/215
-  for (std::vector<std::pair<fs::path, fs::path> >::const_iterator i = dirs.begin();
-       i != dirs.end(); ++i)
+
+  for (vector<utf8>::const_iterator i = files.begin(); i != files.end(); ++i)
     {
-      fs::path const & entry = i->first;
-      fs::path const & rel_entry = i->second;
+      // the fs::native is necessary here, or it will bomb out on any paths
+      // that look at it funny.  (E.g., rcs files with "," in the name.)
+      fs::path rel_entry = relative / fs::path((*i)(), fs::native);
+      rel_entry.normalize();
+
       file_path p;
       if (!try_file_pathize(rel_entry, p))
         continue;
+      walker.visit_file(p);
+    }
+
+  for (vector<utf8>::const_iterator i = dirs.begin(); i != dirs.end(); ++i)
+    {
+      // the fs::native is necessary here, or it will bomb out on any paths
+      // that look at it funny.  (E.g., rcs files with "," in the name.)
+      fs::path entry = absolute / fs::path((*i)(), fs::native);
+      fs::path rel_entry = relative / fs::path((*i)(), fs::native);
+      entry.normalize();
+      rel_entry.normalize();
+
+      if (bookkeeping_path::is_bookkeeping_path(rel_entry.string()))
+        {
+          L(FL("ignoring book keeping entry %s") % rel_entry.string());
+          continue;
+        }
+
+      file_path p;
+      if (!try_file_pathize(rel_entry, p))
+        continue;
       walker.visit_dir(p);
       walk_tree_recursive(entry, rel_entry, walker);
     }
============================================================
--- tests/ls_unknown_in_subdir/__driver__.lua	d1cd25cd2101a2f373ab5c8e3280f98980267c5f
+++ tests/ls_unknown_in_subdir/__driver__.lua	2e2e2cd17c2ff3b8aec9167e588ede917e239203
@@ -10,7 +10,7 @@ check(grep('foo/b$', "stdout"), 0, false
 check(grep('foo/a$', "stdout"), 0, false, false)
 check(grep('foo/b$', "stdout"), 0, false, false)
 
-xfail(indir("foo", mtn("ls", "unknown"), 0, true, false))
+xfail(indir("foo", mtn("ls", "unknown")), 0, true, false)
 check(grep('foo$', "stdout"), 0, false, false)
 check(grep('foo/a$', "stdout"), 0, false, false)
 check(grep('foo/b$', "stdout"), 0, false, false)