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

RE: svn commit: r39616 - in trunk/subversion: libsvn_client tests/cmdline

From: Bert Huijben <rhuijben_at_sharpsvn.net>
Date: Sat, 26 Sep 2009 09:02:45 +0200

> -----Original Message-----
> From: Hyrum K. Wright [mailto:hyrum_at_hyrumwright.org]
> Sent: zaterdag 26 september 2009 2:51
> To: svn_at_subversion.tigris.org
> Subject: svn commit: r39616 - in trunk/subversion: libsvn_client
> tests/cmdline
>
> Author: hwright
> Date: Fri Sep 25 17:51:15 2009
> New Revision: 39616
>
> Log:
> Make 'svn info' use the baton-less entry crawler. This also has the
> side-effect of returning the absolute path back through the info
> callback.
>
> * subversion/tests/cmdline/changelist_tests.py
> (info_with_changelists): Expect an absolute path.
>
> * subversion/tests/cmdline/basic_tests.py
> (basic_info): Same.
>
> * subversion/tests/cmdline/info_tests.py
> (info_on_added_file): Same.

I think this change breaks a lot of user scripts that expected to see the
path that they passed here. We can add an absolute path line if we like. The
status apis had the same internal change, but they still provide identical
relative paths to its users.

>
> * subversion/libsvn_client/info.c
> (build_info_from_entry): Rename to...
> (build_info_for_entry): ...this, and update to implement the correct
> callback
> type. Merge the error handling code into this function.
> (info_error_handler): Remove.
> (crawl_entries): Update to call the node walker.
> (entry_walk_callbacks): Remove.
> (svn_client_info2): Call crawl_entries() with an absolute path.

And I know at least a few of my own scripts aren't going to like info2
returning relative paths. I think there will be hundreds/thousands more.

I think we should make svn_client_info2() pass relative paths again as a
compatibility wrapper and introduce a svn_client_info3() for this and other
changes. (E.g. the new conflicts stuff).

        Bert

