You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@btopenworld.com> on 2008/07/23 09:15:11 UTC

Re: Tree conflicts detection.txt

Hi, tree-conflicts fans, and Stephen in particular.

I'm pleased to see this recent update to
notes/tree-conflicts/detection.txt.

I'm a bit confused about what is being documented here: the intention or
the current implementation?

>  This file describes how tree conflicts described in use-cases.txt
>  can be detected. It documents how detection currently works in the
>  actual code, and also explains the limits of tree conflict detection
>  imposed by Subversion's current design.

But many of the subsequent parts are written like a statement of intent.

> +The current implementation has imperfect tree conflict detection,
> +but it is still better than not handling tree conflicts at all.
> +Once Subversion has been taught about true renames, tree conflict
> +detection can be changed to make use of this and become extremely
> +precise.  See below for further explanation.

Yes. But also, there is a lot more that we can do without true renames.

>  ==========
>  USE CASE 1
>  ==========
> 
> +Files:
> +
>  If 'svn update' modifies a file that has been scheduled for deletion
>  in the working copy, the file is a tree conflict victim.
> 
> +Directories:
> +
> +If 'svn update' modifies any item (including adding or deleting a file
> +or directory) in a directory that has been scheduled for deletion in
> +the working copy, each item modified by the update is a tree conflict
> +victim. The conflict data will be stored in the metadata of the parent
> +of the directory scheduled for deletion.

That would put the metadata two (or more?) levels away from the victims,
which isn't a possibility I had imagined or seen mentioned.

The way I see it is that there is only one victim, and that is the
directory scheduled for deletion. As soon as the "update" tries to
"open" a schedule-delete directory in order to make changes in it, a
conflict should be raised with this directory as the victim (and
metadata stored in its parent), and no further processing of the changes
within this directory should be attempted. If the user then wants to get
at those changes, he would look up the incoming revisions in the
conflict description, and get those changes directly from the
repository.

>  ==========
>  USE CASE 2
>  ==========
> 
> +Files:
> +
>  If 'svn update' deletes a file that has local modifications, the file
>  is a tree conflict victim.
> 
> +Directories:
> +
> +If 'svn update' deletes a directory that has local modifications
> +(including items scheduled to be added or deleted), each locally
> +modified item is a tree conflict victim. The conflict data will be
> +stored in the metadata of the parent of the directory deleted by the
> +update.

Same again.

>  ==========
>  USE CASE 3
>  ==========
> 
> +Files:
>  If 'svn update' deletes a file that has been scheduled for deletion in
>  the working copy, the file is a tree conflict victim.
> 
> +Directories:
> +If 'svn update' deletes a directory that has been scheduled for deletion in
> +the working copy, the directory is a tree conflict victim.
> +
>  ==========
>  USE CASE 4
>  ==========
> 
>  We skip tree conflict detection if the record_only field of the
>  merge-command baton is TRUE. A record-only merge operation updates
>  mergeinfo without touching files.
> 
> +Files:
> +
>  If 'svn merge' tries to modify a file that does not exist in the
>  target working copy, then the target file is a tree conflict victim.

[...]

> +Directories:
> +
> +If 'svn merge' tries to modify a directory that does not exist in the
> +target working copy, then the target directory is a tree conflict victim.

Yes. (Why would you want to treat case 1 differently from this case 4?)


>  ==========
>  USE CASE 5
>  ==========
> 
>  We skip tree conflict detection if the record_only field of the
>  merge-command baton is TRUE. A record-only merge operation updates
>  mergeinfo without touching files.

You didn't recently change that text, but I don't really understand the
first sentence. What's the high-level meaning? Is there a special reason
why it appears here under use case 5 but nowhere else?


> +Files:
> +
> -If 'svn merge' deletes an existing file, the file is a tree conflict
> -victim if its text is different from the corresponding file on the left
> -side of the merge source.
> -
> -To account for uncommitted text modifications in the working copy,
> -we should do any text comparisons against the WORKING revision.
> +If 'svn merge' deletes an existing file, the file is a tree conflict
> +victim if its text is different from the corresponding file on the
> +left side of the merge source. The merge-left source file will be
> +compared to the "WORKING revision", which includes uncommitted local
> +changes.
> 
>  Rationale:
> 
>  We don't want to flag every file deletion as a tree conflict.  We
>  want to warn the user if the file to be deleted locally is different
>  from the file deleted in the merge source.  The user then has a chance
> @@ -107,20 +130,32 @@
> 
>  Implementation:
> 
>  Call svn_client_diff_summarize2() to compare the target file to the
>  file at the left side of the merge source.
> 
> +
> +Directories:
> +
> +If 'svn merge' deletes an existing directory, the directory is a tree
> +conflict victim if its content is different from the corresponding
> +directory on the left side of the merge source.

