dir_delta behavior

[Ben Collins-Sussman <su...@collab.net>]

Yo Cmike:

When I run a command like:

  svn switch A/D/H file:///.../A/B

dir_delta() seems to work fine.  It happily adds all of B's entries,
then it happily deletes all of H's entries.  The anchor of the update
(A/D/H) never changes its name, and it's entries-url is modified to
point to B.

But I forsee future problems:  specifically, it seems Bad that
dir_delta is sending additions before it sends deletions.  What if I'm
switching to a directory that contains a file with an identical name
to one I already have?

  * In the common case, the two files with identical names will be
    related.  (If say, I'm switching to some branch.)  In that case,
    I'll just see a "U" on the file.

  * In the uncommon case, the files may be totally unrelated, in which
    case dir_delta wants to do to a true replacement.  But to do that,
    shouldn't it be sending a delete before an add?  Otherwise we're
    in for an "obstructed update" error.

Cmpilato, please share your wisdom.  Does dir_delta need to be tweaked
to send deletes before adds?  If so, will this screw anything else up?


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

Re: dir_delta behavior

[cm...@collab.net]

Karl Fogel <kf...@newton.ch.collab.net> writes:

> Mmmm, but it sounds like Ben needs to know more than just "driving an
> editor".  He wants to know in what order certain editor operations
> (adds and deletes?) will happen over a given directory baton, and
> nothing in the editor spec itself forces a particular order.
> Therefore, the driver's documentation would have to say.

No.  Ben needed to know only that the function works.  He merely had a
lack of confidence in the implementation.  Reading and understanding
the code would have (and hopefully has) cleared that up.  The
implementation details have no place in the docstring.


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

Re: dir_delta behavior

[Karl Fogel <kf...@newton.ch.collab.net>]

cmpilato@collab.net writes:
> > If you're depending on a certain behavior from dir_deltas, and that
> > behavior can't be discovered from its doc string, than the doc needs
> > to be fixed.
> 
> In this case, all is well.  Ben's depending on dir_deltas driving an
> editor to convert one dir into another.  And it does.  Just like the
> docstring says it will.

Mmmm, but it sounds like Ben needs to know more than just "driving an
editor".  He wants to know in what order certain editor operations
(adds and deletes?) will happen over a given directory baton, and
nothing in the editor spec itself forces a particular order.
Therefore, the driver's documentation would have to say.

Almost by definition, if one needs to know something about a
function's behavior, and can't figure it out from the docstring, then
the docstring needs to say more.  

That's what's going on here, right?  (Or am I misunderstanding Ben?)

[Note, in case it wasn't clear: none of this is a criticism of the
dir_deltas doc string.  It's fine to start out with a certain level of
detail, and then refine on demand.]

-K

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

Re: dir_delta behavior

[cm...@collab.net]

Karl Fogel <kf...@newton.ch.collab.net> writes:

> Ben Collins-Sussman <su...@collab.net> writes:
> > > Benny didn't read the code.
> > 
> > What, I'm *not* supposed to treat dir_delta like a mysterious magical
> > black box??   :-)
> 
> Beyond the doc string, it should be a black box, sure!
> 
> If you're depending on a certain behavior from dir_deltas, and that
> behavior can't be discovered from its doc string, than the doc needs
> to be fixed.

In this case, all is well.  Ben's depending on dir_deltas driving an
editor to convert one dir into another.  And it does.  Just like the
docstring says it will.

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

Re: dir_delta behavior

[Karl Fogel <kf...@newton.ch.collab.net>]

Ben Collins-Sussman <su...@collab.net> writes:
> > Benny didn't read the code.
> 
> What, I'm *not* supposed to treat dir_delta like a mysterious magical
> black box??   :-)

Beyond the doc string, it should be a black box, sure!

If you're depending on a certain behavior from dir_deltas, and that
behavior can't be discovered from its doc string, than the doc needs
to be fixed.

-K

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

Re: dir_delta behavior

[Ben Collins-Sussman <su...@collab.net>]

cmpilato@collab.net writes:

> Benny didn't read the code.

What, I'm *not* supposed to treat dir_delta like a mysterious magical
black box??   :-)

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

Re: dir_delta behavior

[cm...@collab.net]

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

