Question about commit-dependencies branch - Oswald/Michael?

[Max O Bowsher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

It seems Michael has deliberately _not_ merged the 6 revisions listed
below to the commit-dependencies branch - it looks like they are
probably inapplicable - could one/both of you confirm this? Thanks.

Max.


- -------- Original Message --------
Subject: svn commit: r1741 - branches/commit-dependencies
Date: 3 Dec 2005 19:52:59 -0000
Author: maxb
Date: Sat Dec  3 11:52:59 2005
New Revision: 1741

Modified:
   branches/commit-dependencies/   (props changed)
Log:
Blocked revisions 1703,1705,1708,1710-1711,1713 via svnmerge
Some of these - maybe even all - are inapplicable to the branch.

........
r1703 | mhagger | 2005-11-26 22:14:13 +0000 (Sat, 26 Nov 2005) | 2 lines

* cvs2svn (CVSRevisionAggregator.process_revision): Avoid redundant map
lookup.

........
r1705 | mhagger | 2005-11-26 22:17:49 +0000 (Sat, 26 Nov 2005) | 2 lines

* cvs2svn (CVSRevisionAggregator.__init__): Document cvs_commits field.

........
r1708 | mhagger | 2005-11-26 22:49:16 +0000 (Sat, 26 Nov 2005) | 5 lines

Improved comments for CVSRevisionAggregator fields.

* cvs2svn (CVSRevisionAggregator.__init__): Added comment for
  pending_symbols field.  Improved comment for done_symbols field.

........
r1710 | mhagger | 2005-11-26 23:16:06 +0000 (Sat, 26 Nov 2005) | 5 lines

Simplify CVSRevisionAggregator.process_revision().

* cvs2svn (CVSRevisionAggregator.process_revision): Simplify the logic
  for dealing with ready_queue.

........
r1711 | mhagger | 2005-11-26 23:23:58 +0000 (Sat, 26 Nov 2005) | 5 lines

Simplify CVSRevisionAggregator.flush().

* cvs2svn (CVSRevisionAggregator.flush): Simplify the logic for
  dealing with ready_queue.

........
r1713 | mhagger | 2005-11-27 20:11:53 +0000 (Sun, 27 Nov 2005) | 8 lines

Extracted method for getting ready commits.

This also separates this action into a separate loop, instead of
within the same loop that looks for commit dependencies.

* cvs2svn (CVSRevisionAggregator._extract_ready_commits): New method
  (extracted from process_revision()).

........


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (Cygwin)

iD8DBQFDkfiVfFNSmcDyxYARAoe/AJ9pZwiWGxG8isVfrYDJLTeajv5haACfeiGj
oyfyKRFM7W0+dov/6bvryio=
=hJlK
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

On Sat, Dec 03, 2005 at 07:57:09PM +0000, Max Bowsher wrote:
> it looks like they are probably inapplicable - could one/both of you
> confirm this?
>
at a first look, yes. that's what my patch from yesterday rectified
among other things. :)

--
Hi! I'm a .signature virus! Copy me into your ~/.signature, please!
--
Chaos, panic, and disorder - my work here is done.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

[mhagger]

Max Bowsher wrote:
> It seems Michael has deliberately _not_ merged the 6 revisions listed
> below to the commit-dependencies branch - it looks like they are
> probably inapplicable - could one/both of you confirm this? Thanks.

Yes.  Those six revisions had no obvious parallel in Oswald's changed
code, and so far I haven't had time to look hard for nonobvious
parallels.  I had been refactoring trunk while trying to understand that
part of the code, and then tried to get trunk and the
commit-dependencies branch back within sight of each other so that I
could understand Oswald's changes.

Sorry I didn't finish the job.

I'm not familiar with svnmerge; it looks like I could have made good use
of it.

Michael

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

On Sat, Dec 03, 2005 at 09:16:45PM +0100, Oswald Buddenhagen wrote:
> On Sat, Dec 03, 2005 at 07:57:09PM +0000, Max Bowsher wrote:
> > it looks like they are probably inapplicable - could one/both of you
> > confirm this?
> >
> at a first look, yes. that's what my patch from yesterday rectified
> among other things. :)
>
ok, here's a more worthy followup:

1708 is just an omission and can be merged verbatim.
(1)  <--- these numbers are the commit order

1703,1705,1713 are in fact backmerges from the branch, just that at
this time the branch did not even exist. :)

_1713.diff
[[[
Match up with the version commited to trunk in r1708.
]]]
(2)

_1713plus.diff
[[[
Rename commit_ready and get_deps to match up with the naming conventions
from r1713.
]]]
maybe it should be _get_dependencies instead of _get_deps. i for one
find the shorter name sufficiently self-explanatory and friendlier to
the eye, but opinions might differ ...
(3)

_2:1711.diff
[[[
Merge r1710:1711.
]]]
these two are functionally the same change at different locations. i'd
have commited them in one go in the first place.
(4)

_2:1716.diff
[[[
Re-add a comment that got lost in r1716.
]]]
(5)

the commit logs still don't conform to the official guidelines, but i
think it's total waste of time to drag this further on a short-lived
branch.

--
Hi! I'm a .signature virus! Copy me into your ~/.signature, please!
--
Chaos, panic, and disorder - my work here is done.

--- cvs2svn_1708 2005-12-04 18:36:52.000000000 +0100
+++ cvs2svn 2005-12-04 18:52:22.000000000 +0100
@@ -2692,7 +2692,9 @@
       self.get_deps(r, deps)
     return dep
 