Yes.

> +See the "NOTES ON DIRECTORIES" section for details on what it means
> +for a directory to be "different".
> +
>  ==========
>  USE CASE 6
>  ==========
> 
>  We skip tree conflict detection if the record_only field of the
>  merge-command baton is TRUE. A record-only merge operation updates
>  mergeinfo without touching files.
> 
> +Files:
> +
>  If 'svn merge' tries to delete a file that does not exist in the
>  target working copy, then the target file is a tree conflict victim.
> 
>  This is similar to UC4.
> 
>  Rationale:
> @@ -130,34 +165,171 @@
>  target branch's history, or tries to apply a simple "delete" onto a file
>  that has been moved in the target branch, or tries to move a file that
>  has already been moved to a different name in the target branch.
> 
>  Notes on Resolution
>  -------------------
> +
>  Some users may want to skip the tree conflict and have the result automatically
>  resolved if two rename operations have the same destination, or if a file is
>  simply deleted on both branches. But we have to mark these as tree conflicts
>  due to the current lack of "true rename" support. It does not appear to be
>  feasible to detect more than the double-delete aspect of the move operation.
> 
> +Directories:
> +
> +If 'svn merge' tries to delete a directory that does not exist in the
> +target working copy, then the target directory is a tree conflict victim.

Yes.

