You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Shlomi Fish <sh...@vipe.stud.technion.ac.il> on 2003/07/01 11:04:06 UTC

Tackling Issue 571

I'm trying to solve issue 571:

http://subversion.tigris.org/issues/show_bug.cgi?id=571

Using gdb, I've tracked the offending code to libsvn_repos/delta.c:914 :

              else if ((distance == -1) && (! c->ignore_ancestry))
                {
                  SVN_ERR (delete (c, dir_baton, t_fullpath, subpool));
                  SVN_ERR (add_file_or_dir (c, dir_baton, target_path,
                                            t_entry->name, subpool));
                }

Now, delete and add_file_or_dir call callbacks in the upper layers, which
do not accept a flag that indicates the distance variable.

So, how should I tell delete no to emit a message, and tell
add_file_or_dir to emit a replace one? Should I implement the same logic
in the delete_entry and close_file handlers as exists here to get the
distance? Or should I add a parameter to the callback? (thus maybe
breaking the API)

Regards,

	Shlomi Fish



----------------------------------------------------------------------
Shlomi Fish        shlomif@vipe.technion.ac.il
Home Page:         http://t2.technion.ac.il/~shlomif/

An apple a day will keep a doctor away. Two apples a day will keep two
doctors away.

	Falk Fish

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

Re: Tackling Issue 571

Posted by cm...@collab.net.
Shlomi Fish <sh...@vipe.stud.technion.ac.il> writes:

> I'm trying to solve issue 571:
> 
> http://subversion.tigris.org/issues/show_bug.cgi?id=571
> 
> Using gdb, I've tracked the offending code to libsvn_repos/delta.c:914 :

Oh, no you haven't.   You just think you have.

> So, how should I tell delete no to emit a message, and tell
> add_file_or_dir to emit a replace one? Should I implement the same logic
> in the delete_entry and close_file handlers as exists here to get the
> distance? Or should I add a parameter to the callback? (thus maybe
> breaking the API)

Dude, you're looking on the other side of the network for a
client-side UI bug.  dir_delta() and friends is doing the right
thing.  This "bug" can be handled in one of two ways:

   - close it as invalid, because the notification is a true
     representation of what's going on (a deletion of a path followed
     be a re-addition of that same path), or

   - cache the fact that a path is being deleted, and hold off on
     displaying that fact until the last possible moment, or until
     that path is re-added, at which time use the 'R' notification
     once instead of a 'D' and an 'A'.

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

Re: Tackling Issue 571

Posted by kf...@collab.net.
Shlomi Fish <sh...@vipe.stud.technion.ac.il> writes:
> > +0.  I think the current behavior is juuuuuust fine.
> 
> So, what's the final decision? What is the desired behaviour?

You're not under the impression that there is a command structure
here, are you? :-)

The decision is, some developers are okay with the status quo but
would also be okay with certain proposed changes, and other developers
may be dissatisfied enough with the status quo that they go ahead and
commit those changes.  But until they do, the status quo is "the
decision"... if one can call it that.

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

Re: Tackling Issue 571

Posted by Shlomi Fish <sh...@vipe.stud.technion.ac.il>.
On Tue, 1 Jul 2003 cmpilato@collab.net wrote:

> Ben Collins-Sussman <su...@collab.net> writes:
>
> > > I don't have any problem with the existing feedback
> > >
> > >   $ svn up
> > >   D    foo/bar
> > >   A    foo/bar
> > >
> > > but if it has to change how about retaining the 'D' and changing 'A'
> > > to 'R'?  That could be implemented without changing the the order of
> > > the feedback.
> >
> > I'm okay with that.  Don't care much one way or another.
>
> +0.  I think the current behavior is juuuuuust fine.
>

So, what's the final decision? What is the desired behaviour?

Regards,

	Shlomi Fish

----------------------------------------------------------------------
Shlomi Fish        shlomif@vipe.technion.ac.il
Home Page:         http://t2.technion.ac.il/~shlomif/

