You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ko...@apache.org on 2014/09/10 16:25:49 UTC
svn commit: r1624011 - in /subversion/trunk/subversion: libsvn_fs_fs/util.c
tests/cmdline/svnadmin_tests.py
Author: kotkov
Date: Wed Sep 10 14:25:49 2014
New Revision: 1624011
URL: http://svn.apache.org/r1624011
Log:
Fix a broken assumption within the r1603605 changeset.
When format 1 and 2 filesystems are being upgraded, the upgrade routine
leaves the db/current contents as is. As a consequence, there is a window
when a filesystem has a new format, but the 'current' file still contains the
additional IDs left from an old format, i.e. it could look like "359 j5 v\n"
(last values are bogus and won't be there after the next transaction commit).
We should be able to parse it, otherwise any upgraded pre-1.4-compatible
repository will block the commits.
Loosen the corresponding checks.
* subversion/libsvn_fs_fs/util.c
(svn_fs_fs__read_current): Do not be too strict for new filesystem formats
and only expect a parseable revision number. Adjust a scope of the local
variable.
* subversion/tests/cmdline/svnadmin_tests.py
(upgrade): New regression test.
(test_list): Reference the new test.
Modified:
subversion/trunk/subversion/libsvn_fs_fs/util.c
subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py
Modified: subversion/trunk/subversion/libsvn_fs_fs/util.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/util.c?rev=1624011&r1=1624010&r2=1624011&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/util.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/util.c Wed Sep 10 14:25:49 2014
@@ -439,7 +439,6 @@ svn_fs_fs__read_current(svn_revnum_t *re
{
fs_fs_data_t *ffd = fs->fsap_data;
svn_stringbuf_t *content;
- const char *str;
SVN_ERR(svn_fs_fs__read_content(&content,
svn_fs_fs__path_current(fs, pool),
@@ -447,16 +446,20 @@ svn_fs_fs__read_current(svn_revnum_t *re
if (ffd->format >= SVN_FS_FS__MIN_NO_GLOBAL_IDS_FORMAT)
{
- SVN_ERR(svn_revnum_parse(rev, content->data, &str));
- if (*str != '\n')
- return svn_error_create(SVN_ERR_FS_CORRUPT, NULL,
- _("Corrupt 'current' file"));
+ /* When format 1 and 2 filesystems are upgraded, the 'current' file is
+ left intact. As a consequence, there is a window when a filesystem
+ has a new format, but this file still contains the IDs left from an
+ old format, i.e. looks like "359 j5 v\n". Do not be too strict here
+ and only expect a parseable revision number. */
+ SVN_ERR(svn_revnum_parse(rev, content->data, NULL));
*next_node_id = 0;
*next_copy_id = 0;
}
else
{
+ const char *str;
+
SVN_ERR(svn_revnum_parse(rev, content->data, &str));
if (*str != ' ')
return svn_error_create(SVN_ERR_FS_CORRUPT, NULL,
Modified: subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py?rev=1624011&r1=1624010&r2=1624011&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py Wed Sep 10 14:25:49 2014
@@ -2928,6 +2928,20 @@ def freeze_same_uuid(sbox):
sys.executable, '-c', 'True')
+@Skip(svntest.main.is_fs_type_fsx)
+def upgrade(sbox):
+ "upgrade --compatible-version=1.3"
+
+ sbox.build(create_wc=False, minor_version=3)
+ svntest.actions.run_and_verify_svnadmin(None, None, [], "upgrade",
+ sbox.repo_dir)
+ # Does the repository work after upgrade?
+ svntest.actions.run_and_verify_svn(None, ['Committing transaction...\n',
+ 'Committed revision 2.\n'], [], 'mkdir',
+ '-m', svntest.main.make_log_msg(),
+ sbox.repo_url + '/dir')
+
+
########################################################################
# Run the tests
@@ -2980,6 +2994,7 @@ test_list = [ None,
fsfs_hotcopy_progress_with_revprop_changes,
fsfs_hotcopy_progress_old,
freeze_same_uuid,
+ upgrade,
]
if __name__ == '__main__':
Re: svn commit: r1624011 - in /subversion/trunk/subversion:
libsvn_fs_fs/util.c tests/cmdline/svnadmin_tests.py
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 10 September 2014 20:12, Evgeny Kotkov <ev...@visualsvn.com> wrote:
> Ivan Zhakov <iv...@visualsvn.com> writes:
>
>>> +@Skip(svntest.main.is_fs_type_fsx)
>>>
>> I think this test should be skipped for all FS types except FSFS. I.e.
>> @SkipUnless(svntest.main.is_fs_type_fsfs)
>>
>> Did I miss something?
>
> This was intentional.
>
> Prior to this change, we actually had no tests covering the 'svnadmin upgrade'
> behavior, and I took this opportunity to cover all possible --fs-types with the
> new test. We cannot really do this for FSX, because there are no "compatible
> versions", but can do it for BDB and FSFS. Although BDB is deprecated, the
> upgrade code indeed does some amount of work, so I thought there is nothing
> wrong with also testing it.
>
> What I could have done, however, is state this information in the log message
> (for the future readers' sake). I tweaked it accordingly.
>
Ok, that's makes sense. Thanks for explanation.
--
Ivan Zhakov
Re: svn commit: r1624011 - in /subversion/trunk/subversion:
libsvn_fs_fs/util.c tests/cmdline/svnadmin_tests.py
Posted by Evgeny Kotkov <ev...@visualsvn.com>.
Ivan Zhakov <iv...@visualsvn.com> writes:
>> +@Skip(svntest.main.is_fs_type_fsx)
>>
> I think this test should be skipped for all FS types except FSFS. I.e.
> @SkipUnless(svntest.main.is_fs_type_fsfs)
>
> Did I miss something?
This was intentional.
Prior to this change, we actually had no tests covering the 'svnadmin upgrade'
behavior, and I took this opportunity to cover all possible --fs-types with the
new test. We cannot really do this for FSX, because there are no "compatible
versions", but can do it for BDB and FSFS. Although BDB is deprecated, the
upgrade code indeed does some amount of work, so I thought there is nothing
wrong with also testing it.
What I could have done, however, is state this information in the log message
(for the future readers' sake). I tweaked it accordingly.
Regards,
Evgeny Kotkov
Re: svn commit: r1624011 - in /subversion/trunk/subversion:
libsvn_fs_fs/util.c tests/cmdline/svnadmin_tests.py
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 10 September 2014 18:25, <ko...@apache.org> wrote:
> Author: kotkov
> Date: Wed Sep 10 14:25:49 2014
> New Revision: 1624011
>
> URL: http://svn.apache.org/r1624011
> Log:
> Fix a broken assumption within the r1603605 changeset.
>
> When format 1 and 2 filesystems are being upgraded, the upgrade routine
> leaves the db/current contents as is. As a consequence, there is a window
> when a filesystem has a new format, but the 'current' file still contains the
> additional IDs left from an old format, i.e. it could look like "359 j5 v\n"
> (last values are bogus and won't be there after the next transaction commit).
> We should be able to parse it, otherwise any upgraded pre-1.4-compatible
> repository will block the commits.
>
> Loosen the corresponding checks.
>
[...]
>
> Modified: subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py?rev=1624011&r1=1624010&r2=1624011&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py (original)
> +++ subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py Wed Sep 10 14:25:49 2014
> @@ -2928,6 +2928,20 @@ def freeze_same_uuid(sbox):
> sys.executable, '-c', 'True')
>
>
> +@Skip(svntest.main.is_fs_type_fsx)
I think this test should be skipped for all FS types except FSFS. I.e.
@SkipUnless(svntest.main.is_fs_type_fsfs)
Did I miss something?
--
Ivan Zhakov