[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Monotone-devel] fix for change_set.cc invariant bug, tests 54 55 56
From: |
Matt Johnston |
Subject: |
Re: [Monotone-devel] fix for change_set.cc invariant bug, tests 54 55 56 57? |
Date: |
Sat, 18 Dec 2004 23:44:13 +0800 |
User-agent: |
Mutt/1.4i |
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