[Monotone-devel] [PATCH] lca selector RFC

[Emile Snyder]

Hi all,

I think I still have a valid commit key for venge.net, but it's been a
really long time since I've been active with monotone and feels impolite
to start committing again without checking in ;)

The attached patch implements an "LCA selector," ie. it lets you do

mtn diff -r p:net.venge.monotone

to see the changes you have in your working copy vs. the ancestor in
net.venge.monotone which would be selected if you merged the branch
you're on with n.v.m.

Issues remaining with the patch:

* The name.  'p' seems odd to say the least (ostensibly for branch
divergence Point).  Would people prefer allowing multi letter selector
naming, ie. just use lca:<branchname>?  Or just some other single
character choice?

* Working copy reliance.  At the moment it find your workspace parent
revision, the head of the specified branch, and then calls
find_ancestor_for_merge(parent, branch_head).  Does this make sense?
Should it try to allow for specifying both sides of the
find_ancestor...() call like
lca:h:net.venge.monotone.win32,h:net.venge.monotone (or something?)

* Finds merge ancestor, not propagate ancestor.  It doesn't do the smart
thing about figuring out if either the left or right revision is an
ancestor of the other.  I just have to fix that.

* Needs some more cases in the test (doing it outside workspace, the
left or right being ancestor of the other thing, etc..)

So, thoughts?  Thanks,
-emile

patch follows:
================================================================

# 
# old_revision [547ec9b32c9a55b715513e023b4e8b7fd78db1f8]
# 
# add_dir "tests/p_selector"
# 
# add_file "tests/p_selector/__driver__.lua"
#  content [db86017be8442ef6b0cb339a82e258814989bd3c]
# 
# patch "monotone.texi"
#  from [80457540a0a71d8a71a9b00644ed24dcd950e109]
#    to [8f534af5406da86c2dcd8021b75c3757133fb165]
# 
# patch "selectors.cc"
#  from [89c9ef4a38bbad56a692229b389d698e34426aef]
#    to [dc4c080c242cf2bc6cbf2457e5317c5a9d85ae82]
# 
# patch "selectors.hh"
#  from [f3f93e168438c3dd49111a2e4aff5a444e01994d]
#    to [eb102536f83a7f111b0589381060b4c49a93eb5f]
# 
============================================================
--- tests/p_selector/__driver__.lua
db86017be8442ef6b0cb339a82e258814989bd3c
+++ tests/p_selector/__driver__.lua
db86017be8442ef6b0cb339a82e258814989bd3c
@@ -0,0 +1,73 @@
+--
+-- the various operations below result in the following
+-- revision graph (nodes given as id:branch)
+--
+--     REV1:main     REV4:disconnected
+--      |    \
+--  REV2:main \
+--      |    REV3:main.b
+--      |     /        \
+--      |  REV6:main.b  \
+--      |   /           REV5:main.b
+--    REV7:main
+--
+
+include("common/selectors.lua")
+mtn_setup()
+
+addfile("testfile", "blah blah")
+commit("main")
+REV1=base_revision()
+
+-- verify that using it with your own branch gives your own id
+check(mtn("automate", "select", "p:main"), 0, true, false)
+check(qgrep(REV1, "stdout"), 0, false, false)
+
+-- simplest real case, two revs on main, branch main.b from root node
+addfile("testfile2", "stuff stuff")
+commit("main")
+REV2=base_revision()
+check(mtn("update", "--revision", REV1), 0, false, false)
+writefile("testfile", "other stuff")
+commit("main.b")
+REV3=base_revision()
+selmap("p:main", {REV1})
+check(mtn("update", "--revision", REV2), 0, false, false)
+selmap("p:main.b", {REV1})
+
+-- disconnected branch
+check(remove("_MTN"))
+check(mtn("setup", "--branch", "disconnected", "."), 0, false, false)
+addfile("otherfile", "some content")
+commit("disconnected")
+REV4=base_revision()
+check(mtn("automate", "select", "p:main"), 1, false, true)
+check(qgrep("no common ancestor", "stderr"), 0, false, false)
+check(remove("_MTN"))
+check(remove("testfile"))
+check(remove("testfile2"))
+check(mtn("checkout", "--revision", REV3, "."), 0, false, false)
+check(mtn("automate", "select", "p:disconnected"), 1, false, true)
+check(qgrep("no common ancestor", "stderr"), 0, false, false)
+
+-- specified branch must be single headed
+writefile("testfile", "other stuff\nmore other stuff")
+commit("main.b")
+REV5=base_revision()
+check(mtn("update", "--revision", REV3), 0, false, false)
+writefile("testfile", "other stuff\ndifferent other stuff")
+commit("main.b")
+REV6=base_revision()
+check(mtn("update", "--revision", REV2), 0, false, false)
+check(mtn("automate", "select", "p:main.b"), 1, false, true)
+check(qgrep("main.b has 2 heads", "stderr"), 0, false, false)
+check(qgrep("needs exactly 1", "stderr"), 0, false, false)
+
+-- propagate one of the heads of main.b to main, update to
+-- other head, p:main should then give us REV3, the single 
+-- ancestor of the two heads of main.b
+check(mtn("explicit_merge", REV6, REV2, "main"), 0, false, true)
+check(mtn("update", "--revision", "h:main"), 0, false, true)
+REV7=base_revision()
+check(mtn("update", "--revision", REV6), 0, false, true)
+selmap("p:main", {REV3})
============================================================
--- monotone.texi       80457540a0a71d8a71a9b00644ed24dcd950e109
+++ monotone.texi       8f534af5406da86c2dcd8021b75c3757133fb165
@@ -2675,6 +2675,12 @@ @heading Simple examples
 @item graydon/net.venge.monotone.win32/yesterday
 Revisions in the @code{net.venge.monotone.win32} branch, written by
 @code{graydon}, yesterday.
