You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@lyra.org> on 2001/04/17 01:14:59 UTC
Re: CVS update: subversion/subversion/mod_dav_svn version.c
This still isn't quite right. A while back, I asked about an FS API function
to say, "hey! can I modify this?" We punted because it seemed like a simple
ID comparison. I'm finding it isn't :-)
The additional restriction needed is to ask "is D' an immediate child of D,
*AND* is D' mutable?" It is entirely possible for somebody to try and check
out D' (due to monkeyness noted in the comment in code (not shown in the
context)), and then you have D'' when it becomes mutable.
The monkeyness is basically:
1) Jon starts a commit based on rev R
2) Jane commits a change, upping the revision to R'
3) Jon updates D to D'
4) Jon's commit, while trucking along needs to do a DAV CHECKOUT on D' to
make some changes
==> the old code punted (restart the commit (based on R'), hoser). after
this change, it gets through. this may work, maybe not. need to apply
heavy brain power.
Ah well... with this, my complex commit is moving along better, but now it
looks like somebody destroyed a pool on me. Rebuilding the world with
pool debugging enabled...
Cheers,
-g
On Tue, Apr 17, 2001 at 12:58:23AM -0000, gstein@tigris.org wrote:
> User: gstein
> Date: 01/04/16 17:58:23
>
> Modified: subversion/mod_dav_svn version.c
> Log:
> In certain cases, two collection nodes should be treated as equivalent.
> Don't reject this case when a CHECKOUT occurs.
>
> (dav_svn_checkout): check for an immediate parent/child relationship between
> the requested node ID and the transaction's ID. this "change" occurs
> during bubble up scenarios, but also during normal processing (yet it
> would be impossible for a client to specify the node in the txn, so even
> if the node has real changes, they should be considered equivalent
> (although real changes should not have happened before a checkout...))
>
> Revision Changes Path
> 1.25 +37 -8 subversion/subversion/mod_dav_svn/version.c
>
> Index: version.c
> ===================================================================
> RCS file: /cvs/subversion/subversion/mod_dav_svn/version.c,v
> retrieving revision 1.24
> retrieving revision 1.25
> diff -u -r1.24 -r1.25
> --- version.c 2001/04/16 11:31:34 1.24
> +++ version.c 2001/04/17 00:58:22 1.25
> @@ -246,6 +246,8 @@
>
> if (!svn_fs_id_eq(res_id, resource->info->node_id))
> {
> + svn_boolean_t invalid = TRUE;
> +
> /* If the version resource is *newer* than the transaction
> root, then the client started a commit, a new revision was
> created within the repository, the client fetched the new
> @@ -258,15 +260,42 @@
> that. We can stop the commit, and everything will be fine
> again if the user simply restarts it (because we'll use
> that new revision as the transaction root, thus incorporating
> - the new resource). */
> + the new resource).
> +
> + Special exception case:
> +
> + If the nodes are directories, and the transaction's node is
> + an immediate child of the requested node, then we're okay.
> + The issue is that we generate a child for the directory when
> + it goes from an immutable node in a revision root, into a
> + mutable node in the transaction root. We should not consider
> + this a change, and there would be no way for the client to
> + check out the transaction root's directory node anyway.
> +
> + Files do not have this behavior because they are not spuriously
> + placed into the transaction root (directories are because of
> + the bubble up algorithm). The only time a file will shift is
> + when a true change is being made, and we'll see that anyways.
> + */
> + if (svn_fs_is_parent(resource->info->node_id, res_id))
> + {
> + int is_dir;
> +
> + serr = svn_fs_is_dir(&is_dir, txn_root,
> + resource->info->repos_path, resource->pool);
> + if (serr == NULL)
> + invalid = !is_dir;
> + /* else ignore the error and go with the "out of date" msg */
> + }
>
> - return dav_new_error(resource->pool, HTTP_CONFLICT, 0,
> - "The version resource does not correspond "
> - "to the resource within the transaction. "
> - "Either the requested version resource is out "
> - "of date (needs to be updated), or the "
> - "requested version resource is newer than "
> - "the transaction root (restart the commit).");
> + if (invalid)
> + return dav_new_error(resource->pool, HTTP_CONFLICT, 0,
> + "The version resource does not correspond "
> + "to the resource within the transaction. "
> + "Either the requested version resource is "
> + "out of date (needs to be updated), or the "
> + "requested version resource is newer than "
> + "the transaction root (restart the commit).");
> }
> }
>
>
>
>
--
Greg Stein, http://www.lyra.org/
Re: evil commits (was: Re: CVS update: subversion/subversion/mod_dav_svn version.c)
Posted by Jim Blandy <ji...@zwingli.cygnus.com>.
Greg Stein <gs...@lyra.org> writes:
> Now... back to the problem that I mentioned (due to lack of a "real N == N'
> test function"). Let's say that C == B + 1. Jon submits a change to a
> resource in B. The server says "in my TXN, that is C, which is a direct
> child [of B], so I'll let the change occur." Whoops! Of course, the resource
> in the TXN could become B+2 due to bubble up, and *then* punt. But by that
> point, damage could be done. I think we have a case where a change could be
> "legal" when it shouldn't.
Sure thing --- this would be a real bug in the commit/merge algorithm.
Suppose Jon starts a transaction T based on revision B. When Jon
asks, "Can I change path P in T, from node revision NR to whatever
node revision you choose?", shouldn't the server check that B/P is NR,
not T/P?
Re: evil commits (was: Re: CVS update: subversion/subversion/mod_dav_svn version.c)
Posted by Karl Fogel <kf...@collab.net>.
Greg Stein <gs...@lyra.org> writes:
> I know it shouldn't be visible. But the semantic that I'm looking for has
> mutability as an input.
>
> Let me try to explain:
>
> Given node N in revision R. Node N becomes part of txn T when T is
> created, based on R.
>
> Users says "change N". N is in T, so servers says "sure."
>
> Alternatively, N/M is modified, and bubble up puts N' into T. Now the
> user says "change N", but the server says "N != N'. go away."
>
> In reality, N == N', it is just that N' is the mutable form of N.
I think there's a slight error here, though. If bubble up caused a
new node N' to exist in place of N, then N != N'. Specifically, N'
has at least one entry that points to a different node than that entry
pointed to in N.
So we can't just say that N' is the mutable form of N but that
otherwise N == N'. They are not equal.
(I'm also fuzzy on why you would need them to be equal; this kind of
non-equality is what fs merging is supposed to handle, right?)
> In the code that I just checked in, I use svn_fs_is_parent() as a crude
> mechanism for testing N == N'. The right mechanism is a function that says,
> "given T, path P, and node N, can I change it?" That function can
> (internally) use mutability if N' is found.
But I would think the answer is, given txn T, path P, and node N, you
can always change it -- if it's not mutable, it'll become mutable.
The point of svn txns and automatic bubble-up is to save you from
thinking about mutability (your change might conflict with some other
change in the repository, of course, but that fact is external to T).
?
-K
> > > The monkeyness is basically:
> > >
> > > 1) Jon starts a commit based on rev R
> > > 2) Jane commits a change, upping the revision to R'
> > > 3) Jon updates D to D'
> > > 4) Jon's commit, while trucking along needs to do a DAV CHECKOUT on D' to
> > > make some changes
> > > ==> the old code punted (restart the commit (based on R'), hoser). after
> > > this change, it gets through. this may work, maybe not. need to apply
> > > heavy brain power.
> >
> > I don't understand how this case arises. If Jon has a working copy at
> > base revision B, and starts a commit, mod_dav_svn creates a
> > transaction based on the current youngest revision, C. The only
> > comparisons you need to worry about are the changes between B and C
> > vs. the changes between B and Jon's working copy. If Jane commits a
> > new revision C' after Jon creates his transaction, that's irrelevant
> > to you.
>
> No. The evil part is that Jon updates his WC to C' in another window while
> the commit is operating. The commit could then say, "hey! the resource you
> want to change (from C') does not match what is in the transaction (from C)"
> and punts on you.
>
> Ideally, the resources Jon is changing are equal in B and C, so the server
> allows the change.
>
> Now... back to the problem that I mentioned (due to lack of a "real N == N'
> test function"). Let's say that C == B + 1. Jon submits a change to a
> resource in B. The server says "in my TXN, that is C, which is a direct
> child [of B], so I'll let the change occur." Whoops! Of course, the resource
> in the TXN could become B+2 due to bubble up, and *then* punt. But by that
> point, damage could be done. I think we have a case where a change could be
> "legal" when it shouldn't.
>
> The conditions for the false legality are slim (with an unknown impact
> (haven't thought thru it entirely)), but quite present. Fine for M2, but not
> for 1.0.
>
> Cheers,
> -g
>
> --
> Greg Stein, http://www.lyra.org/
evil commits (was: Re: CVS update: subversion/subversion/mod_dav_svn version.c)
Posted by Greg Stein <gs...@lyra.org>.
On Tue, Apr 17, 2001 at 01:30:27AM -0500, Jim Blandy wrote:
> Greg Stein <gs...@lyra.org> writes:
> > This still isn't quite right. A while back, I asked about an FS API function
> > to say, "hey! can I modify this?" We punted because it seemed like a simple
> > ID comparison. I'm finding it isn't :-)
> >
> > The additional restriction needed is to ask "is D' an immediate child of D,
> > *AND* is D' mutable?" It is entirely possible for somebody to try and check
> > out D' (due to monkeyness noted in the comment in code (not shown in the
> > context)), and then you have D'' when it becomes mutable.
>
> Err, mutability shouldn't be visible at all outside the svn_fs.h
> interface. Every node in a transaction can be freely changed.
I know it shouldn't be visible. But the semantic that I'm looking for has
mutability as an input.
Let me try to explain:
Given node N in revision R. Node N becomes part of txn T when T is
created, based on R.
Users says "change N". N is in T, so servers says "sure."
Alternatively, N/M is modified, and bubble up puts N' into T. Now the
user says "change N", but the server says "N != N'. go away."
In reality, N == N', it is just that N' is the mutable form of N.
In the code that I just checked in, I use svn_fs_is_parent() as a crude
mechanism for testing N == N'. The right mechanism is a function that says,
"given T, path P, and node N, can I change it?" That function can
(internally) use mutability if N' is found.
> > The monkeyness is basically:
> >
> > 1) Jon starts a commit based on rev R
> > 2) Jane commits a change, upping the revision to R'
> > 3) Jon updates D to D'
> > 4) Jon's commit, while trucking along needs to do a DAV CHECKOUT on D' to
> > make some changes
> > ==> the old code punted (restart the commit (based on R'), hoser). after
> > this change, it gets through. this may work, maybe not. need to apply
> > heavy brain power.
>
> I don't understand how this case arises. If Jon has a working copy at
> base revision B, and starts a commit, mod_dav_svn creates a
> transaction based on the current youngest revision, C. The only
> comparisons you need to worry about are the changes between B and C
> vs. the changes between B and Jon's working copy. If Jane commits a
> new revision C' after Jon creates his transaction, that's irrelevant
> to you.
No. The evil part is that Jon updates his WC to C' in another window while
the commit is operating. The commit could then say, "hey! the resource you
want to change (from C') does not match what is in the transaction (from C)"
and punts on you.
Ideally, the resources Jon is changing are equal in B and C, so the server
allows the change.
Now... back to the problem that I mentioned (due to lack of a "real N == N'
test function"). Let's say that C == B + 1. Jon submits a change to a
resource in B. The server says "in my TXN, that is C, which is a direct
child [of B], so I'll let the change occur." Whoops! Of course, the resource
in the TXN could become B+2 due to bubble up, and *then* punt. But by that
point, damage could be done. I think we have a case where a change could be
"legal" when it shouldn't.
The conditions for the false legality are slim (with an unknown impact
(haven't thought thru it entirely)), but quite present. Fine for M2, but not
for 1.0.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: CVS update: subversion/subversion/mod_dav_svn version.c
Posted by Jim Blandy <ji...@zwingli.cygnus.com>.
Greg Stein <gs...@lyra.org> writes:
> This still isn't quite right. A while back, I asked about an FS API function
> to say, "hey! can I modify this?" We punted because it seemed like a simple
> ID comparison. I'm finding it isn't :-)
>
> The additional restriction needed is to ask "is D' an immediate child of D,
> *AND* is D' mutable?" It is entirely possible for somebody to try and check
> out D' (due to monkeyness noted in the comment in code (not shown in the
> context)), and then you have D'' when it becomes mutable.
Err, mutability shouldn't be visible at all outside the svn_fs.h
interface. Every node in a transaction can be freely changed.
> The monkeyness is basically:
>
> 1) Jon starts a commit based on rev R
> 2) Jane commits a change, upping the revision to R'
> 3) Jon updates D to D'
> 4) Jon's commit, while trucking along needs to do a DAV CHECKOUT on D' to
> make some changes
> ==> the old code punted (restart the commit (based on R'), hoser). after
> this change, it gets through. this may work, maybe not. need to apply
> heavy brain power.
I don't understand how this case arises. If Jon has a working copy at
base revision B, and starts a commit, mod_dav_svn creates a
transaction based on the current youngest revision, C. The only
comparisons you need to worry about are the changes between B and C
vs. the changes between B and Jon's working copy. If Jane commits a
new revision C' after Jon creates his transaction, that's irrelevant
to you.