Re: branch tree-conflicts-notify: how many columns?

[Neels J Hofmeyr <ne...@elego.de>]

Neels J. Hofmeyr wrote:
> 
> Stephen Butler wrote:
[...]
>>> The check_tree_conflict() vs. svn_wc_conflicted_p() discussion made me
>>> think: In the tests, I guess we should also try to run the same operation
>>> that causes the conflicts again, to check that the persisting conflicts
>>> aren't re-raised or missed altogether. Is this accounted for yet?
>> No, currently we test only fresh tree conflicts.  Good point.
> 
> I'll try that now if you haven't already.

No, actually, got caught up in more pressing matters, as you can see in the
commit log of r33891.

~Neels

Re: branch tree-conflicts-notify: how many columns?

["Neels J. Hofmeyr" <ne...@elego.de>]

Stephen Butler wrote:
> Quoting Neels J Hofmeyr <ne...@elego.de>:
> 
>> Neels J. Hofmeyr wrote:
>>>
>>> Stephen Butler wrote:
>> [...]
>>>>> The check_tree_conflict() vs. svn_wc_conflicted_p() discussion made me
>>>>> think: In the tests, I guess we should also try to run the same
>>>>> operation
>>>>> that causes the conflicts again, to check that the persisting
>>>>> conflicts
>>>>> aren't re-raised or missed altogether. Is this accounted for yet?
>>>> No, currently we test only fresh tree conflicts.  Good point.
>>>
>>> I'll try that now if you haven't already.
>>
>> No, actually, got caught up in more pressing matters, as you can see
>> in the
>> commit log of r33891.
> 
> Hello tree-conflict fans,
> 
> Neels, I believe the changes in r33891 are actually a part of checking that
> persistent tree conflicts "aren't re-raised or missed altogether."  But I

No, they are not:
[[[
Skip files added into tree-conflicted dirs (and don't notify).
Skip deletion of tree-conflicted nodes.
...
]]]

Firstly, it fixes a notification that I added wrongly, but I left some code
in a comment because we will most certainly want to use it if encountering a
persistent conflict, once we want to deal with that stuff.

Secondly, it skips the actual deletion if the delete was discovered to cause
a tree-conflict, which has an effect on notification, which is the reason
why I included that.

> think we should solve that problem in one go rather than piecemeal.
> Otherwise we'll waste a lot of time massaging test expectations from one
> intermediate (bad) state to another.
+1
But on certain changes it is good to review most relevant test suites to
find errors or understand situations. It helped me today ;)

> 
> Attached is a diff to the tree-conflicts-notify branch, just some hacking
> I've done today to try to skip changes to the working copy in the presence
> of existing tree conflicts.  BTW I cribbed a few good things from Julian's
> skip-inside-conflicts-3.patch from a couple days ago.

So, you're not quite done with it yet, right? Let me know if you'd be happy
to continue on trunk (or whether you've completed it on the branch), see below.

> 
> But I think we should tidy up and merge-to-trunk the pre-r33891 state of
> our branch, because we've accomplished our first goal: per-victim tree
> conflict notification in update/switch output.  Skipping conflicted trees,
> handling merge output, UI improvements (e.g. 'CDM'), etc. can be done in
> future patches to trunk or in short-term branches as needed.
> 
> What do you (all) think?

There's just one case I found today that we'd break when merging as-is:
we've got per-victim conflict notification for update/switch, and we changed
the notification code to count them by the new tree_conflicted flag.
However, merge doesn't use that flag yet and displays tree-conflicts as a
"text-conflict" on the parent dir. The result can be seen in merge_tests.py
103 (on branch tree-conflicts-notify r33895): A tree conflict is counted as
a text conflict.

So do we want to tackle merge notification on trunk? I don't mind either
way. But we're slowly drifting farther and farther from trunk. And it's
slowly becoming a "let's do everything on the branch"-branch instead of an
"I just want to try this first"-branch. If no-one objects (or is quicker
than me), I'll reintegrate.

~Neels

Re: branch tree-conflicts-notify: how many columns?

[Stephen Butler <sb...@elego.de>]

Quoting Neels J Hofmeyr <ne...@elego.de>:

> Neels J. Hofmeyr wrote:
>>
>> Stephen Butler wrote:
> [...]
>>>> The check_tree_conflict() vs. svn_wc_conflicted_p() discussion made me
>>>> think: In the tests, I guess we should also try to run the same operation
>>>> that causes the conflicts again, to check that the persisting conflicts
>>>> aren't re-raised or missed altogether. Is this accounted for yet?
>>> No, currently we test only fresh tree conflicts.  Good point.
>>
>> I'll try that now if you haven't already.
>
> No, actually, got caught up in more pressing matters, as you can see in the
> commit log of r33891.

Hello tree-conflict fans,

Neels, I believe the changes in r33891 are actually a part of checking that
persistent tree conflicts "aren't re-raised or missed altogether."  But I
think we should solve that problem in one go rather than piecemeal.
Otherwise we'll waste a lot of time massaging test expectations from one
intermediate (bad) state to another.

Attached is a diff to the tree-conflicts-notify branch, just some hacking
I've done today to try to skip changes to the working copy in the presence
of existing tree conflicts.  BTW I cribbed a few good things from Julian's
skip-inside-conflicts-3.patch from a couple days ago.

But I think we should tidy up and merge-to-trunk the pre-r33891 state of
our branch, because we've accomplished our first goal: per-victim tree
conflict notification in update/switch output.  Skipping conflicted trees,
handling merge output, UI improvements (e.g. 'CDM'), etc. can be done in
future patches to trunk or in short-term branches as needed.

What do you (all) think?

Steve


Draft commit message for attached patch:

On the tree-conflicts-notify branch:  Try to skip tree-conflicted
items.  So far, I have edited the dir callbacks only.  The file
callbacks, test expectations, etc. remain.  Oh, and testing to see
if it works at all.

[[[

* subversion/libsvn_wc/update_editor.c
   (edit_baton): New boolean field inside_a_tree_conflict.
   (maybe_bump_dir_info): Don't call complete_directory() if inside a
    tree conflict.
   (check_tree_conflict): Rename to ...
   (maybe_raise_tree_conflict): ... and add a todo comment.
   (tree_has_local_mods,
    add_file,
    open_file): Track rename of check_tree_conflict().
   (do_entry_deletion,
    add_directory,
    open_directory,
    close_directory): Track rename of check_tree_conflict().  Skip
    almost everything if inside a tree conflict.  Otherwise, if the dir
    has an existing or freshly-raised tree conflict, notify the user
    and skip almost everything else.  Otherwise proceed normally.
   (change_dir_prop): Skip everything if inside a tree conflict.

]]]



Stephen Butler | Software Developer
elego Software Solutions GmbH
Gustav-Meyer-Allee 25 | 13355 Berlin | Germany
fon: +49 30 2345 8696 | mobile: +49 163 25 45 015
fax: +49 30 2345 8695 | http://www.elegosoft.com
Geschäftsführer: Olaf Wagner | Sitz der Gesellschaft: Berlin
Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194