> Yo Cmike:
> 
> When I run a command like:
> 
>   svn switch A/D/H file:///.../A/B
> 
> dir_delta() seems to work fine.  It happily adds all of B's entries,
> then it happily deletes all of H's entries.  The anchor of the update
> (A/D/H) never changes its name, and it's entries-url is modified to
> point to B.
> 
> But I forsee future problems:  specifically, it seems Bad that
> dir_delta is sending additions before it sends deletions.  What if I'm
> switching to a directory that contains a file with an identical name
> to one I already have?
> 
>   * In the common case, the two files with identical names will be
>     related.  (If say, I'm switching to some branch.)  In that case,
>     I'll just see a "U" on the file.
> 
>   * In the uncommon case, the files may be totally unrelated, in which
>     case dir_delta wants to do to a true replacement.  But to do that,
>     shouldn't it be sending a delete before an add?  Otherwise we're
>     in for an "obstructed update" error.
> 
> Cmpilato, please share your wisdom.  Does dir_delta need to be tweaked
> to send deletes before adds?  If so, will this screw anything else up?

This from delta_dirs():

  /* Loop over the hash of entries in the target, searching for its
     partner in the source.  If we find the matching partner entry,
     use editor calls to replace the one in target with a new version
     if necessary, then remove that entry from the source entries
     hash.  If we can't find a related node in the source, we use
     editor calls to add the entry as a new item in the target.
     Having handled all the entries that exist in target, any entries
     still remaining the source entries hash represent entries that no
     longer exist in target.  Use editor calls to delete those entries
     from the target tree. */

For all children of the directories being compared that have the same
name, ancestry comparisons are performed, and one of three conclusions
made:

1.  Are they the same node?  Do nothing.

2.  Are they related?  Then just call editor->open_xxx(), sending
    textdeltas in the file case, and recursing into delta_dirs in the
    directory case.

3.  Are they unrelated?  editor->delete(); editor->add().

Here's the code (with some non-operative #defines stripped out for
your reading convenience):

              if (distance == 0)
                {
                  /* no-op */
                }
              else if (distance == -1)
                {
                    {
                      /* We found no ancestral match at all.  Delete this
                         entry and create a new one from scratch. */
                      SVN_ERR (delete (c, dir_baton, t_entry->name, subpool));
                      SVN_ERR (add_file_or_dir 
                               (c, dir_baton, NULL, NULL, 
                                target_path, t_entry->name, subpool));
                    }
                }
              else
                {
                  SVN_ERR (replace_file_or_dir
                           (c, dir_baton, 
                            source_path,
                            s_entry->name,
                            target_path,
                            t_entry->name,
                            subpool));
                }

Once things with the same name are out of the way, we're left with two
sets of entries:  thing in the target not in the source, and things in
the source not in the target.  Add the former, delete the latter, and
the order doesn't really matter because they have necessarily
different names.

Benny didn't read the code.

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

Re: dir_delta behavior

[Ben Collins-Sussman <su...@collab.net>]

Branko Čibej <br...@xbc.nu> writes:

> >Cmpilato, please share your wisdom.  Does dir_delta need to be tweaked
> >to send deletes before adds?  If so, will this screw anything else up?
> >
> 
> $ cd /some/wc
> $ svn mv B tmp
> $ svn mv A B
> $ svn mv tmp A
> $ svn ci
> ...
> $ cd /another/wc
> $ svn up
> 
> 
> Just throwing a spaner in the works,


The 'svn mv tmp A' command above wouldn't work right now anyway.  As a
general rule in our wc library, if 'tmp' is already scheduled for
addition, you can't do a copy (another schedule-for-addition).

Here's the comment at the bottom of libsvn_wc/copy.c:

/*
  Rabbinic Commentary


  Q:  Why can't we 'svn cp' something that we just copied?
      i.e.  'svn cp foo foo2;  svn cp foo2 foo3"

  A:  It leads to inconsistencies.

      In the example above, foo2 has no associated repository URL,
      because it hasn't been committed yet.  But suppose foo3 simply
      inherited foo's URL (i.e. foo3 'pointed' to foo as a copy
      ancestor by virtue of transitivity.)
 
      For one, this is not what the user would expect.  That's
      certainly not what the user typed!  Second, suppose that the
      user did a commit between the two 'svn cp' commands.  Now foo3
      really *would* point to foo2, but without that commit, it
      pointed to foo.  Ugly inconsistency, and the user has no idea
      that foo3's ancestor would be different in each case.

      And even if somehow we *could* make foo3 point to foo2 before
      foo2 existed in the repository... what's to prevent a user from
      committing foo3 first?  That would break.

*/

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

Re: dir_delta behavior

[Branko Čibej <br...@xbc.nu>]

Ben Collins-Sussman wrote:

>Yo Cmike:
>
>When I run a command like:
>
>  svn switch A/D/H file:///.../A/B
>
>dir_delta() seems to work fine.  It happily adds all of B's entries,
>then it happily deletes all of H's entries.  The anchor of the update
>(A/D/H) never changes its name, and it's entries-url is modified to
>point to B.
>
>But I forsee future problems:  specifically, it seems Bad that
>dir_delta is sending additions before it sends deletions.  What if I'm
>switching to a directory that contains a file with an identical name
>to one I already have?
>
>  * In the common case, the two files with identical names will be
>    related.  (If say, I'm switching to some branch.)  In that case,
>    I'll just see a "U" on the file.
>
>  * In the uncommon case, the files may be totally unrelated, in which
>    case dir_delta wants to do to a true replacement.  But to do that,
>    shouldn't it be sending a delete before an add?  Otherwise we're
>    in for an "obstructed update" error.
>
>Cmpilato, please share your wisdom.  Does dir_delta need to be tweaked
>to send deletes before adds?  If so, will this screw anything else up?
>

$ cd /some/wc
$ svn mv B tmp
$ svn mv A B
$ svn mv tmp A
$ svn ci
...
$ cd /another/wc
$ svn up


Just throwing a spaner in the works,

-- 
Brane Čibej   <br...@xbc.nu>   http://www.xbc.nu/brane/




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