You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ben Collins-Sussman <su...@collab.net> on 2002/05/16 20:09:16 UTC
solution to issue 704
Just wanted to present the latest solution gstein and I came up with
regarding issue 704.
The bug is that when you 'switch' part of a working copy to a new URL
(which is just a fancy type of update), most of the cached
vsn-rsc-urls are no longer valid. The items that were patched get new
wcprop values; but items that merely have their working revs and
URL's 'tweaked' still have secret vsn-rsc-urls that are left pointing
to old locations. That's the reason why cmpilato accidentally
committed changes to /trunk from his switched libsvn_fs working copy.
It's not enough to just erase all wcprops when doing post-update
revision bumping; that implies that libsvn_wc understands what the
wcprops are doing. It shouldn't be that aware; the wcprops should be
*strictly* managed by the RA layer that created them. libsvn_wc just
blindly stores and retrieves them on demand of RA.
So the proposed solution is to extend our update-report-response a
bit. Right now, it looks like this, just an XML version of an editor
drive:
<S:update-report xmlns:S="svn:" xmlns:D="DAV:">
<S:target-revision rev="1971"/>
<S:replace-directory rev="1965">
...lots of opened files and dirs, props, and 'fetch-file' commands...
</S:replace-directory>
</S:update-report>
The new format would allow a new 2nd tree-walk to be embedded at the
end of the first walk:
<S:update-report xmlns:S="svn:" xmlns:D="DAV:">
<S:target-revision rev="1971"/>
<S:replace-directory rev="1965">
...lots of opened files and dirs, props, and 'fetch-file' commands...
</S:replace-directory>
<S:resource-walk>
...lots of opened files and dirs, *nothing* but vsn-rsc-url props...
</S:resource-walk>
</S:update-report>
The second "walk" would actually describe the vsn-rsc-url for *every*
resource below the anchor directory, rather than just those resources
that need updating.
Obviously, the 2nd tree-walk would only happen during an 'svn switch',
not a normal update.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: solution to issue 704
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Ben Collins-Sussman <su...@collab.net> writes:
> Except that the open_* calls all require a 'base revision' argument,
> which we don't have. This argument is supposed to be a sanity-check
> for the editor -- to make sure that the driver and the editor both
> agree on what's being changed. Now granted, our libsvn_wc
> update-editor currently ignores the base-revision argument, but I just
> can't bring myself to passing INVALID_REVNUM to every open_* call. I
> mean, someday the update-editor may very well want to do sanity
> checking. I don't want to violate the editor interface by driving it
> badly.
Is this really so bad?
The editor interface just needs to document what `base_revision'
means, and what happens (or doesn't happen) if you pass
SVN_INVALID_REVNUM.
The real problem is that the current doc string is very vague, not
really saying anything concrete about base_revision. Maybe this is
part of why you're so reluctant to "violate" it -- one can't even tell
what is a violation and what's not! But if the interface were clear
about this, you wouldn't have to worry.
I say: decide & document what base_revision means, and make sure that
the definition allows you to do what you need to solve this particular
problem :-). It's silly for an optional sanity check to force us away
from a good solution...
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: solution to issue 704
Posted by Greg Stein <gs...@lyra.org>.
The refetch is expensive (right now, it is 3 requests per URL fetched; we
could drop that to two, and in some cases down to 1; with a bunch more work,
we could drop the "average" below 1). It is *way* better to return them in a
single operation.
Of course, the URLs are only needed at commit time. It is feasible that you
could 'svn update' a particular resource before needing to commit it, thus
you would be getting the new URL anyways. It is actually somewhat of a
gamble, with very imperfect information.
Cheers,
-g
On Mon, May 20, 2002 at 12:32:09PM +0300, Alon Ziv wrote:
> Just a dumb question from a lurker: can't the (relatively) new
> 'refetch-invalid-urls'
> mechanism solve this far more cleanly? We can just clear all URLs of the
> switched area,
> and they will be automatically re-fetched as needed, no?
>
> (Disclaimer: I haven't tried this...)
>
> -az
>
> ----- Original Message -----
> From: "Greg Stein" <gs...@lyra.org>
> To: "Ben Collins-Sussman" <su...@collab.net>
> Cc: "SVN Dev List" <de...@subversion.tigris.org>
> Sent: Monday, May 20, 2002 11:53 AM
> Subject: Re: solution to issue 704
>
>
> > On Sun, May 19, 2002 at 12:43:57PM -0500, Ben Collins-Sussman wrote:
> > >...
> > > To generate this post-switch <resource-walk>, we're running dir_delta,
> > > comparing revision 0 against the new switched-subtree. So dir_delta
> > > is sending out an editor 'add' command for every single item in the
> > > switch-destination.
> >
> > Oh, geez. "Hey, I've got a hammer. That looks like a nail!"
> >
> > You're producing a tree. That is done by walking the tree using the
> > svn_fs_dir_entries() call. It is certainly not a delta operation.
> >
> > >...
> > > It's a shame, because I've been able to essentially re-use both
> > > mod_dav_svn's and ra_dav's editors to do the resource-walk, with
> > > almost no modifications. It's been very nice. But now it seems that
> >
> > "almost". But what you're doing is forcing an editor and a driver to do
> > double-duty across two entirely different semantic concepts. But with a
> > tweak. Not quite the same. IMO, it is a big recipe for confusion.
> >
> > On the ra_dav side, you're simply going to get a bunch of elements for the
> > walk. Don't try and drive an editor. Just start calling set_wc_prop. This
> is
> > NOT an editor. It is a simple walk of the tree. Don't try to pretend
> > otherwise.
> >
> > >...
> > > Instead, it will just call the 'set_wc_prop' callback sitting in the
> > > RA session baton directly. Does that sound reasonable?
> >
> > Absolutely.
> >
> > Screw dir_delta. It isn't a "walk" system. It is about deltas. On the
> > client, just map the incoming data to set_wc_prop. Clean and simple; and
> > even better: understandable.
> >
> > Cheers,
> > -g
> >
> > --
> > Greg Stein, http://www.lyra.org/
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> > For additional commands, e-mail: dev-help@subversion.tigris.org
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: solution to issue 704
Posted by Alon Ziv <al...@nolaviz.org>.
Just a dumb question from a lurker: can't the (relatively) new
'refetch-invalid-urls'
mechanism solve this far more cleanly? We can just clear all URLs of the
switched area,
and they will be automatically re-fetched as needed, no?
(Disclaimer: I haven't tried this...)
-az
----- Original Message -----
From: "Greg Stein" <gs...@lyra.org>
To: "Ben Collins-Sussman" <su...@collab.net>
Cc: "SVN Dev List" <de...@subversion.tigris.org>
Sent: Monday, May 20, 2002 11:53 AM
Subject: Re: solution to issue 704
> On Sun, May 19, 2002 at 12:43:57PM -0500, Ben Collins-Sussman wrote:
> >...
> > To generate this post-switch <resource-walk>, we're running dir_delta,
> > comparing revision 0 against the new switched-subtree. So dir_delta
> > is sending out an editor 'add' command for every single item in the
> > switch-destination.
>
> Oh, geez. "Hey, I've got a hammer. That looks like a nail!"
>
> You're producing a tree. That is done by walking the tree using the
> svn_fs_dir_entries() call. It is certainly not a delta operation.
>
> >...
> > It's a shame, because I've been able to essentially re-use both
> > mod_dav_svn's and ra_dav's editors to do the resource-walk, with
> > almost no modifications. It's been very nice. But now it seems that
>
> "almost". But what you're doing is forcing an editor and a driver to do
> double-duty across two entirely different semantic concepts. But with a
> tweak. Not quite the same. IMO, it is a big recipe for confusion.
>
> On the ra_dav side, you're simply going to get a bunch of elements for the
> walk. Don't try and drive an editor. Just start calling set_wc_prop. This
is
> NOT an editor. It is a simple walk of the tree. Don't try to pretend
> otherwise.
>
> >...
> > Instead, it will just call the 'set_wc_prop' callback sitting in the
> > RA session baton directly. Does that sound reasonable?
>
> Absolutely.
>
> Screw dir_delta. It isn't a "walk" system. It is about deltas. On the
> client, just map the incoming data to set_wc_prop. Clean and simple; and
> even better: understandable.
>
> Cheers,
> -g
>
> --
> Greg Stein, http://www.lyra.org/
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: solution to issue 704
Posted by Ben Collins-Sussman <su...@collab.net>.
Greg Stein <gs...@lyra.org> writes:
> Screw dir_delta. It isn't a "walk" system. It is about deltas. On the
> client, just map the incoming data to set_wc_prop. Clean and simple; and
> even better: understandable.
Hold on... instead of re-using editors on both sides of the network,
now you're telling me to re-use *neither*?
If mod_dav_svn doesn't attempt to re-use the xml-output-editor and
dir_delta, then fine: it can walk the fs manually. But now this
*guarantees* that I can produce a tree walk that is perfectly
re-usable by ra_dav's parser, and so ra_dav *can* simply re-drive the
update-editor with no hassle. (That's why the <resource-walk> in my
example was deliberately re-using the same xml elements as the
<update-report>!)
Conversely, if I allow dir_delta to send a tree full of 'add'
commands, I can write new parser code in ra_dav that avoids driving
the update-editor, and just calls the set_wc_prop callback.
Either of these solutions seems reasonable: there's no 'distortion' of
editor semantics going on, as long as one side or the other is
customized. It seems to me that the only 'distortion' is when we
attempt to marshal an editor drive on both ends. I don't understand
why you think *both* ends need custom code -- one end is enough, no?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: solution to issue 704
Posted by Greg Stein <gs...@lyra.org>.
On Sun, May 19, 2002 at 12:43:57PM -0500, Ben Collins-Sussman wrote:
>...
> To generate this post-switch <resource-walk>, we're running dir_delta,
> comparing revision 0 against the new switched-subtree. So dir_delta
> is sending out an editor 'add' command for every single item in the
> switch-destination.
Oh, geez. "Hey, I've got a hammer. That looks like a nail!"
You're producing a tree. That is done by walking the tree using the
svn_fs_dir_entries() call. It is certainly not a delta operation.
>...
> It's a shame, because I've been able to essentially re-use both
> mod_dav_svn's and ra_dav's editors to do the resource-walk, with
> almost no modifications. It's been very nice. But now it seems that
"almost". But what you're doing is forcing an editor and a driver to do
double-duty across two entirely different semantic concepts. But with a
tweak. Not quite the same. IMO, it is a big recipe for confusion.
On the ra_dav side, you're simply going to get a bunch of elements for the
walk. Don't try and drive an editor. Just start calling set_wc_prop. This is
NOT an editor. It is a simple walk of the tree. Don't try to pretend
otherwise.
>...
> Instead, it will just call the 'set_wc_prop' callback sitting in the
> RA session baton directly. Does that sound reasonable?
Absolutely.
Screw dir_delta. It isn't a "walk" system. It is about deltas. On the
client, just map the incoming data to set_wc_prop. Clean and simple; and
even better: understandable.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: solution to issue 704
Posted by Ben Collins-Sussman <su...@collab.net>.
Ben Collins-Sussman <su...@collab.net> writes:
> The new format would allow a new 2nd tree-walk to be embedded at the
> end of the first walk:
>
> <S:update-report xmlns:S="svn:" xmlns:D="DAV:">
> <S:target-revision rev="1971"/>
> <S:replace-directory rev="1965">
>
> ...lots of opened files and dirs, props, and 'fetch-file' commands...
>
> </S:replace-directory>
>
> <S:resource-walk>
>
> ...lots of opened files and dirs, *nothing* but vsn-rsc-url props...
>
> </S:resource-walk>
> </S:update-report>
There's a theoretical problem I'm running into, and I think it's going
to be a little weirder than I expected.
To generate this post-switch <resource-walk>, we're running dir_delta,
comparing revision 0 against the new switched-subtree. So dir_delta
is sending out an editor 'add' command for every single item in the
switch-destination.
Now mod_dav_svn and ra_dav are responsible for marshalling this editor
drive over the network, to the *real* update-editor that's tweaking
the working copy. Obviously, the true update-editor doesn't want to
see any add_* calls -- it will get angry about obstructed updates. So
I've toyed with making either mod_dav_svn or ra_dav tweak the
marshalling so as to convert the add_* calls into open_* calls
instead. So the true update-editor just sees "open_*, set a wcprop,
close_*". Very simple.
Except that the open_* calls all require a 'base revision' argument,
which we don't have. This argument is supposed to be a sanity-check
for the editor -- to make sure that the driver and the editor both
agree on what's being changed. Now granted, our libsvn_wc
update-editor currently ignores the base-revision argument, but I just
can't bring myself to passing INVALID_REVNUM to every open_* call. I
mean, someday the update-editor may very well want to do sanity
checking. I don't want to violate the editor interface by driving it
badly.
It's a shame, because I've been able to essentially re-use both
mod_dav_svn's and ra_dav's editors to do the resource-walk, with
almost no modifications. It's been very nice. But now it seems that
I'm going to have to write a completely different parser in ra_dav for
the resource walk -- it won't drive the update-editor at *all*.
Instead, it will just call the 'set_wc_prop' callback sitting in the
RA session baton directly. Does that sound reasonable?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org