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

Re: svn commit: r1525460 - in /subversion/trunk/subversion: include/ libsvn_ra/ libsvn_ra_local/ libsvn_ra_serf/ libsvn_ra_svn/ mod_dav_svn/ mod_dav_svn/reports/ svnserve/

From: Ivan Zhakov <ivan_at_visualsvn.com>
Date: Mon, 24 Feb 2014 17:45:42 +0400

On 23 September 2013 02:10, <stefan2_at_apache.org> wrote:
> Author: stefan2
> Date: Sun Sep 22 22:10:53 2013
> New Revision: 1525460
>
> URL: http://svn.apache.org/r1525460
> Log:
> Support the MOVes in the RA log API:
> Bump svn_ra_get_log to support the move_behavior option.
>
Hi Stefan, see my comments inline.

> For ra_serf, we add an optional move-behavior element to the LOG report.
> If not given, it defaults to 1.8 behavior reporting all moves as adds.
>
> For ra_svn, we append an optional integer parameter determining the
> move-behavior option. Same default behavior as above.
>
mod_dav_svn change is not mention in log message btw.

>
> Modified: subversion/trunk/subversion/libsvn_ra_svn/protocol
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/protocol?rev=1525460&r1=1525459&r2=1525460&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_ra_svn/protocol (original)
> +++ subversion/trunk/subversion/libsvn_ra_svn/protocol Sun Sep 22 22:10:53 2013
> @@ -382,11 +382,13 @@ second place for auth-request point as n
> [ end-rev:number ] changed-paths:bool strict-node:bool
> ? limit:number
> ? include-merged-revisions:bool
> - all-revprops | revprops ( revprop:string ... ) )
> + all-revprops | revprops ( revprop:string ... )
> + ? move-behavior:number )
> Before sending response, server sends log entries, ending with "done".
> If a client does not want to specify a limit, it should send 0 as the
> limit parameter. rev-props excludes author, date, and log; they are
> sent separately for backwards-compatibility.
> + Move-behavior is encoded like enum svn_move_behavior_t.
> log-entry: ( ( change:changed-path-entry ... ) rev:number
> [ author:string ] [ date:string ] [ message:string ]
> ? has-children:bool invalid-revnum:bool
>
Currently Subversion uses symbolic names to marshal enums over the
wire. See svn_depth_t for example. I don't see reason why
svn_move_behavior_t should be different.

> Modified: subversion/trunk/subversion/mod_dav_svn/merge.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/merge.c?rev=1525460&r1=1525459&r2=1525460&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/mod_dav_svn/merge.c (original)
> +++ subversion/trunk/subversion/mod_dav_svn/merge.c Sun Sep 22 22:10:53 2013
> @@ -115,6 +115,7 @@ static svn_error_t *
> do_resources(const dav_svn_repos *repos,
> svn_fs_root_t *root,
> svn_revnum_t revision,
> + svn_move_behavior_t move_behavior,
> ap_filter_t *output,
> apr_bucket_brigade *bb,
> apr_pool_t *pool)
> @@ -129,7 +130,7 @@ do_resources(const dav_svn_repos *repos,
> and deleted things. Also, note that deleted things don't merit
> responses of their own -- they are considered modifications to
> their parent. */
> - SVN_ERR(svn_fs_paths_changed2(&changes, root, pool));
> + SVN_ERR(svn_fs_paths_changed3(&changes, root, move_behavior, pool));
>
> for (hi = apr_hash_first(pool, changes); hi; hi = apr_hash_next(hi))
> {
> @@ -155,6 +156,8 @@ do_resources(const dav_svn_repos *repos,
>
> case svn_fs_path_change_add:
> case svn_fs_path_change_replace:
> + case svn_fs_path_change_move:
> + case svn_fs_path_change_movereplace:
> send_self = TRUE;
> send_parent = TRUE;
> break;
> @@ -360,7 +363,10 @@ dav_svn__merge_response(ap_filter_t *out
> ### we can pass back the new version URL */
>
> /* and go make me proud, boy! */
> - serr = do_resources(repos, root, new_rev, output, bb, pool);
> + serr = do_resources(repos, root, new_rev,
> + /* report changes with no further interpretation */
> + svn_move_behavior_explicit_moves,
> + output, bb, pool);
> if (serr != NULL)
> {
> return dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
>
> Modified: subversion/trunk/subversion/mod_dav_svn/reports/log.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/reports/log.c?rev=1525460&r1=1525459&r2=1525460&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/mod_dav_svn/reports/log.c (original)
> +++ subversion/trunk/subversion/mod_dav_svn/reports/log.c Sun Sep 22 22:10:53 2013
> @@ -301,6 +301,9 @@ dav_svn__log_report(const dav_resource *
> svn_boolean_t discover_changed_paths = FALSE; /* off by default */
> svn_boolean_t strict_node_history = FALSE; /* off by default */
> svn_boolean_t include_merged_revisions = FALSE; /* off by default */
> + svn_move_behavior_t move_behavior = svn_move_behavior_no_moves;
> + /* no moves by default */
> +
> apr_array_header_t *revprops = apr_array_make(resource->pool, 3,
> sizeof(const char *));
> apr_array_header_t *paths
> @@ -395,6 +398,24 @@ dav_svn__log_report(const dav_resource *
> resource->pool);
> APR_ARRAY_PUSH(paths, const char *) = target;
> }
> + else if (strcmp(child->name, "move-behavior") == 0)
> + {
> + int move_behavior_param;
> + serr = svn_cstring_atoi(&move_behavior_param,
> + dav_xml_get_cdata(child, resource->pool, 1));
> + if (serr)
> + return dav_svn__convert_err(serr, HTTP_BAD_REQUEST,
> + "Malformed CDATA in element "
> + "\"move-behavior\"", resource->pool);
> +
> + if ( move_behavior_param < 0
> + || move_behavior_param > svn_move_behavior_auto_moves)
> + return dav_svn__convert_err(serr, HTTP_BAD_REQUEST,
> + "Invalid CDATA in element "
> + "\"move-behavior\"", resource->pool);
> +
> + move_behavior = (svn_move_behavior_t) move_behavior_param;
> + }
Same is here: why use numbers to encode svn_move_behavior_t instead of
symbolic name?

-- 
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com
Received on 2014-02-24 14:46:44 CET

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.