You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bert Huijben <be...@qqmail.nl> on 2010/03/10 16:57:38 UTC

RE: svn commit: r921445 - in /subversion/trunk/subversion: include/private/svn_wc_private.h include/svn_wc.h libsvn_client/commit.c libsvn_wc/entries.c


> -----Original Message-----
> From: philip@apache.org [mailto:philip@apache.org]
> Sent: woensdag 10 maart 2010 17:47
> To: commits@subversion.apache.org
> Subject: svn commit: r921445 - in /subversion/trunk/subversion:
> include/private/svn_wc_private.h include/svn_wc.h libsvn_client/commit.c
> libsvn_wc/entries.c
> 
> Author: philip
> Date: Wed Mar 10 16:47:27 2010
> New Revision: 921445
> 
> URL: http://svn.apache.org/viewvc?rev=921445&view=rev
> Log:
> Remove some access batons from post-commit processing.
> 
> * subversion/include/svn_wc.h
>   (svn_wc_mark_missing_deleted): Deprecate.
> 
> * subversion/include/private/svn_wc_private.h
>   (svn_wc__temp_mark_missing_not_present): New.
> 
> * subversion/libsvn_wc/entries.c
>   (svn_wc__temp_mark_missing_not_present): New.
>   (svn_wc_mark_missing_deleted): Call
> svn_wc__temp_mark_missing_not_present.
> 
> * subversion/libsvn_client/commit.c
>   (struct post_commit_baton): Replace access baton member with wc
> context.
>   (post_process_commit_item): Use db functions instead of access baton.
>   (svn_client_commit4): Store wc context instead of access baton.
> 
> Modified:
>     subversion/trunk/subversion/include/private/svn_wc_private.h
>     subversion/trunk/subversion/include/svn_wc.h
>     subversion/trunk/subversion/libsvn_client/commit.c
>     subversion/trunk/subversion/libsvn_wc/entries.c
> 
> Modified: subversion/trunk/subversion/include/private/svn_wc_private.h
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private
> /svn_wc_private.h?rev=921445&r1=921444&r2=921445&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/include/private/svn_wc_private.h (original)
> +++ subversion/trunk/subversion/include/private/svn_wc_private.h Wed
> Mar 10 16:47:27 2010
> @@ -552,6 +552,18 @@ svn_wc__call_with_write_lock(svn_wc__wit
>                               apr_pool_t *result_pool,
>                               apr_pool_t *scratch_pool);
> 
> +
> +/** Mark missing, deleted directory @a local_abspath as 'not-present'
> + * in its parent's list of entries.
> + *
> + * Return #SVN_ERR_WC_PATH_FOUND if @a local_abspath isn't actually a
> + * missing, deleted directory.
> + */
> +svn_error_t *
> +svn_wc__temp_mark_missing_not_present(const char *local_abspath,
> +                                      svn_wc_context_t *wc_ctx,
> +                                      apr_pool_t *scratch_pool);
> +
>  #ifdef __cplusplus
>  }
>  #endif /* __cplusplus */
> 
> Modified: subversion/trunk/subversion/include/svn_wc.h
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_wc
> .h?rev=921445&r1=921444&r2=921445&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/include/svn_wc.h (original)
> +++ subversion/trunk/subversion/include/svn_wc.h Wed Mar 10 16:47:27
> 2010
> @@ -3312,7 +3312,10 @@ svn_wc_walk_entries(const char *path,
>  /** Mark missing @a path as 'deleted' in its @a parent's list of entries.
>   *
>   * Return #SVN_ERR_WC_PATH_FOUND if @a path isn't actually missing.
> + *
> + * @deprecated Provided for backward compatibility with the 1.6 API.
>   */
> +SVN_DEPRECATED
>  svn_error_t *
>  svn_wc_mark_missing_deleted(const char *path,
>                              svn_wc_adm_access_t *parent,
> 
> Modified: subversion/trunk/subversion/libsvn_client/commit.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/co
> mmit.c?rev=921445&r1=921444&r2=921445&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_client/commit.c (original)
> +++ subversion/trunk/subversion/libsvn_client/commit.c Wed Mar 10
> 16:47:27 2010
> @@ -935,7 +935,7 @@ struct post_commit_baton
>  {
>    svn_wc_committed_queue_t *queue;
>    apr_pool_t *qpool;
> -  svn_wc_adm_access_t *base_dir_access;
> +  svn_wc_context_t *wc_ctx;
>    svn_boolean_t keep_changelists;
>    svn_boolean_t keep_locks;
>    apr_hash_t *checksums;
> @@ -950,35 +950,23 @@ post_process_commit_item(void *baton, vo
>    svn_client_commit_item3_t *item =
>      *(svn_client_commit_item3_t **)this_item;
>    svn_boolean_t loop_recurse = FALSE;
> -  const char *adm_access_path;
> -  svn_wc_adm_access_t *adm_access;
>    svn_boolean_t remove_lock;
> -  svn_error_t *bump_err;
> -
> -  if (item->kind == svn_node_dir)
> -    adm_access_path = item->path;
> -  else
> -    adm_access_path = svn_dirent_dirname(item->path, pool);
> 
> -  bump_err = svn_wc_adm_retrieve(&adm_access, btn->base_dir_access,
> -                                 adm_access_path, pool);
> -  if (bump_err
> -      && bump_err->apr_err == SVN_ERR_WC_NOT_LOCKED)
> +  /* Is it a missing, deleted directory?
> +
> +     ### Temporary: once we centralise this sort of node is just a
> +     normal delete and will get handled by the post-commit queue. */
> +  if (item->kind == svn_node_dir
> +      && item->state_flags & SVN_CLIENT_COMMIT_ITEM_DELETE)
>      {
> -      /* Is it a directory that was deleted in the commit?
> -         Then we probably committed a missing directory. */
> -      if (item->kind == svn_node_dir
> -          && item->state_flags & SVN_CLIENT_COMMIT_ITEM_DELETE)
> -        {
> -          /* Mark it as deleted in the parent. */
> -          svn_error_clear(bump_err);
> -          return svn_wc_mark_missing_deleted(item->path,
> -                                             btn->base_dir_access, pool);
> -        }
> -    }
> -  if (bump_err)
> -    return bump_err;
> +      svn_boolean_t obstructed;
> 
> +      SVN_ERR(svn_wc__node_is_status_obstructed(&obstructed,
> +                                                btn->wc_ctx, item->path, pool));
> +      if (obstructed)
> +        return svn_wc__temp_mark_missing_not_present(item->path,
> +                                                     btn->wc_ctx, pool);
> +    }
> 
>    if ((item->state_flags & SVN_CLIENT_COMMIT_ITEM_ADD)
>        && (item->kind == svn_node_dir)
> @@ -1282,7 +1270,7 @@ svn_client_commit4(svn_commit_info_t **c
> 
>        btn.queue = queue;
>        btn.qpool = pool;
> -      btn.base_dir_access = base_dir_access;
> +      btn.wc_ctx = ctx->wc_ctx;
>        btn.keep_changelists = keep_changelists;
>        btn.keep_locks = keep_locks;
>        btn.checksums = checksums;
> 
> Modified: subversion/trunk/subversion/libsvn_wc/entries.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/entri
> es.c?rev=921445&r1=921444&r2=921445&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_wc/entries.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/entries.c Wed Mar 10 16:47:27
> 2010
> @@ -3365,36 +3365,60 @@ svn_wc_walk_entries3(const char *path,
>  }
> 
>  svn_error_t *
> -svn_wc_mark_missing_deleted(const char *path,
> -                            svn_wc_adm_access_t *parent,
> -                            apr_pool_t *pool)
> +svn_wc__temp_mark_missing_not_present(const char *local_abspath,
> +                                      svn_wc_context_t *wc_ctx,
> +                                      apr_pool_t *scratch_pool)
>  {
> -  svn_node_kind_t pkind;
> -  const char *local_abspath;
> -  svn_wc__db_t *db = svn_wc__adm_get_db(parent);
> -
> -  SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, pool));
> +  svn_wc__db_status_t status;
> +  svn_wc__db_kind_t kind;
> 
> -  SVN_ERR(svn_io_check_path(path, &pkind, pool));
> -
> -  if (pkind == svn_node_none)
> +  SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath));
> +  SVN_ERR(svn_wc__db_read_info(&status, &kind,
> +                               NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> +                               NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> +                               NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> +                               NULL,
> +                               wc_ctx->db, local_abspath,
> +                               scratch_pool, scratch_pool));
> +  if (kind == svn_wc__db_kind_dir
> +      && status == svn_wc__db_status_obstructed_delete)
>      {
>        svn_wc_entry_t tmp_entry;
> 
>        tmp_entry.deleted = TRUE;
>        tmp_entry.schedule = svn_wc_schedule_normal;
> 
> -      return svn_error_return(
> -        svn_wc__entry_modify2(db, local_abspath, svn_node_unknown,
> FALSE,
> -                              &tmp_entry,
> -                              (SVN_WC__ENTRY_MODIFY_DELETED
> -                               | SVN_WC__ENTRY_MODIFY_SCHEDULE
> -                               | SVN_WC__ENTRY_MODIFY_FORCE),
> -                              pool));
> +      SVN_ERR(svn_wc__entry_modify2(wc_ctx->db, local_abspath,
> +                                    svn_node_unknown, FALSE, &tmp_entry,
> +                                    (SVN_WC__ENTRY_MODIFY_DELETED
> +                                     | SVN_WC__ENTRY_MODIFY_SCHEDULE
> +                                     | SVN_WC__ENTRY_MODIFY_FORCE),
> +                                    scratch_pool));

Shouldn't this be svn_node_directory and TRUE for parent_stub instead of unknown and FALSE?

Only directories can be in obstructed_delete mode, and in this case I don't think you can't update the node in the directory itself.

	Bert


Re: svn commit: r921445 - in /subversion/trunk/subversion: include/private/svn_wc_private.h include/svn_wc.h libsvn_client/commit.c libsvn_wc/entries.c

Posted by Greg Stein <gs...@gmail.com>.
On Wed, Mar 10, 2010 at 12:52, Philip Martin <ph...@wandisco.com> wrote:
> Philip Martin <ph...@wandisco.com> writes:
>
>> "Bert Huijben" <be...@qqmail.nl> writes:
>>>
>>> Shouldn't this be svn_node_directory and TRUE for parent_stub
>>> instead of unknown and FALSE?
>>>
>>> Only directories can be in obstructed_delete mode, and in this case
>>> I don't think you can't update the node in the directory itself.
>>
>> Dunno. I just moved the existing call.  I'll investigate.
>
> Seems to work either way, I guess modify2 has logic to deal with
> it. I've committed the change.

Yes, it does. With the svn_node_unknown in there, the code will look
at the disk to try and figure out what is going on. In this case, it
finds nothing on disk, so it says "read metadata from the parent, and
you'll find NAME in there". Reading the metadata can then tell you
whether NAME was supposed to be a file, a directory, or not even under
version control.

But it is MUCH better to pass what you know, rather than
svn_node_unknown. We won't stat() the disk in that case.

(see entries.c::get_entry_access_info())

Cheers,
-g

Re: svn commit: r921445 - in /subversion/trunk/subversion: include/private/svn_wc_private.h include/svn_wc.h libsvn_client/commit.c libsvn_wc/entries.c

Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:

> "Bert Huijben" <be...@qqmail.nl> writes:
>>
>> Shouldn't this be svn_node_directory and TRUE for parent_stub
>> instead of unknown and FALSE?
>>
>> Only directories can be in obstructed_delete mode, and in this case
>> I don't think you can't update the node in the directory itself.
>
> Dunno. I just moved the existing call.  I'll investigate.

Seems to work either way, I guess modify2 has logic to deal with
it. I've committed the change.

-- 
Philip

Re: svn commit: r921445 - in /subversion/trunk/subversion: include/private/svn_wc_private.h include/svn_wc.h libsvn_client/commit.c libsvn_wc/entries.c

Posted by Philip Martin <ph...@wandisco.com>.
"Bert Huijben" <be...@qqmail.nl> writes:

>>        svn_wc_entry_t tmp_entry;
>> 
>>        tmp_entry.deleted = TRUE;
>>        tmp_entry.schedule = svn_wc_schedule_normal;
>> 
>> -      return svn_error_return(
>> -        svn_wc__entry_modify2(db, local_abspath, svn_node_unknown,
>> FALSE,
>> -                              &tmp_entry,
>> -                              (SVN_WC__ENTRY_MODIFY_DELETED
>> -                               | SVN_WC__ENTRY_MODIFY_SCHEDULE
>> -                               | SVN_WC__ENTRY_MODIFY_FORCE),
>> -                              pool));
>> +      SVN_ERR(svn_wc__entry_modify2(wc_ctx->db, local_abspath,
>> +                                    svn_node_unknown, FALSE, &tmp_entry,
>> +                                    (SVN_WC__ENTRY_MODIFY_DELETED
>> +                                     | SVN_WC__ENTRY_MODIFY_SCHEDULE
>> +                                     | SVN_WC__ENTRY_MODIFY_FORCE),
>> +                                    scratch_pool));
>
> Shouldn't this be svn_node_directory and TRUE for parent_stub
> instead of unknown and FALSE?
>
> Only directories can be in obstructed_delete mode, and in this case
> I don't think you can't update the node in the directory itself.

Dunno. I just moved the existing call.  I'll investigate.

-- 
Philip