[Monotone-devel] Please review: initial Attic patch

[Tom Tromey]

Here's the Attic patch.  I haven't had much time to work on it lately
(and probably won't in the near future), and it is at least better
than what we currently do, so I thought I'd post it for comments.

Take a look at the FIXME comments in t_cvsimport2.at to see various
cvs import failure modes I've found.  I think all of these represent
real bugs.

The cvs_history::set_filename part of the patch is basically
independent.  It could go in separately if desired.  It just
canonicalizes file names by removing the "Attic" component.


A quick note on Attic directories: a ,v file is moved into Attic if
and only if the tip of the trunk is dead.  However, the location of
the ,v file is redundant -- cvs also marks the revision as "State:
dead".  The existence of Attic is just a backward compatibility hack
with versions of cvs predating the "dead" state support.


Anyway, what do you think?  I don't want to commit this without
somebody else reading over it.  If the main bits look bad, I'd still
like to check in the test case at the very least.

Tom


# Old manifest: ce5403dbdbe8ba8b6e228d14c791628a1835ab52
# New manifest: ff1d04bbc64faa614d81c251bbe503af09bf27e7
# Summary of changes:
#   add tests/t_cvsimport2.at as 8a6628bf2871292034deaf1f0962857f53fcc4e7
#   patch ChangeLog 7b966c9c3a7b63ba61afd026fe84d469638b6174 -> 
f55501a774d7dd0a81be8c1da6b55aaa1d0b95af
#   patch rcs_import.cc d2535758d21aaaf36c96415e1ebc0f4700361074 -> 
47ec16d0e8c048ef129d7e45b08c3c2f36c25b32
#   patch testsuite.at db82390ede299cfa11af8a1c07f189a2d824bbef -> 
9f0608acd0521168135756bf795c7ec45f1fc3d7
--- ChangeLog
+++ ChangeLog
@@ -1,3 +1,18 @@
+2003-12-05  Tom Tromey  <address@hidden>
+
+       * testsuite.at: Include new test.
+       * tests/t_cvsimport2.at: New file.
+       * rcs_import.cc (cvs_history::set_filename): Rewrite name if file
+       is in Attic.
+       (delta_live_p): New function.
+       (note_file_edge): Added prev_living and curr_living arguments.
+       Update deletions list when required.  Don't add file to head
+       manifest if it has been deleted.
+       (struct cvs_state): New field deletions.
+       (build_parent_state): Process deletions.
+       (build_child_state): Process deletions.
+       (struct cvs_history): New field head_deletions.
+
 2003-11-29  Tom Tromey  <address@hidden>
 
        * rcs_import.cc (store_auxiliary_certs): Renamed to fix typo.
--- rcs_import.cc
+++ rcs_import.cc
@@ -121,6 +121,7 @@
 {
   set<cvs_file_edge> in_edges;
   map< cvs_key, shared_ptr<cvs_state> > substates;
+  set<file_path> deletions;
 };
 
 struct cvs_history
@@ -145,6 +146,7 @@
   state_stack stk;
   file_path curr_file;
   manifest_map head_manifest;
+  set<file_path> head_deletions;
   string base_branch;
 
   ticker n_versions;
@@ -155,11 +156,13 @@
   void set_filename(string const & file,
                    file_id const & ident);
   void push_branch(rcs_file const & r, string const & rcs_version_num);
-  void note_file_edge(rcs_file const & r, 
+  void note_file_edge(rcs_file const & r,
                      string const & prev_rcs_version_num,
                      string const & next_rcs_version_num,
                      file_id const & prev_version,
-                     file_id const & next_version);
+                     file_id const & next_version,
+                     bool prev_living,
+                     bool next_living);
   void pop_branch();
 };
 
@@ -373,6 +375,12 @@
   rcs_put_raw_file_edge(next_id, curr_id, del, db);
 }
 