An apple a day will keep a doctor away. Two apples a day will keep two
doctors away.

	Falk Fish

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

Re: Tackling Issue 571

Posted by cm...@collab.net.
Ben Collins-Sussman <su...@collab.net> writes:

> > I don't have any problem with the existing feedback
> > 
> >   $ svn up
> >   D    foo/bar
> >   A    foo/bar
> > 
> > but if it has to change how about retaining the 'D' and changing 'A'
> > to 'R'?  That could be implemented without changing the the order of
> > the feedback.
> 
> I'm okay with that.  Don't care much one way or another.

+0.  I think the current behavior is juuuuuust fine.

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

Re: Tackling Issue 571

Posted by Ben Collins-Sussman <su...@collab.net>.
Philip Martin <ph...@codematters.co.uk> writes:

> In my view this solution leads to worse behaviour than the original
> problem.  As described above the feedback messages will not match the
> order of the operations on the working copy.  An update that is
> interrupted, or fails, may not provide feedback for deletes that have
> been made.
> 
> I don't have any problem with the existing feedback
> 
>   $ svn up
>   D    foo/bar
>   A    foo/bar
> 
> but if it has to change how about retaining the 'D' and changing 'A'
> to 'R'?  That could be implemented without changing the the order of
> the feedback.

I'm okay with that.  Don't care much one way or another.

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

Re: Tackling Issue 571

Posted by Philip Martin <ph...@codematters.co.uk>.
Ben Collins-Sussman <su...@collab.net> writes:

> The logic you want is this:
>
> * if a file is opened/closed  (update_editor.c:open_file/close_file),
>   then we send a 'U'pdated notification.  That's what we already do.
>
> * if a file is deleted (update_editor.c:delete_entry), remember this
>   fact in the edit_baton (the major context).  Don't print anything.
>
> * if a file is added, look in the edit_baton to see if the same path
>   was previously deleted.  If so, send an 'R'eplaced notification
>   instead of an 'A'dded notification.
>
> * if a directory is closed, and there are still paths that were
>   deleted but never re-added, *then* send all of the 'D'eleted
>   signals.

In my view this solution leads to worse behaviour than the original
problem.  As described above the feedback messages will not match the
order of the operations on the working copy.  An update that is
interrupted, or fails, may not provide feedback for deletes that have
been made.

I don't have any problem with the existing feedback

  $ svn up
  D    foo/bar
  A    foo/bar

but if it has to change how about retaining the 'D' and changing 'A'
to 'R'?  That could be implemented without changing the the order of
the feedback.


-- 
Philip Martin

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

Re: Tackling Issue 571

Posted by Ben Collins-Sussman <su...@collab.net>.
Shlomi Fish <sh...@vipe.technion.ac.il> writes:

> I'm trying to solve issue 571:
> 
> Now, delete and add_file_or_dir call callbacks in the upper layers, which
> do not accept a flag that indicates the distance variable.
> 
> So, how should I tell delete no to emit a message, and tell
> add_file_or_dir to emit a replace one? Should I implement the same logic
> in the delete_entry and close_file handlers as exists here to get the
> distance? Or should I add a parameter to the callback? (thus maybe
> breaking the API)

I don't think the libsvn_wc 'update editor' should have any knowledge
of the node identities.  There's no way it can know, and there's no
reaso it *needs* to know.  If a path is deleted, and the exact same
path is added, we should print an 'R' instead of a 'D, A'.

The logic you want is this:

* if a file is opened/closed  (update_editor.c:open_file/close_file),
  then we send a 'U'pdated notification.  That's what we already do.

* if a file is deleted (update_editor.c:delete_entry), remember this
  fact in the edit_baton (the major context).  Don't print anything.

* if a file is added, look in the edit_baton to see if the same path
  was previously deleted.  If so, send an 'R'eplaced notification
  instead of an 'A'dded notification.

* if a directory is closed, and there are still paths that were
  deleted but never re-added, *then* send all of the 'D'eleted
  signals.

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