You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ivan Zhakov <iv...@visualsvn.com> on 2015/03/11 12:03:57 UTC

Re: svn commit: r1664523 - /subversion/trunk/subversion/tests/cmdline/upgrade_tests.py

On 6 March 2015 at 02:17,  <ph...@apache.org> wrote:
> Author: philip
> Date: Thu Mar  5 23:17:26 2015
> New Revision: 1664523
>
> URL: http://svn.apache.org/r1664523
> Log:
> * subversion/tests/cmdline/upgrade_tests.py
>   (auto_analyze): Relocate without using the client.
>
> Modified:
>     subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
>
> Modified: subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/upgrade_tests.py?rev=1664523&r1=1664522&r2=1664523&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/cmdline/upgrade_tests.py (original)
> +++ subversion/trunk/subversion/tests/cmdline/upgrade_tests.py Thu Mar  5 23:17:26 2015
> @@ -1446,9 +1446,11 @@ def auto_analyze(sbox):
>    replace_sbox_with_tarfile(sbox, 'wc-without-stat1.tar.bz2')
>    svntest.main.run_svnadmin('setuuid', sbox.repo_dir,
>                              '52ec7e4b-e5f0-451d-829f-f05d5571b4ab')
> -  svntest.actions.run_and_verify_svn(None, [], 'relocate',
> -                                     'file:///tmp/repos', sbox.repo_url,
> -                                     sbox.wc_dir)
> +
> +  # Don't use svn to do relocate as that will add the table.
> +  val = svntest.wc.sqlite_stmt(sbox.wc_dir,
> +                               "update repository "
> +                               "set root ='" + sbox.repo_url + "'");
>
The upgrade_tests.py#36 fails on Windows (sqlite 3.8.8.2), Python 2.6.7.
[[[
Testing Debug configuration on local repository.
START: upgrade_tests.py
W: CWD: M:\svn\test_trunk\subversion\tests\cmdline
Traceback (most recent call last):
  File "C:\Ivan\SVN\trunk\subversion\tests\cmdline\svntest\main.py",
line 1744, in run
    rc = self.pred.run(sandbox)
  File "C:\Ivan\SVN\trunk\subversion\tests\cmdline\svntest\testcase.py",
line 178, in run
    result = self.func(sandbox)
  File "C:\Ivan\SVN\trunk\subversion\tests\cmdline\upgrade_tests.py",
line 1453, in auto_analyze
    "set root ='" + sbox.repo_url + "'");
  File "C:\Ivan\SVN\trunk\subversion\tests\cmdline\svntest\wc.py",
line 1044, in sqlite_exec
    c.execute(stmt)
DatabaseError: malformed database schema
(nodes_update_checksum_trigger) - near "OLD": syntax error
FAIL:  upgrade_tests.py 36: automatic SQLite ANALYZE
END: upgrade_tests.py
ELAPSED: upgrade_tests.py 0:00:00.758000
]]]

I'm also noted that wc.db in wc-without-stat1.tar.bz2 contains
sqlite_stat1 table:
[[[
sqlite> select * from sqlite_stat1;
NODES|sqlite_autoindex_NODES_1|8000 8000 2 1
NODES|I_NODES_PARENT|8000 8000 10 2 1
NODES|I_NODES_MOVED|8000 8000 1 1
ACTUAL_NODE|sqlite_autoindex_ACTUAL_NODE_1|8000 8000 1
ACTUAL_NODE|I_ACTUAL_PARENT|8000 8000 10 1
LOCK|sqlite_autoindex_LOCK_1|100 100 1
WC_LOCK|sqlite_autoindex_WC_LOCK_1|100 100 1
]]]

As far I understand that it should not be there, otherwise I don't
understand what test is supposed to test.

-- 
Ivan Zhakov

Re: svn commit: r1664523 - /subversion/trunk/subversion/tests/cmdline/upgrade_tests.py

Posted by Philip Martin <ph...@wandisco.com>.
Ivan Zhakov <iv...@visualsvn.com> writes:

> It's very likely that Python compiled to different version of sqlite.
> But how it supposed to work?  Is not sqlite compatible between
> different versions? What is the minimum requirements? How packagers
> should deal with such test failures?

