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

Re: svn commit: r27170 - trunk/subversion/libsvn_wc

From: Ben Collins-Sussman <sussman_at_red-bean.com>
Date: 2007-10-13 15:47:01 CEST

On 10/13/07, lgo@tigris.org <lgo@tigris.org> wrote:
> Author: lgo
> Date: Sat Oct 13 03:49:30 2007
> New Revision: 27170
>
> Log:
> Fix fetching a file from the server during copy-on-update over ra_neon &
> ra_serf.
>
> * subversion/libsvn_wc/update_editor.c
> (add_file_with_history): copyfrom_path is absolute but fetch_func requires
> a relative path, so skip the first '/'.
>
>
> Modified:
> trunk/subversion/libsvn_wc/update_editor.c
>
> Modified: trunk/subversion/libsvn_wc/update_editor.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/update_editor.c?pathrev=27170&r1=27169&r2=27170
> ==============================================================================
> --- trunk/subversion/libsvn_wc/update_editor.c (original)
> +++ trunk/subversion/libsvn_wc/update_editor.c Sat Oct 13 03:49:30 2007
> @@ -3061,7 +3061,9 @@
> (APR_WRITE | APR_TRUNCATE | APR_CREATE),
> pool));
>
> - SVN_ERR(eb->fetch_func(eb->fetch_baton, copyfrom_path, copyfrom_rev,
> + /* copyfrom_path is a absolute path, fetch_func requires a path relative
> + to the root of the repository so skip the first '/'. */
> + SVN_ERR(eb->fetch_func(eb->fetch_baton, copyfrom_path + 1, copyfrom_rev,
> svn_stream_from_aprfile(textbase_file, pool),
> NULL, &base_props, pool));
> }

I'm trying to grok this change. Is it true that each RA layer sends a
copyfrom_path to the update_editor which starts with '/'? If so,
that's good.

But it sounds like the bug here is that the four implementations of
svn_ra_get_file() are inconsistent. It sounds like two of the
implementations have no problem with svn_ra_get_file(session, "/foo")
-- interpreting the file to fetch as "SessionRootURL/foo"... but the
two http implementations interpret this invocation as just "/foo",
ignoring the SessionRootURL?

So while this change ensures that we always call
svn_ra_get_file(session, "foo"), it seems to cover up the underlying
problem of inconsistency. Can we fix the inconsistency? Is
svn_ra_get_file(session, "/foo") a legal invocation? If not, then
maybe libsvn_ra.so should reject it?

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 13 15:47:10 2007

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