You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "C. Michael Pilato" <cm...@collab.net> on 2004/12/03 04:38:55 UTC
Re: svn commit: r12136 - trunk/subversion/libsvn_client
philip@tigris.org writes:
> Author: philip
> Date: Thu Dec 2 21:20:00 2004
> New Revision: 12136
>
> Modified:
> trunk/subversion/libsvn_client/ra.c
> Log:
> Add a bit of sanity checking, and get rid of a bit of silly disk IO.
>
> * subversion/libsvn_client/ra.c (set_wc_prop): Make sure there is a
> versioned entry before setting the property.
See my mail to the dev-list about why this bug exists. I hadn't
thought of this solution. But now that you mention (by way of making)
it, this seems a pretty safe thing to do. It will likely mean that
switched working copies made with 1.2 clients against 1.0 or 1.1
servers will have no dav wcprops, but that's only a one time
performance hit.
(Heh. In fact, this makes me wonder why we bother doing the resource
walk during 'svn switch' at all -- we really could speed up switch by
*not* doing it, though at the cost of subsequent operations fetching
wcprops on an as-needed basis.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r12136 - trunk/subversion/libsvn_client
Posted by Ben Collins-Sussman <su...@collab.net>.
On Dec 3, 2004, at 9:41 AM, C. Michael Pilato wrote:
> The
> simplest compatibility step that solves all the problems is to simply
> turn off the resource walk in 1.2
+1, I like the idea of killing the resource-walk.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r12136 - trunk/subversion/libsvn_client
Posted by Philip Martin <ph...@codematters.co.uk>.
"C. Michael Pilato" <cm...@collab.net> writes:
> Care to apply, build, and test?
Will do.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r12136 - trunk/subversion/libsvn_client
Posted by "C. Michael Pilato" <cm...@collab.net>.
Philip Martin <ph...@codematters.co.uk> writes:
> > The
> > simplest compatibility step that solves all the problems is to simply
> > turn off the resource walk in 1.2 and re-enable it some time in the
> > future. Note that ra-dav will still, at the beginning of a switch,
> > invalidate all wcprops in the switched target tree.
>
> Ignoring the error is quite simple, however turning off the resource
> walk sounds like a better idea. Is it something the client requests
> or something the server sends automatically? I assume the client
> could ignore it if it's the latter.
I *think* this is the right boolean. :-) I didn't compile or test
this.
Index: subversion/libsvn_ra_dav/fetch.c
===================================================================
--- subversion/libsvn_ra_dav/fetch.c (revision 12078)
+++ subversion/libsvn_ra_dav/fetch.c (working copy)
@@ -2635,7 +2635,7 @@
switch_url,
recurse,
TRUE,
- TRUE,
+ FALSE,
wc_update,
wc_update_baton,
TRUE, /* fetch_content */
Care to apply, build, and test?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r12136 - trunk/subversion/libsvn_client
Posted by Philip Martin <ph...@codematters.co.uk>.
"C. Michael Pilato" <cm...@collab.net> writes:
> r12006 makes both the client and server do the right thing when both
> are running r12006. In the case where the server isn't running the
> new code (as your mail demonstrates), r12006 plus your idea of ignore
> the "not versioned" error still makes a huge improvement for most
> items in the switched try, but yes, can still be fooled by the
> existence of similarly-named things up a directory level.
I see, thanks!
> The
> simplest compatibility step that solves all the problems is to simply
> turn off the resource walk in 1.2 and re-enable it some time in the
> future. Note that ra-dav will still, at the beginning of a switch,
> invalidate all wcprops in the switched target tree.
Ignoring the error is quite simple, however turning off the resource
walk sounds like a better idea. Is it something the client requests
or something the server sends automatically? I assume the client
could ignore it if it's the latter.
At present we have reduced functionality since my new check is raising
an error in cases where previously the client carried on after writing
a spurious file. I have code locally to restore that behaviour by
ignoring the error in ra_dav/fetch.c, I'll commit it if for some
reason we choose not to turn off the resource walk.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r12136 - trunk/subversion/libsvn_client
Posted by "C. Michael Pilato" <cm...@collab.net>.
Philip Martin <ph...@codematters.co.uk> writes:
> Now I'm worried that r12006 doesn't fix the problem it's supposed to
> fix.
r12006 makes both the client and server do the right thing when both
are running r12006. In the case where the server isn't running the
new code (as your mail demonstrates), r12006 plus your idea of ignore
the "not versioned" error still makes a huge improvement for most
items in the switched try, but yes, can still be fooled by the
existence of similarly-named things up a directory level. The
simplest compatibility step that solves all the problems is to simply
turn off the resource walk in 1.2 and re-enable it some time in the
future. Note that ra-dav will still, at the beginning of a switch,
invalidate all wcprops in the switched target tree.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r12136 - trunk/subversion/libsvn_client
Posted by Philip Martin <ph...@codematters.co.uk>.
"C. Michael Pilato" <cm...@collab.net> writes:
> Philip Martin <ph...@codematters.co.uk> writes:
>
>> I supose we could catch/ignore the error somewhere (libsvn_ra_dav
>> perhaps) if that's the only thing going wrong. I don't understand the
>> problem sufficiently to determine whether ignoring the error is safe.
>
> I believe it would be fine for libsvn_ra_dav to catch and ignore this
> error. It's just trying to write an unnecessary cache to save itself
> some work in future operations.
>
> Or we could go all extreme and simply turn off the resource walk for
> switches in 1.2. This way, the wcprop callback would never get
> invoked during a switch, and you shouldn't see any of these problems.
> This would:
>
> - fix the compatibility problem.
>
> - speed up ra-dav switches.
>
> - add a few small extra network turnarounds to the first ra-dav
> operation that needs a vsn-rsrc-url for each path in the tree
> (but afterwards it would be cached). and actually, it's not even
> that bad, but because any paths touched during an update would
> get their vsn-rsrc-urls cached at no additional cost.
>
> We could always re-enable the resource walk again in the future if we
> wanted to.
Now I'm worried that r12006 doesn't fix the problem it's supposed to
fix. Consider the following (I'm using a schedule add file because we
don't have items with suitable names in the Subversion repo):
$ svn co https://svn.collab.net/repos/svn/trunk/tools qq
$ touch qq/random-commits.py
$ svn add qq/random-commits.py
$ svn sw https://svn.collab.net/repos/svn/branches/1.1.x/tools/dev qq/dev
U qq/dev/verify-history.py
U qq/dev/scramble-tree.py
U qq/dev/stress.pl
D qq/dev/lock-check.py
D qq/dev/po-merge.py
../svn/subversion/libsvn_client/ra.c:179: (apr_err=200005)
svn: 'qq/gnuify-changelog.pl' is not under version control
It's hit my new check on gnuify-changelog.pl and that's good,
previously it would have written a spurious file in qq/.svn/wcprops.
However it didn't stop on random-commits.py because a suitable
versioned item exists, so that file has switched wcprops even though
it didn't get switched:
$ cat qq/.svn/wcprops/random-commits.py.svn-work
K 25
svn:wc:ra_dav:version-url
V 64
/repos/svn/!svn/ver/12140/branches/1.1.x/tools/random-commits.py
END
That might be harmless for this schedule add file, but if that were a
committed file things would fall over.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r12136 - trunk/subversion/libsvn_client
Posted by "C. Michael Pilato" <cm...@collab.net>.
Philip Martin <ph...@codematters.co.uk> writes:
> I supose we could catch/ignore the error somewhere (libsvn_ra_dav
> perhaps) if that's the only thing going wrong. I don't understand the
> problem sufficiently to determine whether ignoring the error is safe.
I believe it would be fine for libsvn_ra_dav to catch and ignore this
error. It's just trying to write an unnecessary cache to save itself
some work in future operations.
Or we could go all extreme and simply turn off the resource walk for
switches in 1.2. This way, the wcprop callback would never get
invoked during a switch, and you shouldn't see any of these problems.
This would:
- fix the compatibility problem.
- speed up ra-dav switches.
- add a few small extra network turnarounds to the first ra-dav
operation that needs a vsn-rsrc-url for each path in the tree
(but afterwards it would be cached). and actually, it's not even
that bad, but because any paths touched during an update would
get their vsn-rsrc-urls cached at no additional cost.
We could always re-enable the resource walk again in the future if we
wanted to.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r12136 - trunk/subversion/libsvn_client
Posted by Philip Martin <ph...@codematters.co.uk>.
"C. Michael Pilato" <cm...@collab.net> writes:
> philip@tigris.org writes:
>
>> Author: philip
>> Date: Thu Dec 2 21:20:00 2004
>> New Revision: 12136
>>
>> Modified:
>> trunk/subversion/libsvn_client/ra.c
>> Log:
>> Add a bit of sanity checking, and get rid of a bit of silly disk IO.
>>
>> * subversion/libsvn_client/ra.c (set_wc_prop): Make sure there is a
>> versioned entry before setting the property.
>
> See my mail to the dev-list about why this bug exists. I hadn't
> thought of this solution. But now that you mention (by way of making)
> it, this seems a pretty safe thing to do. It will likely mean that
> switched working copies made with 1.2 clients against 1.0 or 1.1
> servers will have no dav wcprops, but that's only a one time
> performance hit.
No, it still errors out when it tries to write the wrong props file.
Before this change I got this error
$ svn sw http://svn.collab.net/repos/svn/trunk/tools/dev tools/dev
../svn/subversion/libsvn_wc/lock.c:642: (apr_err=155005)
svn: Working copy 'tools/iz' not locked
now I get
$ svn sw https://svn.collab.net/repos/svn/trunk/tools/dev tools/dev
../svn/subversion/libsvn_client/ra.c:179: (apr_err=200005)
svn: 'tools/random-commits.py' is not under version control
and it happens earlier, before the client has written spurious wcprops
into tools/.svn/wcprops/ for files in tools/dev/.
I supose we could catch/ignore the error somewhere (libsvn_ra_dav
perhaps) if that's the only thing going wrong. I don't understand the
problem sufficiently to determine whether ignoring the error is safe.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org