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

Re: svn commit: r26503 - in trunk/subversion: include libsvn_client libsvn_ra libsvn_ra_local libsvn_ra_neon libsvn_ra_serf libsvn_ra_svn libsvn_repos mod_dav_svn/reports svnserve tests/libsvn_repos

From: Karl Fogel <kfogel_at_red-bean.com>
Date: 2007-09-11 20:28:58 CEST

sussman@tigris.org writes:
> --- trunk/subversion/include/svn_ra.h (original)
> +++ trunk/subversion/include/svn_ra.h Sun Sep 9 21:56:55 2007
> @@ -826,6 +826,11 @@
> *
> * Update the target only as deeply as @a depth indicates.
> *
> + * If @a send_copyfrom_args is true, then ask the server to send
> + * copyfrom arguments to add_file() and add_directory() when possible.
> + * (Note: this means that any subsequent txdeltas coming from the
> + * server are presumed to apply against the copied file!)
> + *
> * ### TODO(sd): Make sure the behavior described above is what happens.
> *
> * The working copy will be updated to @a revision_to_update_to, or the

Oops, that pushed a "TODO(sd)" comment to a place where it doesn't
make the same sense it used to. No problem, fixed in rXXXXX.

> @@ -852,6 +857,7 @@
> svn_revnum_t revision_to_update_to,
> const char *update_target,
> svn_depth_t depth,
> + svn_boolean_t send_copyfrom_args,
> const svn_delta_editor_t *update_editor,
> void *update_baton,
> apr_pool_t *pool);

This might seem like an odd nitpick, but I'd be happier calling this
new parameter 'send_copyfrom_info', which would be a less
implementationy name. For old clients, the problem is the presence of
the information itself, not the fact that it arrives in the form of
arguments to functions.

Mind if I change them, or rather, ask Emacs 'tags-query-replace' to
change them? :-)

> --- trunk/subversion/libsvn_client/export.c (original)
> +++ trunk/subversion/libsvn_client/export.c Sun Sep 9 21:56:55 2007
> @@ -899,6 +899,7 @@
> revnum,
> "", /* no sub-target */
> depth,
> + FALSE, /* don't want copyfrom-args */
> export_editor, edit_baton, pool));
>
> SVN_ERR(reporter->set_path(report_baton, "", revnum,

(Note to self: don't forget to find the comments too.)

> --- trunk/subversion/libsvn_client/update.c (original)
> +++ trunk/subversion/libsvn_client/update.c Sun Sep 9 21:56:55 2007
> @@ -211,6 +211,7 @@
> revnum,
> target,
> depth,
> + TRUE, /* send copyfrom args, please */
> update_editor, update_edit_baton, pool));
>
> /* Drive the reporter structure, describing the revisions within

(Ditto, and I'll stop mentioning the comments now.)

> --- trunk/subversion/libsvn_ra_local/ra_plugin.c (original)
> +++ trunk/subversion/libsvn_ra_local/ra_plugin.c Sun Sep 9 21:56:55 2007
> @@ -727,6 +731,7 @@
> switch_url,
> TRUE,
> depth,
> + FALSE, /* ### TODO(sussman): take new arg */
> TRUE,
> update_editor,
> update_baton,

Might want to update the log message to give a summary of what these
"for now" cases amount to. It appears that some discrete chunk of
work has been left for a later commit? That's fine, it would just be
good to know what it is.

I've left my share of "kff" comments in the code too, but seeing this
makes me realize that we should try, as a general policy, to put the
situation -- rather than the developer's name -- in parens in this
kind of TODO comment. Thus: "TODO(copyfrom)", or whatever. That way,
if "sussman" (or "kfogel", or whoever) has five unrelated things he's
planning to finish up in the code, it's possible for someone to search
for every instance of just one of those things, without being
distracted by all the other things.

> --- trunk/subversion/libsvn_ra_neon/fetch.c (original)
> +++ trunk/subversion/libsvn_ra_neon/fetch.c Sun Sep 9 21:56:55 2007
> @@ -3149,6 +3157,17 @@
> NULL, pool));
> }
>
> + /* mod_dav_svn 1.5 and later won't send copyfrom args unless it
> + finds this element. older mod_dav_svn modules should just
> + ignore the unknown element. */
> + if (ignore_ancestry)
> + {
> + const char *data =
> + "<S:send-copyfrom-args>yes</S:send-copyfrom_args>" DEBUG_CR;
> + SVN_ERR(svn_io_file_write_full(rb->tmpfile, data, strlen(data),
> + NULL, pool));
> + }
> +
> /* If we want a resource walk to occur, note that now. */
> if (resource_walk)
> {

If we do change the parameter name, we should change the protocol
elements too, of course.

> --- trunk/subversion/libsvn_ra_serf/update.c (original)
> +++ trunk/subversion/libsvn_ra_serf/update.c Sun Sep 9 21:56:55 2007
> @@ -2482,6 +2487,13 @@
> report->sess->bkt_alloc);
> }
>
> + if (report->send_copyfrom_args)
> + {
> + svn_ra_serf__add_tag_buckets(report->buckets,
> + "S:send-copyfrom-args", "yes",
> + report->sess->bkt_alloc);
> + }
> +
> /* Old servers know "recursive" but not "depth"; help them DTRT. */
> if (depth == svn_depth_files || depth == svn_depth_empty)
> {

Likewise here.

> ==============================================================================
> --- trunk/subversion/svnserve/serve.c (original)
> +++ trunk/subversion/svnserve/serve.c Sun Sep 9 21:56:55 2007
> @@ -1305,13 +1307,14 @@
> svn_revnum_t rev;
> const char *target, *full_path, *depth_word;
> svn_boolean_t recurse;
> + svn_boolean_t send_copyfrom_args = FALSE;
> /* Default to unknown. Old clients won't send depth, but we'll
> handle that by converting recurse if necessary. */
> svn_depth_t depth = svn_depth_unknown;
>
> /* Parse the arguments. */
> - SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "(?r)cb?w", &rev, &target,
> - &recurse, &depth_word));
> + SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "(?r)cb?w?b", &rev, &target,
> + &recurse, &depth_word, &send_copyfrom_args));
> target = svn_path_canonicalize(target, pool);
>
> if (depth_word)

And I see that Hyrum already raised the boolean issue.

-K

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Sep 11 20:25:38 2007

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