Re: [Monotone-devel] Re: Stable version?

[Richard Levitte - VMS Whacker]

In message <address@hidden> on Sat, 18 Dec 2004 17:16:40 -0800, Nathaniel Smith 
<address@hidden> said:

njs> Then making sure the 'disapprove' worked, I almost immediately
njs> found one bug of this class: it seems that in the current
njs> changeset concatenation logic, "patch [a] -> [b]" and
njs> "patch [b] -> [a]" concatenate to "patch [a] -> [a]", rather than
njs> "" (i.e., nothing).  This is at the least not normalized... and
njs> normalize_change_set doesn't notice this case.

I've a change prepared that adds a function that clears empty deltas,
and that function is called from normalize_change_sets.  It looks to
me like that would be the cleanest and most readable change, even
though it would probably be faster to change concatenate_change_sets
to detect that delta_entry_src(existing) and delta_entry_dst(del) are
the same and only erase delta_entry_path(del) in that case instead of
creating a fused edge.

njs> Also, we should probably detect ill-formedness of this type in
njs> the changeset printer and parser, because we never want to commit
njs> such things to text... (except in the debug messages spit out
njs> when such ill-formedness is detected, I suppose).

Hmm, if we make clearing of these empty deltas a part of
normalization, they shouldn't reach the printer, right?  I haven't
looked at the parser yet, so I won't comment on that.

njs> 
njs> So, I count:
njs>   - need to fix concatenation logic
njs>   - should normalize_change_set check for this case?

I'm assuming changing normalize_change_set makes it less important to
fix the concatenation logic...  or?

njs>   - need to fix assertion logic

Uhmm, in connection to this specific bug, I'm not sure what needs to
be changed.


Here's the change I've made (but haven't commited yet):

# 
# patch "change_set.cc"
#  from [4df69b585314be7983ae42499593d009fb6f57f4]
#    to [090dbadbff7751cc82e5a9e0221b5ad5d6a7c0d1]
# 
--- change_set.cc
+++ change_set.cc
@@ -964,7 +964,27 @@
   sanity_check_path_analysis (pa);
 }
 
+static void
+clear_empty_deltas(change_set::delta_map & dm)
+{
+  for(change_set::delta_map::const_iterator del = dm.begin();
+      del != dm.end(); del++)
+    {
+      L(F("path %s: %s -> %s\n")
+       % delta_entry_path(del)
+       % delta_entry_src(del)
+       % delta_entry_dst(del));
+      if (delta_entry_src(del) == delta_entry_dst(del))
+       {
+         L(F("removing empty delta for %s [%s]\n")
+           % delta_entry_path(del)
+           % delta_entry_src(del));
+         dm.erase(delta_entry_path(del));
+       }
+    }
+}
 
+
 void
 normalize_change_set(change_set & norm)
 {  
@@ -975,6 +995,7 @@
   analyze_rearrangement(norm.rearrangement, tmp, ts);
   clear_rearrangement(norm.rearrangement);
   compose_rearrangement(tmp, norm.rearrangement);
+  clear_empty_deltas(norm.deltas);
 }
 
 

Cheers,
Richard

-----
Please consider sponsoring my work on free software.
See http://www.free.lp.se/sponsoring.html for details.

-- 
Richard Levitte                         address@hidden
                                        http://richard.levitte.org/

"When I became a man I put away childish things, including
 the fear of childishness and the desire to be very grown up."
                                                -- C.S. Lewis