+static bool
+delta_live_p (shared_ptr<rcs_delta> &delta)
+{
+  // cvs sets state to "dead" when a file is removed.
+  return delta->state != string("dead");
+}
 
 static void 
 process_branch(string const & begin_version, 
@@ -397,9 +403,19 @@
       L(F("version %s has %d lines\n") % curr_version % curr_lines->size());
       
       // construct this edge on our own branch
-      string next_version = r.deltas.find(curr_version)->second->next;
+      shared_ptr<rcs_delta> delta = r.deltas.find(curr_version)->second;
+      string next_version = delta->next;
       L(F("following RCS edge %s -> %s\n") % curr_version % next_version);
 
+      bool curr_living = delta_live_p(delta);
+
+      bool next_living = true;
+      if (r.deltas.find(next_version) != r.deltas.end())
+       {
+         shared_ptr<rcs_delta> d2 = r.deltas.find(next_version)->second;
+         next_living = delta_live_p(d2);
+       }
+
       construct_version(*curr_lines, next_version, *next_lines, r);
       L(F("constructed RCS version %s, inserting into database\n") % 
        next_version);
@@ -408,15 +418,18 @@
                     *next_lines, next_data, next_id, db);
 
       cvs.note_file_edge (r, curr_version, next_version, 
-                         file_id(curr_id), file_id(next_id));
+                         file_id(curr_id), file_id(next_id),
+                         curr_living, next_living);
 
       // recursively follow any branches rooted here
-      boost::shared_ptr<rcs_delta> curr_delta = 
r.deltas.find(curr_version)->second;
+      shared_ptr<rcs_delta> curr_delta = r.deltas.find(curr_version)->second;
       for(vector<string>::const_iterator i = curr_delta->branches.begin();
          i != curr_delta->branches.end(); ++i)
        {
          L(F("following RCS branch %s\n") % (*i));
          vector< piece > branch_lines;
+         shared_ptr<rcs_delta> curr_delta = r.deltas.find(*i)->second;
+         bool branch_living = delta_live_p(curr_delta);
          construct_version(*curr_lines, *i, branch_lines, r);
          
          data branch_data;
@@ -426,7 +429,8 @@
          cvs.push_branch (r, curr_version);
 
          cvs.note_file_edge (r, curr_version, *i,
-                             file_id(curr_id), file_id(branch_id));
+                             file_id(curr_id), file_id(branch_id),
+                             curr_living, branch_living);
 
          process_branch(*i, branch_lines, branch_data, branch_id, r, db, cvs);
          cvs.pop_branch();
@@ -641,8 +642,35 @@
   I(file.size() > 2);
   I(file.substr(file.size() - 2) == string(",v"));
   string ss = file;
+
+  // A file in a directory named Attic is really the same as a file
+  // outside the attic.  In the Attic, the head main branch revision
+  // is dead, but this is also noted in the ,v file itself.  FIXME:
+  // should probably notice and give an error if a file appears both
+  // in the Attic and outside it; this shouldn't happen.
+  string::size_type spos = ss.rfind('/');
+  if (spos != string::npos)
+    {
+      if (spos > 5 && ss.substr(spos - 6, 6) == string("/Attic"))
+       {
+         string nf = ss;
+         nf.erase(spos - 6, 6);
+         L(F("rewriting %s to %s\n") % ss % nf);
+         ss = nf;
+       }
+      else if (spos == 5 && ss.substr(0, 5) == string("Attic"))
+       {
+         string nf = ss;
+         // Also remove the trailing "/".
+         nf.erase(0, 6);
+         L(F("rewriting %s to %s\n") % ss % nf);
+         ss = nf;
+       }
+    }
+
   ui.set_tick_trailer(ss);
   ss.resize(ss.size() - 2);
+
   curr_file = file_path(ss);
 }
 
@@ -713,15 +740,18 @@
   stk.push(s);
 }
 
