Re: [Monotone-devel] fix for change_set.cc invariant bug, tests 54 55 56 57?

[Matt Johnston]

On Thu, Dec 16, 2004 at 10:27:03PM -0800, Emile Snyder wrote:
> Hi all,
> 
> I spent some time trying to figure this one out, and I think I
> understand.  I've attached a patch that causes the tests to pass, and
> looks right to me.  It's a one line fix, but the patch has all the
> logging I put in to convince myself of it.
> 
> In change_set.cc::concatenate_change_sets, during the fusing of changes,
> we're walking over all the deltas in b's changeset.  However, a file add
> shows up as both an add for the filename, and then a delta from an empty
> file_id to the file_id of the added file.
> 
> I believe the invariant line should be switched to 
> if (b.rearrangement.added_files.find(delta_entry_path(del)) ==
> b.rearrangement.added_files.end()) {
>       I(delta_entry_dst(existing) == delta_entry_src(del));
> }
> 
> Seem right?

I've been looking at this for a bit, and I think that
more is required to fix the problem. The real problem
appears to be related to "patch" operations remaining in
change_sets with a "delete_file". 

Your patch can end up with inconsistent change_sets in some
circumstances - if you modify t_patch_drop_add.at by removing
the COMMIT after 'drop testfile', you end up with an
inconsistent revision which includes:

delete_file "testfile"
add_file "testfile"
patch "testfile"
 from [046c6ae2d5713d17eefbd9e44456cf731028a332]
   to [2836cefb15dae58c3ac859d1ace659d3c6860380]

The patch should be []->[283...], as the newly added
'testfile' is treated as a seperate file to the deleted file
(which the 046c... came from).



I've committed a new revision
cdd90448ae6e33d9ba86e3ee5a9e4859808f6cb6 which passes all
the tests 54 through 59, and seems to behave correctly when
merging between distant parts of the monotone tree.  (Test
53 fails due to a difference in the final tree layout,
though I'm not certain how it should end up). I think I've
now covered all the paths in change_set where deltas can
occur which would conflict with deletes
(project_missing_deltas also needed some work).

The revision could do with a bit more testing, so if
anyone sees problems please let me know.


Matt