I cannot really help here, but I have run the test on Centos 6 with
Python 2.6.6 and SQLite 3.6.20, and FreeBSD 10 with Python 2.7.9 and
SQLite 3.8.8.2.

> I am also find test the actually doesn't test what is supposed to test
> as false sense of security. I'm ok if you think that we don't need
> test for this case, but we should not pretend that we have test which
> actually doesn't test anything.

The test has been improved.

-- 
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*

Re: svn commit: r1664523 - /subversion/trunk/subversion/tests/cmdline/upgrade_tests.py

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 11 March 2015 at 15:56, Bert Huijben <be...@qqmail.nl> wrote:
>
>> -----Original Message-----
>> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
>> Sent: woensdag 11 maart 2015 13:03
>> To: Bert Huijben
>> Cc: dev@subversion.apache.org; philip@apache.org
>> Subject: Re: svn commit: r1664523 -
>> /subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
>>
[...]

>> It's very likely that Python compiled to different version of sqlite.
>> But how it supposed to work?  Is not sqlite compatible between
>> different versions? What is the minimum requirements? How packagers
>> should deal with such test failures?
>
> My guess would be that you need Sqlite 3.6.18 (2009-09-11) or later.
> https://www.sqlite.org/changes.html#version_3_6_18
>
> My Python on my primary test VM has Sqlite 3.6.21 builtin and has no problem running these tests.
>
> $ python
> ActivePython 2.7.2.5 (ActiveState Software Inc.) based on
> Python 2.7.2 (default, Jun 24 2011, 12:21:10) [MSC v.1500 32 bit (Intel)] on win
> 32
> Type "help", "copyright", "credits" or "license" for more information.
>>>> import sqlite3;
>>>> sqlite3.sqlite_version
> '3.6.21'
>
>
[[[
ActivePython 2.6.7.20 (ActiveState Software Inc.) based on
Python 2.6.7 (r267:88850, Jun 27 2011, 13:20:48) [MSC v.1500 64 bit
(AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import sqlite3;
>>> sqlite3.sqlite_version
'3.5.9'
>>>
]]]

And error is "DatabaseError: malformed database schema
(nodes_update_checksum_trigger) - near "OLD": syntax error"

Most likely our database schema is not compatible with SQLite 3.5.x.
sqlite.c has compile-time check to build with SQLite 3.7.12 at least,
but even latest Python from python.org has sqlite 3.6.21 bundled. That
means we could expect similiar problems in future.

I've updated to Python 2.7.8 for now and continue to review the change itself.

-- 
Ivan Zhakov

RE: svn commit: r1664523 - /subversion/trunk/subversion/tests/cmdline/upgrade_tests.py

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
> Sent: woensdag 11 maart 2015 13:03
> To: Bert Huijben
> Cc: dev@subversion.apache.org; philip@apache.org
> Subject: Re: svn commit: r1664523 -
> /subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
> 
> On 11 March 2015 at 14:20, Bert Huijben <be...@qqmail.nl> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
> >> Sent: woensdag 11 maart 2015 12:04
> >> To: dev@subversion.apache.org; philip@apache.org; Bert Huijben
> >> Subject: Re: svn commit: r1664523 -
> >> /subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
> >>
> >> On 6 March 2015 at 02:17,  <ph...@apache.org> wrote:
> >> > Author: philip
> >> > Date: Thu Mar  5 23:17:26 2015
> >> > New Revision: 1664523
> >> >
> >> > URL: http://svn.apache.org/r1664523
> >> > Log:
> >> > * subversion/tests/cmdline/upgrade_tests.py
> >> >   (auto_analyze): Relocate without using the client.
> >> >
> >> > Modified:
> >> >     subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
> >> >
> >> > Modified: subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
> >> > URL:
> >>
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/upgra
> >> de_tests.py?rev=1664523&r1=1664522&r2=1664523&view=diff
> >> >
> >>
> ================================================================
> >> ==============
> >> > --- subversion/trunk/subversion/tests/cmdline/upgrade_tests.py (original)
> >> > +++ subversion/trunk/subversion/tests/cmdline/upgrade_tests.py Thu Mar
> 5
> >> 23:17:26 2015
> >> > @@ -1446,9 +1446,11 @@ def auto_analyze(sbox):
> >> >    replace_sbox_with_tarfile(sbox, 'wc-without-stat1.tar.bz2')
> >> >    svntest.main.run_svnadmin('setuuid', sbox.repo_dir,
> >> >                              '52ec7e4b-e5f0-451d-829f-f05d5571b4ab')
> >> > -  svntest.actions.run_and_verify_svn(None, [], 'relocate',
> >> > -                                     'file:///tmp/repos', sbox.repo_url,
> >> > -                                     sbox.wc_dir)
> >> > +
> >> > +  # Don't use svn to do relocate as that will add the table.
> >> > +  val = svntest.wc.sqlite_stmt(sbox.wc_dir,
> >> > +                               "update repository "
> >> > +                               "set root ='" + sbox.repo_url + "'");
> >> >
> >> The upgrade_tests.py#36 fails on Windows (sqlite 3.8.8.2), Python 2.6.7.
> >> [[[
> >> Testing Debug configuration on local repository.
> >> START: upgrade_tests.py
> >> W: CWD: M:\svn\test_trunk\subversion\tests\cmdline
> >> Traceback (most recent call last):
> >>   File "C:\Ivan\SVN\trunk\subversion\tests\cmdline\svntest\main.py",
> >> line 1744, in run
> >>     rc = self.pred.run(sandbox)
> >>   File "C:\Ivan\SVN\trunk\subversion\tests\cmdline\svntest\testcase.py",
> >> line 178, in run
> >>     result = self.func(sandbox)
> >>   File "C:\Ivan\SVN\trunk\subversion\tests\cmdline\upgrade_tests.py",
> >> line 1453, in auto_analyze
> >>     "set root ='" + sbox.repo_url + "'");
> >>   File "C:\Ivan\SVN\trunk\subversion\tests\cmdline\svntest\wc.py",
> >> line 1044, in sqlite_exec
> >>     c.execute(stmt)
> >> DatabaseError: malformed database schema
> >> (nodes_update_checksum_trigger) - near "OLD": syntax error
> >> FAIL:  upgrade_tests.py 36: automatic SQLite ANALYZE
> >> END: upgrade_tests.py
> >> ELAPSED: upgrade_tests.py 0:00:00.758000
> >> ]]]
> >
> > This is caused by the Sqlite that is linked into your python version...
> > (Unrelated to your separate sqlite version, which you quoted)
> >
> It's very likely that Python compiled to different version of sqlite.
> But how it supposed to work?  Is not sqlite compatible between
> different versions? What is the minimum requirements? How packagers
> should deal with such test failures?

My guess would be that you need Sqlite 3.6.18 (2009-09-11) or later.
https://www.sqlite.org/changes.html#version_3_6_18

My Python on my primary test VM has Sqlite 3.6.21 builtin and has no problem running these tests.

$ python
ActivePython 2.7.2.5 (ActiveState Software Inc.) based on
Python 2.7.2 (default, Jun 24 2011, 12:21:10) [MSC v.1500 32 bit (Intel)] on win
32
Type "help", "copyright", "credits" or "license" for more information.
>>> import sqlite3;
>>> sqlite3.sqlite_version
'3.6.21'


	Bert


Re: svn commit: r1664523 - /subversion/trunk/subversion/tests/cmdline/upgrade_tests.py

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 11 March 2015 at 14:20, Bert Huijben <be...@qqmail.nl> wrote:
>
>
>> -----Original Message-----
>> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
>> Sent: woensdag 11 maart 2015 12:04
>> To: dev@subversion.apache.org; philip@apache.org; Bert Huijben
>> Subject: Re: svn commit: r1664523 -
>> /subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
>>
>> On 6 March 2015 at 02:17,  <ph...@apache.org> wrote:
>> > Author: philip
>> > Date: Thu Mar  5 23:17:26 2015
>> > New Revision: 1664523
>> >
>> > URL: http://svn.apache.org/r1664523
>> > Log:
>> > * subversion/tests/cmdline/upgrade_tests.py
>> >   (auto_analyze): Relocate without using the client.
>> >
>> > Modified:
>> >     subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
>> >
>> > Modified: subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
>> > URL:
>> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/upgra
>> de_tests.py?rev=1664523&r1=1664522&r2=1664523&view=diff
>> >
>> ================================================================
>> ==============
>> > --- subversion/trunk/subversion/tests/cmdline/upgrade_tests.py (original)
>> > +++ subversion/trunk/subversion/tests/cmdline/upgrade_tests.py Thu Mar  5
>> 23:17:26 2015
>> > @@ -1446,9 +1446,11 @@ def auto_analyze(sbox):
>> >    replace_sbox_with_tarfile(sbox, 'wc-without-stat1.tar.bz2')
>> >    svntest.main.run_svnadmin('setuuid', sbox.repo_dir,
>> >                              '52ec7e4b-e5f0-451d-829f-f05d5571b4ab')
>> > -  svntest.actions.run_and_verify_svn(None, [], 'relocate',
>> > -                                     'file:///tmp/repos', sbox.repo_url,
>> > -                                     sbox.wc_dir)
>> > +
>> > +  # Don't use svn to do relocate as that will add the table.
>> > +  val = svntest.wc.sqlite_stmt(sbox.wc_dir,
>> > +                               "update repository "
>> > +                               "set root ='" + sbox.repo_url + "'");
>> >
>> The upgrade_tests.py#36 fails on Windows (sqlite 3.8.8.2), Python 2.6.7.
>> [[[
>> Testing Debug configuration on local repository.
>> START: upgrade_tests.py
>> W: CWD: M:\svn\test_trunk\subversion\tests\cmdline
>> Traceback (most recent call last):
>>   File "C:\Ivan\SVN\trunk\subversion\tests\cmdline\svntest\main.py",
>> line 1744, in run
>>     rc = self.pred.run(sandbox)
>>   File "C:\Ivan\SVN\trunk\subversion\tests\cmdline\svntest\testcase.py",
>> line 178, in run
>>     result = self.func(sandbox)
>>   File "C:\Ivan\SVN\trunk\subversion\tests\cmdline\upgrade_tests.py",
>> line 1453, in auto_analyze
>>     "set root ='" + sbox.repo_url + "'");
>>   File "C:\Ivan\SVN\trunk\subversion\tests\cmdline\svntest\wc.py",
>> line 1044, in sqlite_exec
>>     c.execute(stmt)
>> DatabaseError: malformed database schema
>> (nodes_update_checksum_trigger) - near "OLD": syntax error
>> FAIL:  upgrade_tests.py 36: automatic SQLite ANALYZE
>> END: upgrade_tests.py
>> ELAPSED: upgrade_tests.py 0:00:00.758000
>> ]]]
>
> This is caused by the Sqlite that is linked into your python version...
> (Unrelated to your separate sqlite version, which you quoted)
>
It's very likely that Python compiled to different version of sqlite.
But how it supposed to work?  Is not sqlite compatible between
different versions? What is the minimum requirements? How packagers
should deal with such test failures?

