You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bert Huijben <be...@vmoo.com> on 2012/05/18 07:34:06 UTC
RE: svn commit: r1339892 -
/subversion/trunk/subversion/libsvn_fs/editor.c
MIME-Version: 1.0
Content-Type: multipart/alternative; boundary=f46d04428c9c9dfedd04c046d8da
--f46d04428c9c9dfedd04c046d8da
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Editor v1 only requires the revision for base node replacements and I
don't see why we need to change that for v2.
If we copied a subtree from a revision we know that that revision
didn't change. How could it be out of date?
Bert Huijben (Cell phone)
From: Greg Stein
Sent: 18-5-2012 5:07
To: Hyrum K Wright
Cc: dev@subversion.apache.org
Subject: Re: svn commit: r1339892
- /subversion/trunk/subversion/libsvn_fs/editor.c
On May 17, 2012 10:38 PM, "Hyrum K Wright" <hy...@wandisco.com>
wrote:
>
> Thanks for taking a look. It feels like all of these are issues with
> the complex copies and inheriting the correct replacement revision.
> The problem then lies back in the commit driver, but I don't think it
> will be too hard to track down.
Yeah. I seem to recall the specific code where a revision is tested against
its patent. Where those happen, simply use the parent rev for REPLACES_REV.
I think that might be in the harvester, where we set the COPY flag. Not
sure how that will translate over to the driver, or if it is even
applicable. I guess any copy-dest-within-dest needs the parent rev.
> The blame test failure is an unrelated EOL repairing issue.
Yeah, so I skipped looking into that one.
Cheers,
-g
--f46d04428c9c9dfedd04c046d8da
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
<html><head><meta content=3D"text/html; charset=3Dutf-8" http-equiv=3D"Cont=
ent-Type"></head><body><div><div style=3D"font-family: Calibri,sans-serif; =
font-size: 11pt;">Editor v1 only requires the revision for base node replac=
ements and I don't see why we need to change that for v2.<br><br>If we copi=
ed a subtree from a revision we know that that revision didn't change. How =
could it be out of date?<br><br>Bert Huijben (Cell phone)<br></div></div><h=
r><span style=3D"font-family: Tahoma,sans-serif; font-size: 10pt; font-weig=
ht: bold;">From: </span><span style=3D"font-family: Tahoma,sans-serif; font=
-size: 10pt;">Greg Stein</span><br><span style=3D"font-family: Tahoma,sans-=
serif; font-size: 10pt; font-weight: bold;">Sent: </span><span style=3D"fon=
t-family: Tahoma,sans-serif; font-size: 10pt;">18-5-2012 5:07</span><br><sp=
an style=3D"font-family: Tahoma,sans-serif; font-size: 10pt; font-weight: b=
old;">To: </span><span style=3D"font-family: Tahoma,sans-serif; font-size: =
10pt;">Hyrum K Wright</span><br><span style=3D"font-family: Tahoma,sans-ser=
if; font-size: 10pt; font-weight: bold;">Cc: </span><span style=3D"font-fam=
ily: Tahoma,sans-serif; font-size: 10pt;">dev@subversion.apache.org</span><=
br><span style=3D"font-family: Tahoma,sans-serif; font-size: 10pt; font-wei=
ght: bold;">Subject: </span><span style=3D"font-family: Tahoma,sans-serif; =
font-size: 10pt;">Re: svn commit: r1339892 - /subversion/trunk/subversion/l=
ibsvn_fs/editor.c</span><br><br></body></html><p><br>
On May 17, 2012 10:38 PM, "Hyrum K Wright" <<a href=3D"mailto:=
hyrum.wright@wandisco.com">hyrum.wright@wandisco.com</a>> wrote:<br>
><br>
> Thanks for taking a look. =C2=A0It feels like all of these are issues =
with<br>
> the complex copies and inheriting the correct replacement revision.<br=
>
> The problem then lies back in the commit driver, but I don't think=
it<br>
> will be too hard to track down.</p>
<p>Yeah. I seem to recall the specific code where a revision is tested agai=
nst its patent. Where those happen, simply use the parent rev for REPLACES_=
REV.</p>
<p>I think that might be in the harvester, where we set the COPY flag. Not =
sure how that will translate over to the driver, or if it is even applicabl=
e. I guess any copy-dest-within-dest needs the parent rev.</p>
<p>> The blame test failure is an unrelated EOL repairing issue.</p>
<p>Yeah, so I skipped looking into that one.</p>
<p>Cheers,<br>
-g<br>
</p>
--f46d04428c9c9dfedd04c046d8da--
RE: svn commit: r1339892 -
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Greg Stein [mailto:gstein@gmail.com]
> Sent: vrijdag 18 mei 2012 8:07
> To: Bert Huijben
> Cc: Hyrum K Wright; dev@subversion.apache.org
> Subject: Re: svn commit: r1339892 -
>
> On Fri, May 18, 2012 at 1:57 AM, Greg Stein <gs...@gmail.com> wrote:
> > On Fri, May 18, 2012 at 1:34 AM, Bert Huijben <be...@vmoo.com> wrote:
> >>
> >> Editor v1 only requires the revision for base node replacements and I
> >> don't see why we need to change that for v2.
> >>
> >> If we copied a subtree from a revision we know that that revision
> >> didn't change. How could it be out of date?
> >
> > The FS does not remember where changes came from, without some deep
> > inspection. If we see "/some/path", then we don't know how it got
> > there. Whether it started there, or whether it was copied there.
> >
> > Now... that said, I just realized that passing the parent revision
> > will not work, because the path will not be found in that target
> > revision.
> >
> > So, yeah: we need to somehow enable passing SVN_INVALID_REVNUM for the
> > children of copies. That is equivalent to the Ev1 behavior: "I'm
> > replacing an uncommitted item". It looks like we can use
> > svn_fs_closest_copy() to determine if a txn node (or an ancestor) has
> > been copied to that target location.
>
> Some more thinking...
>
> But we do need some sort of check in here. We should not allow add_*()
> to overwrite a copied-child. But a copy() can do so IFF it is part of
> a mixed-rev copy. But what about a secondary copy operation over a
> child?
+1 I agree that we should still check for overwriting existing nodes (and
anything else we can think of that doesn't really hurt performance)
> At some point, we just assume the client is working properly. Maybe we
> just omit all OOD checks under copies. But that doesn't really manager
> improper overwrites...
>
> Hrm...
Bert
Re: svn commit: r1339892 -
Posted by Greg Stein <gs...@gmail.com>.
On Fri, May 18, 2012 at 1:57 AM, Greg Stein <gs...@gmail.com> wrote:
> On Fri, May 18, 2012 at 1:34 AM, Bert Huijben <be...@vmoo.com> wrote:
>>
>> Editor v1 only requires the revision for base node replacements and I
>> don't see why we need to change that for v2.
>>
>> If we copied a subtree from a revision we know that that revision
>> didn't change. How could it be out of date?
>
> The FS does not remember where changes came from, without some deep
> inspection. If we see "/some/path", then we don't know how it got
> there. Whether it started there, or whether it was copied there.
>
> Now... that said, I just realized that passing the parent revision
> will not work, because the path will not be found in that target
> revision.
>
> So, yeah: we need to somehow enable passing SVN_INVALID_REVNUM for the
> children of copies. That is equivalent to the Ev1 behavior: "I'm
> replacing an uncommitted item". It looks like we can use
> svn_fs_closest_copy() to determine if a txn node (or an ancestor) has
> been copied to that target location.
Some more thinking...
But we do need some sort of check in here. We should not allow add_*()
to overwrite a copied-child. But a copy() can do so IFF it is part of
a mixed-rev copy. But what about a secondary copy operation over a
child?
At some point, we just assume the client is working properly. Maybe we
just omit all OOD checks under copies. But that doesn't really manager
improper overwrites...
Hrm...
Re: svn commit: r1339892 -
Posted by Greg Stein <gs...@gmail.com>.
On Fri, May 18, 2012 at 1:34 AM, Bert Huijben <be...@vmoo.com> wrote:
>
> Editor v1 only requires the revision for base node replacements and I
> don't see why we need to change that for v2.
>
> If we copied a subtree from a revision we know that that revision
> didn't change. How could it be out of date?
The FS does not remember where changes came from, without some deep
inspection. If we see "/some/path", then we don't know how it got
there. Whether it started there, or whether it was copied there.
Now... that said, I just realized that passing the parent revision
will not work, because the path will not be found in that target
revision.
So, yeah: we need to somehow enable passing SVN_INVALID_REVNUM for the
children of copies. That is equivalent to the Ev1 behavior: "I'm
replacing an uncommitted item". It looks like we can use
svn_fs_closest_copy() to determine if a txn node (or an ancestor) has
been copied to that target location.
Cheers,
-g