>  =========================
>  OBSTRUCTIONS DURING MERGE
>  =========================
> 
> +Files:
> +
>  If 'svn merge' fails to apply an operation to a file because the
>  file is obstructed (i.e. an unversioned item of the same name is
>  in the file's place), the obstructed file is a tree conflict victim.
> 
> -Rationale:
> +Directories:
> +
> +If 'svn merge' fails to apply an operation to a directory because the
> +directory is obstructed (i.e. an unversioned item of the same name is
> +in the directory's place), the obstructed directory is a tree conflict
> +victim.
> +
> +Rationale for both files and directories:
> 
>  We want to make sure that a merge either completes successfully
>  or any problems found during a merge are flagged as conflicts.
>  Skipping obstructed items during merge is no longer acceptable
>  behaviour, since users might not be aware of obstructions that were
>  skipped when they commit the result of a merge.
> 
> +====================
> +NOTES ON DIRECTORIES
> +====================
> +
> +===========================
> +Modification of directories
> +===========================
> +
> +How do we define a modification of a directory?
> +
> +A directory with no subdirectories has been modified if any changes
> +were made to files and properties inside the directory, or if any
> +files or directories were added or deleted inside it.
> +
> +A directory with subdirectories has been modified if any changes
> +were made to files and properties inside the directory, or if any
> +files or directories were added or deleted inside it, of if any
> +subdirectories were modified.
> +
> +=======================
> +Equality of directories
> +=======================
> +
> +How do we define equality between directories?
> +
> +Two directories with no subdirectories are equal if they
> +contain the same files with the same content, and the
> +same properties with the same content.
> +
> +Two directories with subdirectories are equal if they
> +contain the same files with the same content, and the
> +same properties with the same content, and all their
> +subdirectories are equal.
> +
> +How can this be implemented?
> +
> +For each directory, it could retrieve the corresponding dir entry from
> +the repository as it existed in the merge-left source of the merge,
> +and compare the two for equality, i.e. check whether all fields in
> +the svn_dirent_t returned by the repo match their corresponding
> +attributes of the directory as found in the working copy.
> +
> +The left-merge revision shall be a new additional parameter to
> +merge_dir_deleted(). The ra session needed to contact the repository
> +via the get_dir() method is already contained in the merge baton which
> +is passed to merge_dir_deleted().
> +
> +The last two paragraphs were taken from:
> +http://subversion.tigris.org/servlets/ReadMsg?listName=dev&msgNo=136794

Thanks. That's useful.

> +=======================================
> +Deep tree conflict example (use case 1)
> +=======================================
> +
> +In a working copy, a directory named B is scheduled for deletion.
> +Running 'svn status' lists the entire tree rooted at B.
> +
> +  D      A/B/E/alpha
> +  D      A/B/E/beta
> +  D      A/B/E
> +  D      A/B/F
> +  D      A/B
> +
> +Running 'svn status -uq' warns that the repository contains changes to
> +the locally-deleted directory.
> +
> +  D      *        1   A/B
> +  Status against revision:      2
> +
> +Updating the working copy modifies one file, deletes another file,
> +deletes a directory, and adds a file and a directory.
> +
> +  U    A/B/E/alpha
> +  D    A/B/E/beta
> +  A    A/B/E/gamma
> +  D    A/B/F
> +  A    A/B/G
> +  C    A
> +
> +The 5 tree conflicts revealed by the update are recorded in the
> +metadata of directory A.  They are described by 'svn info'.
> +
> +  The update attempted to edit 'A/B/E/alpha'.
> +  You have deleted or renamed 'A/B' locally.
> +
> +  The update has deleted 'A/B/E/beta'
> +  (possibly as part of a rename operation).
> +  You have deleted or renamed 'A/B' locally.
> +
> +  The update has added 'A/B/E/gamma'
> +  You have deleted or renamed 'A/B' locally.
> +
> +  The update has deleted 'A/B/F'
> +  (possibly as part of a rename operation).
> +  You have deleted or renamed 'A/B' locally.
> +
> +  The update has added 'A/B/G'
> +  You have deleted or renamed 'A/B' locally.
> +
> +(The wording of the warnings is not yet settled.  Also, it looks like
> +we'll have to extend the conflict struct to include a 'target' field,
> +since in this common case the target of the action is not the victim.)
> +
> +What is the current state of the working copy?
> +
> +* F and beta are completely gone from the working copy because they
> +  don't exist in the target revision.
> +
> +* B, E, and alpha are still scheduled for deletion.

OK.

> +* The update's edit to alpha was applied to the "pristine copy" hidden
> +  in A/B/E/.svn/text-base, for use by the diff and revert commands.

...

> +* The newly-added G and gamma are not scheduled for deletion, but will
> +  be deleted if the user commits the deletion of B.

Don't you think that either they should be scheduled for deletion and
then deleted by a commit, or not scheduled for deletion and not deleted
by a commit?

> +* Any commit of A (including any commit in a parent directory or
> +  subdirectory of A) will be blocked by the tree conflicts.

But a commit of B won't be blocked? That's not good. An attempt to
commit anything inside A should be blocked too, in my opinion.


> +Except for the blocked commit (and the output of update, status, and
> +info) the behavior is the same as Subversion 1.5.
> +

- Julian



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Tree conflicts detection.txt

Posted by Stephen Butler <sb...@elego.de>.
Quoting Stefan Sperling <st...@elego.de>:

> On Wed, Jul 23, 2008 at 10:15:11AM +0100, Julian Foad wrote:

[...]

>> >  ==========
>> >  USE CASE 1
>> >  ==========
>> >
>> > +Files:
>> > +
>> >  If 'svn update' modifies a file that has been scheduled for deletion
>> >  in the working copy, the file is a tree conflict victim.
>> >
>> > +Directories:
>> > +
>> > +If 'svn update' modifies any item (including adding or deleting a file
>> > +or directory) in a directory that has been scheduled for deletion in
>> > +the working copy, each item modified by the update is a tree conflict
>> > +victim. The conflict data will be stored in the metadata of the parent
>> > +of the directory scheduled for deletion.
>>
>> That would put the metadata two (or more?) levels away from the victims,
>> which isn't a possibility I had imagined or seen mentioned.
>>
>> The way I see it is that there is only one victim, and that is the
>> directory scheduled for deletion. As soon as the "update" tries to
>> "open" a schedule-delete directory in order to make changes in it, a
>> conflict should be raised with this directory as the victim (and
>> metadata stored in its parent), and no further processing of the changes
>> within this directory should be attempted. If the user then wants to get
>> at those changes, he would look up the incoming revisions in the
>> conflict description, and get those changes directly from the
>> repository.
>
> Hmmm. I'm not sure if what we meant to say is exactly what you describe.
> I don't think we were really going to separately store the victim status
> of each item in the subtree being deleted, at least I can't remember a
> good reason for doing so. Steve, do you remember why we wrote this the
> way we did?

Interrupting the update, as Julian suggests, would be simpler.
There would be less for the user to read.  Sometimes less info
is better!  In my example of a deep tree conflict, there are five
conflict description like this one:

   The update attempted to edit 'A/B/E/alpha'.
   You have deleted or renamed 'A/B' locally.

By interrupting the update, these are reduced to one description:

   The update wants to modify something inside 'A/B'.
   You have deleted or renamed 'A/B' locally.

Also, the interruption avoids awkward aspects of the current 1.5
behavior.  For instance, the inconsistent 'svn status' output for
items added by update in a tree scheduled for deletion.

So far, we've avoided interrupting the update, because update is
a one-way operation.  In general, a working copy contains mixed
revisions, so the update operation is applied to each item
individually.  There's no holistic diff-and-apply.  If an update
is interrupted, there's no revert command to restore the working
copy to what it was before.  An interruption would leave the
working copy in a state where the revisions are even more
mixed-up than usual.

But maybe that argument is outweighed by the simplicity argument.

[...]

>> >  ==========
>> >  USE CASE 4
>> >  ==========
>> >
>> >  We skip tree conflict detection if the record_only field of the
>> >  merge-command baton is TRUE. A record-only merge operation updates
>> >  mergeinfo without touching files.
>> >
>> > +Files:
>> > +
>> >  If 'svn merge' tries to modify a file that does not exist in the
>> >  target working copy, then the target file is a tree conflict victim.
>>
>> [...]
>>
>> > +Directories:
>> > +
>> > +If 'svn merge' tries to modify a directory that does not exist in the
>> > +target working copy, then the target directory is a tree conflict victim.
>>
>> Yes. (Why would you want to treat case 1 differently from this case 4?)
>
> Merge behaves differently than update when faced with deleted
> directories. I keep forgetting the details about this, even
> though Steve has explained them to me a couple of times already.
>
> Steve, has this got to do with the difference between update and
> merge when deleting? Can you explain the details again? We should
> probably add such a detailed description to the file itself so I
> don't keep forgetting them ;)

The difference is that in use case 1, the directory tree is in the
working copy (scheduled for deletion, but still there), while in
use case 4 the directory tree is simply not there.

>
>> >  ==========
>> >  USE CASE 5
>> >  ==========
>> >
>> >  We skip tree conflict detection if the record_only field of the
>> >  merge-command baton is TRUE. A record-only merge operation updates
>> >  mergeinfo without touching files.
>>
>> You didn't recently change that text, but I don't really understand the
>> first sentence. What's the high-level meaning? Is there a special reason
>> why it appears here under use case 5 but nowhere else?
>
> AFAIK the high-level meaning is that a --record-only merge is being done.

Yes.  The three copies of this text in the document ought to be demoted
to a footnote.

>
>> > +=======================================
>> > +Deep tree conflict example (use case 1)
>> > +=======================================
>
>> > +* The newly-added G and gamma are not scheduled for deletion, but will
>> > +  be deleted if the user commits the deletion of B.
>>
>> Don't you think that either they should be scheduled for deletion and
>> then deleted by a commit, or not scheduled for deletion and not deleted
>> by a commit?
>
>> > +* Any commit of A (including any commit in a parent directory or
>> > +  subdirectory of A) will be blocked by the tree conflicts.
>>
>> But a commit of B won't be blocked? That's not good. An attempt to
>> commit anything inside A should be blocked too, in my opinion.

By "subdirectory of A", I meant A/B, A/B/E, etc.  I'll rewrite that
sentence to make that more obvious.

Thanks for all the feedback!

Regards,
Steve

-- 
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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: Tree conflicts detection.txt

Posted by Stefan Sperling <st...@elego.de>.
On Wed, Jul 23, 2008 at 10:15:11AM +0100, Julian Foad wrote:
> Hi, tree-conflicts fans, and Stephen in particular.
> 
> I'm pleased to see this recent update to
> notes/tree-conflicts/detection.txt.
> 
> I'm a bit confused about what is being documented here: the intention or
> the current implementation?

Oh, you're right, we mixed that up a bit.
Everything we've added documents our intentions.

> >  This file describes how tree conflicts described in use-cases.txt
> >  can be detected. It documents how detection currently works in the
> >  actual code, and also explains the limits of tree conflict detection
> >  imposed by Subversion's current design.
> 
> But many of the subsequent parts are written like a statement of intent.

Yes, you're right. We should adjust the first paragraph for the time
being.
 
> > +The current implementation has imperfect tree conflict detection,
> > +but it is still better than not handling tree conflicts at all.
> > +Once Subversion has been taught about true renames, tree conflict
> > +detection can be changed to make use of this and become extremely
> > +precise.  See below for further explanation.
> 
> Yes. But also, there is a lot more that we can do without true renames.

I would not mind removing that paragraph altogether. It's not that
informative, and you're right that it might be worded a bit too broadly.
 
> >  ==========
> >  USE CASE 1
> >  ==========
> > 
> > +Files:
> > +
> >  If 'svn update' modifies a file that has been scheduled for deletion
> >  in the working copy, the file is a tree conflict victim.
> > 
> > +Directories:
> > +
> > +If 'svn update' modifies any item (including adding or deleting a file
> > +or directory) in a directory that has been scheduled for deletion in
> > +the working copy, each item modified by the update is a tree conflict
> > +victim. The conflict data will be stored in the metadata of the parent
> > +of the directory scheduled for deletion.
> 
> That would put the metadata two (or more?) levels away from the victims,
> which isn't a possibility I had imagined or seen mentioned.
> 
> The way I see it is that there is only one victim, and that is the
> directory scheduled for deletion. As soon as the "update" tries to
> "open" a schedule-delete directory in order to make changes in it, a
> conflict should be raised with this directory as the victim (and
> metadata stored in its parent), and no further processing of the changes
> within this directory should be attempted. If the user then wants to get
> at those changes, he would look up the incoming revisions in the
> conflict description, and get those changes directly from the
> repository.

Hmmm. I'm not sure if what we meant to say is exactly what you describe.
I don't think we were really going to separately store the victim status
of each item in the subtree being deleted, at least I can't remember a
good reason for doing so. Steve, do you remember why we wrote this the
way we did?

This paragraph definitely needs fixing either way, it is not
precise enough to serve as a spec to work from.

> >  ==========
> >  USE CASE 2
> >  ==========
> > 
> > +Files:
> > +
> >  If 'svn update' deletes a file that has local modifications, the file
> >  is a tree conflict victim.
> > 
> > +Directories:
> > +
> > +If 'svn update' deletes a directory that has local modifications
> > +(including items scheduled to be added or deleted), each locally
> > +modified item is a tree conflict victim. The conflict data will be
> > +stored in the metadata of the parent of the directory deleted by the
> > +update.
> 
> Same again.

ACK.

> >  ==========
> >  USE CASE 4
> >  ==========
> > 
> >  We skip tree conflict detection if the record_only field of the
> >  merge-command baton is TRUE. A record-only merge operation updates
> >  mergeinfo without touching files.
> > 
> > +Files:
> > +
> >  If 'svn merge' tries to modify a file that does not exist in the
> >  target working copy, then the target file is a tree conflict victim.
> 
> [...]
> 
> > +Directories:
> > +
> > +If 'svn merge' tries to modify a directory that does not exist in the
> > +target working copy, then the target directory is a tree conflict victim.
> 
> Yes. (Why would you want to treat case 1 differently from this case 4?)

Merge behaves differently than update when faced with deleted
directories. I keep forgetting the details about this, even
though Steve has explained them to me a couple of times already.

Steve, has this got to do with the difference between update and
merge when deleting? Can you explain the details again? We should
probably add such a detailed description to the file itself so I
don't keep forgetting them ;)

> >  ==========
> >  USE CASE 5
> >  ==========
> > 
> >  We skip tree conflict detection if the record_only field of the
> >  merge-command baton is TRUE. A record-only merge operation updates
> >  mergeinfo without touching files.
> 
> You didn't recently change that text, but I don't really understand the
> first sentence. What's the high-level meaning? Is there a special reason
> why it appears here under use case 5 but nowhere else?

AFAIK the high-level meaning is that a --record-only merge is being done.

> > +=======================================
> > +Deep tree conflict example (use case 1)
> > +=======================================

> > +* The newly-added G and gamma are not scheduled for deletion, but will
> > +  be deleted if the user commits the deletion of B.
> 
> Don't you think that either they should be scheduled for deletion and
> then deleted by a commit, or not scheduled for deletion and not deleted
> by a commit?

> > +* Any commit of A (including any commit in a parent directory or
> > +  subdirectory of A) will be blocked by the tree conflicts.
> 
> But a commit of B won't be blocked? That's not good. An attempt to
> commit anything inside A should be blocked too, in my opinion.
 
Steve mentioned to me that the example still had errors he was
going to fix. I will refrain from commenting until his fixes
are in.

Thanks for your feedback, Julian!

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org