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

RE: svn commit: r1078497 - in /subversion/trunk/subversion: libsvn_wc/adm_crawler.c libsvn_wc/update_editor.c tests/cmdline/update_tests.py

From: Bert Huijben <bert_at_qqmail.nl>
Date: Sun, 6 Mar 2011 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 18:39:55 CET

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