[Monotone-devel] missing directories

[Derek Scherger]

I was thinking a bit today about the various possible workspace conflict 
cases that are possible and how we might deal with them. One of the 
cases I can imagine is attempting to add a file to an empty versioned 
directory when this directory is missing from the workspace.

At the moment monotone won't notice this problem until it tries to 
attach a file to the missing directory. This is too late, other changes 
to the workspace may already have been made and failing at this point 
may leave the workspace in an unhappy state.

I vaguely recall someone wondering why monotone doesn't notice when an 
empty versioned directory is missing a while ago and this seems like 
something we probably should be doing. i.e. monotone will currently 
abort in update_current_roster_from_filesystem if there are any missing 
files but will not notice if there are missing directories if they don't 
happen to also contain versioned files.

The attached patch changes update_current_roster_from_filesystem 
slightly so that it will abort if there are missing directories, whether 
they contained files or not. It also makes some slight tweaks to the 
associated messages so they're a bit more specific about missing things 
verses things that are not what they should be, like files that should 
be directories and vice-versa.

Are there any objections to this change? Should I hold off until 0.32 
has been rolled? It will probably have a slight impact on translations 
because of the messaging changes.

See http://www.venge.net/monotone/wiki/NonMergeConflicts if you're 
interested in some background on workspace conflicts.

Cheers,
Derek

# 
# old_revision [58e9c2947e2da3844b22916eb4117a2404bf452e]
# 
# add_dir "tests/missing_empty_dir"
# 
# add_file "tests/missing_empty_dir/__driver__.lua"
#  content [852bbd4441b9b858223f48074a6763fc644ada95]
# 
# patch "testsuite.lua"
#  from [528db1ed137b479e223d9830d535348e677fb216]
#    to [d3183be3af86c27e3c66dd069726de3ee97e5f2d]
# 
# patch "work.cc"
#  from [aa552b8a8355ad6c7c39921004f5026535a10345]
#    to [c20b991d1a7de36863728fd2effca7b07b54fc8b]
# 
============================================================
--- tests/missing_empty_dir/__driver__.lua      
852bbd4441b9b858223f48074a6763fc644ada95
+++ tests/missing_empty_dir/__driver__.lua      
852bbd4441b9b858223f48074a6763fc644ada95
@@ -0,0 +1,18 @@
+
+mtn_setup()
+
+mkdir("foo")
+mkdir("bar")
+
+check(mtn("add", "foo"), 0, false, true)
+check(mtn("add", "bar"), 0, false, true)
+commit()
+
+remove("foo")
+
+check(mtn("status"), 1, false, false)
+
+writefile("foo", "foo")
+
+check(mtn("status"), 1, false, false)
+
============================================================
--- testsuite.lua       528db1ed137b479e223d9830d535348e677fb216
+++ testsuite.lua       d3183be3af86c27e3c66dd069726de3ee97e5f2d
@@ -697,4 +697,5 @@ table.insert(tests, "add_ignores__MTN")
 table.insert(tests, "mkdir")
 table.insert(tests, "fail_cleanly_when__MTN_format_corrupt")
 table.insert(tests, "add_ignores__MTN")
+table.insert(tests, "missing_empty_dir")
 
============================================================
--- work.cc     aa552b8a8355ad6c7c39921004f5026535a10345
+++ work.cc     c20b991d1a7de36863728fd2effca7b07b54fc8b
@@ -947,6 +947,7 @@ workspace::update_current_roster_from_fi
     }
 
   size_t missing_files = 0;
+  size_t missing_dirs = 0;
 
   // this code is speed critical, hence the use of inode fingerprints so be
   // careful when making changes in here and preferably do some timing tests
@@ -960,11 +961,7 @@ workspace::update_current_roster_from_fi
       node_id nid = i->first;
       node_t node = i->second;
 
-      // Only analyze files further, not dirs.
-      if (! is_file_t(node))
-        continue;
-
-      // Only analyze restriction-included files.
+      // Only analyze restriction-included files and dirs
       if (!mask.includes(ros, nid))
         continue;
 
@@ -972,28 +969,53 @@ workspace::update_current_roster_from_fi
       ros.get_name(nid, sp);
       file_path fp(sp);
 
-      // Only analyze changed files (or all files if inodeprints mode
-      // is disabled).
-      if (inodeprint_unchanged(ipm, fp))
-        continue;
-
-      file_t file = downcast_to_file_t(node);
-      if (!ident_existing_file(fp, file->content, lua))
+      if (is_dir_t(node))
         {
-          W(F("missing %s") % (fp));
-          missing_files++;
+          if (!path_exists(fp))
+            {
+              W(F("missing directory '%s'") % (fp));
+              missing_dirs++;
+            }
+          else if (!directory_exists(fp))
+            {
+              W(F("not a directory '%s'") % (fp));
+              missing_dirs++;
+            }
         }
+      else
+        {
+          // Only analyze changed files (or all files if inodeprints mode
+          // is disabled).
+          if (inodeprint_unchanged(ipm, fp))
+            continue;
+
+          if (!path_exists(fp))
+            {
+              W(F("missing file '%s'") % (fp));
+              missing_files++;
+            }
+          else if (!file_exists(fp))
+            {
+              W(F("not a file '%s'") % (fp));
+              missing_files++;
+            }
+
+          file_t file = downcast_to_file_t(node);
+          ident_existing_file(fp, file->content, lua);
+        }
+
     }
 
-  N(missing_files == 0,
-    F("%d missing files; use '%s ls missing' to view\n"
+  N(missing_files == 0 && missing_dirs == 0,
+    F("%d missing files; %d missing directories; use '%s ls missing' to view\n"
       "To restore consistency, on each missing file run either\n"
       " '%s drop FILE' to remove it permanently, or\n"
       " '%s revert FILE' to restore it.\n"
       "To handle all at once, simply use\n"
       " '%s drop --missing' or\n"
       " '%s revert --missing'")
-    % missing_files % ui.prog_name % ui.prog_name % ui.prog_name
+    % missing_files % missing_dirs 
+    % ui.prog_name % ui.prog_name % ui.prog_name
     % ui.prog_name % ui.prog_name);
 }