-  def close_expired(self, timestamp):
+  def _extract_ready_commits(self, timestamp):
+    """Extract any active commits that expire by TIMESTAMP."""
+
     # First take all expired commits out of the pool of available commits.
     for digest_key, cvs_commits in self.cvs_commits.items():
       for cvs_commit in cvs_commits[:]:
@@ -2747,9 +2749,9 @@
       self._attempt_to_commit_symbols()
 
   def process_revision(self, c_rev):
-    # Each time we read a new line, we scan the commits we've
-    # accumulated so far to see if any are ready for processing now.
-    self.close_expired(c_rev.timestamp)
+    # Each time we read a new line, scan the accumulating commits to
+    # see if any are ready for processing.
+    self._extract_ready_commits(c_rev.timestamp)
 
     # Add this item into the set of still-available commits.
     deps = {}
@@ -2779,7 +2781,7 @@
     """Commit anything left in self.cvs_commits.  Then inform the
     SymbolingsLogger that all commits are done."""
 
-    self.close_expired(1L<<32)
+    self._extract_ready_commits(1L<<32)
     self.commit_ready(1L<<32)
 
     if not Ctx().trunk_only:

--- cvs2svn_1713 2005-12-04 18:52:22.000000000 +0100
+++ cvs2svn 2005-12-04 19:03:29.000000000 +0100
@@ -2678,7 +2678,7 @@
     Ctx()._default_branches_db = SDatabase(temp(DEFAULT_BRANCHES_DB),
                                            DB_OPEN_READ)
 
-  def get_deps(self, c_rev, deps):
+  def _get_deps(self, c_rev, deps):
     """Populate DEPS with a list of all pending commits this C_REV depends on
     directly or indirectly. Return the commit C_REV depends on directly."""
     if not c_rev.prev_rev:
@@ -2689,7 +2689,7 @@
       return None
     deps[dep] = None
     for r in dep.revisions():
-      self.get_deps(r, deps)
+      self._get_deps(r, deps)
     return dep
 
   def _extract_ready_commits(self, timestamp):
@@ -2726,7 +2726,7 @@
       if not chg:
         break
 
-  def commit_ready(self, timestamp, c_rev=None):
+  def _commit_ready_commits(self, timestamp, c_rev=None):
     # Make sure we _attempt_to_commit_symbols for this c_rev, even if no
     # commits are ready.
     syms_committed = False
@@ -2755,7 +2755,7 @@
 
     # Add this item into the set of still-available commits.
     deps = {}
-    dep = self.get_deps(c_rev, deps)
+    dep = self._get_deps(c_rev, deps)
     cvs_commits = self.cvs_commits.setdefault(c_rev.digest, [])
     # This is pretty silly; it will add the revision to the oldest pending
     # commit. It might be wiser to do time range matching to avoid stretching
@@ -2775,14 +2775,14 @@
     # If there are any elements in the ready_queue at this point, they
     # need to be processed, because this latest rev couldn't possibly
     # be part of any of them.
-    self.commit_ready(c_rev.timestamp, c_rev)
+    self._commit_ready_commits(c_rev.timestamp, c_rev)
 
   def flush(self):
     """Commit anything left in self.cvs_commits.  Then inform the
     SymbolingsLogger that all commits are done."""
 
     self._extract_ready_commits(1L<<32)
-    self.commit_ready(1L<<32)
+    self._commit_ready_commits(1L<<32)
 
     if not Ctx().trunk_only:
       Ctx()._symbolings_logger.close()

--- cvs2svn_1713plus 2005-12-04 19:03:29.000000000 +0100
+++ cvs2svn 2005-12-04 19:14:06.000000000 +0100
@@ -2732,13 +2732,14 @@
     syms_committed = False
     # Sort them into time-order, then process 'em.
     self.ready_queue.sort()
-    for cvs_commit in self.ready_queue[:]:
+    while self.ready_queue:
+      cvs_commit = self.ready_queue[0]
       if cvs_commit.t_max > timestamp:
         # Re-stamping according to a commit's dependencies can cause this.
         break
-      self.latest_primary_svn_commit \
-          = cvs_commit.process_revisions(self.done_symbols)
-      self.ready_queue.remove(cvs_commit)
+      del self.ready_queue[0]
+      self.latest_primary_svn_commit = \
+          cvs_commit.process_revisions(self.done_symbols)
       if c_rev is not None:
         self._add_pending_symbols(c_rev)
       self._attempt_to_commit_symbols()

--- cvs2svn_2:1711 2005-12-04 19:14:06.000000000 +0100
+++ cvs2svn 2005-12-04 19:18:20.000000000 +0100
@@ -2745,6 +2745,8 @@
       self._attempt_to_commit_symbols()
       syms_committed = True
     if not syms_committed:
+      # Make sure we _add_pending_symbols() for this c_rev and
+      # _attempt_to_commit_symbols(), even if no commits are ready.
       if c_rev is not None:
         self._add_pending_symbols(c_rev)
       self._attempt_to_commit_symbols()


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

[Max O Bowsher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Oswald Buddenhagen wrote:
>
> ok, here's a more worthy followup:
...

Most worthy indeed!

Thanks for that thoughrough analysis, all committed.

Max.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (Cygwin)

iD8DBQFDk4rmfFNSmcDyxYARAsO8AKDMimnEmbnTHlPZoZbP3F8igr4RvACcDSRL
7/c5HqdF1/XcUXPHp73O71g=
=sk1t
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...