-void cvs_history::note_file_edge(rcs_file const & r, 
+void cvs_history::note_file_edge(rcs_file const & r,
                                 string const & prev_rcs_version_num,
                                 string const & next_rcs_version_num,
                                 file_id const & prev_version,
-                                file_id const & next_version) 
+                                file_id const & next_version,
+                                bool prev_living,
+                                bool next_living)
 {
 
   cvs_key k;
   shared_ptr<cvs_state> s;
+  bool living;
 
   I(stk.size() > 0);
   I(! curr_file().empty());
@@ -734,11 +737,11 @@
       L(F("noting trunk edge %s : %s -> %s\n") % curr_file
        % next_rcs_version_num
        % prev_rcs_version_num);
-      // find_key_and_state (r, next_rcs_version_num, k, s); // just to create 
it if necessary
       find_key_and_state (r, prev_rcs_version_num, k, s);
       s->in_edges.insert(cvs_file_edge(next_version, curr_file, 
                                       prev_version, curr_file, 
                                       *this));
+      living = next_living;
     }
   else
     {
@@ -750,11 +750,42 @@
       s->in_edges.insert(cvs_file_edge(prev_version, curr_file, 
                                       next_version, curr_file, 
                                       *this));
+      living = prev_living;
     }
   
-  // add this file and youngest version to the manifest if we've never seen it 
before
-  if (head_manifest.find(curr_file) ==  head_manifest.end())
-    head_manifest.insert(make_pair(curr_file, prev_version));
+  // the logic here is a bit obscure.  if we've never seen this file
+  // before, we add its youngest version to the manifest.  however, if
+  // we've seen an explicit deletion for this file, then we *don't*
+  // add the file to the head manifest.  in any case, if this is a
+  // deletion, we make a note to delete the file again when walking
+  // the cvs state tree.  we only bother with this on the trunk, as
+  // branches are taken care of when walking them later.
+  if (stk.size () == 1)
+    {
+      if (head_manifest.find(curr_file) == head_manifest.end())
+       {
+         if (! prev_living)
+           {
+             L(F("file %s at revision %s is dead at first sight\n")
+               % curr_file % prev_version);
+             head_deletions.insert(curr_file);
+           }
+         else if (head_deletions.find(curr_file) == head_deletions.end())
+           {
+             // never saw, and live.
+             L(F("adding file %s at revision %s to manifest\n") % curr_file
+               % prev_version);
+             head_manifest.insert(make_pair(curr_file, prev_version));
+           }
+       }
+    }
+
+  if (! living)
+    {
+      L(F("removing file %s at revision %s from manifest\n") % curr_file
+       % prev_version);
+      s->deletions.insert(curr_file);
+    }
   
   ++n_versions;
 }
@@ -908,6 +939,14 @@
       file_path pth(cvs.path_interner.lookup(f->parent_path));
       state_map[pth] = fid;
     }  
+  for (set<file_path>::const_iterator i = state->deletions.begin();
+       i != state->deletions.end(); ++i)
+    {
+      string s = (*i)();
+      L(F("deleting file %s from manifest\n") % s);
+      state_map.erase(*i);
+    }
+
   L(F("logical changeset from child -> parent has %d file deltas\n")
     % state->in_edges.size());
 }
@@ -925,6 +933,14 @@
       file_path pth(cvs.path_interner.lookup(f->child_path));
       state_map[pth] = fid;
     }  
+  for (set<file_path>::const_iterator i = state->deletions.begin();
+       i != state->deletions.end(); ++i)
+    {
+      string s = (*i)();
+      L(F("deleting file %s from manifest\n") % s);
+      state_map.erase(*i);
+    }
+
   L(F("logical changeset from parent -> child has %d file deltas\n") 
     % state->in_edges.size());
 }
--- testsuite.at
+++ testsuite.at
@@ -109,4 +109,5 @@
 m4_include(tests/t_erename.at)
 m4_include(tests/t_smtp.at)
 m4_include(tests/t_cvsimport.at)
+m4_include(tests/t_cvsimport2.at)
 m4_include(tests/t_proxy.at)