[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: RFC: Reverting the semantic change made in r31101

From: Paul Burba <ptburba_at_gmail.com>
Date: Thu, 9 Apr 2009 14:27:56 -0400

On Thu, Apr 9, 2009 at 11:52 AM, C. Michael Pilato <cmpilato_at_collab.net> wrote:
> I'm strongly considering reverting the behavioral change made in r31101, by
> making the --depth option as applied to 'svn add' cover the depth of the
> crawl (an operational depth) but still always set the recorded depth of the
> newly added items to "infinity".
>
> The change in r31101 was discussed here:
>
>   http://svn.haxx.se/dev/archive-2008-04/0444.shtml
>
> I contributed to the thread, but wish to recant the poor advice I gave there.
>
> Here's the problem:  GUI clients often perform their operations using
> shallow operational depths (because individual targets can be selected from
> lists thereof).  This means that any new directories added by such a client
> essentially remain "unsubscribed" from changes which happen under them
> later.  That's inconsistent with Subversion's mission of promoting
> collaboration and avoiding unexpected behaviors.  I submit that just because
> I was the guy that added an empty "trunk" to my repository doesn't mean I
> don't want to see all the files and directories my colleagues add to in
> future commits when I update.
>
> There are three approaches I could take here.
>
> The first is a slightly complex approach which allows a syntax like this:
> 'svn add --depth OPERATION-DEPTH --set-depth RECORDED-DEPTH' (where both
> depths default to "infinity").  As you can probably guess, this would
> empower folks (including GUIs, though the APIs) to control both the
> operational depth of the crawl for newly added items, as well as the
> recorded depth of the newly added items.  But I'm not sure there's much
> value for the complexity here in terms of real-world use-cases, though, and
> frankly I tied my brain in knot just considering the various interpretations
> of that syntax.

Our definitions of "slightly complex" clearly differ :-P I am under
the impression that some of our depth behavior is fairly
non-intuitive, e.g.
http://subversion.tigris.org/ds/viewMessage.do?dsMessageId=1614711&dsForumId=462.
 I think a change like this goes too far to the right along the
"easily does what I need" --> "does *something* through an absurd
number of confusing options that may or may not be what I want"
continuum.

> The second is the simple approach and that I one I lean towards:  always add
> new items with a recorded depth of "infinity".  This is just a partial
> reversion of r31101 back to Subversion 1.5.x functionality.  (I've attached
> a patch for this.)
>
> The third approach is the simplest (for us):  do nothing, and tell GUI
> clients that after committing adds of new items of non-infinite depth
> (resulting in revision X), they should then 'svn update --set-depth infinity
> --revision X' each of those items behind the scenes so the user is
> "subscribed" to future changes in those directories.

I lean slightly to the do nothing approach, but since I can count on
one hand the number of times I've used a GUI client to add a subtree
my opinion probably isn't worth much. I certainly have no objections
to making the default depth infinity.

Paul

> Also floating in my thinkspace is the fact that 'svn mkdir' doesn't let you
> choose the recorded depth of the new directory you've created.  You could
> say that this makes 'svn mkdir FOO' inconsistent with 'mkdir FOO; svn add -N
> FOO'.  You could also say, "So what, that just gives folks more behavioral
> options."
>
> Thoughts?
>
> --
> C. Michael Pilato <cmpilato_at_collab.net>
> CollabNet   <>   www.collab.net   <>   Distributed Development On Demand
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1615432
> When doing 'svn add', always set the recorded depth of newly added
> items to "infinity".  This is a partial reversion of r31101.
>
> * subversion/libsvn_client/add.c
>  (add_dir_recursive): Perform the add of the directory, setting its
>    recorded depth to infinity.
>
> * subversion/tests/cmdline/depth_tests.py
>  (add_tree_with_depth): Change test expectations.
>
> Index: subversion/libsvn_client/add.c
> ===================================================================
> --- subversion/libsvn_client/add.c      (revision 37119)
> +++ subversion/libsvn_client/add.c      (working copy)
> @@ -349,8 +349,8 @@
>     SVN_ERR(ctx->cancel_func(ctx->cancel_baton));
>
>   /* Add this directory to revision control. */
> -  err = svn_wc_add3(dirname, adm_access, depth, NULL, SVN_INVALID_REVNUM,
> -                    ctx->cancel_func, ctx->cancel_baton,
> +  err = svn_wc_add3(dirname, adm_access, svn_depth_infinity, NULL,
> +                    SVN_INVALID_REVNUM, ctx->cancel_func, ctx->cancel_baton,
>                     ctx->notify_func2, ctx->notify_baton2, pool);
>   if (err && err->apr_err == SVN_ERR_ENTRY_EXISTS && force)
>     svn_error_clear(err);
>
> Index: subversion/tests/cmdline/depth_tests.py
> ===================================================================
> --- subversion/tests/cmdline/depth_tests.py     (revision 37119)
> +++ subversion/tests/cmdline/depth_tests.py     (working copy)
> @@ -1238,22 +1238,22 @@
>   # Simple case, add new1 only, set depth to files
>   svntest.actions.run_and_verify_svn(None, None, [],
>                                      "add", "--depth", "files", new1_path)
> -  verify_depth(None, "files", new1_path)
> +  verify_depth(None, "infinity", new1_path)
>
>   # Force add new1 at new1 again, should include new2 at empty, the depth of
>   # new1 should not change
>   svntest.actions.run_and_verify_svn(None, None, [],
>                                      "add", "--depth", "immediates",
>                                      "--force", new1_path)
> -  verify_depth(None, "files", new1_path)
> -  verify_depth(None, "empty", new2_path)
> +  verify_depth(None, "infinity", new1_path)
> +  verify_depth(None, "infinity", new2_path)
>
>   # add new4 with intermediate path, the intermediate path is added at empty
>   svntest.actions.run_and_verify_svn(None, None, [],
>                                      "add", "--depth", "immediates",
>                                      "--parents", new4_path)
>   verify_depth(None, "infinity", new3_path)
> -  verify_depth(None, "immediates", new4_path)
> +  verify_depth(None, "infinity", new4_path)
>
>  def upgrade_from_above(sbox):
>   "upgrade a depth=empty wc from above"
>
>

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1617669
Received on 2009-04-09 22:35:28 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.