>
> Modified:
> trunk/subversion/libsvn_client/info.c
> trunk/subversion/tests/cmdline/basic_tests.py
> trunk/subversion/tests/cmdline/changelist_tests.py
> trunk/subversion/tests/cmdline/info_tests.py
>
> Modified: trunk/subversion/libsvn_client/info.c
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/info.c?
> pathrev=39616&r1=39615&r2=39616
> =======================================================================
> =======
> --- trunk/subversion/libsvn_client/info.c Fri Sep 25 17:39:00 2009
> (r39615)
> +++ trunk/subversion/libsvn_client/info.c Fri Sep 25 17:51:15 2009
> (r39616)
> @@ -82,16 +82,17 @@ build_info_from_dirent(svn_info_t **info
> allocated in POOL. Pointer fields are copied by reference, not
> dup'd.
> PATH is the path of the WC node that ENTRY represents. */
> static svn_error_t *
> -build_info_from_entry(svn_info_t **info,
> - svn_wc_context_t *wc_ctx,
> - const svn_wc_entry_t *entry,
> - const char *path,
> - apr_pool_t *pool)
> +build_info_for_entry(svn_info_t **info,
> + svn_wc_context_t *wc_ctx,
> + const char *local_abspath,
> + apr_pool_t *pool)
> {
> - const char *local_abspath;
> svn_info_t *tmpinfo = apr_pcalloc(pool, sizeof(*tmpinfo));
> + const svn_wc_entry_t *entry;
>
> - SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, pool));
> + SVN_ERR(svn_wc__get_entry_versioned(&entry, wc_ctx, local_abspath,
> + svn_node_unknown, TRUE, FALSE,
> + pool, pool));
>
> SVN_ERR(svn_wc__node_get_kind(&tmpinfo->kind, wc_ctx, local_abspath,
> TRUE,
> pool));
> @@ -269,123 +270,115 @@ struct found_entry_baton
> svn_wc_context_t *wc_ctx;
> };
>
> -/* An svn_wc_entry_callbacks2_t callback function. */
> +/* An svn_wc__node_found_func_t callback function. */
> static svn_error_t *
> -info_found_entry_callback(const char *path,
> - const svn_wc_entry_t *entry,
> - void *walk_baton,
> - apr_pool_t *pool)
> +info_found_node_callback(const char *local_abspath,
> + void *walk_baton,
> + apr_pool_t *pool)
> {
> struct found_entry_baton *fe_baton = walk_baton;
> - const char *local_abspath;
> -
> - /* We're going to receive dirents twice; we want to ignore the
> - first one (where it's a child of a parent dir), and only print
> - the second one (where we're looking at THIS_DIR.) */
> - if ((entry->kind == svn_node_dir)
> - && (strcmp(entry->name, SVN_WC_ENTRY_THIS_DIR)))
> - return SVN_NO_ERROR;
>
> - SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, pool));
> if (svn_wc__changelist_match(fe_baton->wc_ctx, local_abspath,
> fe_baton->changelist_hash, pool))
> {
> svn_info_t *info;
> const svn_wc_conflict_description2_t *tmp_conflict;
> + svn_error_t *err;
> +
> + err = build_info_for_entry(&info, fe_baton->wc_ctx,
> local_abspath,
> + pool);
> + if (err && err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND)
> + {
> + /* Check for a tree conflict, and if there is one, send a
> minimal
> + info struct. */
> + const svn_wc_conflict_description2_t *tree_conflict;
> +
> + SVN_ERR(svn_wc__get_tree_conflict(&tree_conflict, fe_baton-
> >wc_ctx,
> + local_abspath, pool,
> pool));
> +
> + if (tree_conflict)
> + {
> + svn_error_clear(err);
> +
> + SVN_ERR(build_info_for_unversioned(&info, pool));
> + SVN_ERR(svn_wc__node_get_repos_info(&(info-
> >repos_root_URL),
> + NULL,
> + fe_baton->wc_ctx,
> + local_abspath,
> + pool, pool));
> + }
> + else
> + return svn_error_return(err);
> + }
> + else if (err)
> + return svn_error_return(err);
>
> - SVN_ERR(build_info_from_entry(&info, fe_baton->wc_ctx, entry,
> path,
> - pool));
> SVN_ERR(svn_wc__get_tree_conflict(&tmp_conflict, fe_baton-
> >wc_ctx,
> local_abspath, pool, pool));
> if (tmp_conflict)
> info->tree_conflict = svn_wc__cd2_to_cd(tmp_conflict, pool);
> - SVN_ERR(fe_baton->receiver(fe_baton->receiver_baton, path, info,
> pool));
> + SVN_ERR(fe_baton->receiver(fe_baton->receiver_baton,
> local_abspath,
> + info, pool));
> }
> return SVN_NO_ERROR;
> }
>
> -/* An svn_wc_entry_callbacks2_t callback function.
> - Handle an error encountered by the walker.
> - If the error is "unversioned resource" and, upon checking the
> - parent dir's tree conflict data, we find that PATH is a tree
> - conflict victim, cancel the error and send a minimal info struct.
> - Otherwise re-raise the error.
> -*/
> +
> +/* Helper function: push the svn_wc_entry_t for WCPATH at
> + RECEIVER/BATON, and possibly recurse over more entries. */
> static svn_error_t *
> -info_error_handler(const char *path,
> - svn_error_t *err,
> - void *walk_baton,
> - apr_pool_t *pool)
> +crawl_entries(const char *local_abspath,
> + svn_info_receiver_t receiver,
> + void *receiver_baton,
> + svn_depth_t depth,
> + apr_hash_t *changelist_hash,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool)
> {
> - if (err && (err->apr_err == SVN_ERR_UNVERSIONED_RESOURCE))
> + struct found_entry_baton fe_baton;
> + svn_error_t *err;
> +
> + fe_baton.changelist_hash = changelist_hash;
> + fe_baton.receiver = receiver;
> + fe_baton.receiver_baton = receiver_baton;
> + fe_baton.wc_ctx = ctx->wc_ctx;
> +
> + err = svn_wc__node_walk_children(ctx->wc_ctx, local_abspath, FALSE,
> + info_found_node_callback,
> &fe_baton, depth,
> + ctx->cancel_func, ctx-
> >cancel_baton, pool);
> +
> + if (err && err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND)
> {
> - struct found_entry_baton *fe_baton = walk_baton;
> + /* Check for a tree conflict on the root node of the info, and
> if there
> + is one, send a minimal info struct. */
> const svn_wc_conflict_description2_t *tree_conflict;
> - const char *local_abspath;
>
> - SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, pool));
> - SVN_ERR(svn_wc__get_tree_conflict(&tree_conflict, fe_baton-
> >wc_ctx,
> + SVN_ERR(svn_wc__get_tree_conflict(&tree_conflict, ctx->wc_ctx,
> local_abspath, pool, pool));
>
> if (tree_conflict)
> {
> svn_info_t *info;
> -
> svn_error_clear(err);
>
> SVN_ERR(build_info_for_unversioned(&info, pool));
> info->tree_conflict = svn_wc__cd2_to_cd(tree_conflict,
> pool);
> -
> +
> SVN_ERR(svn_wc__node_get_repos_info(&(info->repos_root_URL),
> NULL,
> - fe_baton->wc_ctx,
> + ctx->wc_ctx,
> local_abspath,
> pool, pool));
>
> - SVN_ERR(fe_baton->receiver(fe_baton->receiver_baton, path,
> info,
> - pool));
> - return SVN_NO_ERROR;
> + SVN_ERR(receiver(receiver_baton, local_abspath, info,
> pool));
> }
> + else
> + return svn_error_return(err);
> }
> -
> - return svn_error_return(err);
> -}
> -
> -static const svn_wc_entry_callbacks2_t
> -entry_walk_callbacks =
> - {
> - info_found_entry_callback,
> - info_error_handler
> - };
> -
> -
> -/* Helper function: push the svn_wc_entry_t for WCPATH at
> - RECEIVER/BATON, and possibly recurse over more entries. */
> -static svn_error_t *
> -crawl_entries(const char *wcpath,
> - svn_info_receiver_t receiver,
> - void *receiver_baton,
> - svn_depth_t depth,
> - apr_hash_t *changelist_hash,
> - svn_client_ctx_t *ctx,
> - apr_pool_t *pool)
> -{
> - svn_wc_adm_access_t *adm_access;
> - int adm_lock_level = SVN_WC__LEVELS_TO_LOCK_FROM_DEPTH(depth);
> - struct found_entry_baton fe_baton;
> -
> - SVN_ERR(svn_wc__adm_probe_in_context(&adm_access, ctx->wc_ctx,
> wcpath, FALSE,
> - adm_lock_level, ctx-
> >cancel_func,
> - ctx->cancel_baton, pool));
> -
> - fe_baton.changelist_hash = changelist_hash;
> - fe_baton.receiver = receiver;
> - fe_baton.receiver_baton = receiver_baton;
> - fe_baton.wc_ctx = ctx->wc_ctx;
> - return svn_wc_walk_entries3(wcpath, adm_access,
> - &entry_walk_callbacks, &fe_baton,
> - depth, FALSE, ctx->cancel_func,
> - ctx->cancel_baton, pool);
> + else if (err)
> + return svn_error_return(err);
> +
> + return SVN_NO_ERROR;
> }
>
> /* Set *SAME_P to TRUE if URL exists in the head of the repository and
> @@ -463,12 +456,17 @@ svn_client_info2(const char *path_or_url
> || peg_revision->kind == svn_opt_revision_unspecified))
> {
> /* Do all digging in the working copy. */
> + const char *local_abspath;
> +
> apr_hash_t *changelist_hash = NULL;
> if (changelists && changelists->nelts)
> SVN_ERR(svn_hash_from_cstring_keys(&changelist_hash,
> changelists, pool));
> - return crawl_entries(path_or_url, receiver, receiver_baton,
> - depth, changelist_hash, ctx, pool);
> +
> + SVN_ERR(svn_dirent_get_absolute(&local_abspath, path_or_url,
> pool));
> + return svn_error_return(
> + crawl_entries(local_abspath, receiver, receiver_baton,
> + depth, changelist_hash, ctx, pool));
> }
>
> /* Go repository digging instead. */
>
> Modified: trunk/subversion/tests/cmdline/basic_tests.py
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/tests/cmdline/basic_t
> ests.py?pathrev=39616&r1=39615&r2=39616
> =======================================================================
> =======
> --- trunk/subversion/tests/cmdline/basic_tests.py Fri Sep 25
> 17:39:00 2009 (r39615)
> +++ trunk/subversion/tests/cmdline/basic_tests.py Fri Sep 25
> 17:51:15 2009 (r39616)
> @@ -1632,11 +1632,11 @@ def basic_info(sbox):
>
> # Check that "info" works with 0, 1 and more than 1 explicit
> targets.
> exit_code, output, errput = svntest.main.run_svn(None, 'info')
> - check_paths(output, ['.'])
> + check_paths(output, [os.path.abspath('.')])
> exit_code, output, errput = svntest.main.run_svn(None, 'info',
> 'iota')
> - check_paths(output, ['iota'])
> + check_paths(output, [os.path.abspath('iota')])
> exit_code, output, errput = svntest.main.run_svn(None, 'info',
> 'iota', '.')
> - check_paths(output, ['iota', '.'])
> + check_paths(output, [os.path.abspath('iota'), os.path.abspath('.')])
>
> def repos_root(sbox):
> "check that repos root gets set on checkout"
>
> Modified: trunk/subversion/tests/cmdline/changelist_tests.py
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/tests/cmdline/changel
> ist_tests.py?pathrev=39616&r1=39615&r2=39616
> =======================================================================
> =======
> --- trunk/subversion/tests/cmdline/changelist_tests.py Fri Sep 25
> 17:39:00 2009 (r39615)
> +++ trunk/subversion/tests/cmdline/changelist_tests.py Fri Sep 25
> 17:51:15 2009 (r39616)
> @@ -507,7 +507,7 @@ def info_with_changelists(sbox):
> expected_paths.append('A/D/G/pi')
> expected_paths.append('A/D/H/chi')
> expected_paths.append('A/D/H/psi')
> - expected_paths = sorted([os.path.join(wc_dir, x.replace('/',
> os.sep)) for x in expected_paths])
> + expected_paths = sorted([os.path.abspath(os.path.join(wc_dir,
> x.replace('/', os.sep))) for x in expected_paths])
>
> # Build the command line.
> args = ['info', wc_dir]
>
> Modified: trunk/subversion/tests/cmdline/info_tests.py
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/tests/cmdline/info_te
> sts.py?pathrev=39616&r1=39615&r2=39616
> =======================================================================
> =======
> --- trunk/subversion/tests/cmdline/info_tests.py Fri Sep 25
> 17:39:00 2009 (r39615)
> +++ trunk/subversion/tests/cmdline/info_tests.py Fri Sep 25
> 17:51:15 2009 (r39616)
> @@ -239,7 +239,7 @@ def info_on_added_file(sbox):
>
> verify_xml_elements(output,
> [('entry', {'kind' : 'file',
> - 'path' : new_file,
> + 'path' :
> os.path.abspath(new_file),
> 'revision' : '0'}),
> ('url', {}, '.*/new_file'),
> ('root', {}, '.*'),
> @@ -277,7 +277,7 @@ def info_on_mkdir(sbox):
> '--
> xml')
> verify_xml_elements(output,
> [('entry', {'kind' : 'dir',
> - 'path' : new_dir,
> + 'path' :
> os.path.abspath(new_dir),
> 'revision' : '0'}),
> ('url', {}, '.*/new_dir'),
> ('root', {}, '.*'),
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageI
> d=2400467

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2400500
Received on 2009-09-26 09:03:13 CEST

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