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

Re: svn commit: r1126525 - in /subversion/trunk/subversion: libsvn_client/delete.c tests/cmdline/basic_tests.py

From: Greg Stein <gstein_at_gmail.com>
Date: Mon, 23 May 2011 11:15:08 -0400

On Mon, May 23, 2011 at 11:00, <hwright_at_apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_client/delete.c Mon May 23 15:00:50 2011
> @@ -141,36 +141,22 @@ path_driver_cb_func(void **dir_baton,
>   return editor->delete_entry(path, SVN_INVALID_REVNUM, parent_baton, pool);
>  }
>
> -
>  static svn_error_t *
> -delete_urls(const apr_array_header_t *paths,
> -            const apr_hash_t *revprop_table,
> -            svn_commit_callback2_t commit_callback,
> -            void *commit_baton,
> -            svn_client_ctx_t *ctx,
> -            apr_pool_t *pool)
> +single_repos_delete(svn_ra_session_t *ra_session,
> +                    const char *repos_root,
> +                    const apr_array_header_t *relpaths,
> +                    const apr_hash_t *revprop_table,
> +                    svn_commit_callback2_t commit_callback,
> +                    void *commit_baton,
> +                    svn_client_ctx_t *ctx,
> +                    apr_pool_t *pool)
>  {
> -  svn_ra_session_t *ra_session = NULL;
>   const svn_delta_editor_t *editor;
> +  apr_hash_t *commit_revprops;
>   void *edit_baton;
>   const char *log_msg;
> -  svn_node_kind_t kind;
> -  apr_array_header_t *targets;
> -  apr_hash_t *commit_revprops;
> -  svn_error_t *err;
> -  const char *common_url;
>   int i;
> -  apr_pool_t *iterpool = svn_pool_create(pool);
> -
> -  /* Condense our list of deletion targets. */
> -  SVN_ERR(svn_uri_condense_targets(&common_url, &targets, paths, TRUE,
> -                                   pool, iterpool));
> -  if (! targets->nelts)
> -    {
> -      const char *bname;
> -      svn_uri_split(&common_url, &bname, common_url, pool);
> -      APR_ARRAY_PUSH(targets, const char *) = bname;
> -    }
> +  svn_error_t *err;
>
>   /* Create new commit items and add them to the array. */
>   if (SVN_CLIENT__HAS_LOG_MSG_FUNC(ctx))
> @@ -178,24 +164,21 @@ delete_urls(const apr_array_header_t *pa
>       svn_client_commit_item3_t *item;
>       const char *tmp_file;
>       apr_array_header_t *commit_items
> -        = apr_array_make(pool, targets->nelts, sizeof(item));
> +        = apr_array_make(pool, relpaths->nelts, sizeof(item));
>
> -      for (i = 0; i < targets->nelts; i++)
> +      for (i = 0; i < relpaths->nelts; i++)
>         {
> -          const char *relpath = APR_ARRAY_IDX(targets, i, const char *);
> +          const char *relpath = APR_ARRAY_IDX(relpaths, i, const char *);
>
>           item = svn_client_commit_item3_create(pool);
> -          item->url = svn_path_url_add_component2(common_url, relpath, pool);
> +          item->url = svn_path_url_add_component2(repos_root, relpath, pool);
>           item->state_flags = SVN_CLIENT_COMMIT_ITEM_DELETE;
>           APR_ARRAY_PUSH(commit_items, svn_client_commit_item3_t *) = item;
>         }
>       SVN_ERR(svn_client__get_log_msg(&log_msg, &tmp_file, commit_items,
>                                       ctx, pool));
>       if (! log_msg)
> -        {
> -          svn_pool_destroy(iterpool);
> -          return SVN_NO_ERROR;
> -        }
> +        return SVN_NO_ERROR;
>     }
>   else
>     log_msg = "";
> @@ -203,30 +186,8 @@ delete_urls(const apr_array_header_t *pa
>   SVN_ERR(svn_client__ensure_revprop_table(&commit_revprops, revprop_table,
>                                            log_msg, ctx, pool));
>
> -  /* Open ra session to the common parent of our deletes */
> -  SVN_ERR(svn_client__open_ra_session_internal(&ra_session, NULL,
> -                                               common_url, NULL, NULL,
> -                                               FALSE, TRUE, ctx, pool));
> -
> -  /* Verify that each thing to be deleted actually exists (to prevent
> -     the creation of a revision that has no changes, since the
> -     filesystem allows for no-op deletes).  */
> -  for (i = 0; i < targets->nelts; i++)
> -    {
> -      const char *relpath = APR_ARRAY_IDX(targets, i, const char *);
> -
> -      svn_pool_clear(iterpool);
> -
> -      SVN_ERR(svn_ra_check_path(ra_session, relpath, SVN_INVALID_REVNUM,
> -                                &kind, iterpool));
> -      if (kind == svn_node_none)
> -        {
> -          return svn_error_createf(SVN_ERR_FS_NOT_FOUND, NULL,
> -                                   _("URL '%s' does not exist"),
> -                                   svn_path_url_add_component2(common_url,
> -                                                               relpath, pool));
> -        }
> -    }
> +  /* Reparent the RA_session to the repos_root. */
> +  SVN_ERR(svn_ra_reparent(ra_session, repos_root, pool));

You shouldn't need to do this. See below.

>
>   /* Fetch RA commit editor */
>   SVN_ERR(svn_ra_get_commit_editor3(ra_session, &editor, &edit_baton,
> @@ -238,10 +199,9 @@ delete_urls(const apr_array_header_t *pa
>
>   /* Call the path-based editor driver. */
>   err = svn_delta_path_driver(editor, edit_baton, SVN_INVALID_REVNUM,
> -                              targets, path_driver_cb_func,
> -                              (void *)editor, iterpool);
> +                              relpaths, path_driver_cb_func,
> +                              (void *)editor, pool);
>
> -  svn_pool_destroy(iterpool);
>   if (err)
>     {
>       return svn_error_return(
> @@ -253,6 +213,100 @@ delete_urls(const apr_array_header_t *pa
>   return svn_error_return(editor->close_edit(edit_baton, pool));
>  }
>
> +static svn_error_t *
> +delete_urls_multi_repos(const apr_array_header_t *uris,
> +                        const apr_hash_t *revprop_table,
> +                        svn_commit_callback2_t commit_callback,
> +                        void *commit_baton,
> +                        svn_client_ctx_t *ctx,
> +                        apr_pool_t *pool)
> +{
> +  apr_hash_t *sessions = apr_hash_make(pool);
> +  apr_hash_t *relpaths = apr_hash_make(pool);
> +  apr_hash_index_t *hi;
> +  int i;
> +
> +  /* Create a hash of repos_root -> ra_session maps and repos_root -> relpaths
> +     maps, used to group the various targets. */

It isn't clear that "relpaths" is an array of char* relpaths.

> +  for (i = 0; i < uris->nelts; i++)
> +    {
> +      const char *uri = APR_ARRAY_IDX(uris, i, const char *);
> +      svn_ra_session_t *ra_session = NULL;
> +      const char *repos_root = NULL;
> +      const char *repos_relpath = NULL;
> +      apr_array_header_t *relpaths_list;
> +      svn_node_kind_t kind;
> +
> +      for (hi = apr_hash_first(pool, sessions); hi; hi = apr_hash_next(hi))
> +        {
> +          repos_root = svn__apr_hash_index_key(hi);
> +          repos_relpath = svn_uri_is_child(repos_root, uri, pool);
> +
> +          if (repos_relpath)
> +            {
> +              /* Great!  We've found another uri underneath this session,
> +                 store it and move on. */
> +              ra_session = svn__apr_hash_index_val(hi);
> +              relpaths_list = apr_hash_get(relpaths, repos_root,
> +                                           APR_HASH_KEY_STRING);
> +
> +              APR_ARRAY_PUSH(relpaths_list, const char *) =
> +                                    svn_path_uri_decode(repos_relpath, pool);

svn_uri_is_child() already decodes its result. You're double-decoding.

> +
> +              break;
> +            }
> +        }
> +
> +      if (!ra_session)
> +        {
> +          /* If we haven't found a session yet, we need to open one up.
> +             Note that we don't have a local directory, nor a place
> +             to put temp files. */
> +          SVN_ERR(svn_client__open_ra_session_internal(&ra_session, NULL, uri,
> +                                                       NULL, NULL, FALSE,
> +                                                       TRUE, ctx, pool));
> +          SVN_ERR(svn_ra_get_repos_root2(ra_session, &repos_root, pool));
> +
> +          apr_hash_set(sessions, repos_root, APR_HASH_KEY_STRING, ra_session);
> +          repos_relpath = svn_uri_is_child(repos_root, uri, pool);
> +
> +          relpaths_list = apr_array_make(pool, 1, sizeof(const char *));
> +          apr_hash_set(relpaths, repos_root, APR_HASH_KEY_STRING,
> +                       relpaths_list);
> +          APR_ARRAY_PUSH(relpaths_list, const char *) =
> +                                    svn_path_uri_decode(repos_relpath, pool);

Double-decode again.

> +        }
> +
> +      /* Now, test to see if the thing actually exists. */
> +      SVN_ERR(svn_ra_reparent(ra_session, uri, pool));
> +      SVN_ERR(svn_ra_check_path(ra_session, "", SVN_INVALID_REVNUM, &kind,
> +                                pool));

Rather than reparenting to the URI and checking "" ... why not
reparent the session (when created) to REPOS_ROOT and then use the
REPOS_RELPATH variable? You could then remove the reparent from the
single_repos_delete() function.

> +      if (kind == svn_node_none)
> +        return svn_error_createf(SVN_ERR_FS_NOT_FOUND, NULL,
> +                                 "URL '%s' does not exist", uri);
> +    }
> +
> +  /* At this point, we should have two hashs:
> +      SESSIONS maps repos_roots to ra_sessions.
> +      RELPATHS maps repos_roots to a list of decoded relpaths for that root.
> +
> +     Now we iterate over the collection of sessions and do a commit for each
> +     one with the collected relpaths. */
> +  for (hi = apr_hash_first(pool, sessions); hi; hi = apr_hash_next(hi))
> +    {
> +      const char *repos_root = svn__apr_hash_index_key(hi);
> +      svn_ra_session_t *ra_session = svn__apr_hash_index_val(hi);
> +      apr_array_header_t *relpaths_list = apr_hash_get(relpaths, repos_root,
> +                                                       APR_HASH_KEY_STRING);

const!

>...

Cheers,
-g
Received on 2011-05-23 17:15:39 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.