Rev 3892: Better explanation of the differences between path and file-id in file:///net/bigmamac/Volumes/home/vila/src/bzr/cases/3232-spurious-conflict/

[Vincent Ladeuil]

At file:///net/bigmamac/Volumes/home/vila/src/bzr/cases/3232-spurious-conflict/

------------------------------------------------------------
revno: 3892
revision-id: v.ladeuil+lp at free.fr-20081215143819-pocdcxw3so8ppjyk
parent: v.ladeuil+lp at free.fr-20081212130309-e524phx1dic9ic4e
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: log-deep
timestamp: Mon 2008-12-15 15:38:19 +0100
message:
  Better explanation of the differences between path and file-id
  based implementations of 'bzr log <FILE>'.
  
  * bzrlib/log.py:
  (_show_log): Clean up the trivial implementation a bit. It's not
  buggy finally, it just happens that the file-id based one doesn't
  output the removed revisions and that this implementation doesn't
  output the mainline revisions that merged the revisions that
  delete the files.
  (_filter_revisions_touching_file_id): Add more comments explaining
  the subtle consequences of the implementation.
-------------- next part --------------
=== modified file 'bzrlib/log.py'
--- a/bzrlib/log.py	2008-12-12 11:53:07 +0000
+++ b/bzrlib/log.py	2008-12-15 14:38:19 +0000
@@ -230,16 +230,10 @@
         for (rev_id, revno, merge_depth), rev, delta in revs:
             if _path_filter is not None:
                 matches = 0
-                for item in delta.added:
-                    if _path_filter(item[0], item[1]): matches += 1
-                for item in delta.removed:
-                    if _path_filter(item[0], item[1]): matches += 1
-                for item in delta.renamed:
-                    if _path_filter(item[0], item[1]): matches += 1
-                for item in delta.kind_changed:
-                    if _path_filter(item[0], item[1]): matches += 1
-                for item in delta.modified:
-                    if _path_filter(item[0], item[1]): matches += 1
+                for l in (delta.added, delta.removed,
+                          delta.kind_changed, delta.modified):
+                    for item in l:
+                        if _path_filter(item[0], item[1]): matches += 1
                 for item in delta.renamed:
                     if (_path_filter(item[0], item[2])
                         or _path_filter(item[1], item[2])):
@@ -603,7 +597,10 @@
     :return: A list of (revision_id, dotted_revno, merge_depth) tuples.
     """
     # Lookup all possible text keys to determine which ones actually modified
-    # the file.
+    # the file. Note that this function use the fact that we create a new text
+    # only when a file is created or modified. We don't need to access every
+    # inventory to find them, we just use the text index (which is the heart of
+    # the optimization).
     text_keys = [(file_id, rev_id) for rev_id, revno, depth in view_revisions]
     # Looking up keys in batches of 1000 can cut the time in half, as well as
     # memory consumption. GraphIndex *does* like to look for a few keys in
@@ -624,7 +621,12 @@
 
     result = []
     # Track what revisions will merge the current revision, replace entries
-    # with 'None' when they have been added to result
+    # with 'None' when they have been added to result. We do that to give an
+    # output that shows where the changes have been integrated to
+    # mainline. Failing to do so will produce outputs where merged revisions
+    # will appear next after the other *without* the mainline revisions in
+    # between which may give the wrong impression that they occur
+    # consecutively.
     current_merge_stack = [None]
     for info in view_revisions:
         rev_id, revno, depth = info
@@ -635,7 +637,10 @@
             current_merge_stack[-1] = info
 
         if rev_id in modified_text_revisions:
-            # This needs to be logged, along with the extra revisions
+            # This needs to be logged, along with the extra revisions It's
+            # important to understand that the test above identify all the
+            # revisions that add or modify the given file-id but *not* the ones
+            # that *remove* it.
             for idx in xrange(len(current_merge_stack)):
                 node = current_merge_stack[idx]
                 if node is not None: