On Mon, Feb 24, 2014 at 2:45 PM, Ivan Zhakov <i...@visualsvn.com> wrote:
> On 23 September 2013 02:10, <stef...@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. > Hm. I see: * subversion/mod_dav_svn/merge.c > (do_resources): add move_behavior in case we want to call this function > from places other than dav_svn__merge_response; > handle the new change types > (dav_svn__merge_response): report commit result as recorded > What part is missing? > > > > > 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. > Fair point. That should be modeled similarly to e.g. svn_depth_t. The code actually gets simpler using the string encoding. > > 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 > > @@ -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? > Mainly quick coding followed by a mere oversight. Both protocols have been updated in r1572044. Thanks for the review! -- Stefan^2.