On 03/13/2013 11:37 AM, Julian Foad wrote:
> Intuitively I expect 'update' and 'switch' APIs to be the same except for
> their essential differences which are:
>
> - 'update' changes rev; 'switch' changes rev and URL.
>
> - 'switch' switches every node in the tree to a child of the same URL;
> 'update' changes only the revision of each node, leaving any switched
> subtrees switched, so it isn't the same as switching to the current URL.
That's correct. (Switching the the current URL has the effect of
*unswitching* any switched subtrees + the other stuff update does.)
> And possibly one other difference: 'svn update --set-depth=x [--parents]'
> is used to prune a tree or to extend a tree that was previously excluded
> or shallow. 'switch' also supports the '--set-depth' option, but not the
> '--parents' options, so perhaps it is not intended to fully support this
> usage, or is the lack of '--parents' just incidental?
Right again.
> Some APIs currently have extra parameters that are specific to one or the
> other:
>
> svn_ra_do_update2(..., pool)
> svn_ra_do_switch3(..., ignore_ancestry, ..., result_pool, scratch_pool)
>
> - switch3() is recently revised from switch2(), specifically to add the
> 'ignore_ancestry' option to allow it to *not* ignore ancestry.
> Previously switch2() always ignored ancestry while update2() did not, so
> this is a step towards consistency. Further consistency would be
> attained by providing the option to update as well.
Hrm. Ignoring ancestry in the switch sense is a workaround provided for
folks who got into the habit of switching a working copy from any ol'
repository location to any other ol' repository location intentionally.
There should be no ignore_ancestry option to update, as its primary
functionality is and has always been intimately bound to the ancestry of the
target being updated.
In other words, you could add this option to update today, but it would have
exactly one meaningful and supported value (FALSE). And in fact, were we
starting from scratch here, there would be ignore_ancestry option to switch,
either.
> svn_wc__get_update_editor(..., adds_as_modification, clean_checkout,
> ...)
> svn_wc__get_switch_editor(...)
>
> - These APIs are the 1.8 replacements for deprecated
> svn_wc_get_{update,switch}_editor4().
>
> - 'clean_checkout' appears to be just an optimisation hint to bypass some
> unnecessary checks when we're using 'update' to do a checkout, and as
> such it doesn't need to be available in 'switch'.
>
> - 'adds_as_modification' controls whether a tree conflict is raised for
> an incoming add onto a local add, and makes just as much sense for
> 'switch' as for 'update'.
Taking your word on this. I have no experience with these flags.
> svn_client__update_internal(..., adds_as_modification, make_parents,
> ...)
> svn_client__switch_internal(..., ignore_ancestry, ...)
> svn_client_update4(..., adds_as_modification, make_parents, ...)
> svn_client_switch3(..., ignore_ancestry, ...)
>
> - 'adds_as_modification' and 'ignore_ancestry' are as described above.
>
> - I'm not sure whether 'make_parents' would make sense for 'switch'.
Not so much.
> PROPOSAL
>
> - svn_ra_do_update2(): revise to svn_ra_do_update3(), add the
> 'ignore_ancestry' option there too, and use two pools while we're at it.
> Track this change in the RA vtable 'update' method, protocols, etc.
>
> - svn_wc__get_switch_editor(): add 'adds_as_modification'.
>
> - svn_client__update_internal(), svn_client_update4(): add
> 'ignore_ancestry'.
>
> - svn_client__switch_internal(), svn_client_switch4(): add
> 'adds_as_modification'.
>
>
> RATIONALE
>
> Do I really have to explain why Consistency is Good?
Well, actually, yes. :-) You need to establish that the consistency you
seeks brings some other value -- easier maintenance, more code sharing, etc.
-- beyond mere similarity.
As I've already pointed out, some of your suggestions don't make sense. The
dual-pool thing for update, and the 'adds_as_modification' for switch,
remain as the ones that *do* make sense. So +1 to make those changes.
--
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Received on 2013-03-13 17:23:39 CET