Re: Tracking "branch heads"

Greg Ward wrote:
> [...]
> Cool.  OK, here is a preliminary draft.  This passes its test, but I
> have not yet used it in production.  I'll see how the design actually
> works within the next 24 hours.  For your amusement...
> 
> """
> # HG changeset patch
> # User Greg Ward <[email protected]>
> # Date 1260821833 18000
> # Node ID 1763b1aba3259d59ed84141f046f73a35331a7e0
> # Parent  64442bf496d83c4a1883bddc51a387493edeabd1
> Record the head of each branch as we generate Subversion revisions.
> This should make life easier for DVCS conversions, where it's often
> desirable to do special things with branch heads (close them, merge
> them, etc.)
> 
> * config.py: add SVN_BRANCH_HEADS file (a pickled dictionary)
> * passes.py: modify CreateRevsPass to record and write branch heads
> * run-tests.py: add a test
> 
> diff --git a/cvs2svn_lib/config.py b/cvs2svn_lib/config.py
> --- a/cvs2svn_lib/config.py
> +++ b/cvs2svn_lib/config.py
> @@ -159,6 +159,12 @@
>  # pickled SVNCommit instances.
>  CVS_REVS_TO_SVN_REVNUMS = 'cvs-revs-to-svn-revnums.dat'
> 
> +# Pickled dictionary mapping LOD as (id, name) tuple to branch head
> +# (highest svn revision number on that branch).  Only Trunk and Branch LODs
> +# will be recorded here.  The LOD name is only recorded for testing
> +# purposes; if you use this dict, you really should use the LOD id.
> +SVN_BRANCH_HEADS = 'svn-branch-heads.pck'
> +

OK, this answers the question in my last email: the name is included for
testing purposes.  Hmmm....

>  # This database maps Subversion revision numbers to pickled SVNCommit
>  # instances.
>  SVN_COMMITS_INDEX_TABLE = 'svn-commits-index.dat'
> diff --git a/cvs2svn_lib/passes.py b/cvs2svn_lib/passes.py
> --- a/cvs2svn_lib/passes.py
> +++ b/cvs2svn_lib/passes.py
> @@ -1541,6 +1541,7 @@
>      self._register_temp_file(config.SVN_COMMITS_STORE)
>      self._register_temp_file(config.CVS_REVS_TO_SVN_REVNUMS)
>      self._register_temp_file(config.SYMBOL_OPENINGS_CLOSINGS)
> +    self._register_temp_file(config.SVN_BRANCH_HEADS)
>      self._register_temp_file_needed(config.PROJECTS)
>      self._register_temp_file_needed(config.CVS_FILES_DB)
>      self._register_temp_file_needed(config.CVS_ITEMS_SORTED_STORE)
> @@ -1585,6 +1586,23 @@
>        for cvs_rev in svn_commit.cvs_revs:
>          Log().verbose(' %s %s' % (cvs_rev.cvs_path, cvs_rev.rev,))
> 
> +  def update_branch_heads(self, branch_heads, svn_commit):
> +    for cvs_rev in svn_commit.get_cvs_items():
> +      if isinstance(cvs_rev, CVSRevision):  # ordinary commit
> +        lod = cvs_rev.lod
> +        id = lod.id
> +        if isinstance(lod, Trunk):
> +          name = str(lod)
> +        elif isinstance(lod, Branch):
> +          name = lod.name
> +        else:
> +          continue
> +      else:
> +        continue
> +
> +      branch_heads[(id, name)] = svn_commit.revnum
> +
> +

Shouldn't this rather be "branch_heads[id] = (svn_commit.revnum, name)"?
 I don't see much purpose to requiring both id and name to look up an
entry in the map...

For that matter, you could just write "branch_heads[lod] =
svn_commit.revnum", since LinesOfDevelopment are hashable.  Then when
outputting the results you can add the extra information.

>    def run(self, run_options, stats_keeper):
>      Log().quiet("Mapping CVS revisions to Subversion commits...")
> 
> @@ -1599,6 +1617,7 @@
>          DB_OPEN_READ)
> 
>      Ctx()._symbolings_logger = SymbolingsLogger()
> +    branch_heads = {}                   # map (LOD id, name) to head revnum
> 
>      persistence_manager = PersistenceManager(DB_OPEN_NEW)
> 
> @@ -1606,6 +1625,12 @@
>      for svn_commit in self.get_svn_commits(creator):
>        self.log_svn_commit(svn_commit)
>        persistence_manager.put_svn_commit(svn_commit)
> +      self.update_branch_heads(branch_heads, svn_commit)
> +
> +    # Write the branch_heads dict.
> +    file = open(artifact_manager.get_temp_file(config.SVN_BRANCH_HEADS), 'wb')
> +    cPickle.dump(branch_heads, file, -1)
> +    file.close()

Then this could become

    cPickle.dump(
        [(lod.id, lod.name, revnum) for (lod, revnum) in branch_heads],
        file, -1,
        )

In other words, there is no need to commit to which direction the
dictionary should run, because there is no need to store this as a
dictionary at all.

> 
>      stats_keeper.set_svn_rev_count(creator.revnum_generator.get_last_id())
>      del creator
> @@ -1712,6 +1737,7 @@
>      self._register_temp_file_needed(config.SVN_COMMITS_INDEX_TABLE)
>      self._register_temp_file_needed(config.SVN_COMMITS_STORE)
>      self._register_temp_file_needed(config.CVS_REVS_TO_SVN_REVNUMS)
> +    self._register_temp_file_needed(config.SVN_BRANCH_HEADS)
>      Ctx().output_option.register_artifacts(self)
> 
>    def run(self, run_options, stats_keeper):
> diff --git a/run-tests.py b/run-tests.py
> --- a/run-tests.py
> +++ b/run-tests.py
> @@ -4584,6 +4584,33 @@
>          ('trunk/proj/sub1/default', [None]),
>          ])
> 
> +@Cvs2SvnTestFunction
> +def branch_heads():
> +  "record head of each branch"
> +  import cPickle
> +  from cvs2svn_lib.symbol import Trunk, Branch

cPickle can be imported at the top of the file.  Trunk and Branch don't
seem to be used at all.

> +
> +  conv = ensure_conversion('main', args=['--skip-cleanup',])
> +  file = open('cvs2svn-tmp/svn-branch-heads.pck', 'rb')
> +  branch_heads = cPickle.load(file)
> +  file.close()
> +
> +  expect = {
> +    # these are easy and obvious, since they all have CVS commits
> +    "Trunk":    33,
> +    "B_SPLIT":  32,
> +    "B_MIXED":  24,
> +
> +    }
> +
> +  actual = {}
> +  for ((id, name), revnum) in branch_heads.items():
> +    actual[name] = revnum
> +
> +  if expect != actual:
> +    raise Failure("branch heads: expected:\n%s\nbut got:\n%s"
> +                  % (expect, actual))
> +
> 
>  ########################################################################
>  # Run the tests
> @@ -4802,6 +4829,7 @@
>      collision_with_unlabeled_branch_name,
>      many_deletes,
>      cvs_description,
> +    branch_heads,
>      ]
> 
>  if __name__ == '__main__':
> """
> 
> Of note: I do *not* use branch creation events to update branch_heads.
>  I have a patch that adds that, but:
> 
>   * it ensures that CVS branches that never actually had any commits
> are present in
>     branch_heads... but it's not clear if that is a good thing or a bad thing
>   * it complicates matters by recording "split" CVS branches under two distinct
>     LOD IDs
> 
> Think about it.  I'll send the second patch if you want, but it feels
> wrong to me.

The user has a chance to change commitless branches into tags.  If he
hasn't, it might be because it is a legitimate branch that has been
created but not yet used.

I don't understand your second point.  What do you mean by "split" CVS
branch here?

Michael

------------------------------------------------------
http://cvs2svn.tigris.org/ds/viewMessage.do?dsForumId=1667&dsMessageId=2431079

To unsubscribe from this discussion, e-mail: [[email protected]].