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: danielsh_at_apache.org [mailto:danielsh_at_apache.org]
> > Sent: zondag 6 maart 2011 16:44
> > To: commits_at_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_at_1035343, such an update
> > # within a copied directory triggered an assertion failure.
> > -_at_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
>
>
Received on 2011-03-06 19:04:19 CET