[Monotone-devel] Small bug fixes

[Eric Kidd]

These apply to 16290fd6ccb8612c3b41a580f1716b0a1d142bb4.  I'd put them
in a depot, but I haven't quite figured out how get depot.cgi to link
statically.  Sorry for the patch at the end; my e-mail program seems to
be grumpy, too.

This fixes the following bugs as best I could.

EK005: 'monotone propagate net.test.test net.test.test.hacking' creates a
new child manifest with no branch name.  Updates on either
net.test.test.hacking OR net.test.test will pull in this manifest.

EK006: Agraph fails immediately after first check-in.

EK007: Update does not remain on the current branch!

EK010: Can't check out anything until *after* the second commit:
monotone: fetching heads of branch 'net.test.retest'
monotone: misuse: branch net.test.retest has multiple heads

In particular, EK010 modifies how branch heads are computed (the current
algorithm requires all heads to have an ancestor), and may not be the
ideal fix.  But it allows quite a few operations to work correctly
immediately after first check-in.  EK007 modifies how update selects
branches--it will no longer wander off branches silently.  EK005
involves a case where monotone sends packets to the network, but not to
the database.  If a single composite object wrapped both the database
writer and the network packet writer, these sorts of errors would be
impossible.

Many thanks for such a cool program!  Several of my co-workers and
friends seem pretty impressed by the architecture.

Cheers,
Eric

# Old manifest: 16290fd6ccb8612c3b41a580f1716b0a1d142bb4
# New manifest: 89203632315a82d092845756219da87ee77962e2
# Summary of changes:
#   patch ChangeLog 72ea3ad0142f5db29317a278009fb27fe4cfc2cd -> 
e5b677c7950f1c5ee401088594bc9efc308f8a8e
#   patch cert.cc 87bd7914545275ab77ac0382a521063f697607d9 -> 
22b292de2afb49908e4548eb6652a36eb3236ada
#   patch commands.cc 5b53cf83adb38927c313cfafdde77dee927e8a5a -> 
77ab829e868861252e6baa98e046310f31c42621
#   patch database.cc bfd2f4bf03b6be81df99aff3cc54a0a44501cec8 -> 
5c0a0d6043e2fc2d054cc7b6a327ef8b0f976463
#   patch update.cc 1d50bf0e067c056233fb5affbd58883953de62b3 -> 
48b49886555d055fc4c0402bc9894f44e0c8c0ad
#   patch update.hh 65bf13cb8db7f9d40b9182c188b0c2946ab6a45b -> 
d7e728ea07accf498da091dae24aa3f1807a2ff4
--- ChangeLog
+++ ChangeLog
@@ -1,3 +1,19 @@
+2003-12-07  Eric Kidd  <address@hidden>
+
+       * commands.cc (agraph): Handle repositories with a single version.
+       * database.cc (get_head_candidates): Handle heads with no
+       ancestors.
+       * cert.cc (get_branch_heads): Handle heads with no
+       ancestors.
+       
+2003-12-06  Eric Kidd  <address@hidden>
+
+       * update.hh, update.cc (pick_update_target): Return current
+       version if no better update candidates available.
+       * update.cc (pick_update_target): Always do branch filtering.
+       * commands.cc (update): Notice when we're already up-to-date.
+       * commands.cc (propagate): Assign branch name correctly when merging.
+
 2003-12-02  graydon hoare  <address@hidden>
 
        * database.cc, database.hh (reverse_queue): Copy constructor.