>> I'm also noted that wc.db in wc-without-stat1.tar.bz2 contains
>> sqlite_stat1 table:
>> [[[
>> sqlite> select * from sqlite_stat1;
>> NODES|sqlite_autoindex_NODES_1|8000 8000 2 1
>> NODES|I_NODES_PARENT|8000 8000 10 2 1
>> NODES|I_NODES_MOVED|8000 8000 1 1
>> ACTUAL_NODE|sqlite_autoindex_ACTUAL_NODE_1|8000 8000 1
>> ACTUAL_NODE|I_ACTUAL_PARENT|8000 8000 10 1
>> LOCK|sqlite_autoindex_LOCK_1|100 100 1
>> WC_LOCK|sqlite_autoindex_WC_LOCK_1|100 100 1
>> ]]]
>>
>> As far I understand that it should not be there, otherwise I don't
>> understand what test is supposed to test.
>
> I understand your review points, but why does this cause you to issue a veto?
> There are other known test failures in 1.9.x
>
> None of these points are about the actual patch/change... just about the test philip added.
>
I consider test failures on a stabilization branch to be unacceptable.

I am also find test the actually doesn't test what is supposed to test
as false sense of security. I'm ok if you think that we don't need
test for this case, but we should not pretend that we have test which
actually doesn't test anything.