address@hidden p:net.venge.monotone.win32
+The least common ancestor node between the workspace, and the 
+head of the branch @code{net.venge.monotone.win32}; ie. the ancestor
+that monotone would find if you were to commit your working 
+directory and then merge with head of the
@code{net.venge.monotone.win32} 
+branch.
 @end table
 
 A moment's examination reveals that these specifications are ``fuzzy''
@@ -2743,6 +2749,12 @@ @heading Selectors in detail
 Uses selector type @code{t}. For example, @code{t:monotone-0.11}
matches
 @code{tag} certs where the cert value begins with @code{monotone-0.11}.
 Values to match for can have shell wildcards.
address@hidden Branch Divergence Point selection
+Uses selector type @code{p}.  For example, 
address@hidden diff -r p:net.venge.monotone} run from a workspace would 
+show you all changes that exist in your working copy which have not
+been propagated to the @code{net.venge.monotone} branch yet.  So far
+this is the only selector that relies on a workspace.
 @end table
 
 Further selector types may be added in the future. 
============================================================
--- selectors.cc        89c9ef4a38bbad56a692229b389d698e34426aef
+++ selectors.cc        dc4c080c242cf2bc6cbf2457e5317c5a9d85ae82
@@ -11,12 +11,15 @@
 #include "sanity.hh"
 #include "app_state.hh"
 #include "constants.hh"
+#include "revision.hh"
+#include "database.hh"
 
 using std::make_pair;
 using std::pair;
 using std::set;
 using std::string;
 using std::vector;
+using std::ostringstream;
 
 namespace selectors
 {
@@ -76,6 +79,9 @@ namespace selectors
           case 'e':
             type = sel_earlier;
             break;
+          case 'p':
+            type = sel_divergence_point;
+            break;
           default:
             W(F("unknown selector type: %c") % sel[0]);
             break;
@@ -114,6 +120,46 @@ namespace selectors
     app.db.complete(type, sel, limit, completions);
   }
 
+  void
+  preprocess_selector(selector_type & type, string & str, app_state &
app)
+  {
+    //L(FL("preprocess_selector called with type %d, str '%d'") % type
% str);
+    switch (type)
+      {
+      case sel_divergence_point:
+        {
+          parent_map parents;
+          app.work.get_parent_rosters(parents);
+          N(parents.size() == 1,
+            F("branch divergence point selector needs a single parent
workspace"));
+
+          revision_id work_base_id(parent_id(parents.begin()));
+
+          std::set<std::string> rev_completions;
+          std::vector<std::pair<selector_type, std::string> > limit;
+          limit.push_back(std::make_pair(sel_head, str));
+          app.db.complete(sel_ident, "", limit, rev_completions);
+          N(rev_completions.size() == 1,
+            F("branch %s has %d heads, branch divergence point selector
needs exactly 1") % str % rev_completions.size());
+
+          revision_id branch_head_id(*rev_completions.begin());
+          revision_id anc_id;
+          find_common_ancestor_for_merge(work_base_id, branch_head_id,
anc_id, app);
+
+          N(false == null_id(anc_id),
+            F("no common ancestor exists for branch %s head %s and
workspace parent %s") 
+              % str % branch_head_id.inner()() %
work_base_id.inner()());
+          
+          type = sel_ident;
+          str = anc_id.inner()();
+          //L(FL("preprocess_selector transformed to type %d, str '%
d'") % type % str);
+        }
+        break;
+      default:
+        break;
+      }
+  }
+
   vector<pair<selector_type, string> >
   parse_selector(string const & str,
                  app_state & app)
@@ -143,6 +189,7 @@ namespace selectors
             selector_type type = sel_unknown;
 
             decode_selector(*i, type, sel, app);
+            preprocess_selector(type, sel, app);
             sels.push_back(make_pair(type, sel));
           }
       }
============================================================
--- selectors.hh        f3f93e168438c3dd49111a2e4aff5a444e01994d
+++ selectors.hh        eb102536f83a7f111b0589381060b4c49a93eb5f
@@ -31,6 +31,7 @@ namespace selectors
       sel_cert,
       sel_earlier,
       sel_later,
+      sel_divergence_point, // workspace dependent.  should do more
reworking?
       sel_unknown
     }
   selector_type;
@@ -41,6 +42,9 @@ namespace selectors
                     selector_type & type,
                     std::set<std::string> & completions,
                     app_state & app);
+  void
+  preprocess_selector(selector_type & type, std::string & str,
app_state & app);
+
   std::vector<std::pair<selector_type, std::string> >
   parse_selector(std::string const & str,
                  app_state & app);