[Monotone-commits-diffs] net.venge.monotone: 327090160ca10bd6df9e9132c3e39cea5abc88fe

[code]

revision:            327090160ca10bd6df9e9132c3e39cea5abc88fe
date:                2012-06-23T11:57:56
author:              address@hidden
branch:              net.venge.monotone
changelog:
fix issue 207 (partial revert with/without inodeprints) and related xfail test 
restricted_commit_with_inodeprints.

* src/cmd_ws_commit.cc (revert, perform_commit): pass mask to 
work.maybe_update_inodeprints

* src/work.hh:
* src/work.cc (maybe_update_inodeprints): add mask param

* test/func/restricted_commit_with_inodeprints/__driver__.lua: bug now fixed

* test/func/revert_with_inodeprints: New directory, for issue 207.

manifest:
format_version "1"

new_manifest [71dba9796e9ad43a09b43028803b1e98086840d5]

old_revision [3b2a2ec027f659b2a11f02f794a8edd4884c4be5]

add_dir "test/func/revert_with_inodeprints"

add_file "test/func/revert_with_inodeprints/__driver__.lua"
 content [d3ad794b0a265fb5fa4eee3e264eefc6931d9340]

patch "src/cmd_ws_commit.cc"
 from [431aff612ada586c56b63247423ae72df1650b88]
   to [78e02629916d649d7eed18480e05ede5f4b19971]

patch "src/work.cc"
 from [36e5dcda8cf09c9054cb88e6165707112ba9ac03]
   to [324996c07f4fc5bda6e32d20eac172b35db759cf]

patch "src/work.hh"
 from [0ca2a0f0ab94c2421db119216af9373b81f38e61]
   to [520ff77cc8bc9655d3826388f06dda29de9292d0]

patch "test/func/restricted_commit_with_inodeprints/__driver__.lua"
 from [b11b4fbd4ed1b88ccb61d2b9ea5347c61a360c47]
   to [48dda63d86651a117b978054e362f4ed056f3e47]
============================================================
--- src/cmd_ws_commit.cc	431aff612ada586c56b63247423ae72df1650b88
+++ src/cmd_ws_commit.cc	78e02629916d649d7eed18480e05ede5f4b19971
@@ -1,4 +1,4 @@
-// Copyright (C) 2010, 2011 Stephen Leake <address@hidden>
+// Copyright (C) 2010, 2011, 2012 Stephen Leake <address@hidden>
 // Copyright (C) 2002 Graydon Hoare <address@hidden>
 //
 // This program is made available under the GNU GPL version 2.0 or
@@ -530,9 +530,8 @@ revert(app_state & app,
   revision_t remaining;
   make_revision_for_workspace(parent_id(parents.begin()), preserved, remaining);
 
-  // Race.
   work.put_work_rev(remaining);
-  work.maybe_update_inodeprints(db);
+  work.maybe_update_inodeprints(db, mask);
 }
 
 CMD(revert, "revert", "", CMD_REF(workspace), N_("[PATH]..."),
@@ -1801,7 +1800,7 @@ void perform_commit(app_state & app,
       % prog_name);
   }
 