-- 
Ivan Zhakov

RE: svn commit: r1664523 - /subversion/trunk/subversion/tests/cmdline/upgrade_tests.py

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
> Sent: woensdag 11 maart 2015 12:04
> To: dev@subversion.apache.org; philip@apache.org; Bert Huijben
> Subject: Re: svn commit: r1664523 -
> /subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
> 
> On 6 March 2015 at 02:17,  <ph...@apache.org> wrote:
> > Author: philip
> > Date: Thu Mar  5 23:17:26 2015
> > New Revision: 1664523
> >
> > URL: http://svn.apache.org/r1664523
> > Log:
> > * subversion/tests/cmdline/upgrade_tests.py
> >   (auto_analyze): Relocate without using the client.
> >
> > Modified:
> >     subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
> >
> > Modified: subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
> > URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/upgra
> de_tests.py?rev=1664523&r1=1664522&r2=1664523&view=diff
> >
> ================================================================
> ==============
> > --- subversion/trunk/subversion/tests/cmdline/upgrade_tests.py (original)
> > +++ subversion/trunk/subversion/tests/cmdline/upgrade_tests.py Thu Mar  5
> 23:17:26 2015
> > @@ -1446,9 +1446,11 @@ def auto_analyze(sbox):
> >    replace_sbox_with_tarfile(sbox, 'wc-without-stat1.tar.bz2')
> >    svntest.main.run_svnadmin('setuuid', sbox.repo_dir,
> >                              '52ec7e4b-e5f0-451d-829f-f05d5571b4ab')
> > -  svntest.actions.run_and_verify_svn(None, [], 'relocate',
> > -                                     'file:///tmp/repos', sbox.repo_url,
> > -                                     sbox.wc_dir)
> > +
> > +  # Don't use svn to do relocate as that will add the table.
> > +  val = svntest.wc.sqlite_stmt(sbox.wc_dir,
> > +                               "update repository "
> > +                               "set root ='" + sbox.repo_url + "'");
> >
> The upgrade_tests.py#36 fails on Windows (sqlite 3.8.8.2), Python 2.6.7.
> [[[
> Testing Debug configuration on local repository.
> START: upgrade_tests.py
> W: CWD: M:\svn\test_trunk\subversion\tests\cmdline
> Traceback (most recent call last):
>   File "C:\Ivan\SVN\trunk\subversion\tests\cmdline\svntest\main.py",
> line 1744, in run
>     rc = self.pred.run(sandbox)
>   File "C:\Ivan\SVN\trunk\subversion\tests\cmdline\svntest\testcase.py",
> line 178, in run
>     result = self.func(sandbox)
>   File "C:\Ivan\SVN\trunk\subversion\tests\cmdline\upgrade_tests.py",
> line 1453, in auto_analyze
>     "set root ='" + sbox.repo_url + "'");
>   File "C:\Ivan\SVN\trunk\subversion\tests\cmdline\svntest\wc.py",
> line 1044, in sqlite_exec
>     c.execute(stmt)
> DatabaseError: malformed database schema
> (nodes_update_checksum_trigger) - near "OLD": syntax error
> FAIL:  upgrade_tests.py 36: automatic SQLite ANALYZE
> END: upgrade_tests.py
> ELAPSED: upgrade_tests.py 0:00:00.758000
> ]]]

This is caused by the Sqlite that is linked into your python version... 
(Unrelated to your separate sqlite version, which you quoted)

I'm surprised that you don't see other test errors, as there are other tests that also open wc.db, which wouldn't work with this sqlite version.

> I'm also noted that wc.db in wc-without-stat1.tar.bz2 contains
> sqlite_stat1 table:
> [[[
> sqlite> select * from sqlite_stat1;
> NODES|sqlite_autoindex_NODES_1|8000 8000 2 1
> NODES|I_NODES_PARENT|8000 8000 10 2 1
> NODES|I_NODES_MOVED|8000 8000 1 1
> ACTUAL_NODE|sqlite_autoindex_ACTUAL_NODE_1|8000 8000 1
> ACTUAL_NODE|I_ACTUAL_PARENT|8000 8000 10 1
> LOCK|sqlite_autoindex_LOCK_1|100 100 1
> WC_LOCK|sqlite_autoindex_WC_LOCK_1|100 100 1
> ]]]
> 
> As far I understand that it should not be there, otherwise I don't
> understand what test is supposed to test.

I understand your review points, but why does this cause you to issue a veto?
There are other known test failures in 1.9.x


None of these points are about the actual patch/change... just about the test philip added.

	Bert