--- cert.cc
+++ cert.cc
@@ -453,7 +453,6 @@
 {
   heads.clear();
   vector< manifest<cert> > branch_certs, ancestor_certs;
-  set<manifest_id> candidates;
   base64<cert_value> branch_encoded;
   encode_base64(branchname, branch_encoded);
 
@@ -462,40 +461,32 @@
   app.db.get_head_candidates(branch_encoded(), branch_certs, ancestor_certs);
   bogus_cert_p bogus(app);
 
-  for (vector< manifest<cert> >::const_iterator i = ancestor_certs.begin();
-       i != ancestor_certs.end(); ++i)
+  for (vector< manifest<cert> >::const_iterator i = branch_certs.begin();
+       i != branch_certs.end(); ++i)
     {
-      candidates.insert(i->inner().ident);
+      if (!bogus(*i))
+       {
+         heads.insert(i->inner().ident);
+       }      
     }
 
-  L(F("began with %d candidate heads\n") % candidates.size());
+  L(F("began with %d candidate heads\n") % heads.size());
 
+  // Remove every manifest with descendents.
   for (vector< manifest<cert> >::const_iterator i = ancestor_certs.begin();
        i != ancestor_certs.end(); ++i)
     {
       cert_value tv;
       decode_base64(i->inner().value, tv);
       manifest_id parent(tv());
-      set<manifest_id>::const_iterator j = candidates.find(parent);
-      if (j != candidates.end() && !bogus(*i))
+      set<manifest_id>::const_iterator j = heads.find(parent);
+      if (j != heads.end() && !bogus(*i))
        {
-         candidates.erase(j);
+         heads.erase(j);
        }
     }
 
-  L(F("reduced to %d candidate heads\n") % candidates.size());
-
-  for (vector< manifest<cert> >::const_iterator i = branch_certs.begin();
-       i != branch_certs.end(); ++i)
-    {
-      set<manifest_id>::const_iterator j = candidates.find(i->inner().ident);
-      if (j != candidates.end() && !bogus(*i))
-       {
-         heads.insert(*j);
-       }      
-    }
-
-  L(F("finalized %d heads\n") % heads.size());
+  L(F("reduced to %d heads\n") % heads.size());
 }
                   
 void cert_file_ancestor(file_id const & parent, 
--- commands.cc
+++ commands.cc
@@ -1273,6 +1273,11 @@
   calculate_new_manifest_map(m_old, m_working);
   
   pick_update_target(m_old_id, args, app, m_chosen_id);
+  if (m_old_id == m_chosen_id)
+    {
+      P(F("already up to date at %s\n") % m_old_id);
+      return;
+    }
   P(F("selected update target %s\n") % m_chosen_id);
   app.db.get_manifest_version(m_chosen_id, m_chosen_data);
   read_manifest_map(m_chosen_data, m_chosen);
@@ -1801,17 +1806,23 @@
       transaction_guard guard(app.db);
       try_one_merge (*src_i, *dst_i, merged, app, targets);      
 
+      packet_db_writer dbw(app);
       queueing_packet_writer qpw(app, targets);
-      cert_manifest_in_branch(merged, app.branch_name, app, qpw);
-      cert_manifest_changelog(merged, 
-                             "propagate of " 
-                             + src_i->inner()() 
-                             + " and " 
-                             + dst_i->inner()()
-                             + "\n"
-                             + "from branch " 
-                             + idx(args, 0) + " to " + idx(args, 1) + "\n", 
-                             app, qpw);          
+
+      cert_manifest_in_branch(merged, idx(args, 1), app, dbw);
+      cert_manifest_in_branch(merged, idx(args, 1), app, qpw);
+
+      string log = ("propagate of " 
+                    + src_i->inner()()
+                    + " and " 
+                    + dst_i->inner()()
+                    + "\n"
+                    + "from branch "
+                    + idx(args, 0) + " to " + idx(args, 1) + "\n");
+
+      cert_manifest_changelog(merged, log, app, dbw);
+      cert_manifest_changelog(merged, log, app, qpw);
+
       guard.commit();      
     }
 }
@@ -2541,6 +2547,7 @@
     {
       cert_value tv;
       decode_base64(i->inner().value, tv);
+      nodes.insert(i->inner().ident()); // in case no edges were connected
       branches.insert(make_pair(i->inner().ident(), tv()));
     }  
 
--- database.cc
+++ database.cc
@@ -1074,8 +1074,6 @@
        "WHERE (name = 'ancestor' OR name = 'branch') "
        "AND id IN "
        "("
-       "SELECT id FROM manifest_certs WHERE name = 'ancestor' "
-       "INTERSECT "
        "SELECT id FROM manifest_certs WHERE name = 'branch' "
        "AND value = '%q'"
        ")",
--- update.cc
+++ update.cc
@@ -293,19 +293,17 @@
     }
   
   find_descendents(base_ident, app, candidates);
-  if (candidates.size() == 0 &&
-      app.db.manifest_version_exists(base_ident))
+  if (app.db.manifest_version_exists(base_ident))
+    // We can almost always "update" to what we're starting with.
     candidates.insert(base_ident);
   
-  if (candidates.size() > 1
-      && (app.branch_name != ""))
-    {
-      set<manifest_id> branch;
-      filter_by_branch(app, candidates, branch);
-      N(branch.size() != 0,
-       F("no update candidates after selecting branch"));
-      candidates = branch;
-    }
+  N(app.branch_name != "",
+    F("cannot determine branch for update"));
+  set<manifest_id> branch;
+  filter_by_branch(app, candidates, branch);
+  N(branch.size() != 0,
+    F("no update candidates after selecting branch"));
+  candidates = branch;
     
   if (candidates.size() > 1)
     {
--- update.hh
+++ update.hh
@@ -17,6 +17,9 @@
 // behind picking an update target. the actual updating takes
 // place in commands.cc, along with most other file-modifying
 // actions.
+//
+// if no version is better than base_ident, then this function
+// will set chosen to base_ident.
 
 void pick_update_target(manifest_id const & base_ident,
                        std::vector<std::string> const & sort_certs,