On IRC, Bert and I were in disagreement as to how 'svn up' should behave in intersection with locally-added trees: whether 'update' should always affect the BASE tree (irrespective of any locally-replaced trees), or whether it should be interpreted as applying to the overlaying tree (i.e., the higher op_depth).
Another point that Bert raised was whether 'svn up addeddir' and 'svn up addeddir/some/child' should both work, in light of the semantics (i.e., invalidity) of rooting an update editor at an added path. Some scenarios: % svn cp A A2; svn up A2/ % svn cp A A2; svn up A2/mu % svn rm A2; svn cp A A2; svn up A2/ % svn rm A2; svn cp A A2; cd A2; svn up % svn rm A2; svn cp A A2; cd A2; svn up mu Bert Huijben wrote on Sun, Mar 06, 2011 at 18:38:59 +0100: > > -----Original Message----- > > From: danie...@apache.org [mailto:danie...@apache.org] > > Sent: zondag 6 maart 2011 16:44 > > To: comm...@subversion.apache.org > > Subject: svn commit: r1078497 - in /subversion/trunk/subversion: > > libsvn_wc/adm_crawler.c libsvn_wc/update_editor.c > > tests/cmdline/update_tests.py > > > > Author: danielsh > > Date: Sun Mar 6 15:43:34 2011 > > New Revision: 1078497 > > > > URL: http://svn.apache.org/viewvc?rev=1078497&view=rev > > Log: > > Fix issue #3807, "'svn up' of a nonexistent child in a copied dir triggers > > an > > assertion". This patch makes a couple of places treat added or absent nodes > > explicitly. > > > > It's possible that in a few of these places, the handling of added nodes > > should > > be done by svn_wc__db_read_info() rather than by its callers --- but that > > patch > > implements the handling in the callers. > > > > * subversion/libsvn_wc/update_editor.c > > (make_dir_baton): > > Scan more than the BASE tree when computing NEW_RELPATH. > > (make_editor): > > Look for REPOS_ROOT_URL and REPOS_UUID for added nodes, too. > > > > * subversion/libsvn_wc/adm_crawler.c > > (svn_wc_crawl_revisions5): > > Consider svn_wc__db_status_absent the same as > > svn_wc__db_status_not_present. > > > > * subversion/tests/cmdline/update_tests.py > > (update_nonexistent_child_of_copy): Expect it to pass. > > > > Modified: > > subversion/trunk/subversion/libsvn_wc/adm_crawler.c > > subversion/trunk/subversion/libsvn_wc/update_editor.c > > subversion/trunk/subversion/tests/cmdline/update_tests.py > > > > Modified: subversion/trunk/subversion/libsvn_wc/adm_crawler.c > > URL: > > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm > > _crawler.c?rev=1078497&r1=1078496&r2=1078497&view=diff > > ========================================================== > > ==================== > > --- subversion/trunk/subversion/libsvn_wc/adm_crawler.c (original) > > +++ subversion/trunk/subversion/libsvn_wc/adm_crawler.c Sun Mar 6 > > 15:43:34 2011 > > @@ -786,7 +786,8 @@ svn_wc_crawl_revisions5(svn_wc_context_t > > status = svn_wc__db_status_not_present; /* As checkout */ > > } > > > > - if ((status == svn_wc__db_status_not_present) > > + if (status == svn_wc__db_status_not_present > > + || status == svn_wc__db_status_absent > > || (target_kind == svn_wc__db_kind_dir > > && status != svn_wc__db_status_normal > > && status != svn_wc__db_status_incomplete)) > > > > Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c > > URL: > > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/upd > > ate_editor.c?rev=1078497&r1=1078496&r2=1078497&view=diff > > ========================================================== > > ==================== > > --- subversion/trunk/subversion/libsvn_wc/update_editor.c (original) > > +++ subversion/trunk/subversion/libsvn_wc/update_editor.c Sun Mar 6 > > 15:43:34 2011 > > @@ -600,9 +600,33 @@ make_dir_baton(struct dir_baton **d_p, > > { > > /* Get the original REPOS_RELPATH. An update will not be > > changing its value. */ > > - SVN_ERR(svn_wc__db_scan_base_repos(&d->new_relpath, NULL, > > NULL, > > + svn_wc__db_status_t status; > > + const char *repos_relpath, *original_repos_relpath; > > + SVN_ERR(svn_wc__db_read_info(&status, NULL, NULL, > > &repos_relpath, > > + NULL, NULL, NULL, NULL, NULL, NULL, > > + NULL, NULL, NULL, NULL, NULL, > > + &original_repos_relpath, > > + NULL, NULL, NULL, NULL, NULL, NULL, > > + NULL, NULL, > > + eb->db, d->local_abspath, > > + dir_pool, scratch_pool)); > > + if (status == svn_wc__db_status_added) > > + SVN_ERR(svn_wc__db_scan_addition(NULL, NULL, > > + &repos_relpath, NULL, NULL, > > + &original_repos_relpath, > > NULL, NULL, > > + NULL, > > eb->db, d->local_abspath, > > dir_pool, scratch_pool)); > > The update editor should not look at layers above op_depth 0. Nodes can only > be added below an existing op_depth 0 node. > > If the update editor would touch something in the higher layers just a (tree) > conflict should be raised. > (And in some specific cases things might be automatically handled for > compatibility reasons) > > > + if (original_repos_relpath) > > + d->new_relpath = original_repos_relpath; > > + else if (repos_relpath) > > + d->new_relpath = repos_relpath; > > + else > > + SVN_ERR(svn_wc__db_scan_base_repos(&d->new_relpath, NULL, > > NULL, > > + eb->db, d->local_abspath, > > + dir_pool, scratch_pool)); > > + SVN_ERR_ASSERT(d->new_relpath); > > } > > } > > > > @@ -4830,17 +4854,27 @@ make_editor(svn_revnum_t *target_revisio > > svn_delta_editor_t *tree_editor = svn_delta_default_editor(edit_pool); > > const svn_delta_editor_t *inner_editor; > > const char *repos_root, *repos_uuid; > > + svn_wc__db_status_t status; > > > > /* An unknown depth can't be sticky. */ > > if (depth == svn_depth_unknown) > > depth_is_sticky = FALSE; > > > > /* Get the anchor's repository root and uuid. */ > > - SVN_ERR(svn_wc__db_read_info(NULL,NULL, NULL, NULL, &repos_root, > > &repos_uuid, > > + SVN_ERR(svn_wc__db_read_info(&status, NULL, NULL, NULL, > > + &repos_root, &repos_uuid, > > NULL, NULL, NULL, NULL, NULL, NULL, NULL, > > NULL, > > NULL, NULL, NULL, NULL, NULL, NULL, NULL, > > NULL, > > NULL, NULL, wc_ctx->db, anchor_abspath, > > result_pool, scratch_pool)); > > + > > + /* ### For adds, REPOS_ROOT and REPOS_UUID would be NULL now. */ > > + if (status == svn_wc__db_status_added) > > + SVN_ERR(svn_wc__db_scan_addition(NULL, NULL, NULL, > > + &repos_root, &repos_uuid, > > + NULL, NULL, NULL, NULL, > > + wc_ctx->db, anchor_abspath, > > + result_pool, scratch_pool)); > > Only the op_depth 0 ancestor is interesting. The update editor should not > look at the higher layers. > > > > > /* With WC-NG we need a valid repository root */ > > SVN_ERR_ASSERT(repos_root != NULL && repos_uuid != NULL); > > > > Modified: subversion/trunk/subversion/tests/cmdline/update_tests.py > > URL: > > http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/ > > update_tests.py?rev=1078497&r1=1078496&r2=1078497&view=diff > > ========================================================== > > ==================== > > --- subversion/trunk/subversion/tests/cmdline/update_tests.py (original) > > +++ subversion/trunk/subversion/tests/cmdline/update_tests.py Sun Mar 6 > > 15:43:34 2011 > > @@ -5346,7 +5346,6 @@ def update_with_file_lock_and_keywords_p > > # Updating a nonexistent or deleted path should be a successful no-op, > > # when there is no incoming change. In trunk@1035343, such an update > > # within a copied directory triggered an assertion failure. > > -@XFail() > > @Issue(3807) > > def update_nonexistent_child_of_copy(sbox): > > """update a nonexistent child of a copied dir""" > > > > I haven't looked at this issue, but I think the expected behavior should be > an error that explains why you shouldn't do this. > > Updating nodes that don't exist in op_depth 0 is only valid for pulling in > unexisting child nodes. In this case it would be a child a few levels down > and this scenario is not supported by editor v1. > (But svn up --parents would allow it by using multiple updates) > > Bert > >
