[PATCH] convert: make convert from cvs recognise branch points correctly [revised, 3]

Patrick Mézard pmezard at gmail.com
Tue Apr 21 04:35:41 CDT 2009


Henrik Stuart a écrit :
> # HG changeset patch
> # User Henrik Stuart <henrik.stuart at edlund.dk>
> # Date 1239706830 -7200
> # Node ID f8fd162954633e67b27605b2541eb46e155d4780
> # Parent  aece3c9e62f1a685e4cebd3ecf0b22e1c8101ae1
> convert: make convert from cvs recognise branch points correctly
> 
> This patch fixes issue 1447 by using the symbolic names for branches
> in the rlog output to determine the branch point for each file and
> then finding the latest possible branch point in the given changesets
> such that all the file revisions match and the date is earlier than
> the branch commit. For commits on non-branches, the current
> functionality is maintained.
> 
> Co-contributor: Greg Ward <greg-hg at gerg.ca>
> 

[snip]

My opinion for today (or today morning, let's not be too optimistic) is the patch looks correct but uses too many side-effects/implicit assumptions. To answer my own question about the reuse of "e" in:


+                        if [e.file for e in qcs.entries if 
+                                e.file in seen_branched_files]:
+                            break # we cannot progress past this place
+
+                        branchesto = getattr(e, 'branchesto', None)


I think the behaviour is correct because:
- changesets always have at least one log entry
- you make the implicity assumption later that all "branchesto" of log entries of a given changeset are equal. I think we should expect this, reason to fail would be some issue with the changeset per date clustering code, or the possibility of branching a subset of the working directory. I don't know if we should be concerned with the latter or not, you tell me. The "branchesto" equality assumption should be made explicit somewhere.

What still worries me is we scan the [p, i) revision index range without checking the revision branches. I think the code mostly works by relying on strong implicity graph properties about the revision ordering. Here is case I failed to test for my cvs-fu is too weak. I try to make the parent selection code pick up the wrong branch, by playing on the updated file dependencies (nodes are changesets, annotated with changed file versions):


. b,1.3.2.1.2.1
|
|
|                   . c,1.4
|                   |
|                   |
\_____. a,1.3.2.1   |
      |             |
      \_____________.
                    |
                    |
                    . a,1.3
                    |
                    |
                    . b,1.3
                    |
                    |
                    . c,1.3
                    |             

Why should the revision containing b,1.3.2.1.2.1 pick the revision containing a,1.3.2.1 as parent instead of the one containing c,1.4 (assuming the initial  wrong parent selected is the revision containing b,1.3).

--
Patrick Mézard





  


More information about the Mercurial-devel mailing list