http://bugs.grommit.com/show_bug.cgi?id=432

           Summary: cdm may lose files removed in confusing merges
           Product: SCM Migration
           Version: unspecified
          Platform: All
        OS/Version: Solaris 11/Nevada
            Status: NEW
          Severity: major
          Priority: P1
         Component: cdm
        AssignedTo: scm-migration-dev at opensolaris.org
        ReportedBy: richlowe at richlowe.net


The duckwater gate contains a twisty set of changes represented thusly by
graphlog:

    o    6066:7897448f55b0 - branch merge
    |\
    | o    6065:e5a899f965e3 - branch merge
    | |\
    o---+  6064:a5d56bffd893 - Igor's per user test fixes.
    |/ /
    | o    6063:2b9855581c1f - branch merge
    | |\
    | | o    6062:afe598168e96 - branch merge
    | | |\
    | | | o    6061:6b0397a70cef - branch merge
    | | | |\

6065 is a merge of 6063 and the revision in onnv-gate at the time
6064 is a merge of those same revisions, but with some additional changes
6066 is a merge of 6064 and 6065

(you can see these in more detail if you bringover the duckwater gate
that's on opensolaris.org)

During this, 3 files were removed (probably by mistake), because both
parents of the merge in 6066 are local, and because the removals are
only present in that particular changeset, we lose them due to the
logic in ActiveList._build which attempts to prevent showing phantom
renames
(http://src.opensolaris.org/source/xref/scm-migration/onnv-scm/usr/src/tools/onbld/Scm/WorkSpace.py?r=6166#200)

The problem that caused this code to be introduced (a rename in the
parent being shown as local after a merge with it) is something I can
no longer reproduce.

The logic in the link above is unpleasantly heuristic, and also
incorrect.

The least invasive fix here would be to add a check as to whether p[1]
was outgoing, and only drop a file if it was not.  That just makes it
more heuristic however, and I'm not convinced it is correct (though I
can't find a situation it fails, that doesn't mean there isn't one).

Given I can't reproduce the original problem, I'm tempted to remove
that code (given its unpleasant nature), and, should the
problem it was added to fix resurface, I'll have a means to reproduce
that issue (or at least a workspace exhibiting it) by which to improve
the code to avoid it.

For reference, the situation in which we would see phantom renames is,
I believe, thus:

  Removal (and addition, in some cases) become part of both the
  changeset where they were initially done, and a merge changeset if
  the original change is in the lineage of the second parent of the
  merge.

  To see a removal as 'local', that in fact happened in the parent,
  the initial removal would not be an outgoing change, and thus would
  not be in the active list.

  A merge, with that removal in the history behind the 2nd parent,
  would be outgoing.

  The status (hg status, actually mercurial.localrepo.status())
  comparing the parenttip, to the localttip would have to include
  that file as a removal, 'R'.

  We would see the removal in both the status, and the changeset, and
  would thus list it as removed as compared to the parent.

  In no situation I can currently contrive does the above happen, I
  cannot find a way for the status call we make to include the already
  removed file as 'removed'.

  (note, that 'hg status' may, the call we make does not, hg status
  --rev seems to use the first parent revision, when given the '.'
  revision, we compare the workspace parenttip to the working copy)

Opinion on which of the above options to take are solicited (remove the code,
since it's nasty and the situation it 'fixes' can't be reproduced, or fix this
specific case)  I think both are somewhat equal in risk, I think the removal is
probably preferable given I'm familiar with the symptoms there.

In the interest of full disclosure, I have also in the past thought that this
case no longer applied and removed the code locally, and soon after saw the
issue again.  I can't say for sure the same thing won't happen again.


-- 
Configure bugmail: http://bugs.grommit.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

Reply via email to