You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2010/07/18 12:47:40 UTC
[PATCH] fsfs: add two "format new enough" checks
This script:
[[[
#!/bin/sh
svnadmin=subversion/svnadmin/svnadmin
rm -rf r
$svnadmin create --pre-1.6-compatible r
./subversion/svnrdump/svnrdump file://`pwd`/r -r1:2
]]]
complains about 'min-unpacked-revprop' not found:
[[[
SVN-fs-dump-format-version: 3
subversion/svnrdump/svnrdump.c:136: (apr_err=2)
subversion/libsvn_ra/ra_loader.c:1084: (apr_err=2)
subversion/libsvn_repos/fs-wrap.c:406: (apr_err=2)
subversion/libsvn_fs_fs/fs_fs.c:2832: (apr_err=2)
subversion/libsvn_fs_fs/fs_fs.c:2832: (apr_err=2)
subversion/libsvn_fs_fs/fs_fs.c:1156: (apr_err=2)
subversion/libsvn_subr/io.c:2724: (apr_err=2)
svn: Can't open file '/home/daniel/src/svn/trunk.d/r/db/min-unpacked-revprop': No such file or directory
shell returned 1
]]]
even though it shouldn't touch that file at all, because that a format-3
filesystem isn't expected to contain that file.
The last hunk of the following patch fixes this issue (it causes a proper
"No such revision 1" error).
The remaining hunks add assertions(!) and fix a potential similar issue in
svn_fs_fs__path_rev_absolute().
Comments? More places that need to be updated?
[[[
Index: subversion/libsvn_fs_fs/fs_fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.c (revision 965128)
+++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
@@ -250,20 +250,25 @@ svn_fs_fs__path_rev_absolute(const char **path,
svn_revnum_t rev,
apr_pool_t *pool)
{
+ fs_fs_data_t *ffd = fs->fsap_data;
+
+ /* Initialize the return variable. */
+ *path = path_rev(fs, rev, pool);
+
+ if (ffd->format < SVN_FS_FS__MIN_PACKED_FORMAT)
+ return SVN_NO_ERROR;
+
if (! is_packed_rev(fs, rev))
{
svn_node_kind_t kind;
- /* Initialize the return variable. */
- *path = path_rev(fs, rev, pool);
-
SVN_ERR(svn_io_check_path(*path, &kind, pool));
if (kind == svn_node_file)
{
/* *path is already set correctly. */
return SVN_NO_ERROR;
}
- else
+ else if (ffd->format >= SVN_FS_FS__MIN_PACKED_FORMAT)
{
/* Someone must have run 'svnadmin pack' while this fs object
* was open. */
@@ -1167,6 +1172,8 @@ update_min_unpacked_rev(svn_fs_t *fs, apr_pool_t *
{
fs_fs_data_t *ffd = fs->fsap_data;
+ SVN_ERR_ASSERT(ffd->format >= SVN_FS_FS__MIN_PACKED_FORMAT);
+
return read_min_unpacked_rev(&ffd->min_unpacked_rev,
path_min_unpacked_rev(fs, pool),
pool);
@@ -1177,6 +1184,8 @@ update_min_unpacked_revprop(svn_fs_t *fs, apr_pool
{
fs_fs_data_t *ffd = fs->fsap_data;
+ SVN_ERR_ASSERT(ffd->format >= SVN_FS_FS__MIN_PACKED_REVPROP_FORMAT);
+
return read_min_unpacked_rev(&ffd->min_unpacked_revprop,
path_min_unpacked_revprop(fs, pool),
pool);
@@ -2821,9 +2830,11 @@ svn_fs_fs__revision_proplist(apr_hash_t **proplist
apr_pool_t *pool)
{
svn_error_t *err;
+ fs_fs_data_t *ffd = fs->fsap_data;
err = revision_proplist(proplist_p, fs, rev, pool);
- if (err && err->apr_err == SVN_ERR_FS_NO_SUCH_REVISION)
+ if (err && err->apr_err == SVN_ERR_FS_NO_SUCH_REVISION
+ && ffd->format >= SVN_FS_FS__MIN_PACKED_REVPROP_FORMAT)
{
/* If a pack is occurring simultaneously, the min-unpacked-revprop value
could change, so reload it and then attempt to fetch these revprops
]]]
Re: [PATCH] fsfs: add two "format new enough" checks
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Committed r967194 with minor modifications.
Daniel Shahaf wrote on Sun, Jul 18, 2010 at 15:47:40 +0300:
> This script:
>
> [[[
> #!/bin/sh
> svnadmin=subversion/svnadmin/svnadmin
> rm -rf r
> $svnadmin create --pre-1.6-compatible r
> ./subversion/svnrdump/svnrdump file://`pwd`/r -r1:2
> ]]]
>
> complains about 'min-unpacked-revprop' not found:
>
> [[[
> SVN-fs-dump-format-version: 3
> subversion/svnrdump/svnrdump.c:136: (apr_err=2)
> subversion/libsvn_ra/ra_loader.c:1084: (apr_err=2)
> subversion/libsvn_repos/fs-wrap.c:406: (apr_err=2)
> subversion/libsvn_fs_fs/fs_fs.c:2832: (apr_err=2)
> subversion/libsvn_fs_fs/fs_fs.c:2832: (apr_err=2)
> subversion/libsvn_fs_fs/fs_fs.c:1156: (apr_err=2)
> subversion/libsvn_subr/io.c:2724: (apr_err=2)
> svn: Can't open file '/home/daniel/src/svn/trunk.d/r/db/min-unpacked-revprop': No such file or directory
> shell returned 1
> ]]]
>
> even though it shouldn't touch that file at all, because that a format-3
> filesystem isn't expected to contain that file.
>
> The last hunk of the following patch fixes this issue (it causes a proper
> "No such revision 1" error).
>
> The remaining hunks add assertions(!) and fix a potential similar issue in
> svn_fs_fs__path_rev_absolute().
>
> Comments? More places that need to be updated?
>
> [[[
> Index: subversion/libsvn_fs_fs/fs_fs.c
> ===================================================================
> --- subversion/libsvn_fs_fs/fs_fs.c (revision 965128)
> +++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
> @@ -250,20 +250,25 @@ svn_fs_fs__path_rev_absolute(const char **path,
> svn_revnum_t rev,
> apr_pool_t *pool)
> {
> + fs_fs_data_t *ffd = fs->fsap_data;
> +
> + /* Initialize the return variable. */
> + *path = path_rev(fs, rev, pool);
> +
> + if (ffd->format < SVN_FS_FS__MIN_PACKED_FORMAT)
> + return SVN_NO_ERROR;
> +
> if (! is_packed_rev(fs, rev))
> {
> svn_node_kind_t kind;
>
> - /* Initialize the return variable. */
> - *path = path_rev(fs, rev, pool);
> -
> SVN_ERR(svn_io_check_path(*path, &kind, pool));
> if (kind == svn_node_file)
> {
> /* *path is already set correctly. */
> return SVN_NO_ERROR;
> }
> - else
> + else if (ffd->format >= SVN_FS_FS__MIN_PACKED_FORMAT)
> {
> /* Someone must have run 'svnadmin pack' while this fs object
> * was open. */
> @@ -1167,6 +1172,8 @@ update_min_unpacked_rev(svn_fs_t *fs, apr_pool_t *
> {
> fs_fs_data_t *ffd = fs->fsap_data;
>
> + SVN_ERR_ASSERT(ffd->format >= SVN_FS_FS__MIN_PACKED_FORMAT);
> +
> return read_min_unpacked_rev(&ffd->min_unpacked_rev,
> path_min_unpacked_rev(fs, pool),
> pool);
> @@ -1177,6 +1184,8 @@ update_min_unpacked_revprop(svn_fs_t *fs, apr_pool
> {
> fs_fs_data_t *ffd = fs->fsap_data;
>
> + SVN_ERR_ASSERT(ffd->format >= SVN_FS_FS__MIN_PACKED_REVPROP_FORMAT);
> +
> return read_min_unpacked_rev(&ffd->min_unpacked_revprop,
> path_min_unpacked_revprop(fs, pool),
> pool);
> @@ -2821,9 +2830,11 @@ svn_fs_fs__revision_proplist(apr_hash_t **proplist
> apr_pool_t *pool)
> {
> svn_error_t *err;
> + fs_fs_data_t *ffd = fs->fsap_data;
>
> err = revision_proplist(proplist_p, fs, rev, pool);
> - if (err && err->apr_err == SVN_ERR_FS_NO_SUCH_REVISION)
> + if (err && err->apr_err == SVN_ERR_FS_NO_SUCH_REVISION
> + && ffd->format >= SVN_FS_FS__MIN_PACKED_REVPROP_FORMAT)
> {
> /* If a pack is occurring simultaneously, the min-unpacked-revprop value
> could change, so reload it and then attempt to fetch these revprops
> ]]]
Re: [PATCH] fsfs: add two "format new enough" checks
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
By the way, I was seeing this with a "-DSVN_FS_FS_DEFAULT_MAX_FILES_PER_DIR=4
-DPACK_AFTER_EVERY_COMMIT" build. (I always use these two flags in my builds.)
Running tests now.
Daniel