You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Paul Burba <pt...@gmail.com> on 2009/10/16 16:53:48 UTC
Re: svn commit: r40025 - trunk/subversion/libsvn_wc
On Wed, Oct 14, 2009 at 11:27 AM, Bert Huijben <rh...@sharpsvn.net> wrote:
> Author: rhuijben
> Date: Wed Oct 14 08:27:43 2009
> New Revision: 40025
>
> Log:
> Following up on issues discovered/worked around by gstein in r40000
> and in preparation for moving all property operations in the database,
> take baton lifetime in the update editor in our own hands. Construct per
> dir and per file pools in their parent pool and the top level directory
> in the editor pool.
>
> This moves the final cleanup for the directories to the close/bump of
> their parent or of the editor in case of the root, instead of at closing
> of the parent pool of the editor (read: at svn exit time).
>
> Use this functionality to introduce a per file log and a per file/dir
> wq item accumulator, which will be used for property operations soon.
>
> * subversion/libsvn_wc/update_editor.c
> (dir_baton): Remove copy of edit_baton->db, add wq_accumulator.
> (bump_dir_info): Add pool.
> (flush_log): Use db from edit baton. Also flush the wq_accumulator.
> (cleanup_dir_baton): Update user.
>
> (make_dir_baton): Create baton in parent and allocate everything in the
> baton pool.
> (maybe_bump_dir_info): Destroy the directory pool after bumping.
> (close_directory): Flush the log if skipping the directory.
>
> (file_baton): Add log and wq accumulator.
> (make_file_baton): Create pool in parent dir pool and allocate everything in
> this pool.
> (flush_file_log): New function, like flush_log()
>
> (open_file, add_file, merge_file):
> Add accumulated work to file accumulator.
>
> (close_file): If not skipping, send accumulated operations to the wq. Destroy
> file pool.
>
> (make_editor): Rename pools to match their usage.
>
> Modified:
> trunk/subversion/libsvn_wc/update_editor.c
>
> Modified: trunk/subversion/libsvn_wc/update_editor.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/update_editor.c?pathrev=40025&r1=40024&r2=40025
> ==============================================================================
> --- trunk/subversion/libsvn_wc/update_editor.c Wed Oct 14 07:59:44 2009 (r40024)
> +++ trunk/subversion/libsvn_wc/update_editor.c Wed Oct 14 08:27:43 2009 (r40025)
> @@ -265,10 +265,6 @@ remember_skipped_tree(struct edit_baton
>
> struct dir_baton
> {
> - /* Keep a local copy of this because ->edit_baton might be garbage when
> - we need this in cleanup_dir_baton(). */
> - svn_wc__db_t *db;
> -
> /* Basename of this directory. */
> const char *name;
>
> @@ -328,6 +324,11 @@ struct dir_baton
> svn_stringbuf_appendstr. */
> svn_stringbuf_t *log_accum;
>
> + /* wq item accumulator. Array of const svn_skel_t * instances.
> + will be flushed and run in close_file(). Items should be allocated
> + in the dir pool */
> + apr_array_header_t *wq_accum;
> +
> /* The depth of the directory in the wc (or inferred if added). Not
> used for filtering; we have a separate wrapping editor for that. */
> svn_depth_t ambient_depth;
> @@ -366,6 +367,9 @@ struct bump_dir_info
> /* Set if this directory was previously recorded as excluded,
> but is pulled back in the working copy */
> svn_boolean_t was_excluded;
> +
> + /* Pool that should be cleared after the dir is bumped */
> + apr_pool_t *pool;
> };
>
>
> @@ -431,13 +435,24 @@ get_entry_url(svn_wc_context_t *wc_ctx,
> static svn_error_t *
> flush_log(struct dir_baton *db, apr_pool_t *pool)
> {
> + int i;
> if (! svn_stringbuf_isempty(db->log_accum))
> {
> - SVN_ERR(svn_wc__wq_add_loggy(db->db, db->local_abspath,
> + SVN_ERR(svn_wc__wq_add_loggy(db->edit_baton->db, db->local_abspath,
> db->log_accum, pool));
> svn_stringbuf_setempty(db->log_accum);
> }
>
> + for (i = 0; i < db->wq_accum->nelts; i++)
> + {
> + SVN_ERR(svn_wc__db_wq_add(db->edit_baton->db, db->local_abspath,
> + APR_ARRAY_IDX(db->wq_accum, i,
> + const svn_skel_t *),
> + pool));
> + }
> +
> + apr_array_clear(db->wq_accum);
> +
> return SVN_NO_ERROR;
> }
>
> @@ -447,14 +462,13 @@ static apr_status_t
> cleanup_dir_baton(void *dir_baton)
> {
> struct dir_baton *db = dir_baton;
> + struct edit_baton *eb = db->edit_baton;
> svn_error_t *err;
> apr_pool_t *pool = apr_pool_parent_get(db->pool);
>
> - /* ### this function CANNOT reference ->edit_baton. it is garbage. */
> -
> err = flush_log(db, pool);
> if (!err)
> - err = svn_wc__run_log2(db->db, db->local_abspath, pool);
> + err = svn_wc__run_log2(eb->db, db->local_abspath, pool);
>
> if (err)
> {
> @@ -487,22 +501,27 @@ make_dir_baton(struct dir_baton **d_p,
> struct edit_baton *eb,
> struct dir_baton *pb,
> svn_boolean_t added,
> - apr_pool_t *pool)
> + apr_pool_t *scratch_pool)
> {
> + apr_pool_t *dir_pool;
> struct dir_baton *d;
> struct bump_dir_info *bdi;
>
> + if (pb != NULL)
> + dir_pool = svn_pool_create(pb->pool);
> + else
> + dir_pool = svn_pool_create(eb->pool);
> +
> SVN_ERR_ASSERT(path || (! pb));
>
> /* Okay, no easy out, so allocate and initialize a dir baton. */
> - d = apr_pcalloc(pool, sizeof(*d));
> - d->db = eb->db;
> + d = apr_pcalloc(dir_pool, sizeof(*d));
>
> /* Construct the PATH and baseNAME of this directory. */
> if (path)
> {
> - d->name = svn_dirent_basename(path, pool);
> - d->local_abspath = svn_dirent_join(pb->local_abspath, d->name, pool);
> + d->name = svn_dirent_basename(path, dir_pool);
> + d->local_abspath = svn_dirent_join(pb->local_abspath, d->name, dir_pool);
> d->inside_deleted_subtree = pb->inside_deleted_subtree;
> }
> else
> @@ -522,9 +541,9 @@ make_dir_baton(struct dir_baton **d_p,
> if (! pb)
> {
> if (! *eb->target_basename) /* anchor is also target */
> - d->new_URL = apr_pstrdup(pool, eb->switch_url);
> + d->new_URL = apr_pstrdup(dir_pool, eb->switch_url);
> else
> - d->new_URL = svn_uri_dirname(eb->switch_url, pool);
> + d->new_URL = svn_uri_dirname(eb->switch_url, dir_pool);
> }
> /* Else this directory is *not* the root (has a parent). If it
> is the target (there is a target, and this directory has no
> @@ -533,10 +552,10 @@ make_dir_baton(struct dir_baton **d_p,
> else
> {
> if (*eb->target_basename && (! pb->parent_baton))
> - d->new_URL = apr_pstrdup(pool, eb->switch_url);
> + d->new_URL = apr_pstrdup(dir_pool, eb->switch_url);
> else
> d->new_URL = svn_path_url_add_component2(pb->new_URL,
> - d->name, pool);
> + d->name, dir_pool);
> }
> }
> else /* must be an update */
> @@ -544,18 +563,21 @@ make_dir_baton(struct dir_baton **d_p,
> /* updates are the odds ones. if we're updating a path already
> present on disk, we use its original URL. otherwise, we'll
> telescope based on its parent's URL. */
> - d->new_URL = get_entry_url(eb->wc_ctx, d->local_abspath, pool, pool);
> + d->new_URL = get_entry_url(eb->wc_ctx, d->local_abspath,
> + dir_pool, scratch_pool);
> if ((! d->new_URL) && pb)
> - d->new_URL = svn_path_url_add_component2(pb->new_URL, d->name, pool);
> + d->new_URL = svn_path_url_add_component2(pb->new_URL, d->name,
> + dir_pool);
> }
>
> /* the bump information lives in the edit pool */
> - bdi = apr_pcalloc(eb->pool, sizeof(*bdi));
> + bdi = apr_pcalloc(dir_pool, sizeof(*bdi));
> bdi->parent = pb ? pb->bump_info : NULL;
> bdi->ref_count = 1;
> bdi->local_abspath = apr_pstrdup(eb->pool, d->local_abspath);
> bdi->skipped = FALSE;
> bdi->was_excluded = FALSE;
> + bdi->pool = dir_pool;
>
> /* the parent's bump info has one more referer */
> if (pb)
> @@ -563,20 +585,21 @@ make_dir_baton(struct dir_baton **d_p,
>
> d->edit_baton = eb;
> d->parent_baton = pb;
> - d->pool = svn_pool_create(pool);
> - d->propchanges = apr_array_make(pool, 1, sizeof(svn_prop_t));
> + d->pool = dir_pool;
> + d->propchanges = apr_array_make(dir_pool, 1, sizeof(svn_prop_t));
> d->added = added;
> d->existed = FALSE;
> d->add_existed = FALSE;
> d->bump_info = bdi;
> - d->log_accum = svn_stringbuf_create("", pool);
> + d->log_accum = svn_stringbuf_create("", dir_pool);
> + d->wq_accum = apr_array_make(dir_pool, 4, sizeof(const svn_skel_t *));
> d->old_revision = SVN_INVALID_REVNUM;
>
> /* The caller of this function needs to fill these in. */
> d->ambient_depth = svn_depth_unknown;
> d->was_incomplete = FALSE;
>
> - apr_pool_cleanup_register(d->pool, d, cleanup_dir_baton,
> + apr_pool_cleanup_register(dir_pool, d, cleanup_dir_baton,
> cleanup_dir_baton_child);
>
> *d_p = d;
> @@ -843,6 +866,8 @@ maybe_bump_dir_info(struct edit_baton *e
> SVN_ERR(complete_directory(eb, bdi->local_abspath,
> bdi->parent == NULL,
> bdi->was_excluded, pool));
> +
> + svn_pool_destroy(bdi->pool);
> }
> /* we exited the for loop because there are no more parents */
>
> @@ -944,6 +969,12 @@ struct file_baton
>
> /* Bump information for the directory this file lives in */
> struct bump_dir_info *bump_info;
> +
> + /* log accumulator; will be flushed and run in close_file(). */
> + svn_stringbuf_t *log_accum;
> + /* wq item accumulator. Array of const svn_skel_t * instances.
> + will be flushed and run in close_file() */
> + apr_array_header_t *wq_accum;
> };
>
>
> @@ -955,33 +986,36 @@ make_file_baton(struct file_baton **f_p,
> struct dir_baton *pb,
> const char *path,
> svn_boolean_t adding,
> - apr_pool_t *pool)
> + apr_pool_t *scratch_pool)
> {
> - struct file_baton *f = apr_pcalloc(pool, sizeof(*f));
> + apr_pool_t *file_pool = svn_pool_create(pb->pool);
> +
> + struct file_baton *f = apr_pcalloc(file_pool, sizeof(*f));
>
> SVN_ERR_ASSERT(path);
>
> /* Make the file's on-disk name. */
> - f->name = svn_dirent_basename(path, pool);
> + f->name = svn_dirent_basename(path, file_pool);
> f->old_revision = SVN_INVALID_REVNUM;
> - f->local_abspath = svn_dirent_join(pb->local_abspath, f->name, pool);
> + f->local_abspath = svn_dirent_join(pb->local_abspath, f->name, file_pool);
>
> /* Figure out the new_URL for this file. */
> if (pb->edit_baton->switch_url)
> {
> - f->new_URL = svn_path_url_add_component2(pb->new_URL, f->name, pool);
> + f->new_URL = svn_path_url_add_component2(pb->new_URL, f->name,
> + file_pool);
> }
> else
> {
> f->new_URL = get_entry_url(pb->edit_baton->wc_ctx,
> svn_dirent_join(pb->local_abspath,
> - f->name, pool),
> - pool, pool);
> + f->name, scratch_pool),
> + file_pool, scratch_pool);
> }
>
> - f->pool = pool;
> + f->pool = file_pool;
> f->edit_baton = pb->edit_baton;
> - f->propchanges = apr_array_make(pool, 1, sizeof(svn_prop_t));
> + f->propchanges = apr_array_make(file_pool, 1, sizeof(svn_prop_t));
> f->bump_info = pb->bump_info;
> f->added = adding;
> f->existed = FALSE;
> @@ -989,6 +1023,9 @@ make_file_baton(struct file_baton **f_p,
> f->deleted = FALSE;
> f->dir_baton = pb;
>
> + f->log_accum = svn_stringbuf_create("", file_pool);
> + f->wq_accum = apr_array_make(file_pool, 4, sizeof(const svn_skel_t *));
> +
> /* No need to initialize f->digest, since we used pcalloc(). */
>
> /* the directory's bump info has one more referer now */
> @@ -998,6 +1035,32 @@ make_file_baton(struct file_baton **f_p,
> return SVN_NO_ERROR;
> }
>
> +static svn_error_t *
> +flush_file_log(struct file_baton *fb, apr_pool_t *pool)
> +{
> + int i;
> + if (! svn_stringbuf_isempty(fb->log_accum))
> + {
> + SVN_ERR(svn_wc__wq_add_loggy(fb->edit_baton->db,
> + fb->dir_baton->local_abspath,
> + fb->log_accum, pool));
> + svn_stringbuf_setempty(fb->log_accum);
> + }
> +
> + for (i = 0; i < fb->wq_accum->nelts; i++)
> + {
> + SVN_ERR(svn_wc__db_wq_add(fb->edit_baton->db,
> + fb->dir_baton->local_abspath,
> + APR_ARRAY_IDX(fb->wq_accum, i,
> + const svn_skel_t *),
> + pool));
> + }
> +
> + apr_array_clear(fb->wq_accum);
> +
> + return SVN_NO_ERROR;
> +}
> +
>
>
> /*** Helpers for the editor callbacks. ***/
> @@ -2926,6 +2989,8 @@ close_directory(void *dir_baton,
> {
> db->bump_info->skipped = TRUE;
>
> + SVN_ERR(flush_log(db, pool));
> +
> /* Allow the parent to complete its update. */
> SVN_ERR(maybe_bump_dir_info(db->edit_baton, db->bump_info, db->pool));
>
> @@ -3718,7 +3783,7 @@ add_file(const char *path,
> svn_wc_conflict_description2_t *tree_conflict;
>
> SVN_ERR(check_tree_conflict(&tree_conflict, eb, fb->local_abspath,
> - pb->log_accum,
> + fb->log_accum,
> svn_wc_conflict_action_add,
> svn_node_file, fb->new_URL,
> pb->inside_deleted_subtree, subpool));
> @@ -3829,7 +3894,7 @@ open_file(const char *path,
>
> /* Is this path the victim of a newly-discovered tree conflict? */
> SVN_ERR(check_tree_conflict(&tree_conflict, eb, fb->local_abspath,
> - pb->log_accum,
> + fb->log_accum,
> svn_wc_conflict_action_edit,
> svn_node_file, fb->new_URL,
> pb->inside_deleted_subtree, pool));
> @@ -4645,7 +4710,7 @@ merge_file(svn_wc_notify_state_t *conten
> /* Now that we've built up *all* of the loggy commands for this
> file, add them to the directory's log accumulator in one fell
> swoop. */
> - svn_stringbuf_appendstr(fb->dir_baton->log_accum, log_accum);
> + svn_stringbuf_appendstr(fb->log_accum, log_accum);
>
> return SVN_NO_ERROR;
> }
> @@ -4667,7 +4732,12 @@ close_file(void *file_baton,
> const char *new_base_path;
>
> if (fb->skip_this)
> - return maybe_bump_dir_info(eb, fb->bump_info, pool);
> + {
> + SVN_ERR(flush_file_log(fb, pool));
> + SVN_ERR(maybe_bump_dir_info(eb, fb->bump_info, pool));
> + svn_pool_destroy(fb->pool);
> + return SVN_NO_ERROR;
> + }
>
> if (expected_hex_digest)
> SVN_ERR(svn_checksum_parse_hex(&expected_checksum, svn_checksum_md5,
> @@ -4711,6 +4781,7 @@ close_file(void *file_baton,
> new_base_path,
> actual_checksum, pool));
>
> + SVN_ERR(flush_file_log(fb, pool));
> /* We have one less referrer to the directory's bump information. */
> SVN_ERR(maybe_bump_dir_info(eb, fb->bump_info, pool));
>
> @@ -4749,6 +4820,9 @@ close_file(void *file_baton,
>
> eb->notify_func(eb->notify_baton, notify, pool);
> }
> +
> + svn_pool_destroy(fb->pool);
> +
> return SVN_NO_ERROR;
> }
>
> @@ -4851,13 +4925,13 @@ make_editor(svn_revnum_t *target_revisio
> apr_array_header_t *preserved_exts,
> const svn_delta_editor_t **editor,
> void **edit_baton,
> - apr_pool_t *pool, /* = result_pool */
> + apr_pool_t *result_pool,
> apr_pool_t *scratch_pool)
> {
> struct edit_baton *eb;
> void *inner_baton;
> - apr_pool_t *subpool = svn_pool_create(pool);
> - svn_delta_editor_t *tree_editor = svn_delta_default_editor(subpool);
> + apr_pool_t *edit_pool = svn_pool_create(result_pool);
> + 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_adm_access_t *adm_access;
> @@ -4873,7 +4947,8 @@ make_editor(svn_revnum_t *target_revisio
>
> /* Get the anchor entry, so we can fetch the repository root. */
> SVN_ERR(svn_wc__node_get_repos_info(&repos_root, &repos_uuid, wc_ctx,
> - anchor_abspath, pool, scratch_pool));
> + anchor_abspath,
> + result_pool, scratch_pool));
>
> /* Disallow a switch operation to change the repository root of the target,
> if that is known. */
> @@ -4886,8 +4961,8 @@ make_editor(svn_revnum_t *target_revisio
> "'%s'"), switch_url, repos_root);
>
> /* Construct an edit baton. */
> - eb = apr_pcalloc(subpool, sizeof(*eb));
> - eb->pool = subpool;
> + eb = apr_pcalloc(edit_pool, sizeof(*eb));
> + eb->pool = edit_pool;
> eb->use_commit_times = use_commit_times;
> eb->target_revision = target_revision;
> eb->switch_url = switch_url;
> @@ -4903,7 +4978,7 @@ make_editor(svn_revnum_t *target_revisio
> eb->target_abspath = eb->anchor_abspath;
> else
> eb->target_abspath = svn_dirent_join(eb->anchor_abspath, target_basename,
> - pool);
> + edit_pool);
>
> eb->requested_depth = depth;
> eb->depth_is_sticky = depth_is_sticky;
> @@ -4919,7 +4994,7 @@ make_editor(svn_revnum_t *target_revisio
> eb->fetch_func = fetch_func;
> eb->fetch_baton = fetch_baton;
> eb->allow_unver_obstructions = allow_unver_obstructions;
> - eb->skipped_trees = apr_hash_make(subpool);
> + eb->skipped_trees = apr_hash_make(edit_pool);
> eb->ext_patterns = preserved_exts;
>
> /* Construct an editor. */
> @@ -4961,7 +5036,7 @@ make_editor(svn_revnum_t *target_revisio
> anchor,
> target_basename,
> wc_ctx->db,
> - pool));
> + result_pool));
>
> return svn_delta_get_cancellation_editor(cancel_func,
> cancel_baton,
> @@ -4969,7 +5044,7 @@ make_editor(svn_revnum_t *target_revisio
> inner_baton,
> editor,
> edit_baton,
> - pool);
> + result_pool);
> }
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=2407610
Hi Bert,
While testing a patch I noticed merge_tests.py 3 was failing. After
confirming it wasn't due to my changes I went looking for the culprit
and it appears to be r40025. The test fails when a file child *and*
directory child of a subtree are deleted and then committed. The
commit appears to succeed but the deleted file still shows as deleted,
e.g.:
trunk.release.build>svn st -v
1 1 jrandom .
1 1 jrandom A
1 1 jrandom A\mu
1 1 jrandom A\B
1 1 jrandom A\B\lambda
1 1 jrandom A\B\E
1 1 jrandom A\B\E\alpha
1 1 jrandom A\B\E\beta
1 1 jrandom A\B\F
1 1 jrandom A\C
1 1 jrandom A\D
1 1 jrandom A\D\gamma
1 1 jrandom A\D\G
1 1 jrandom A\D\G\rho
1 1 jrandom A\D\G\pi
1 1 jrandom A\D\G\tau
1 1 jrandom A\D\H
1 1 jrandom A\D\H\chi
1 1 jrandom A\D\H\omega
1 1 jrandom A\D\H\psi
1 1 jrandom iota
trunk.release.build>svn del A\B\E A\B\lambda
D A\B\E\alpha
D A\B\E\beta
D A\B\E
D A\B\lambda
trunk.release.build>svn ci -m "delete"
Deleting A\B\E
Deleting A\B\lambda
Committed revision 2.
trunk.release.build>svn st -v
1 1 jrandom .
1 1 jrandom A
1 1 jrandom A\mu
1 1 jrandom A\B
D 1 1 jrandom A\B\lambda
1 1 jrandom A\B\F
1 1 jrandom A\C
1 1 jrandom A\D
1 1 jrandom A\D\gamma
1 1 jrandom A\D\G
1 1 jrandom A\D\G\rho
1 1 jrandom A\D\G\pi
1 1 jrandom A\D\G\tau
1 1 jrandom A\D\H
1 1 jrandom A\D\H\chi
1 1 jrandom A\D\H\omega
1 1 jrandom A\D\H\psi
1 1 jrandom iota
Both the file and directory deletes actually happened:
trunk.release.build>svn log -v -r2
------------------------------------------------------------------------
r2 | pburba | 2009-10-16 12:46:27 -0400 (Fri, 16 Oct 2009) | 1 line
Changed paths:
D /A/B/E
D /A/B/lambda
------------------------------------------------------------------------
trunk.release.build>svn up
C A\B\lambda
At revision 2.
Summary of conflicts:
Tree conflicts: 1
The test *only* fails with a release build. Could you do me a favor
and try running the test with a release build and see if it fails for
you?
There seems to be something wrong with the initial checkout (obvious
given the r40025 changes only libsvn_wc/update_editor.c). If I
perform the checkout using a debug build and then manually perform the
deletes and commit with a release build, then the commit works
correctly.
This is still failing for me as of trunk@40079. Assuming you can
replicate this any insight is appreciated.
Paul
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2408282
Re: svn commit: r40025 - trunk/subversion/libsvn_wc
Posted by Greg Stein <gs...@gmail.com>.
On Fri, Oct 16, 2009 at 20:35, Bert Huijben <be...@qqmail.nl> wrote:
>> -----Original Message-----
>> From: Bert Huijben [mailto:rhuijben@sharpsvn.net]
>> Sent: vrijdag 16 oktober 2009 23:24
>> To: 'Paul Burba'; dev@subversion.tigris.org
>> Cc: svn@subversion.tigris.org; gstein@gmail.com
>> Subject: RE: svn commit: r40025 - trunk/subversion/libsvn_wc
>>
>> > -----Original Message-----
>> > From: Paul Burba [mailto:ptburba@gmail.com]
>> > Sent: vrijdag 16 oktober 2009 18:54
>> > To: dev@subversion.tigris.org
>> > Cc: svn@subversion.tigris.org
>> > Subject: Re: svn commit: r40025 - trunk/subversion/libsvn_wc
>>
>> <snip 25 Kbyte patch>
>>
>> >
>> > Hi Bert,
>> >
>> > While testing a patch I noticed merge_tests.py 3 was failing. After
>> > confirming it wasn't due to my changes I went looking for the culprit
>> > and it appears to be r40025. The test fails when a file child *and*
>> > directory child of a subtree are deleted and then committed. The
>> > commit appears to succeed but the deleted file still shows as
>> deleted,
>> > e.g.:
>> >
>> > trunk.release.build>svn st -v
>> > 1 1 jrandom .
>> > 1 1 jrandom A
>> > 1 1 jrandom A\mu
>> > 1 1 jrandom A\B
>> > 1 1 jrandom A\B\lambda
>> > 1 1 jrandom A\B\E
>> > 1 1 jrandom A\B\E\alpha
>> > 1 1 jrandom A\B\E\beta
>> > 1 1 jrandom A\B\F
>> > 1 1 jrandom A\C
>> > 1 1 jrandom A\D
>> > 1 1 jrandom A\D\gamma
>> > 1 1 jrandom A\D\G
>> > 1 1 jrandom A\D\G\rho
>> > 1 1 jrandom A\D\G\pi
>> > 1 1 jrandom A\D\G\tau
>> > 1 1 jrandom A\D\H
>> > 1 1 jrandom A\D\H\chi
>> > 1 1 jrandom A\D\H\omega
>> > 1 1 jrandom A\D\H\psi
>> > 1 1 jrandom iota
>> >
>> > trunk.release.build>svn del A\B\E A\B\lambda
>> > D A\B\E\alpha
>> > D A\B\E\beta
>> > D A\B\E
>> > D A\B\lambda
>> >
>> > trunk.release.build>svn ci -m "delete"
>> > Deleting A\B\E
>> > Deleting A\B\lambda
>> >
>> > Committed revision 2.
>> >
>> > trunk.release.build>svn st -v
>> > 1 1 jrandom .
>> > 1 1 jrandom A
>> > 1 1 jrandom A\mu
>> > 1 1 jrandom A\B
>> > D 1 1 jrandom A\B\lambda
>> > 1 1 jrandom A\B\F
>> > 1 1 jrandom A\C
>> > 1 1 jrandom A\D
>> > 1 1 jrandom A\D\gamma
>> > 1 1 jrandom A\D\G
>> > 1 1 jrandom A\D\G\rho
>> > 1 1 jrandom A\D\G\pi
>> > 1 1 jrandom A\D\G\tau
>> > 1 1 jrandom A\D\H
>> > 1 1 jrandom A\D\H\chi
>> > 1 1 jrandom A\D\H\omega
>> > 1 1 jrandom A\D\H\psi
>> > 1 1 jrandom iota
>> >
>> > Both the file and directory deletes actually happened:
>> >
>> > trunk.release.build>svn log -v -r2
>> > -------------------------------------------------------------------
>> --
>> > ---
>> > r2 | pburba | 2009-10-16 12:46:27 -0400 (Fri, 16 Oct 2009) | 1 line
>> > Changed paths:
>> > D /A/B/E
>> > D /A/B/lambda
>> >
>> >
>> > -------------------------------------------------------------------
>> --
>> > ---
>> >
>> > trunk.release.build>svn up
>> > C A\B\lambda
>> > At revision 2.
>> > Summary of conflicts:
>> > Tree conflicts: 1
>> >
>> > The test *only* fails with a release build. Could you do me a favor
>> > and try running the test with a release build and see if it fails for
>> > you?
>> >
>> > There seems to be something wrong with the initial checkout (obvious
>> > given the r40025 changes only libsvn_wc/update_editor.c). If I
>> > perform the checkout using a debug build and then manually perform
>> the
>> > deletes and commit with a release build, then the commit works
>> > correctly.
>> >
>> > This is still failing for me as of trunk@40079. Assuming you can
>> > replicate this any insight is appreciated.
>> >
>> > Paul
>> >
>>
>>
>> Strange issue this failure, but thanks for the release mode hint.
>
> After a lot of debugging I found the root cause of this issue and the issue
> itself is unrelated to the patch in r40025. It just happens to trigger the
> error because it had some pool and loggy changes, which reordered and
> splitted some loggy operations in multiple wq items.
>
>
>
> Our working queue handling in wc_db.c works with 'wri_paths', which is a raw
> estimation of in which wcroot a path is located.
>
> The problem in this specific testcase is that the wri_path points to a
> different wcroot before and after the working queue item is processed.
>
> workingqueue.c has a simple loop.
>
> Fetch work item and id from wri_path.
>
> <process item>
>
> Delete work item with id from wri_path.
>
>
> In this specific case the wq operation is deleting the directory after
> committing. Which changes the calculation of the wcroot to the parent. Which
> accidentally has a wq item with the same id. (Handling the commit
> processiong of A\B\lambda).
>
>
> I'm not going to fix this issue on this hour (It's 2:30 AM here), so if
> somebody else wants to fix this? (gstein?)
Sure, I'll get it fixed asap. Thanks for digging into this!
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2408430
RE: svn commit: r40025 - trunk/subversion/libsvn_wc
Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: Bert Huijben [mailto:rhuijben@sharpsvn.net]
> Sent: vrijdag 16 oktober 2009 23:24
> To: 'Paul Burba'; dev@subversion.tigris.org
> Cc: svn@subversion.tigris.org; gstein@gmail.com
> Subject: RE: svn commit: r40025 - trunk/subversion/libsvn_wc
>
> > -----Original Message-----
> > From: Paul Burba [mailto:ptburba@gmail.com]
> > Sent: vrijdag 16 oktober 2009 18:54
> > To: dev@subversion.tigris.org
> > Cc: svn@subversion.tigris.org
> > Subject: Re: svn commit: r40025 - trunk/subversion/libsvn_wc
>
> <snip 25 Kbyte patch>
>
> >
> > Hi Bert,
> >
> > While testing a patch I noticed merge_tests.py 3 was failing. After
> > confirming it wasn't due to my changes I went looking for the culprit
> > and it appears to be r40025. The test fails when a file child *and*
> > directory child of a subtree are deleted and then committed. The
> > commit appears to succeed but the deleted file still shows as
> deleted,
> > e.g.:
> >
> > trunk.release.build>svn st -v
> > 1 1 jrandom .
> > 1 1 jrandom A
> > 1 1 jrandom A\mu
> > 1 1 jrandom A\B
> > 1 1 jrandom A\B\lambda
> > 1 1 jrandom A\B\E
> > 1 1 jrandom A\B\E\alpha
> > 1 1 jrandom A\B\E\beta
> > 1 1 jrandom A\B\F
> > 1 1 jrandom A\C
> > 1 1 jrandom A\D
> > 1 1 jrandom A\D\gamma
> > 1 1 jrandom A\D\G
> > 1 1 jrandom A\D\G\rho
> > 1 1 jrandom A\D\G\pi
> > 1 1 jrandom A\D\G\tau
> > 1 1 jrandom A\D\H
> > 1 1 jrandom A\D\H\chi
> > 1 1 jrandom A\D\H\omega
> > 1 1 jrandom A\D\H\psi
> > 1 1 jrandom iota
> >
> > trunk.release.build>svn del A\B\E A\B\lambda
> > D A\B\E\alpha
> > D A\B\E\beta
> > D A\B\E
> > D A\B\lambda
> >
> > trunk.release.build>svn ci -m "delete"
> > Deleting A\B\E
> > Deleting A\B\lambda
> >
> > Committed revision 2.
> >
> > trunk.release.build>svn st -v
> > 1 1 jrandom .
> > 1 1 jrandom A
> > 1 1 jrandom A\mu
> > 1 1 jrandom A\B
> > D 1 1 jrandom A\B\lambda
> > 1 1 jrandom A\B\F
> > 1 1 jrandom A\C
> > 1 1 jrandom A\D
> > 1 1 jrandom A\D\gamma
> > 1 1 jrandom A\D\G
> > 1 1 jrandom A\D\G\rho
> > 1 1 jrandom A\D\G\pi
> > 1 1 jrandom A\D\G\tau
> > 1 1 jrandom A\D\H
> > 1 1 jrandom A\D\H\chi
> > 1 1 jrandom A\D\H\omega
> > 1 1 jrandom A\D\H\psi
> > 1 1 jrandom iota
> >
> > Both the file and directory deletes actually happened:
> >
> > trunk.release.build>svn log -v -r2
> > -------------------------------------------------------------------
> --
> > ---
> > r2 | pburba | 2009-10-16 12:46:27 -0400 (Fri, 16 Oct 2009) | 1 line
> > Changed paths:
> > D /A/B/E
> > D /A/B/lambda
> >
> >
> > -------------------------------------------------------------------
> --
> > ---
> >
> > trunk.release.build>svn up
> > C A\B\lambda
> > At revision 2.
> > Summary of conflicts:
> > Tree conflicts: 1
> >
> > The test *only* fails with a release build. Could you do me a favor
> > and try running the test with a release build and see if it fails for
> > you?
> >
> > There seems to be something wrong with the initial checkout (obvious
> > given the r40025 changes only libsvn_wc/update_editor.c). If I
> > perform the checkout using a debug build and then manually perform
> the
> > deletes and commit with a release build, then the commit works
> > correctly.
> >
> > This is still failing for me as of trunk@40079. Assuming you can
> > replicate this any insight is appreciated.
> >
> > Paul
> >
>
>
> Strange issue this failure, but thanks for the release mode hint.
After a lot of debugging I found the root cause of this issue and the issue
itself is unrelated to the patch in r40025. It just happens to trigger the
error because it had some pool and loggy changes, which reordered and
splitted some loggy operations in multiple wq items.
Our working queue handling in wc_db.c works with 'wri_paths', which is a raw
estimation of in which wcroot a path is located.
The problem in this specific testcase is that the wri_path points to a
different wcroot before and after the working queue item is processed.
workingqueue.c has a simple loop.
Fetch work item and id from wri_path.
<process item>
Delete work item with id from wri_path.
In this specific case the wq operation is deleting the directory after
committing. Which changes the calculation of the wcroot to the parent. Which
accidentally has a wq item with the same id. (Handling the commit
processiong of A\B\lambda).
I'm not going to fix this issue on this hour (It's 2:30 AM here), so if
somebody else wants to fix this? (gstein?)
Bert
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2408404
RE: svn commit: r40025 - trunk/subversion/libsvn_wc
Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: Paul Burba [mailto:ptburba@gmail.com]
> Sent: vrijdag 16 oktober 2009 18:54
> To: dev@subversion.tigris.org
> Cc: svn@subversion.tigris.org
> Subject: Re: svn commit: r40025 - trunk/subversion/libsvn_wc
<snip 25 Kbyte patch>
>
> Hi Bert,
>
> While testing a patch I noticed merge_tests.py 3 was failing. After
> confirming it wasn't due to my changes I went looking for the culprit
> and it appears to be r40025. The test fails when a file child *and*
> directory child of a subtree are deleted and then committed. The
> commit appears to succeed but the deleted file still shows as deleted,
> e.g.:
>
> trunk.release.build>svn st -v
> 1 1 jrandom .
> 1 1 jrandom A
> 1 1 jrandom A\mu
> 1 1 jrandom A\B
> 1 1 jrandom A\B\lambda
> 1 1 jrandom A\B\E
> 1 1 jrandom A\B\E\alpha
> 1 1 jrandom A\B\E\beta
> 1 1 jrandom A\B\F
> 1 1 jrandom A\C
> 1 1 jrandom A\D
> 1 1 jrandom A\D\gamma
> 1 1 jrandom A\D\G
> 1 1 jrandom A\D\G\rho
> 1 1 jrandom A\D\G\pi
> 1 1 jrandom A\D\G\tau
> 1 1 jrandom A\D\H
> 1 1 jrandom A\D\H\chi
> 1 1 jrandom A\D\H\omega
> 1 1 jrandom A\D\H\psi
> 1 1 jrandom iota
>
> trunk.release.build>svn del A\B\E A\B\lambda
> D A\B\E\alpha
> D A\B\E\beta
> D A\B\E
> D A\B\lambda
>
> trunk.release.build>svn ci -m "delete"
> Deleting A\B\E
> Deleting A\B\lambda
>
> Committed revision 2.
>
> trunk.release.build>svn st -v
> 1 1 jrandom .
> 1 1 jrandom A
> 1 1 jrandom A\mu
> 1 1 jrandom A\B
> D 1 1 jrandom A\B\lambda
> 1 1 jrandom A\B\F
> 1 1 jrandom A\C
> 1 1 jrandom A\D
> 1 1 jrandom A\D\gamma
> 1 1 jrandom A\D\G
> 1 1 jrandom A\D\G\rho
> 1 1 jrandom A\D\G\pi
> 1 1 jrandom A\D\G\tau
> 1 1 jrandom A\D\H
> 1 1 jrandom A\D\H\chi
> 1 1 jrandom A\D\H\omega
> 1 1 jrandom A\D\H\psi
> 1 1 jrandom iota
>
> Both the file and directory deletes actually happened:
>
> trunk.release.build>svn log -v -r2
> ---------------------------------------------------------------------
> ---
> r2 | pburba | 2009-10-16 12:46:27 -0400 (Fri, 16 Oct 2009) | 1 line
> Changed paths:
> D /A/B/E
> D /A/B/lambda
>
>
> ---------------------------------------------------------------------
> ---
>
> trunk.release.build>svn up
> C A\B\lambda
> At revision 2.
> Summary of conflicts:
> Tree conflicts: 1
>
> The test *only* fails with a release build. Could you do me a favor
> and try running the test with a release build and see if it fails for
> you?
>
> There seems to be something wrong with the initial checkout (obvious
> given the r40025 changes only libsvn_wc/update_editor.c). If I
> perform the checkout using a debug build and then manually perform the
> deletes and commit with a release build, then the commit works
> correctly.
>
> This is still failing for me as of trunk@40079. Assuming you can
> replicate this any insight is appreciated.
>
> Paul
>
Strange issue this failure, but thanks for the release mode hint.
On the Windows buildbot -which always uses release mode- this tests passes, but on the x64-ubuntu buildbot -which uses debug mode- the test fails.
But at least I can reproduce the issue now :)
I will look into this issue tomorrow.
Not sure what I will find, because around this commit the commit code was updated too...
Bert
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2408358