You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by gs...@apache.org on 2012/05/18 00:27:03 UTC
svn commit: r1339892 - /subversion/trunk/subversion/libsvn_fs/editor.c
Author: gstein
Date: Thu May 17 22:27:03 2012
New Revision: 1339892
URL: http://svn.apache.org/viewvc?rev=1339892&view=rev
Log:
Generate out-of-date errors for paths which already exist (rather than
waiting for an error when we try to write over existing content).
* subversion/libsvn_fs/editor.c:
(can_create): new helper function
(add_directory_cb, add_file_cb, add_symlink_cb, copy_cb, move_cb):
call the new can_create() helper
Modified:
subversion/trunk/subversion/libsvn_fs/editor.c
Modified: subversion/trunk/subversion/libsvn_fs/editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs/editor.c?rev=1339892&r1=1339891&r2=1339892&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs/editor.c (original)
+++ subversion/trunk/subversion/libsvn_fs/editor.c Thu May 17 22:27:03 2012
@@ -268,6 +268,25 @@ can_modify(svn_fs_root_t *txn_root,
}
+/* Can we create a node at FSPATH in TXN_ROOT? If something already exists
+ at that path, then the client is out of date. */
+static svn_error_t *
+can_create(svn_fs_root_t *txn_root,
+ const char *fspath,
+ apr_pool_t *scratch_pool)
+{
+ svn_node_kind_t kind;
+
+ SVN_ERR(svn_fs_check_path(&kind, txn_root, fspath, scratch_pool));
+ if (kind != svn_node_none)
+ return svn_error_createf(SVN_ERR_FS_OUT_OF_DATE, NULL,
+ _("'%s' already exists, so may be out"
+ " of date; try updating"),
+ fspath);
+ return SVN_NO_ERROR;
+}
+
+
/* This implements svn_editor_cb_add_directory_t */
static svn_error_t *
add_directory_cb(void *baton,
@@ -291,7 +310,10 @@ add_directory_cb(void *baton,
SVN_ERR(can_modify(root, fspath, replaces_rev, scratch_pool));
SVN_ERR(svn_fs_delete(root, fspath, scratch_pool));
}
- /* else better not be there! */
+ else
+ {
+ SVN_ERR(can_create(root, fspath, scratch_pool));
+ }
SVN_ERR(svn_fs_make_dir(root, fspath, scratch_pool));
SVN_ERR(add_new_props(root, fspath, props, scratch_pool));
@@ -321,7 +343,10 @@ add_file_cb(void *baton,
SVN_ERR(can_modify(root, fspath, replaces_rev, scratch_pool));
SVN_ERR(svn_fs_delete(root, fspath, scratch_pool));
}
- /* else better not be there! */
+ else
+ {
+ SVN_ERR(can_create(root, fspath, scratch_pool));
+ }
SVN_ERR(svn_fs_make_file(root, fspath, scratch_pool));
@@ -353,7 +378,10 @@ add_symlink_cb(void *baton,
SVN_ERR(can_modify(root, fspath, replaces_rev, scratch_pool));
SVN_ERR(svn_fs_delete(root, fspath, scratch_pool));
}
- /* else better not be there! */
+ else
+ {
+ SVN_ERR(can_create(root, fspath, scratch_pool));
+ }
/* ### we probably need to construct a file with specific contents
### (until the FS grows some symlink APIs) */
@@ -506,6 +534,10 @@ copy_cb(void *baton,
SVN_ERR(can_modify(root, dst_fspath, replaces_rev, scratch_pool));
SVN_ERR(svn_fs_delete(root, dst_fspath, scratch_pool));
}
+ else
+ {
+ SVN_ERR(can_create(root, dst_fspath, scratch_pool));
+ }
SVN_ERR(svn_fs_revision_root(&src_root, svn_fs_root_fs(root), src_revision,
scratch_pool));
@@ -541,6 +573,10 @@ move_cb(void *baton,
SVN_ERR(can_modify(root, dst_fspath, replaces_rev, scratch_pool));
SVN_ERR(svn_fs_delete(root, dst_fspath, scratch_pool));
}
+ else
+ {
+ SVN_ERR(can_create(root, dst_fspath, scratch_pool));
+ }
/* ### would be nice to have svn_fs_move() */
Re: svn commit: r1339892 - /subversion/trunk/subversion/libsvn_fs/editor.c
Posted by Greg Stein <gs...@gmail.com>.
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
Re: svn commit: r1339892 - /subversion/trunk/subversion/libsvn_fs/editor.c
Posted by Hyrum K Wright <hy...@wandisco.com>.
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.
The blame test failure is an unrelated EOL repairing issue.
-Hyrum
On Thu, May 17, 2012 at 7:48 PM, Greg Stein <gs...@gmail.com> wrote:
> Notes:
>
> copy 29: a mixed-rev child of a copied parent that is re-copied to
> pick up a different source revision needs to specify that parent
> revision for REPLACES_REV. Since it passes SVN_INVALID_REVNUM, the
> existence check comes into play, and the node exists (implicitly as a
> result of copying the parent).
>
> merge 18: same. I@2 and I/theta@6 are copied to L. It copies I@2, then
> it (re)copies I@6 without specifying REPLACES_REV=2.
>
> switch 33: a little bit different, but copied children again. In this
> case, R/G is switched to a different subtree than the R/G@1 subtree.
> So the commit process copies R, and then it must replace R/G with a
> different subtree. Again: it needs to specify REPLACES_REV.
>
> Hope that helps,
> -g
>
> On Thu, May 17, 2012 at 7:32 PM, Greg Stein <gs...@gmail.com> wrote:
>> Hyrum,
>>
>> This fixes the two tree_conflict tests you mentioned, and blame 7
>> continues to fail.
>>
>> However, it introduced failures in copy 29, merge 18, and switch 33.
>> Each one has the error that I just added. I haven't investigated yet:
>> it could be that a different error was expected.
>>
>> (all this on the ev2-export branch, of course, and on ra_local)
>>
>> Cheers,
>> -g
>>
>> On Thu, May 17, 2012 at 6:27 PM, <gs...@apache.org> wrote:
>>> Author: gstein
>>> Date: Thu May 17 22:27:03 2012
>>> New Revision: 1339892
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1339892&view=rev
>>> Log:
>>> Generate out-of-date errors for paths which already exist (rather than
>>> waiting for an error when we try to write over existing content).
>>>
>>> * subversion/libsvn_fs/editor.c:
>>> (can_create): new helper function
>>> (add_directory_cb, add_file_cb, add_symlink_cb, copy_cb, move_cb):
>>> call the new can_create() helper
>>>
>>> Modified:
>>> subversion/trunk/subversion/libsvn_fs/editor.c
>>>
>>> Modified: subversion/trunk/subversion/libsvn_fs/editor.c
>>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs/editor.c?rev=1339892&r1=1339891&r2=1339892&view=diff
>>> ==============================================================================
>>> --- subversion/trunk/subversion/libsvn_fs/editor.c (original)
>>> +++ subversion/trunk/subversion/libsvn_fs/editor.c Thu May 17 22:27:03 2012
>>> @@ -268,6 +268,25 @@ can_modify(svn_fs_root_t *txn_root,
>>> }
>>>
>>>
>>> +/* Can we create a node at FSPATH in TXN_ROOT? If something already exists
>>> + at that path, then the client is out of date. */
>>> +static svn_error_t *
>>> +can_create(svn_fs_root_t *txn_root,
>>> + const char *fspath,
>>> + apr_pool_t *scratch_pool)
>>> +{
>>> + svn_node_kind_t kind;
>>> +
>>> + SVN_ERR(svn_fs_check_path(&kind, txn_root, fspath, scratch_pool));
>>> + if (kind != svn_node_none)
>>> + return svn_error_createf(SVN_ERR_FS_OUT_OF_DATE, NULL,
>>> + _("'%s' already exists, so may be out"
>>> + " of date; try updating"),
>>> + fspath);
>>> + return SVN_NO_ERROR;
>>> +}
>>> +
>>> +
>>> /* This implements svn_editor_cb_add_directory_t */
>>> static svn_error_t *
>>> add_directory_cb(void *baton,
>>> @@ -291,7 +310,10 @@ add_directory_cb(void *baton,
>>> SVN_ERR(can_modify(root, fspath, replaces_rev, scratch_pool));
>>> SVN_ERR(svn_fs_delete(root, fspath, scratch_pool));
>>> }
>>> - /* else better not be there! */
>>> + else
>>> + {
>>> + SVN_ERR(can_create(root, fspath, scratch_pool));
>>> + }
>>>
>>> SVN_ERR(svn_fs_make_dir(root, fspath, scratch_pool));
>>> SVN_ERR(add_new_props(root, fspath, props, scratch_pool));
>>> @@ -321,7 +343,10 @@ add_file_cb(void *baton,
>>> SVN_ERR(can_modify(root, fspath, replaces_rev, scratch_pool));
>>> SVN_ERR(svn_fs_delete(root, fspath, scratch_pool));
>>> }
>>> - /* else better not be there! */
>>> + else
>>> + {
>>> + SVN_ERR(can_create(root, fspath, scratch_pool));
>>> + }
>>>
>>> SVN_ERR(svn_fs_make_file(root, fspath, scratch_pool));
>>>
>>> @@ -353,7 +378,10 @@ add_symlink_cb(void *baton,
>>> SVN_ERR(can_modify(root, fspath, replaces_rev, scratch_pool));
>>> SVN_ERR(svn_fs_delete(root, fspath, scratch_pool));
>>> }
>>> - /* else better not be there! */
>>> + else
>>> + {
>>> + SVN_ERR(can_create(root, fspath, scratch_pool));
>>> + }
>>>
>>> /* ### we probably need to construct a file with specific contents
>>> ### (until the FS grows some symlink APIs) */
>>> @@ -506,6 +534,10 @@ copy_cb(void *baton,
>>> SVN_ERR(can_modify(root, dst_fspath, replaces_rev, scratch_pool));
>>> SVN_ERR(svn_fs_delete(root, dst_fspath, scratch_pool));
>>> }
>>> + else
>>> + {
>>> + SVN_ERR(can_create(root, dst_fspath, scratch_pool));
>>> + }
>>>
>>> SVN_ERR(svn_fs_revision_root(&src_root, svn_fs_root_fs(root), src_revision,
>>> scratch_pool));
>>> @@ -541,6 +573,10 @@ move_cb(void *baton,
>>> SVN_ERR(can_modify(root, dst_fspath, replaces_rev, scratch_pool));
>>> SVN_ERR(svn_fs_delete(root, dst_fspath, scratch_pool));
>>> }
>>> + else
>>> + {
>>> + SVN_ERR(can_create(root, dst_fspath, scratch_pool));
>>> + }
>>>
>>> /* ### would be nice to have svn_fs_move() */
>>>
>>>
>>>
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/
Re: svn commit: r1339892 - /subversion/trunk/subversion/libsvn_fs/editor.c
Posted by Greg Stein <gs...@gmail.com>.
Notes:
copy 29: a mixed-rev child of a copied parent that is re-copied to
pick up a different source revision needs to specify that parent
revision for REPLACES_REV. Since it passes SVN_INVALID_REVNUM, the
existence check comes into play, and the node exists (implicitly as a
result of copying the parent).
merge 18: same. I@2 and I/theta@6 are copied to L. It copies I@2, then
it (re)copies I@6 without specifying REPLACES_REV=2.
switch 33: a little bit different, but copied children again. In this
case, R/G is switched to a different subtree than the R/G@1 subtree.
So the commit process copies R, and then it must replace R/G with a
different subtree. Again: it needs to specify REPLACES_REV.
Hope that helps,
-g
On Thu, May 17, 2012 at 7:32 PM, Greg Stein <gs...@gmail.com> wrote:
> Hyrum,
>
> This fixes the two tree_conflict tests you mentioned, and blame 7
> continues to fail.
>
> However, it introduced failures in copy 29, merge 18, and switch 33.
> Each one has the error that I just added. I haven't investigated yet:
> it could be that a different error was expected.
>
> (all this on the ev2-export branch, of course, and on ra_local)
>
> Cheers,
> -g
>
> On Thu, May 17, 2012 at 6:27 PM, <gs...@apache.org> wrote:
>> Author: gstein
>> Date: Thu May 17 22:27:03 2012
>> New Revision: 1339892
>>
>> URL: http://svn.apache.org/viewvc?rev=1339892&view=rev
>> Log:
>> Generate out-of-date errors for paths which already exist (rather than
>> waiting for an error when we try to write over existing content).
>>
>> * subversion/libsvn_fs/editor.c:
>> (can_create): new helper function
>> (add_directory_cb, add_file_cb, add_symlink_cb, copy_cb, move_cb):
>> call the new can_create() helper
>>
>> Modified:
>> subversion/trunk/subversion/libsvn_fs/editor.c
>>
>> Modified: subversion/trunk/subversion/libsvn_fs/editor.c
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs/editor.c?rev=1339892&r1=1339891&r2=1339892&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_fs/editor.c (original)
>> +++ subversion/trunk/subversion/libsvn_fs/editor.c Thu May 17 22:27:03 2012
>> @@ -268,6 +268,25 @@ can_modify(svn_fs_root_t *txn_root,
>> }
>>
>>
>> +/* Can we create a node at FSPATH in TXN_ROOT? If something already exists
>> + at that path, then the client is out of date. */
>> +static svn_error_t *
>> +can_create(svn_fs_root_t *txn_root,
>> + const char *fspath,
>> + apr_pool_t *scratch_pool)
>> +{
>> + svn_node_kind_t kind;
>> +
>> + SVN_ERR(svn_fs_check_path(&kind, txn_root, fspath, scratch_pool));
>> + if (kind != svn_node_none)
>> + return svn_error_createf(SVN_ERR_FS_OUT_OF_DATE, NULL,
>> + _("'%s' already exists, so may be out"
>> + " of date; try updating"),
>> + fspath);
>> + return SVN_NO_ERROR;
>> +}
>> +
>> +
>> /* This implements svn_editor_cb_add_directory_t */
>> static svn_error_t *
>> add_directory_cb(void *baton,
>> @@ -291,7 +310,10 @@ add_directory_cb(void *baton,
>> SVN_ERR(can_modify(root, fspath, replaces_rev, scratch_pool));
>> SVN_ERR(svn_fs_delete(root, fspath, scratch_pool));
>> }
>> - /* else better not be there! */
>> + else
>> + {
>> + SVN_ERR(can_create(root, fspath, scratch_pool));
>> + }
>>
>> SVN_ERR(svn_fs_make_dir(root, fspath, scratch_pool));
>> SVN_ERR(add_new_props(root, fspath, props, scratch_pool));
>> @@ -321,7 +343,10 @@ add_file_cb(void *baton,
>> SVN_ERR(can_modify(root, fspath, replaces_rev, scratch_pool));
>> SVN_ERR(svn_fs_delete(root, fspath, scratch_pool));
>> }
>> - /* else better not be there! */
>> + else
>> + {
>> + SVN_ERR(can_create(root, fspath, scratch_pool));
>> + }
>>
>> SVN_ERR(svn_fs_make_file(root, fspath, scratch_pool));
>>
>> @@ -353,7 +378,10 @@ add_symlink_cb(void *baton,
>> SVN_ERR(can_modify(root, fspath, replaces_rev, scratch_pool));
>> SVN_ERR(svn_fs_delete(root, fspath, scratch_pool));
>> }
>> - /* else better not be there! */
>> + else
>> + {
>> + SVN_ERR(can_create(root, fspath, scratch_pool));
>> + }
>>
>> /* ### we probably need to construct a file with specific contents
>> ### (until the FS grows some symlink APIs) */
>> @@ -506,6 +534,10 @@ copy_cb(void *baton,
>> SVN_ERR(can_modify(root, dst_fspath, replaces_rev, scratch_pool));
>> SVN_ERR(svn_fs_delete(root, dst_fspath, scratch_pool));
>> }
>> + else
>> + {
>> + SVN_ERR(can_create(root, dst_fspath, scratch_pool));
>> + }
>>
>> SVN_ERR(svn_fs_revision_root(&src_root, svn_fs_root_fs(root), src_revision,
>> scratch_pool));
>> @@ -541,6 +573,10 @@ move_cb(void *baton,
>> SVN_ERR(can_modify(root, dst_fspath, replaces_rev, scratch_pool));
>> SVN_ERR(svn_fs_delete(root, dst_fspath, scratch_pool));
>> }
>> + else
>> + {
>> + SVN_ERR(can_create(root, dst_fspath, scratch_pool));
>> + }
>>
>> /* ### would be nice to have svn_fs_move() */
>>
>>
>>
Re: svn commit: r1339892 - /subversion/trunk/subversion/libsvn_fs/editor.c
Posted by Greg Stein <gs...@gmail.com>.
Hyrum,
This fixes the two tree_conflict tests you mentioned, and blame 7
continues to fail.
However, it introduced failures in copy 29, merge 18, and switch 33.
Each one has the error that I just added. I haven't investigated yet:
it could be that a different error was expected.
(all this on the ev2-export branch, of course, and on ra_local)
Cheers,
-g
On Thu, May 17, 2012 at 6:27 PM, <gs...@apache.org> wrote:
> Author: gstein
> Date: Thu May 17 22:27:03 2012
> New Revision: 1339892
>
> URL: http://svn.apache.org/viewvc?rev=1339892&view=rev
> Log:
> Generate out-of-date errors for paths which already exist (rather than
> waiting for an error when we try to write over existing content).
>
> * subversion/libsvn_fs/editor.c:
> (can_create): new helper function
> (add_directory_cb, add_file_cb, add_symlink_cb, copy_cb, move_cb):
> call the new can_create() helper
>
> Modified:
> subversion/trunk/subversion/libsvn_fs/editor.c
>
> Modified: subversion/trunk/subversion/libsvn_fs/editor.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs/editor.c?rev=1339892&r1=1339891&r2=1339892&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_fs/editor.c (original)
> +++ subversion/trunk/subversion/libsvn_fs/editor.c Thu May 17 22:27:03 2012
> @@ -268,6 +268,25 @@ can_modify(svn_fs_root_t *txn_root,
> }
>
>
> +/* Can we create a node at FSPATH in TXN_ROOT? If something already exists
> + at that path, then the client is out of date. */
> +static svn_error_t *
> +can_create(svn_fs_root_t *txn_root,
> + const char *fspath,
> + apr_pool_t *scratch_pool)
> +{
> + svn_node_kind_t kind;
> +
> + SVN_ERR(svn_fs_check_path(&kind, txn_root, fspath, scratch_pool));
> + if (kind != svn_node_none)
> + return svn_error_createf(SVN_ERR_FS_OUT_OF_DATE, NULL,
> + _("'%s' already exists, so may be out"
> + " of date; try updating"),
> + fspath);
> + return SVN_NO_ERROR;
> +}
> +
> +
> /* This implements svn_editor_cb_add_directory_t */
> static svn_error_t *
> add_directory_cb(void *baton,
> @@ -291,7 +310,10 @@ add_directory_cb(void *baton,
> SVN_ERR(can_modify(root, fspath, replaces_rev, scratch_pool));
> SVN_ERR(svn_fs_delete(root, fspath, scratch_pool));
> }
> - /* else better not be there! */
> + else
> + {
> + SVN_ERR(can_create(root, fspath, scratch_pool));
> + }
>
> SVN_ERR(svn_fs_make_dir(root, fspath, scratch_pool));
> SVN_ERR(add_new_props(root, fspath, props, scratch_pool));
> @@ -321,7 +343,10 @@ add_file_cb(void *baton,
> SVN_ERR(can_modify(root, fspath, replaces_rev, scratch_pool));
> SVN_ERR(svn_fs_delete(root, fspath, scratch_pool));
> }
> - /* else better not be there! */
> + else
> + {
> + SVN_ERR(can_create(root, fspath, scratch_pool));
> + }
>
> SVN_ERR(svn_fs_make_file(root, fspath, scratch_pool));
>
> @@ -353,7 +378,10 @@ add_symlink_cb(void *baton,
> SVN_ERR(can_modify(root, fspath, replaces_rev, scratch_pool));
> SVN_ERR(svn_fs_delete(root, fspath, scratch_pool));
> }
> - /* else better not be there! */
> + else
> + {
> + SVN_ERR(can_create(root, fspath, scratch_pool));
> + }
>
> /* ### we probably need to construct a file with specific contents
> ### (until the FS grows some symlink APIs) */
> @@ -506,6 +534,10 @@ copy_cb(void *baton,
> SVN_ERR(can_modify(root, dst_fspath, replaces_rev, scratch_pool));
> SVN_ERR(svn_fs_delete(root, dst_fspath, scratch_pool));
> }
> + else
> + {
> + SVN_ERR(can_create(root, dst_fspath, scratch_pool));
> + }
>
> SVN_ERR(svn_fs_revision_root(&src_root, svn_fs_root_fs(root), src_revision,
> scratch_pool));
> @@ -541,6 +573,10 @@ move_cb(void *baton,
> SVN_ERR(can_modify(root, dst_fspath, replaces_rev, scratch_pool));
> SVN_ERR(svn_fs_delete(root, dst_fspath, scratch_pool));
> }
> + else
> + {
> + SVN_ERR(can_create(root, dst_fspath, scratch_pool));
> + }
>
> /* ### would be nice to have svn_fs_move() */
>
>
>