You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Sperling <st...@elego.de> on 2008/09/12 11:10:07 UTC
Tree Conflicts and User Interface (was: Re: [PATCH]
Tree-conflicts: do_entry_deletion segfault)
On Fri, Sep 12, 2008 at 12:44:11AM +0200, Neels Hofmeyr wrote:
>
>
> Julian Foad wrote:
> > Bear in mind that "reported as" and "stored as" are two completely
> > different things. I'm not intending to change the storage, only the
> > reporting. The storage can stay in the parent directory.
>
> Oh, I see. In that case...:
This discussion is important!
Please keep in mind that the current way we're reporting tree
conflicts is nearly useless. I don't want it to end up in a
release like this. This is the most urgent problem on the
tree-conflicts branch IMO. We really need to fix this!
> One day, stsp and I were discussing the _reporting_ of tree conflicts, and
> thought that maybe victims and storage holder (i.e. parent directory) should
> have distinct markings; C for parent dir, as in "conflicted", and V for an
> actual victim node where the conflict manifested.
>
> We also thought it would be nice to have an additional column for tree
> conflicts, to be able to still see the other markings on the
> tree-conflict-marked files.
I'm still not entirely convinced about that third column.
There is a bit of a problem with the way Subversion currently
displays information during update/status, and how to encode
tree conflict information in a compatible way.
To recap, what we currently have (in 1.5 and trunk) is two columns.
One column for displaying information about files/directories,
and one for displaying information about properties.
Files and properties can be text conflicted (signalled by 'C'),
directories cannot be conflicted.
Enter tree conflicts:
* Files can now be text conflicted, or be victims of a tree conflict,
or both.
* Properties can be text conflicted.
* Directories can now contain tree conflict victims, or be tree
conflict victims themselves, or both.
Currently, on the tree-conflicts branch we're only displaying
information about directories which contain tree conflicts.
This works sort-of well, by showing a 'C' in the first column
of the status output for a directory.
For example:
CM foo.c (a file with text conflict and property mod)
C dir (a directory containing tree conflict victims)
To get information on individual victims inside a directory,
the user has to run 'svn info' on that directory, and will
then get a display like this:
Tree conflicts:
The update edited the file 'bar.c'.
You have deleted 'bar.c' locally.
Maybe you renamed it?
This UI is less than ideal. It's fine for use during development
to see whether victims have been detected correctly, but it is
not something I'd like to use when actually working with Subversion.
I'd rather have something like this:
$ svn update
CM foo.c (a file with text conflict and property mod)
C dir (a directory containing tree conflict victims)
V dir/bar.c (a tree conflict victim)
Updated to revision 42.
There was 1 text conflict and 1 tree conflict.
$
Note the conflict summary display which should be rather handy
once those numbers get big. Imagine not getting a summary when
you actually run into something like this:
There were 33 text conflicts and 20 tree conflicts.
But there is a problem. What if dir/bar.c was text conflicted,
had property conflicts, and are also the victim of a tree conflict?
Note that such a condition may be the result of multiple updates
or merges without conflict resolution done in between.
If we only had 'C' and 'V' and two columns, we could encode
this situation as:
CC dir/bar.c (hides the tree conflict)
VC dir/bar.c (hides the text conflict)
CV dir/bar.c (ambiguous -- does the 'C' in the first column indicate
text or property conflict? And inconsistent: the second
column is used for something other than information
about properties -- or the first column is used for
something other than files, depending on what 'CV'
should mean)
VV dir/bar.c (Bah, no one will grok this.)
So imagine we had a third column to show tree conflict status:
$ svn update
CM foo.c (a file with text conflict and property mod)
C dir (a directory containing tree conflict victims)
V dir/bar.c (a tree conflict victim)
Updated to revision 42.
There was 1 text conflict and 1 tree conflict.
$
We can now encode every conflict individually and unambiguously.
Encoding all three types of conflict at once is easy:
CCV dir/bar.c (a file that is text conflicted, has property
conflicts, and is the victim of a tree conflict)
CV dir/dir2 (a directory which has property conflicts, and is
the victim of a tree conflict)
CC dir/dir2 (a directory which has property conflicts, and contains
tree conflict victims)
However, I see two drawbacks:
a) For easy parsing, the 3rd column would always have to be present
in the output, even if no tree conflicts are reported. This wastes
a single charater and possibly breaks some wrapper scripts, but
it's not that big of a problem.
b) The 3rd column would only ever show status about conflicts.
The only letters that could appear there would be 'C' and 'V'.
No 'U', not 'M', etc. As such, adding this column breaks with
the current design of the UI (one column for *anything* about
files, one column for *anything* about properties).
And by the way, I think that a proper description of all conflicts
should be printed when the user tries to commit:
$ svn commit
Commit failed (reasons follow):
foo.c is text-conflicted.
dir/bar.c is a tree conflict victim:
dir/bar.c has been edited in the repository.
You have deleted dir/bar.c locally.
Maybe you renamed it?
$
This is only a rough idea, though. We could also sort output by type
of conflict, change the wording, etc. The point is that svn should
become more verbose, to accurately explain complex conflict situations
to the user. Just printing "'foo.c' remains in conflict" as we do now
is not gonna help people much. This is a big chance to improve the
user interface wrt conflicts, we should use it.
Also, an additional way of getting at this information without trying
to commit would be nice. I don't really think that using 'svn info'
for this is right, but I don't have any better ideas either:
$ svn conflicts (new subcommand, name sounds odd)
$ svn show-conflicts (new subcommand, too long to type)
$ svn status --verbose or svn status -v (important information hidden
behind some option switch,
not good)
$ svn resolve --show-conflicts=tree (too long to type, user might not
want to resolve yet)
Hmmm... :-/
> What do you (all) think?
>
> ~Neels
What he asked :)
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Mergeinfo for skipped paths? (was: Re: Tree Conflicts and User Interface)
Posted by Danil Shopyrin <da...@gmail.com>.
> Write access to some siblings of skipped path can be restricted. In that case:
> A) user will be disappointed because commit will be prohibited;
>
> B) inconsistent mergeinfo will be committed even if user will manually
> remove mergeinfo on write-restricted paths.
>
> Write-restriction rarely occurs in the open-source development but
> it's a common practice in "the industry". And write-restricted users
> are usually not-a-very-experienced programmers. And they can be not
> very familiar with advanced Subversion topics.
It's fair to say that this problem can be avoided if we will not write
mergeinfo for unaffected paths. Am I understand properly, that we are
not requested to write explicit mergeinfo to unaffected siblings of
the skipped paths in that case?
E.g. if not-writing-explicit-mergeinfo-for-unaffected-paths discussed
at http://svn.haxx.se/dev/archive-2008-09/0443.shtml will be
implemented, can we get the following mergeinfo with your approach?
Properties on 'branch':
svn:mergeinfo
/trunk:2,4* <--- Non-inheritable mergeinfo.
Properties on 'branch/bar.c':
svn:mergeinfo
/trunk/bar.c:2,4
Properties on 'branch/subdir':
svn:mergeinfo
<null> <-- subdir isn't somehow affected by the merge.
--
Danil
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Mergeinfo for skipped paths? (was: Re: Tree Conflicts and User Interface)
Posted by Paul Burba <pt...@gmail.com>.
On Wed, Sep 17, 2008 at 8:13 AM, Stefan Sperling <st...@elego.de> wrote:
> On Tue, Sep 16, 2008 at 08:22:51PM -0400, Paul Burba wrote:
>> Hi Stefan,
>>
>> Let's assume your target branch looks like this (these other paths
>> under trunk help better illustrate what I'm going to propose):
>>
>> /branch
>> /branch/bar.c
>> /branch/subdir
>>
>> We tweak the merge logic to set non-inheritable mergeinfo on the merge
>> target describing the merge, and normal mergeinfo on all the target's
>> present children, like this:
>>
>> Properties on 'branch':
>> svn:mergeinfo
>> /trunk:2,4* <--- Non-inheritable mergeinfo.
>> Properties on 'branch/bar.c':
>> svn:mergeinfo
>> /trunk/bar.c:2,4
>> Properties on 'branch/subdir':
>> svn:mergeinfo
>> /trunk/subdir:2,4
>
> Sounds like a plan!
>
>> Of course if the skipped path was a deeper subtree and not an
>> immediate child of our merge target we would adjust accordingly and
>> set the non-inheritable mergeinfo on the skipped path's immediate
>> parent and the normal mergeinfo on the skipped path's siblings.
>
> Yes.
>
>> If we commit the above merge and then merge -c3 into branch now foo.c
>> is added but it only has mergeinfo for r2-3:
>>
>> Properties on 'branch':
>> svn:mergeinfo
>> /trunk:2-3,4*
>> Properties on 'branch/foo.c':
>> svn:mergeinfo
>> /trunk/foo.c:2-3
>> Properties on 'branch/bar.c':
>> svn:mergeinfo
>> /trunk/bar.c:2-4
>> Properties on 'branch/subdir':
>> svn:mergeinfo
>> /trunk/subdir:2-4
>>
>> Note: In my ad hoc testing foo.c isn't getting r2 in it's mergeinfo,
>> it should, but that can be fixed. Assuming that we fix this, if we
>> re-merge -r4 to branch then foo.c gets that change and all the
>> mergeinfo elides* up to branch and we are left with is this:
>>
>> Properties on 'branch':
>> svn:mergeinfo
>> /trunk:2-4
>>
>> I think this change is fairly simple, we already catch similar cases -
>> see libsvn_client/merge.c:get_mergeinfo_paths(). The only difference
>> is that we can't identify these paths at the start of the merge but
>> must catch them once we realize they are skipped. The
>> notification_receiver_baton_t struct keeps track of these already in
>> its skipped_paths member so this shouldn't be too hard.
>
> It's nice to know that you have a simple solution.
> I was afraid of the solution being really complicated,
> but your proposal sounds very straightforward.
...and I hope it is straightforward. I'm sure there will be some
complications...there always are with merge tracking.
>> I can crank out a patch that does what I propose in fairly short order
>> if this idea solves your problem.
>
> Great. I'll gladly test anything you throw at me.
I'll start work on a branch for this sometime this week if no
objections come up.
>> * In the interests of full disclosure, in another thread I am
>> proposing the elimination of automatic mergeinfo elision as a fairly
>> useless feature...ironically here is a case where it is nice to have
>> :-\
>
> So we keep it? :)
I think we should keep elision, but not have it happen automatically
but rather upon request. Rather than going into it too much here just
see this thread: http://svn.haxx.se/dev/archive-2008-09/0595.shtml
Paul
>> > XXX big ugly hack idea: Could we extend the mergeinfo
>> > format so that parent directories can record "negative"
>> > mergeinfo for direct children which were skipped?
>>
>> Befitting a "big ugly hack" there's a problem :-) What if, in your
>> example, r4 also affected trunk directly (e.g. we added a svn:ignore
>> property). It could get a bit ugly if we want a way to record that r4
>> affected trunk, but not one of its (currently) non-existent children.
>
> Oh, I was thinking about extending the mergeinfo format to include
> names for skipped child paths, something like
>
> Properties on 'branch':
> svn:mergeinfo
> /trunk:2-4,foo.c:!3
> ^^^^^
> But your solution is much better, so let's forget about this :)
>
> Thanks!
> Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Mergeinfo for skipped paths? (was: Re: Tree Conflicts and User
Interface)
Posted by Stefan Sperling <st...@elego.de>.
On Tue, Sep 16, 2008 at 08:22:51PM -0400, Paul Burba wrote:
> Hi Stefan,
>
> Let's assume your target branch looks like this (these other paths
> under trunk help better illustrate what I'm going to propose):
>
> /branch
> /branch/bar.c
> /branch/subdir
>
> We tweak the merge logic to set non-inheritable mergeinfo on the merge
> target describing the merge, and normal mergeinfo on all the target's
> present children, like this:
>
> Properties on 'branch':
> svn:mergeinfo
> /trunk:2,4* <--- Non-inheritable mergeinfo.
> Properties on 'branch/bar.c':
> svn:mergeinfo
> /trunk/bar.c:2,4
> Properties on 'branch/subdir':
> svn:mergeinfo
> /trunk/subdir:2,4
Sounds like a plan!
> Of course if the skipped path was a deeper subtree and not an
> immediate child of our merge target we would adjust accordingly and
> set the non-inheritable mergeinfo on the skipped path's immediate
> parent and the normal mergeinfo on the skipped path's siblings.
Yes.
> If we commit the above merge and then merge -c3 into branch now foo.c
> is added but it only has mergeinfo for r2-3:
>
> Properties on 'branch':
> svn:mergeinfo
> /trunk:2-3,4*
> Properties on 'branch/foo.c':
> svn:mergeinfo
> /trunk/foo.c:2-3
> Properties on 'branch/bar.c':
> svn:mergeinfo
> /trunk/bar.c:2-4
> Properties on 'branch/subdir':
> svn:mergeinfo
> /trunk/subdir:2-4
>
> Note: In my ad hoc testing foo.c isn't getting r2 in it's mergeinfo,
> it should, but that can be fixed. Assuming that we fix this, if we
> re-merge -r4 to branch then foo.c gets that change and all the
> mergeinfo elides* up to branch and we are left with is this:
>
> Properties on 'branch':
> svn:mergeinfo
> /trunk:2-4
>
> I think this change is fairly simple, we already catch similar cases -
> see libsvn_client/merge.c:get_mergeinfo_paths(). The only difference
> is that we can't identify these paths at the start of the merge but
> must catch them once we realize they are skipped. The
> notification_receiver_baton_t struct keeps track of these already in
> its skipped_paths member so this shouldn't be too hard.
It's nice to know that you have a simple solution.
I was afraid of the solution being really complicated,
but your proposal sounds very straightforward.
> I can crank out a patch that does what I propose in fairly short order
> if this idea solves your problem.
Great. I'll gladly test anything you throw at me.
> * In the interests of full disclosure, in another thread I am
> proposing the elimination of automatic mergeinfo elision as a fairly
> useless feature...ironically here is a case where it is nice to have
> :-\
So we keep it? :)
> > XXX big ugly hack idea: Could we extend the mergeinfo
> > format so that parent directories can record "negative"
> > mergeinfo for direct children which were skipped?
>
> Befitting a "big ugly hack" there's a problem :-) What if, in your
> example, r4 also affected trunk directly (e.g. we added a svn:ignore
> property). It could get a bit ugly if we want a way to record that r4
> affected trunk, but not one of its (currently) non-existent children.
Oh, I was thinking about extending the mergeinfo format to include
names for skipped child paths, something like
Properties on 'branch':
svn:mergeinfo
/trunk:2-4,foo.c:!3
^^^^^
But your solution is much better, so let's forget about this :)
Thanks!
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Mergeinfo for skipped paths? (was: Re: Tree Conflicts and User Interface)
Posted by Danil Shopyrin <da...@gmail.com>.
> There is one serious problem with this solution: It won't work if r4
> affects 'trunk' directly (e.g. a prop change on 'trunk').
>
> Because in that case the prop change in r4 will be merged into
> 'branch' so we need to record it in 'branch's mergeinfo. But if we
> add '/trunk:4' to 'branch's mergeinfo that implies that
> '/branch/foo.c' got r4, which of course it didn't since 'foo.c' isn't
> present yet. I mentioned this problem earlier in this thread - see
> "Befitting a "big ugly hack" there's a problem".
>
> Do you understand why this is a problem? If not let's resolve that first!
We've missed this problem. We understand it and the proposed solution
isn't viable because of it. Sorry.
>> Only part of this revision is merged. It's better
>> than write that r4 is merged "non-inheritable", because
>> 'non-inheritable' merged revisions make no sense for an average user.
>
> So normal 'inheritable' mergeinfo makes sense to users, but
> non-inheritable mergeinfo doesn't? Heck, I'm not very fond of
> mergeinfo as an inheritable property, but it's what we have at the
> moment and I don't see how non-inheritable properties are
> fundamentally more complex.
We're not trying to say that users understand inheritable and don't
understand non-inheritable mergeinfo records. The problem is in the
level of abstraction (keep in mind that we're talking about "very
average Subversion user").
* What's happened: revision r4 is merged incompletely.
* What's recorded: non-inheritable mergeinfo for target and
inheritable mergeinfo for all siblings of the skipped path.
It's not so easy to figure out "what's happened" from "what's
recorded". It will be far more easy for users if "incomplete merge"
will be recorded explicitly.
That's what I'm trying to say.
> Also, am I to take it that you are proposing removing non-inheritable
> mergeinfo?
I'm not proposing to completely remove non-inheritable mergeinfo. I'm
arguing to find a better solution for this problem.
> Unless we are planning on disabling merge-tracking when
> performing shallow merges (i.e. with depths other than infinity), when
> merging into shallow working copies, working copies with switched
> subtrees, or WCs with subtrees missing due to authorization
> restrictions, then we need non-inheritable mergeinfo. These problems
> and the problem we are discussing in this thread are all subsets of
> the same basic issue: How do we account for subtrees that are
> (potentially) affected by a merge but that are missing from the
> working copy. The solution to this class of problems is currently
> non-inheritable mergeinfo. I'd like whatever solution is settled on
> for this problem to the solution we use for this whole class of
> problems.
Am I understand correctly, that with the current approach explicit
mergeinfo will be written to all siblings of skipped (/switched/etc.)
path? There is a problem with access rights management if it is.
Write access to some siblings of skipped path can be restricted. In that case:
A) user will be disappointed because commit will be prohibited;
B) inconsistent mergeinfo will be committed even if user will manually
remove mergeinfo on write-restricted paths.
Write-restriction rarely occurs in the open-source development but
it's a common practice in "the industry". And write-restricted users
are usually not-a-very-experienced programmers. And they can be not
very familiar with advanced Subversion topics.
> Note: This example assumes the ideas proposed in
> http://svn.haxx.se/dev/archive-2008-09/0443.shtml have been
> implemented (but that is hardly a done deal). Specifically, r3
> doesn't affect 'bar.c' so that revision is not recorded in 'bar.c's
> mergeinfo. Not a big deal, but worth noting.
Ok.
>> Now user can find out that r4 is still not merged completely
>
> Seriously? Can you state the general steps one would use to determine
> this? Before you do, keep in mind what "real" mergeinfo looks like;
> go run svn pg svn:mergeinfo -vR on any of the Subversion branches to
> see what I mean. Then tell me how easy it will be to spot a single
> revision or revision range that exists on one subtree but not on the
> others.
As you pointed, the whole proposed approach is not viable (because of
possible target's properties change). But the initial idea will be as
follows:
* user sees that mergeinfo is recorded to some sub-paths but not
recorded to the target;
* this means that the revision isn't merged completely;
* user is not requested to clearly understand the difference between
inheritable and non-inheritable mergeinfo.
What do you think about following solution? In the discussed example
we can do the following:
1) record on target that r4 is merged incompletely
2) record r4 on all affected (and not skipped) paths
Generated mergeinfo can look like this:
Properties on 'branch':
svn:mergeinfo
/trunk:4! (r4 is merged incompletely)
Properties on 'branch/bar.c':
svn:mergeinfo
/trunk/bar.c:4
Properties on 'branch/subdir':
svn:mergeinfo
<null>
I understand that this approach requires introduction of additional
semantics. But I just want to investigate, is it viable or not.
> I'd go as far as to say that the non-inheritable marker '*' makes it
> somewhat easier to spot what revisions haven't been merged completely.
Yes, it can be. But there are following problems:
1) write-restriction problem that's described above;
2) it's better to see 'what's changed' than 'what's not changed'.
Usually, the count of changed paths is smaller than count of unchanged
ones.
>> on branch and merge it once again.
>>
>> We will get the following mergeinfo:
>>
>> Properties on 'branch':
>> svn:mergeinfo
>> /trunk:3,4
>> Properties on 'branch/bar.c':
>> svn:mergeinfo
>> <empty>
>> Properties on 'branch/foo.c':
>> svn:mergeinfo
>> <empty>
>> Properties on 'branch/subdir':
>> svn:mergeinfo
>> <empty>
>>
>> Note than now we can elide mergeinfo on 'branch/bar.c'.
>
> Actually no, until we rewrite the elision code this won't work.
> Because when you perform the merge above, at the end of the merge,
> before elision is attempted you will have mergeinfo that looks like
> this:
I agree. But I think it's not a good idea to discuss elision in
details in this thread.
--
Danil
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Mergeinfo for skipped paths? (was: Re: Tree Conflicts and User Interface)
Posted by Paul Burba <pt...@gmail.com>.
On Mon, Sep 22, 2008 at 8:04 AM, Danil Shopyrin
<da...@gmail.com> wrote:
>> Let's assume your target branch looks like this (these other paths
>> under trunk help better illustrate what I'm going to propose):
>>
>> /branch
>> /branch/bar.c
>> /branch/subdir
>>
>> We tweak the merge logic to set non-inheritable mergeinfo on the merge
>> target describing the merge, and normal mergeinfo on all the target's
>> present children, like this:
>>
>> Properties on 'branch':
>> svn:mergeinfo
>> /trunk:2,4* <--- Non-inheritable mergeinfo.
>> Properties on 'branch/bar.c':
>> svn:mergeinfo
>> /trunk/bar.c:2,4
>> Properties on 'branch/subdir':
>> svn:mergeinfo
>> /trunk/subdir:2,4
>>
>> Of course if the skipped path was a deeper subtree and not an
>> immediate child of our merge target we would adjust accordingly and
>> set the non-inheritable mergeinfo on the skipped path's immediate
>> parent and the normal mergeinfo on the skipped path's siblings.
>>
>> If we commit the above merge and then merge -c3 into branch now foo.c
>> is added but it only has mergeinfo for r2-3:
>>
>> Properties on 'branch':
>> svn:mergeinfo
>> /trunk:2-3,4*
>> Properties on 'branch/foo.c':
>> svn:mergeinfo
>> /trunk/foo.c:2-3
>> Properties on 'branch/bar.c':
>> svn:mergeinfo
>> /trunk/bar.c:2-4
>> Properties on 'branch/subdir':
>> svn:mergeinfo
>> /trunk/subdir:2-4
>>
>> Note: In my ad hoc testing foo.c isn't getting r2 in it's mergeinfo,
>> it should, but that can be fixed. Assuming that we fix this, if we
>> re-merge -r4 to branch then foo.c gets that change and all the
>> mergeinfo elides* up to branch and we are left with is this:
>>
>> Properties on 'branch':
>> svn:mergeinfo
>> /trunk:2-4
>>
>> I think this change is fairly simple, we already catch similar cases -
>> see libsvn_client/merge.c:get_mergeinfo_paths(). The only difference
>> is that we can't identify these paths at the start of the merge but
>> must catch them once we realize they are skipped. The
>> notification_receiver_baton_t struct keeps track of these already in
>> its skipped_paths member so this shouldn't be too hard.
>>
>> I can crank out a patch that does what I propose in fairly short order
>> if this idea solves your problem.
>>
>> * In the interests of full disclosure, in another thread I am
>> proposing the elimination of automatic mergeinfo elision as a fairly
>> useless feature...ironically here is a case where it is nice to have
>> :-\
>>
>>> XXX big ugly hack idea: Could we extend the mergeinfo
>>> format so that parent directories can record "negative"
>>> mergeinfo for direct children which were skipped?
>>
>> Befitting a "big ugly hack" there's a problem :-) What if, in your
>> example, r4 also affected trunk directly (e.g. we added a svn:ignore
>> property). It could get a bit ugly if we want a way to record that r4
>> affected trunk, but not one of its (currently) non-existent children.
>
> We've discussed this informally at VisualSVN and have found
> alternative solution.
>
> Let's assume that our target branch looks like this:
> /branch
> /branch/bar.c
> /branch/subdir
>
> And we have /trunk/foo.c added in r3. Files /trunk/bar.c and
> /trunk/foo.c are both changed in r4.
>
> We are merging r4 to the branch. Since r4 contains changes for an
> absent file (i.e. foo.c will be skipped) this revision can't be
> treated as regularly merged into branch. So the merge should produce
> the following mergeinfo in the trunk:
>
> Properties on 'branch':
> svn:mergeinfo
> <empty> //r4 isn't merged completely so we can't claim that it's merged!
> Properties on 'branch/bar.c':
> svn:mergeinfo
> /trunk/bar.c:4
> Properties on 'branch/subdir':
> svn:mergeinfo
> <empty>
>
> From our point of view, it's far more consistent, because r4 isn't
> actually merged.
There is one serious problem with this solution: It won't work if r4
affects 'trunk' directly (e.g. a prop change on 'trunk').
Because in that case the prop change in r4 will be merged into
'branch' so we need to record it in 'branch's mergeinfo. But if we
add '/trunk:4' to 'branch's mergeinfo that implies that
'/branch/foo.c' got r4, which of course it didn't since 'foo.c' isn't
present yet. I mentioned this problem earlier in this thread - see
"Befitting a "big ugly hack" there's a problem".
Do you understand why this is a problem? If not let's resolve that first!
> Only part of this revision is merged. It's better
> than write that r4 is merged "non-inheritable", because
> 'non-inheritable' merged revisions make no sense for an average user.
So normal 'inheritable' mergeinfo makes sense to users, but
non-inheritable mergeinfo doesn't? Heck, I'm not very fond of
mergeinfo as an inheritable property, but it's what we have at the
moment and I don't see how non-inheritable properties are
fundamentally more complex.
Also, am I to take it that you are proposing removing non-inheritable
mergeinfo? Unless we are planning on disabling merge-tracking when
performing shallow merges (i.e. with depths other than infinity), when
merging into shallow working copies, working copies with switched
subtrees, or WCs with subtrees missing due to authorization
restrictions, then we need non-inheritable mergeinfo. These problems
and the problem we are discussing in this thread are all subsets of
the same basic issue: How do we account for subtrees that are
(potentially) affected by a merge but that are missing from the
working copy. The solution to this class of problems is currently
non-inheritable mergeinfo. I'd like whatever solution is settled on
for this problem to the solution we use for this whole class of
problems.
> Note that we record that /trunk/bar.c has merged changes from r4. So
> there will be no conflicts in the future.
>
> Then we are merging r3 and get the following mergeinfo:
> Properties on 'branch':
> svn:mergeinfo
> /trunk:3
> Properties on 'branch/bar.c':
> svn:mergeinfo
> /trunk/bar.c:4
> Properties on 'branch/foo.c':
> svn:mergeinfo
> <empty>
> Properties on 'branch/subdir':
> svn:mergeinfo
> <empty>
Note: This example assumes the ideas proposed in
http://svn.haxx.se/dev/archive-2008-09/0443.shtml have been
implemented (but that is hardly a done deal). Specifically, r3
doesn't affect 'bar.c' so that revision is not recorded in 'bar.c's
mergeinfo. Not a big deal, but worth noting.
> Now user can find out that r4 is still not merged completely
Seriously? Can you state the general steps one would use to determine
this? Before you do, keep in mind what "real" mergeinfo looks like;
go run svn pg svn:mergeinfo -vR on any of the Subversion branches to
see what I mean. Then tell me how easy it will be to spot a single
revision or revision range that exists on one subtree but not on the
others.
I'd go as far as to say that the non-inheritable marker '*' makes it
somewhat easier to spot what revisions haven't been merged completely.
> on branch and merge it once again.
>
> We will get the following mergeinfo:
>
> Properties on 'branch':
> svn:mergeinfo
> /trunk:3,4
> Properties on 'branch/bar.c':
> svn:mergeinfo
> <empty>
> Properties on 'branch/foo.c':
> svn:mergeinfo
> <empty>
> Properties on 'branch/subdir':
> svn:mergeinfo
> <empty>
>
> Note than now we can elide mergeinfo on 'branch/bar.c'.
Actually no, until we rewrite the elision code this won't work.
Because when you perform the merge above, at the end of the merge,
before elision is attempted you will have mergeinfo that looks like
this:
Properties on 'branch':
svn:mergeinfo
/trunk:3,4
Properties on 'branch/bar.c':
svn:mergeinfo
/trunk/bar.c:4
The mergeinfo on '/branch/bar.c' will not elide to 'branch'. The
elision code needs to be rewritten to contact the server to check that
r3 doesn't impact '/branch/bar.c' and can thus be ignored for purposes
of eliding. Remember that "real" world mergeinfo is not going to be
this simple and thus we might contact the repository a *lot* for this
purpose and pay steep performance penalty for doing so.
> Advantages:
> * user understand that some revisions are merged incompletely;
As stated above I couldn't disagree more with this statement.
> * less mergeinfo records generated;
Agreed, less explicit mergeinfo may be generated. It's not correct in
all situations, but there is less of it.
> * user can easily figure our why r4 mergeinfo is written on /branch/bar.c
I don't want to sound like a broken record, but this behavior doesn't
seem any clearer than what I am proposing.
Paul
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Mergeinfo for skipped paths? (was: Re: Tree Conflicts and User
Interface)
Posted by Stefan Sperling <st...@elego.de>.
On Mon, Sep 15, 2008 at 03:23:36PM +0200, Stefan Sperling wrote:
> Say there was a textdelta for file foo.c in revision 4,
> and the file foo.c was created in revision 3.
>
> I am merging revision 4 into a branch.
>
> In the current implementation, revision 4 (the textdelta)
> will be skipped, but mergeinfo will claim that revision 4
> was merged! (See attached recipe script for reference.)
>
> /trunk:2,4
>
> Instead, I'd expect explicit mergeinfo on foo.c with
> revision 4 omitted:
>
> /trunk/foo.c:2
>
> or even:
>
> /trunk/foo.c:2,!4
>
> if such "negative mergeinfo" was feasible -- it may be easier
> to record "We failed to apply revision X to path Y" rather
> than determining the answer to the question "Which revisions
> in the merge range could not be applied to path Y?" after
> a merge of some arbitrary revision range.
Stephen Butler pointed out to me that the client-side merge
editor does not know which revisions individual changes come from.
The merge only knows the merge-left and merge-right revisions, and
gets the resulting diff without knowing how the diff was composed.
So the client may not even know which particular revision fails
to apply.
Thoughts?
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Mergeinfo for skipped paths? (was: Re: Tree Conflicts and User Interface)
Posted by Danil Shopyrin <da...@gmail.com>.
> Properties on 'branch':
> svn:mergeinfo
> <empty> //r4 isn't merged completely so we can't claim that it's merged!
> Properties on 'branch/bar.c':
> svn:mergeinfo
> /trunk/bar.c:4
> Properties on 'branch/subdir':
> svn:mergeinfo
> <empty>
A little remark. <empty> means <null> or <absent> everywhere in the my
previous post. I forget that there are special <empty> mergeinfos :)
--
Danil
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Mergeinfo for skipped paths? (was: Re: Tree Conflicts and User Interface)
Posted by Danil Shopyrin <da...@gmail.com>.
> Let's assume your target branch looks like this (these other paths
> under trunk help better illustrate what I'm going to propose):
>
> /branch
> /branch/bar.c
> /branch/subdir
>
> We tweak the merge logic to set non-inheritable mergeinfo on the merge
> target describing the merge, and normal mergeinfo on all the target's
> present children, like this:
>
> Properties on 'branch':
> svn:mergeinfo
> /trunk:2,4* <--- Non-inheritable mergeinfo.
> Properties on 'branch/bar.c':
> svn:mergeinfo
> /trunk/bar.c:2,4
> Properties on 'branch/subdir':
> svn:mergeinfo
> /trunk/subdir:2,4
>
> Of course if the skipped path was a deeper subtree and not an
> immediate child of our merge target we would adjust accordingly and
> set the non-inheritable mergeinfo on the skipped path's immediate
> parent and the normal mergeinfo on the skipped path's siblings.
>
> If we commit the above merge and then merge -c3 into branch now foo.c
> is added but it only has mergeinfo for r2-3:
>
> Properties on 'branch':
> svn:mergeinfo
> /trunk:2-3,4*
> Properties on 'branch/foo.c':
> svn:mergeinfo
> /trunk/foo.c:2-3
> Properties on 'branch/bar.c':
> svn:mergeinfo
> /trunk/bar.c:2-4
> Properties on 'branch/subdir':
> svn:mergeinfo
> /trunk/subdir:2-4
>
> Note: In my ad hoc testing foo.c isn't getting r2 in it's mergeinfo,
> it should, but that can be fixed. Assuming that we fix this, if we
> re-merge -r4 to branch then foo.c gets that change and all the
> mergeinfo elides* up to branch and we are left with is this:
>
> Properties on 'branch':
> svn:mergeinfo
> /trunk:2-4
>
> I think this change is fairly simple, we already catch similar cases -
> see libsvn_client/merge.c:get_mergeinfo_paths(). The only difference
> is that we can't identify these paths at the start of the merge but
> must catch them once we realize they are skipped. The
> notification_receiver_baton_t struct keeps track of these already in
> its skipped_paths member so this shouldn't be too hard.
>
> I can crank out a patch that does what I propose in fairly short order
> if this idea solves your problem.
>
> * In the interests of full disclosure, in another thread I am
> proposing the elimination of automatic mergeinfo elision as a fairly
> useless feature...ironically here is a case where it is nice to have
> :-\
>
>> XXX big ugly hack idea: Could we extend the mergeinfo
>> format so that parent directories can record "negative"
>> mergeinfo for direct children which were skipped?
>
> Befitting a "big ugly hack" there's a problem :-) What if, in your
> example, r4 also affected trunk directly (e.g. we added a svn:ignore
> property). It could get a bit ugly if we want a way to record that r4
> affected trunk, but not one of its (currently) non-existent children.
We've discussed this informally at VisualSVN and have found
alternative solution.
Let's assume that our target branch looks like this:
/branch
/branch/bar.c
/branch/subdir
And we have /trunk/foo.c added in r3. Files /trunk/bar.c and
/trunk/foo.c are both changed in r4.
We are merging r4 to the branch. Since r4 contains changes for an
absent file (i.e. foo.c will be skipped) this revision can't be
treated as regularly merged into branch. So the merge should produce
the following mergeinfo in the trunk:
Properties on 'branch':
svn:mergeinfo
<empty> //r4 isn't merged completely so we can't claim that it's merged!
Properties on 'branch/bar.c':
svn:mergeinfo
/trunk/bar.c:4
Properties on 'branch/subdir':
svn:mergeinfo
<empty>
Re: Mergeinfo for skipped paths? (was: Re: Tree Conflicts and User Interface)
Posted by Paul Burba <pt...@gmail.com>.
2008/9/15 Stefan Sperling <st...@elego.de>:
> On Fri, Sep 12, 2008 at 02:34:32PM +0200, Stefan Sperling wrote:
>> On Fri, Sep 12, 2008 at 01:57:53PM +0200, Stefan Sperling wrote:
>> > There are certain types of conflicts that require an additional
>> > merge to be resolved, such as when a textdelta is applied to a file
>> > which is not yet present in the target branch -- the fix is to
>> > *repeat* the merge with a wider revision range, this time including
>> > the revision in which the file was created on the source branch.
>>
>> I just realised that this is wrong.
>> The merge cannot simply be repeated.
>>
>> Because of merge tracking, the file addition would be merged,
>> but the textdelta would not be merged again.
>> So the final merge result would miss the textdelta,
>> unless the user reapplied it manually.
>
> On second thought, this is actually a problem with the way
> we're currently recording mergeinfo. It cannot account
> for failed merges due to missing paths.
>
> Say there was a textdelta for file foo.c in revision 4,
> and the file foo.c was created in revision 3.
>
> I am merging revision 4 into a branch.
>
> In the current implementation, revision 4 (the textdelta)
> will be skipped, but mergeinfo will claim that revision 4
> was merged! (See attached recipe script for reference.)
>
> /trunk:2,4
>
> Instead, I'd expect explicit mergeinfo on foo.c with
> revision 4 omitted:
>
> /trunk/foo.c:2
>
> or even:
>
> /trunk/foo.c:2,!4
>
> if such "negative mergeinfo" was feasible -- it may be easier
> to record "We failed to apply revision X to path Y" rather
> than determining the answer to the question "Which revisions
> in the merge range could not be applied to path Y?" after
> a merge of some arbitrary revision range.
>
> A problem of course is that we can't create create mergeinfo
> for foo.c if it does not yet exist in the branch's working copy :(
>
> So before we can solve this problem, we'd probably need to store
> mergeinfo somewhere else than properties...
>
> Any idea on how we could record mergeinfo for skipped paths
> in the current implementation?
Hi Stefan,
Let's assume your target branch looks like this (these other paths
under trunk help better illustrate what I'm going to propose):
/branch
/branch/bar.c
/branch/subdir
We tweak the merge logic to set non-inheritable mergeinfo on the merge
target describing the merge, and normal mergeinfo on all the target's
present children, like this:
Properties on 'branch':
svn:mergeinfo
/trunk:2,4* <--- Non-inheritable mergeinfo.
Properties on 'branch/bar.c':
svn:mergeinfo
/trunk/bar.c:2,4
Properties on 'branch/subdir':
svn:mergeinfo
/trunk/subdir:2,4
Of course if the skipped path was a deeper subtree and not an
immediate child of our merge target we would adjust accordingly and
set the non-inheritable mergeinfo on the skipped path's immediate
parent and the normal mergeinfo on the skipped path's siblings.
If we commit the above merge and then merge -c3 into branch now foo.c
is added but it only has mergeinfo for r2-3:
Properties on 'branch':
svn:mergeinfo
/trunk:2-3,4*
Properties on 'branch/foo.c':
svn:mergeinfo
/trunk/foo.c:2-3
Properties on 'branch/bar.c':
svn:mergeinfo
/trunk/bar.c:2-4
Properties on 'branch/subdir':
svn:mergeinfo
/trunk/subdir:2-4
Note: In my ad hoc testing foo.c isn't getting r2 in it's mergeinfo,
it should, but that can be fixed. Assuming that we fix this, if we
re-merge -r4 to branch then foo.c gets that change and all the
mergeinfo elides* up to branch and we are left with is this:
Properties on 'branch':
svn:mergeinfo
/trunk:2-4
I think this change is fairly simple, we already catch similar cases -
see libsvn_client/merge.c:get_mergeinfo_paths(). The only difference
is that we can't identify these paths at the start of the merge but
must catch them once we realize they are skipped. The
notification_receiver_baton_t struct keeps track of these already in
its skipped_paths member so this shouldn't be too hard.
I can crank out a patch that does what I propose in fairly short order
if this idea solves your problem.
* In the interests of full disclosure, in another thread I am
proposing the elimination of automatic mergeinfo elision as a fairly
useless feature...ironically here is a case where it is nice to have
:-\
> XXX big ugly hack idea: Could we extend the mergeinfo
> format so that parent directories can record "negative"
> mergeinfo for direct children which were skipped?
Befitting a "big ugly hack" there's a problem :-) What if, in your
example, r4 also affected trunk directly (e.g. we added a svn:ignore
property). It could get a bit ugly if we want a way to record that r4
affected trunk, but not one of its (currently) non-existent children.
Paul
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Mergeinfo for skipped paths? (was: Re: Tree Conflicts and User
Interface)
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Sep 12, 2008 at 02:34:32PM +0200, Stefan Sperling wrote:
> On Fri, Sep 12, 2008 at 01:57:53PM +0200, Stefan Sperling wrote:
> > There are certain types of conflicts that require an additional
> > merge to be resolved, such as when a textdelta is applied to a file
> > which is not yet present in the target branch -- the fix is to
> > *repeat* the merge with a wider revision range, this time including
> > the revision in which the file was created on the source branch.
>
> I just realised that this is wrong.
> The merge cannot simply be repeated.
>
> Because of merge tracking, the file addition would be merged,
> but the textdelta would not be merged again.
> So the final merge result would miss the textdelta,
> unless the user reapplied it manually.
On second thought, this is actually a problem with the way
we're currently recording mergeinfo. It cannot account
for failed merges due to missing paths.
Say there was a textdelta for file foo.c in revision 4,
and the file foo.c was created in revision 3.
I am merging revision 4 into a branch.
In the current implementation, revision 4 (the textdelta)
will be skipped, but mergeinfo will claim that revision 4
was merged! (See attached recipe script for reference.)
/trunk:2,4
Instead, I'd expect explicit mergeinfo on foo.c with
revision 4 omitted:
/trunk/foo.c:2
or even:
/trunk/foo.c:2,!4
if such "negative mergeinfo" was feasible -- it may be easier
to record "We failed to apply revision X to path Y" rather
than determining the answer to the question "Which revisions
in the merge range could not be applied to path Y?" after
a merge of some arbitrary revision range.
A problem of course is that we can't create create mergeinfo
for foo.c if it does not yet exist in the branch's working copy :(
So before we can solve this problem, we'd probably need to store
mergeinfo somewhere else than properties...
Any idea on how we could record mergeinfo for skipped paths
in the current implementation?
XXX big ugly hack idea: Could we extend the mergeinfo
format so that parent directories can record "negative"
mergeinfo for direct children which were skipped?
Stefan
Re: Tree Conflicts and User Interface (was: Re: [PATCH]
Tree-conflicts: do_entry_deletion segfault)
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Sep 12, 2008 at 01:57:53PM +0200, Stefan Sperling wrote:
> There are certain types of conflicts that require an additional
> merge to be resolved, such as when a textdelta is applied to a file
> which is not yet present in the target branch -- the fix is to
> *repeat* the merge with a wider revision range, this time including
> the revision in which the file was created on the source branch.
I just realised that this is wrong.
The merge cannot simply be repeated.
Because of merge tracking, the file addition would be merged,
but the textdelta would not be merged again.
So the final merge result would miss the textdelta,
unless the user reapplied it manually.
Damn, this is really complicated stuff we're dealing with :/
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Tree Conflicts and User Interface (was: Re: [PATCH]
Tree-conflicts: do_entry_deletion segfault)
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Sep 12, 2008 at 12:31:29PM +0100, Julian Foad wrote:
> We won't attempt to apply another update or merge once the
> node is already in some sort of conflict.
Are you really sure about that?
I think we talked about this in the past, and came to the
conclusion that we'd have to allow further merges into
tree-conflicted directories.
There are certain types of conflicts that require an additional
merge to be resolved, such as when a textdelta is applied to a file
which is not yet present in the target branch -- the fix is to
*repeat* the merge with a wider revision range, this time including
the revision in which the file was created on the source branch.
The problem, of course, is what happens when yet another tree
conflict is recorded for the same victim. We currently only
support a one-to-one mapping between victims and conflicts.
But that is more of an implementation detail, right?
Or is there another important reason for this?
If we didn't allow merges into tree-conflicted directories,
people would have to revert -R their working copies, and
then repeat the merge. But if we do that, we're basically
treating the tree conflict as unresolvable.
Unresolvable conflicts would be a totally new concept,
and in a good UI the user would be informed at commit
time that the conflict is not resolvable.
Which implies that we'd need to make a list of unresolvable
tree conflict situations to compare what we find in the
working copy against. Big can of worms :(
> > $ svn update
> > CM foo.c (a file with text conflict and property mod)
> > C dir (a directory containing tree conflict victims)
> > V dir/bar.c (a tree conflict victim)
> > Updated to revision 42.
> > There was 1 text conflict and 1 tree conflict.
>
> Why did you want the notification that 'dir' contains tree conflicts? If
> it's so that you can do a non-recursive status on it and still see
> there's something wrong with it, then wouldn't you want the indication
> to bubble all the way up to the WC root?
>
> I suggest there's no need for reporting 'this contains tree conflict
> victims'.
Yes, agreed. It's redundant and can be omitted.
> > But there is a problem. What if dir/bar.c was text conflicted,
> > had property conflicts, and are also the victim of a tree conflict?
> > Note that such a condition may be the result of multiple updates
> > or merges without conflict resolution done in between.
>
> Let's not allow that. The design we were working to said we would not
> allow that, as I recall. It should bail out if we try to update or merge
> into something that's already conflicted. If that's not done, we'll fix
> it.
I don't think we can simply say "let's not allow that". See above.
> (There's a complication in that the new merge-tracking merges are
> multi-phase and it's not ideal if they bail out after raising a conflict
> on an early phase, but that problem is already present outside of tree
> conflict handling.)
So that's yet another problem with this approach.
> > If we only had 'C' and 'V' and two columns, we could encode
> > this situation as:
>
> I'll follow up with my own proposal.
>
> - Julian
>
I'm waiting in suspense :)
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Tree Conflicts and User Interface
Posted by "Neels J. Hofmeyr" <ne...@elego.de>.
Neels J. Hofmeyr wrote:
> $ svn status
> M beta
> D epsilon
> M V gamma
> M zeta
btw, this example is slightly stupid, I admit. More like:
M beta
D epsilon
M V gamma
M zeta
, so my argument about indenting tree-conflicts only applies to update.
Either way, I'm pretty sure my vote is on an added column.
~Neels
--
Neels Hofmeyr -- elego Software Solutions GmbH
Gustav-Meyer-Allee 25 / Gebäude 12, 13355 Berlin, Germany
phone: +49 30 23458696 mobile: +49 177 2345869 fax: +49 30 23458695
http://www.elegosoft.com | Geschäftsführer: Olaf Wagner | Sitz: Berlin
Handelsreg: Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194
Re: Tree Conflicts and User Interface
Posted by "Neels J. Hofmeyr" <ne...@elego.de>.
Julian Foad wrote:
> A node is either tree-conflicted (as a victim), or can have (text and/or
> prop) conflicts: one of:
>
> - not in conflict
> - tree conflict victim
> - prop conflicts
> - text conflict
> - text and prop conflicts
I'd like to draw attention to D, M: files and directories, when victims of
tree-conflicts, are mostly scheduled for deletion or locally modified. That
is valuable information!
working copy 1: working copy 2: `svn update' in wc 2:
M alpha D alpha D V alpha
D beta M beta M V beta
(committed) (locally) ...or something.
Looking around, I realized this:
$ svn --version
svn, version 1.5.1 (r32289)
compiled Sep 4 2008, 18:42:55
[...]
$ svn help status
[...]
The first six columns in the output are each one character wide:
First column: Says if item was added, deleted, or otherwise changed
' ' no modifications
'A' Added
'C' Conflicted
'D' Deleted
'I' Ignored
'M' Modified
'R' Replaced
'X' item is unversioned, but is used by an externals definition
'?' item is not under version control
'!' item is missing (removed by non-svn command) or incomplete
'~' versioned item obstructed by some item of a different kind
Second column: Modifications of a file's or directory's properties
' ' no modifications
'C' Conflicted
'M' Modified
Third column: Whether the working copy directory is locked
' ' not locked
'L' locked
Fourth column: Scheduled commit will contain addition-with-history
' ' no history scheduled with commit
'+' history scheduled with commit
Fifth column: Whether the item is switched relative to its parent
' ' normal
'S' switched
Sixth column: Repository lock token
(without -u)
' ' no lock token
'K' lock token present
(with -u)
' ' not locked in repository, no lock token
'K' locked in repository, lock toKen present
'O' locked in repository, lock token in some Other working copy
'T' locked in repository, lock token present but sTolen
'B' not locked in repository, lock token present but Broken
[...]
So, there already *IS* a multitude of columns? So what's the big deal with
adding another column for tree-conflicts -- isn't that the obvious choice?
Even update has a third column:
$ svn help update
[...]
A 'B' in the third column signifies that the lock for the file has
been broken or stolen.
[...]
So why not report tree-conflicts in an added column *only* when
tree-conflicts are to be reported? I know that stsp wouldn't like it to look
like this:
$ svn up
A alpha
G beta
D V gamma
D delta
$ svn status
M beta
D epsilon
M V gamma
M zeta
But I myself wouldn't mind it at all: I think it's even an added bonus,
because it visually sets tree-conflicted items apart from "normal" ones.
In a "third" column, couldn't we also actually use a 'T', because it's an
all new column? (Have these things been considered in another thread??)
~Neels
--
Neels Hofmeyr -- elego Software Solutions GmbH
Gustav-Meyer-Allee 25 / Gebäude 12, 13355 Berlin, Germany
phone: +49 30 23458696 mobile: +49 177 2345869 fax: +49 30 23458695
http://www.elegosoft.com | Geschäftsführer: Olaf Wagner | Sitz: Berlin
Handelsreg: Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194
Re: Tree Conflicts and User Interface (was: Re: [PATCH]
Tree-conflicts: do_entry_deletion segfault)
Posted by Julian Foad <ju...@btopenworld.com>.
On Fri, 2008-09-12 at 13:10 +0200, Stefan Sperling wrote:
> On Fri, Sep 12, 2008 at 12:44:11AM +0200, Neels Hofmeyr wrote:
> >
> >
> > Julian Foad wrote:
> > > Bear in mind that "reported as" and "stored as" are two completely
> > > different things. I'm not intending to change the storage, only the
> > > reporting. The storage can stay in the parent directory.
> >
> > Oh, I see. In that case...:
>
> This discussion is important!
>
> Please keep in mind that the current way we're reporting tree
> conflicts is nearly useless. I don't want it to end up in a
> release like this. This is the most urgent problem on the
> tree-conflicts branch IMO. We really need to fix this!
>
> > One day, stsp and I were discussing the _reporting_ of tree conflicts, and
> > thought that maybe victims and storage holder (i.e. parent directory) should
> > have distinct markings; C for parent dir, as in "conflicted", and V for an
> > actual victim node where the conflict manifested.
> >
> > We also thought it would be nice to have an additional column for tree
> > conflicts, to be able to still see the other markings on the
> > tree-conflict-marked files.
>
> I'm still not entirely convinced about that third column.
>
> There is a bit of a problem with the way Subversion currently
> displays information during update/status, and how to encode
> tree conflict information in a compatible way.
>
> To recap, what we currently have (in 1.5 and trunk) is two columns.
> One column for displaying information about files/directories,
> and one for displaying information about properties.
>
> Files and properties can be text conflicted (signalled by 'C'),
> directories cannot be conflicted.
>
> Enter tree conflicts:
>
> * Files can now be text conflicted, or be victims of a tree conflict,
> or both.
No, not both. We won't attempt to apply another update or merge once the
node is already in some sort of conflict.
> * Properties can be text conflicted.
Yes, but not if their node is tree-conflicted.
A node is either tree-conflicted (as a victim), or can have (text and/or
prop) conflicts: one of:
- not in conflict
- tree conflict victim
- prop conflicts
- text conflict
- text and prop conflicts
> * Directories can now contain tree conflict victims, or be tree
> conflict victims themselves, or both.
Yes.
> Currently, on the tree-conflicts branch we're only displaying
> information about directories which contain tree conflicts.
> This works sort-of well, by showing a 'C' in the first column
> of the status output for a directory.
>
> For example:
>
> CM foo.c (a file with text conflict and property mod)
> C dir (a directory containing tree conflict victims)
>
> To get information on individual victims inside a directory,
> the user has to run 'svn info' on that directory, and will
> then get a display like this:
>
> Tree conflicts:
> The update edited the file 'bar.c'.
> You have deleted 'bar.c' locally.
> Maybe you renamed it?
>
> This UI is less than ideal. It's fine for use during development
> to see whether victims have been detected correctly, but it is
> not something I'd like to use when actually working with Subversion.
>
> I'd rather have something like this:
>
> $ svn update
> CM foo.c (a file with text conflict and property mod)
> C dir (a directory containing tree conflict victims)
> V dir/bar.c (a tree conflict victim)
> Updated to revision 42.
> There was 1 text conflict and 1 tree conflict.
Why did you want the notification that 'dir' contains tree conflicts? If
it's so that you can do a non-recursive status on it and still see
there's something wrong with it, then wouldn't you want the indication
to bubble all the way up to the WC root?
I suggest there's no need for reporting 'this contains tree conflict
victims'.
> Note the conflict summary display which should be rather handy
> once those numbers get big. Imagine not getting a summary when
> you actually run into something like this:
> There were 33 text conflicts and 20 tree conflicts.
Yes. Or, more crucially, when 300 files were updated, there was just 1
conflict.
> But there is a problem. What if dir/bar.c was text conflicted,
> had property conflicts, and are also the victim of a tree conflict?
> Note that such a condition may be the result of multiple updates
> or merges without conflict resolution done in between.
Let's not allow that. The design we were working to said we would not
allow that, as I recall. It should bail out if we try to update or merge
into something that's already conflicted. If that's not done, we'll fix
it.
(There's a complication in that the new merge-tracking merges are
multi-phase and it's not ideal if they bail out after raising a conflict
on an early phase, but that problem is already present outside of tree
conflict handling.)
> If we only had 'C' and 'V' and two columns, we could encode
> this situation as:
I'll follow up with my own proposal.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org