Re: r38000 group

On Mon, 2009-09-07 at 18:30 +0200, Stephen Butler wrote:
> Quoting Neels Janosch Hofmeyr <neels_at_elego.de>:
>
> > Hi stsp, julianf,
> >
> > about the r38000 group --
> >
> > - I've fixed another duplicate notification in
> > r39096,39124,39125
> > - Straightened out some related code, making it "safer", in r39127
> > - I've found a couple other bugs still remaining for replace merges,
> > still unfixed. Test created, last tweaked in r39132.
> >
> > I'd say we include
> > r39096,39124,39125,39127
> > in r38000 if they can be merged easily (stsp?), and sign off the r38000
> > group more or less now. r38000's purpose is, after all, fixing the case
> > where a replace merge with TC straight breaks the merge and WC completely.
> >
> > Agreed?
>
> I agree. I've committed the suggested merge from trunk to the
> 1.6.x-r38000 branch. There was only one minor text conflict.
>
> I've voted for the r38000 group in ^/branches/1.6.x/STATUS. Can I get
> a witness?

Hi tree-conflict fans.

I still have some concerns about this back-port branch.

I have just gone through the branch and made a diff that represents the
whole state of the branch, and I have started to go through it and in so
doing I have written a log message based purely on what code changes I
see. I want to compare this with the original log messages of the
individual revisions, and see what light that throws on it, but I
haven't done so yet and it's end of day for me now.

Please look at the "###" concerns in the attached log message, repeated
here:

* subversion/libsvn_wc/update_editor.c
  (schedule_existing_item_for_re_add): Teach it do plain adds (without
    history) as well as copies, controlled by a new "modify_copyfrom"
    parameter.
  (do_entry_deletion): If we raise a tree conflict because the node is locally
    replaced, re-schedule the node for addition (without history) rather than
    trying to delete it.
### Without history sounds wrong. A replacement can be with or without history,
### and the schedule-add should correspondingly be with or without history.

* subversion/libsvn_client/merge.c
  (make_tree_conflict): New function, factored out of tree_conflict().
  (tree_conflict): Factor out a chunk into make_tree_conflict().
  (tree_conflict_on_add): New function, like tree_conflict() but can combine
    an existing action=delete conflict with a new action=add conflict to make
    an action=replace conflict.
### Better doc-string needed.
[But I'm not fussing about that.]

  (merge_file_added): Use tree_conflict_on_add() instead of tree_conflict() so
    an incoming file replacement results in a single tree conflict instead of
    an (abortive) attempt to add two tree conflicts on the same victim. If it
    conflicts with an existing file, determine the "reason" (added, deleted or
    obstructed) from the node's local scheduling.
### This schedule => reason sounds wrong. Expected state is no node present.
### Actual state is there is a file present. Therefore reason for conflict is
### "obstructed" (or we could describe it as "added").

The point here ^^ is that this is a merge, so the target is a branch,
and the state of stuff in the branch is dependent mainly on the branch
history, and only incidentally on the local WC scheduling state as well.
I'm not convinced that this will in any situation generate a wrong
result, however, so it might be just-about-acceptable.

I am concerned about the first "###" point, the "without history".

- Julian

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2398997