-  work.maybe_update_inodeprints(db);
+  work.maybe_update_inodeprints(db, mask);
 
   {
     // Tell lua what happened. Yes, we might lose some information
============================================================
--- src/work.cc	36e5dcda8cf09c9054cb88e6165707112ba9ac03
+++ src/work.cc	324996c07f4fc5bda6e32d20eac172b35db759cf
@@ -1,4 +1,4 @@
-// Copyright (C) 2009, 2010 Stephen Leake <address@hidden>
+// Copyright (C) 2009, 2010, 2012 Stephen Leake <address@hidden>
 // Copyright (C) 2002 Graydon Hoare <address@hidden>
 //
 // This program is made available under the GNU GPL version 2.0 or
@@ -900,15 +900,29 @@ workspace::maybe_update_inodeprints(data
 void
 workspace::maybe_update_inodeprints(database & db)
 {
+  maybe_update_inodeprints(db, node_restriction());
+}
+
+void
+workspace::maybe_update_inodeprints(database & db,
+                                    node_restriction const & mask)
+{
   if (!in_inodeprints_mode())
     return;
 
+  // We update the cache only for files that are included in the
+  // restriction. The only guarantee that inodeprints mode makes is that if
+  // a file's current inodeprint matches its cached inodeprint then it has
+  // not changed. i.e. for a missing file, the cache would not be updated
+  // but the old cached value can't possibly equal the current value since
+  // the file does not exist and cannot have an inodeprint.
+
   inodeprint_map ipm_new;
   temp_node_id_source nis;
   roster_t new_roster;
 
   get_current_roster_shape(db, nis, new_roster);
-  update_current_roster_from_filesystem(new_roster);
+  update_current_roster_from_filesystem(new_roster, mask);
 
   parent_map parents;
   get_parent_rosters(db, parents);
@@ -917,6 +931,10 @@ workspace::maybe_update_inodeprints(data
   for (node_map::const_iterator i = new_nodes.begin(); i != new_nodes.end(); ++i)
     {
       node_id nid = i->first;
+
+      if (!mask.includes(new_roster, nid))
+        continue;
+
       if (!is_file_t(i->second))
         continue;
       file_t new_file = downcast_to_file_t(i->second);
============================================================
--- src/work.hh	0ca2a0f0ab94c2421db119216af9373b81f38e61
+++ src/work.hh	520ff77cc8bc9655d3826388f06dda29de9292d0
@@ -1,3 +1,4 @@
+// Copyright (C) 2012 Stephen Leake <address@hidden>
 // Copyright (C) 2002 Graydon Hoare <address@hidden>
 //
 // This program is made available under the GNU GPL version 2.0 or
@@ -287,6 +288,8 @@ public:
 
   void enable_inodeprints();
   void maybe_update_inodeprints(database &);
+  void maybe_update_inodeprints(database &,
+                                node_restriction const & mask);
 
   // the 'ignore file', .mtn-ignore in the root of the workspace, contains a
   // set of regular expressions that match pathnames.  any file or directory
============================================================
--- test/func/restricted_commit_with_inodeprints/__driver__.lua	b11b4fbd4ed1b88ccb61d2b9ea5347c61a360c47
+++ test/func/restricted_commit_with_inodeprints/__driver__.lua	48dda63d86651a117b978054e362f4ed056f3e47
@@ -1,23 +1,13 @@ mtn_setup()
 
 mtn_setup()
 
--- this test is a bug report. the problem is that inodeprints tries to update its
--- cache for all files in the complete manifest, but a restricted commit can
--- succeed with missing files if they are excluded. subsequently the inodeprint
--- update fails because it can't build a complete manifest due to the missing
--- files.
+-- This test is a bug report, now fixed. The problem was:
+-- 
+-- inodeprints tries to update its cache for all files in the complete
+-- manifest, but a restricted commit can succeed with missing files if
+-- they are excluded. subsequently the inodeprint update fails because
+-- it can't build a complete manifest due to the missing files.
 
--- one solution is to let inodeprints update its cache only for files that are
--- included in the restriction, which seems to be safe. the only gaurentee that
--- inodeprints mode makes is that if a file's current inodeprint matches its
--- cached inodprint then it has not changed. i.e. for a missing file, the cache
--- would not be updated but the old cached value can't possibly equal the current
--- value since the file does not exist and cannot have an inodeprint.
-
--- also, it may be a useful feature (?) to allow refresh_inodeprints to update
--- its cache for a restricted set of files by allowing paths on the command line
--- to establish a restriction.
-
 addfile("file1", "file1")
 
 commit()
@@ -32,6 +22,5 @@ remove("file1")
 remove("file1")
 
 -- restricted commit of file2 succeeds with file1 missing
--- but the corresponding inodeprint update fails
 
-xfail_if(true, mtn("commit", "--message=file2", "file2"),  0, false, false)
+check(mtn("commit", "--message=file2", "file2"),  0, false, false)
============================================================
--- /dev/null	
+++ test/func/revert_with_inodeprints/__driver__.lua	d3ad794b0a265fb5fa4eee3e264eefc6931d9340
@@ -0,0 +1,21 @@
+-- Test for issue 207; partial revert succeeds without inodeprints,
+-- fails with. Now fixed.
+
+mtn_setup()
+
+addfile("file1", "file1")
+addfile("file2", "file2")
+
+commit()
+
+-- enable inodeprints mode
+check(mtn("refresh_inodeprints"), 0, false, false)
+
+-- create two missing files
+
+remove("file1")
+remove("file2")
+
+-- revert only one file
+
+check(mtn("revert", "file2"),  0, false, false)