Re: [PATCH] Improve svn_checksum_t bindings in SWIG
On Tue, Dec 11, 2012 at 4:08 AM, Daniel Shahaf wrote: > Shivani Poddar wrote on Tue, Dec 11, 2012 at 02:22:28 +0530: > > Log Message: > > > > Improve support for svn_checksum.h in SWIG bindings > > * subversion/bindings/swig/python/tests/checksum.py: Improved > test_checksum > > > > Need a blank line before the * line, and to use the "* file\n (symbol)" > syntax --- 'test_checksum' is a symbol. > > > Review by: danielsh > > > > That's inappropriate; I haven't reviewed the patch yet. You might add > this field after I review it, not before. > Since you reviewed the last patch because of which i wrote this 1 i thought that i needed to attribute you at review > > > Index: subversion/bindings/swig/python/tests/checksum.py > > === > > --- subversion/bindings/swig/python/tests/checksum.py (revision 1419694) > > +++ subversion/bindings/swig/python/tests/checksum.py (working copy) > > @@ -20,22 +20,25 @@ > > # > > import unittest, setup_path > > import svn.core > > - > > +LENGTH = 32 or 40; > > This is wrong in two different ways: > > - "32 or 40" is a constant expression that evaluates to 32. > > - You hardcode two values, when you should be hardcoding neither of > them. (The bindings shouldn't need to change if the C library grows > a third svn_checksum_kind_t.) > > the symbolic constants in python are declared as this one. However in this test, since we are checking by only svn_checksum_md5 , we only need the length to be >= 32, i dont know why we would want to include 40 in the length here , since atleast in this test length should always be 32. so maybe LENGTH = svn.core.svn_checksum_to_cstring_display(svn_checksum_create(svn_checksum_md5)) would have been a better thing to do > class ChecksumTestCases(unittest.TestCase): > > def test_checksum(self): > > # Checking primarily the return type for the svn_checksum_create > > # function > > val = svn.core.svn_checksum_create(svn.core.svn_checksum_md5) > > check_val = svn.core.svn_checksum_to_cstring_display(val) > > - > > # The svn_checksum_to_cstring_display should return a str type > object > > # from the check_val object passed to it > > if(type(check_val) == str): > > -# The intialized value created from a checksum should be 0 > > -if(int(check_val) != 0): > > -self.assertRaises(AssertionError) > > +#Check the length of the string check_val > > +if(len(check_val) == LENGTH): > > +# The intialized value created from a checksum should > be 0 > > +if(int(check_val) != 0): > > +raise > > This bare "raise" statement without arguments is itself an error. > > See for yourself: > > % python -c 'raise' > TypeError: exceptions must be old-style classes or derived from > BaseException, not NoneType > > This exception signifies a bug in your program. It has become > a RuntimeError in recent Pythons (and, frankly, could become > a compile-time error as well --- the compiler knows there's no except: > block surrounding this statement). It might work, but not because it's > correct. > > Yes it was working for me in the program, will check how i can fix this one > Daniel > > > +else: > > + raise > > else: > > -self.assertRaises(TypeError, test_checksum) > > +raise > > > > def suite(): > > return > unittest.defaultTestLoader.loadTestsFromTestCase(ChecksumTestCases) > > -- Shivani Poddar, Bachelors in Computer Sciences and MS in Exact Humanities, Sophomore International Institute of Information Technology, Hyderabad
Re: OWP: Introduction for Gabriela Gibson
On 12/10/2012 07:32 PM, Daniel Shahaf wrote: > Gabriela Gibson wrote on Tue, Dec 11, 2012 at 00:21:19 +: >> For my initial submission I have written a test for issue 4263 which >> I'll mail shortly. I admit that I struggled to figure out how to >> connect gdb to svnrdump in my build tree (since svnrdump is actually a >> shell script, and svnrdump reads from stdin) but I am now working on a >> fix for 4263. >> > > libtool --mode=execute gdb -args subversion/svnrdump/svnrdump > > (documentation patches welcome...) I often run gdb using subversion/svnrdump/.libs/lt-svnrdump. But note that that program doesn't exist until after the first post-compilation invocation of subversion/svnrdump/svnrdump itself, so I'm thankful to also learn of the method Daniel reports here. -- C. Michael Pilato CollabNet <> www.collab.net <> Enterprise Cloud Development signature.asc Description: OpenPGP digital signature
Re: 1.7.8 up for testing/signing
On Mon, Dec 10, 2012 at 5:07 PM, Ben Reser wrote: > The 1.7.8 release artifacts are now available for testing/signing. > Please get the tarballs from > https://dist.apache.org/repos/dist/dev/subversion > and add your signatures there. > > Thanks! SUMMARY: - +1 to release VERIFIED: - Other than the expected differences in subversion/include/svn_version.h, https://dist.apache.org/repos/dist/dev/subversion/subversion-1.7.8.zip is identical to https://svn.apache.org/repos/asf/subversion/branches/1.7.x@1419691. SHA1 checksum of https://dist.apache.org/repos/dist/dev/subversion/subversion-1.7.8.zip is 65985725f8138cc18993a9088d4ad70df6c0d816 TESTED: --- [Release-Build] x[ fsfs | bdb ] x [ file | svn | http (neon) | http (serf) ] Ruby bindings (patched as described here: http://svn.haxx.se/dev/archive-2011-06/0682.shtml) JavaHL Bindings RESULTS: All Tests Pass PLATFORM: - MS Windows 7 Home Premium Service Pack 1 Microsoft Visual Studio 2008 Version 9.0.30729.1 SP DEPENDENCIES: - APR: 1.4.6 APR-UTIL: 1.4.1 Neon: 0.29.5 zlib: 1.2.4 OpenSSL: 0.9.8q Apache: 2.2.22 BDB: 4.8.30 sqlite: 3.7.7.1 Python: 2.7.2 (ActivePython 2.7.2.5) Perl: 5.14.2 Ruby: ruby 1.8.7 java: 1.6.0_21 junit: 4.8.2 swig: 1.3.40 serf: 1.1.1 SIGNATURE: -- https://dist.apache.org/repos/dist/dev/subversion/subversion-1.7.8.zip: -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.5 (MingW32) iD8DBQBQxnLZ2RaJMFP83FURAhdgAJ4uhjXgujUQK5MCObZWanCv0y06dgCfcip4 9wYkft1k86LMlDaXYBRZfvk= =EbgM -END PGP SIGNATURE- -- Paul T. Burba CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development Skype: ptburba
[PATCH] Test for line ending bug in svnrdump (issue 4263)
[[[ Test for issue #4263: svnrdump: E125005: Cannot accept non-LF line endings in 'svn:log' property * subversion/tests/cmdline/svnrdump_tests.py copy_bad_line_endings_load: Test for \r line ending bug in svnrdump (issue 4263) ]]] Index: svnrdump_tests.py === --- svnrdump_tests.py (revision 1419536) +++ svnrdump_tests.py (working copy) @@ -356,6 +356,10 @@ def copy_bad_line_endings_dump(sbox): expected_dumpfile_name="copy-bad-line-endings.expected.dump", bypass_prop_validation=True) +def copy_bad_line_endings_load(sbox): + "load: inconsistent line endings in svn:* props" + run_load_test(sbox, "copy-bad-line-endings.dump") + def copy_bad_line_endings2_dump(sbox): "dump: non-LF line endings in svn:* props" run_dump_test(sbox, "copy-bad-line-endings2.dump", @@ -771,6 +775,7 @@ test_list = [ None, move_and_modify_in_the_same_revision_dump, move_and_modify_in_the_same_revision_load, copy_bad_line_endings_dump, + copy_bad_line_endings_load, copy_bad_line_endings2_dump, commit_a_copy_of_root_dump, commit_a_copy_of_root_load,
OWP: Introduction for Gabriela Gibson
Dear All, My name is Gabriela Gibson and I've applied for the internship with Subversion under the Gnome Outreach for Women Program. I am an ex-wizardess who used to haunt a long-lost MUD. I've not programmed in a while, although I've *intended* to join more than one open-source project in the past. I've been using linux since 1997. For my initial submission I have written a test for issue 4263 which I'll mail shortly. I admit that I struggled to figure out how to connect gdb to svnrdump in my build tree (since svnrdump is actually a shell script, and svnrdump reads from stdin) but I am now working on a fix for 4263. BTW, I noticed that the python tests in subversion/tests/cmdline/ don't support a "list" parameter as suggested in subversion/tests/README. Is there a reason for this? Otherwise, this might something I could fix. I don't know what I would be doing for my project yet, because I realise that the Subversion codebase is large and complicated - my learning curve has been both long and steep. However, I feel that I'm reaching the point where I can contribute to the project. Regards Gabriela
Re: svn_client operations start failing - unable to resolve host name
So I managed to figure out the problem. It was a socket leak, caused by me failing to destroy a pool inside a function called in a loop. My apologies for disturbing people. On 2012-11-27 9:33 AM, "Robert Turner" wrote: >Thanks for the reply. > >So I spent ages digging into this (and related issues) last night, and >that is correct. gethostbyname() is indeed failing, spontaneously it >seems. However, as to why, I don't know yet. I'm suspecting something is >getting messed up in the local memory space that is affecting it. Other >processes do not seem to be affected. > > >I also tried using 1.7.7 client libraries (which I eventually got to >compile and more or less work from source once I stopped using a custom >built APR and used the one Apple provides on the platform). However, I was >running into stack corruption issues during repeated svn_client_cat2() >calls (the revision was getting overwritten, or the pointer was getting >changed and ended up being the URL string). I haven't yet figured these >issues out, or tried more current code than 1.7.7. > >Robert > >On 2012-11-27 9:14 AM, "Daniel Shahaf" wrote: > >>Robert Turner wrote on Mon, Nov 26, 2012 at 19:49:35 +: >>> I'm in the process of making some updates to a FUSE svnfs, and while >>>the process is merrily chugging away pulling file information, it starts >>>getting lots of errors from the svn_client commands (not just the one >>>below, but that's an example): >>> >>> DEBUG : [00] svnclient_cache(): svn_client_cat(), err->message='OPTIONS >>>of '<<>>': Could not resolve hostname `<<>>removed>>>': Host not found (<<>>)' >>>err->apr_err=175002,0x2ab9a 'RA layer request failed' >>> >>> There is nothing of note configured related to proxies, or anything. As >>>mentioned above, the operations are performing correctly for a while, >>>then stop, and no further operations succeed until the process exits and >>>is restarted. My process seems to behaving well from a memory >>>perspective, and static analysis tools are coming up clean. >>> >>> I'm suspecting that something is getting confused in the internal state >>>of the SVN client libraries. >>> >> >>And I'm suspecting your C library DNS resolver function is returning an >>error. Have you ruled that out yet? >> >>> Any suggestions on where to look further, or what might be the problem? >>> >>> >>> >>> (I'm using the 1.6.18 client libraries from wanDisco, compiled for OSX >>>-- I have been unable to get locally compiled and working copies of the >>>SVN libraries myself when trying to compile from source, so I have no >>>easy way to debug through the SVN libraries) >>> >>> Thanks for any ideas anyone might have, >>> >>> Robert >
Re: [PATCH] Test for line ending bug in svnrdump (issue 4263)
Gabriela Gibson wrote on Tue, Dec 11, 2012 at 00:21:46 +: > Thanks for the patch, a few quick comments: > [[[ > Test for issue #4263: svnrdump: E125005: Cannot accept non-LF line > endings in 'svn:log' property > > * subversion/tests/cmdline/svnrdump_tests.py >copy_bad_line_endings_load: Test for \r line ending bug in svnrdump > (issue 4263) Need parentheses around the symbol name. Lines should be wrapped at 80 characters and subsequent lines indented. > ]]] > > > > Index: svnrdump_tests.py > === > --- svnrdump_tests.py (revision 1419536) > +++ svnrdump_tests.py (working copy) > @@ -356,6 +356,10 @@ def copy_bad_line_endings_dump(sbox): > expected_dumpfile_name="copy-bad-line-endings.expected.dump", > bypass_prop_validation=True) > > +def copy_bad_line_endings_load(sbox): The test needs an @XFail decorator, since it currently FAILs. And an @Issue decorator, to associate it with #4263. > + "load: inconsistent line endings in svn:* props" > + run_load_test(sbox, "copy-bad-line-endings.dump") FTR, when run over svn://, this currently errors as: subversion/svnrdump/svnrdump.c:554: (apr_err=125005) subversion/libsvn_repos/load.c:583: (apr_err=125005) subversion/libsvn_repos/load.c:260: (apr_err=125005) subversion/svnrdump/load_editor.c:858: (apr_err=125005) subversion/libsvn_repos/fs-wrap.c:193: (apr_err=125005) svnrdump: E125005: Cannot accept non-LF line endings in 'svn:log' property > + > def copy_bad_line_endings2_dump(sbox): >"dump: non-LF line endings in svn:* props" >run_dump_test(sbox, "copy-bad-line-endings2.dump", > @@ -771,6 +775,7 @@ test_list = [ None, >move_and_modify_in_the_same_revision_dump, >move_and_modify_in_the_same_revision_load, >copy_bad_line_endings_dump, > + copy_bad_line_endings_load, >copy_bad_line_endings2_dump, >commit_a_copy_of_root_dump, >commit_a_copy_of_root_load, > In summary, thanks for this contribution. I guess it's correct but I don't want to make that judgement at 2am, so I'll probably apply the patch Wed or Thu after reviewing the relevant parts of the C code too. Re your other mail about OPW, you shouldn't let yourself be blocked by this --- while this patch is outstanding, you should feel free to work on another patch. The natural choice would be the C patch that turns this test from XFAIL to PASS. Cheers, Daniel
Re: OWP: Introduction for Gabriela Gibson
Gabriela Gibson wrote on Tue, Dec 11, 2012 at 00:21:19 +: > For my initial submission I have written a test for issue 4263 which > I'll mail shortly. I admit that I struggled to figure out how to > connect gdb to svnrdump in my build tree (since svnrdump is actually a > shell script, and svnrdump reads from stdin) but I am now working on a > fix for 4263. > libtool --mode=execute gdb -args subversion/svnrdump/svnrdump (documentation patches welcome...) > BTW, I noticed that the python tests in subversion/tests/cmdline/ > don't support a "list" parameter as suggested in > subversion/tests/README. Is there a reason for this? Otherwise, this > might something I could fix. The parameter has been renamed to --list. "foo_tests.py bar" now runs the "bar" function defined in foo_tests.py (usually one of the functions enumerated in test_list). > > I don't know what I would be doing for my project yet, because I > realise that the Subversion codebase is large and complicated - my > learning curve has been both long and steep. However, I feel that I'm > reaching the point where I can contribute to the project. > Great! > Regards > > Gabriela > >
[PATCH] Test for line ending bug in svnrdump (issue 4263)
[[[ Test for issue #4263: svnrdump: E125005: Cannot accept non-LF line endings in 'svn:log' property * subversion/tests/cmdline/svnrdump_tests.py copy_bad_line_endings_load: Test for \r line ending bug in svnrdump (issue 4263) ]]] Index: svnrdump_tests.py === --- svnrdump_tests.py (revision 1419536) +++ svnrdump_tests.py (working copy) @@ -356,6 +356,10 @@ def copy_bad_line_endings_dump(sbox): expected_dumpfile_name="copy-bad-line-endings.expected.dump", bypass_prop_validation=True) +def copy_bad_line_endings_load(sbox): + "load: inconsistent line endings in svn:* props" + run_load_test(sbox, "copy-bad-line-endings.dump") + def copy_bad_line_endings2_dump(sbox): "dump: non-LF line endings in svn:* props" run_dump_test(sbox, "copy-bad-line-endings2.dump", @@ -771,6 +775,7 @@ test_list = [ None, move_and_modify_in_the_same_revision_dump, move_and_modify_in_the_same_revision_load, copy_bad_line_endings_dump, + copy_bad_line_endings_load, copy_bad_line_endings2_dump, commit_a_copy_of_root_dump, commit_a_copy_of_root_load,
OWP: Introduction for Gabriela Gibson
Dear All, My name is Gabriela Gibson and I've applied for the internship with Subversion under the Gnome Outreach for Women Program. I am an ex-wizardess who used to haunt a long-lost MUD. I've not programmed in a while, although I've *intended* to join more than one open-source project in the past. I've been using linux since 1997. For my initial submission I have written a test for issue 4263 which I'll mail shortly. I admit that I struggled to figure out how to connect gdb to svnrdump in my build tree (since svnrdump is actually a shell script, and svnrdump reads from stdin) but I am now working on a fix for 4263. BTW, I noticed that the python tests in subversion/tests/cmdline/ don't support a "list" parameter as suggested in subversion/tests/README. Is there a reason for this? Otherwise, this might something I could fix. I don't know what I would be doing for my project yet, because I realise that the Subversion codebase is large and complicated - my learning curve has been both long and steep. However, I feel that I'm reaching the point where I can contribute to the project. Regards Gabriela
Re: Literals in wc SQLite queries
Philip Martin wrote on Mon, Dec 10, 2012 at 17:49:44 +: > Daniel Shahaf writes: > > > Philip - perhaps you can move the relevant definitions to that header? > > Then I'll follow up with a transform_sql.py patch. > > OK, I've done that. I didn't annotate the maps or elements in any way. Implemented. Please note the question embedded in the r1419771 log message. (The code added in r1419817 might be relevant, too, in case both the node-kind and the depth maps have an "unknown" map member.)
Re: [PATCH] Improve svn_checksum_t bindings in SWIG
Shivani Poddar wrote on Tue, Dec 11, 2012 at 02:22:28 +0530: > Log Message: > > Improve support for svn_checksum.h in SWIG bindings > * subversion/bindings/swig/python/tests/checksum.py: Improved test_checksum > Need a blank line before the * line, and to use the "* file\n (symbol)" syntax --- 'test_checksum' is a symbol. > Review by: danielsh > That's inappropriate; I haven't reviewed the patch yet. You might add this field after I review it, not before. > Index: subversion/bindings/swig/python/tests/checksum.py > === > --- subversion/bindings/swig/python/tests/checksum.py (revision 1419694) > +++ subversion/bindings/swig/python/tests/checksum.py (working copy) > @@ -20,22 +20,25 @@ > # > import unittest, setup_path > import svn.core > - > +LENGTH = 32 or 40; This is wrong in two different ways: - "32 or 40" is a constant expression that evaluates to 32. - You hardcode two values, when you should be hardcoding neither of them. (The bindings shouldn't need to change if the C library grows a third svn_checksum_kind_t.) > class ChecksumTestCases(unittest.TestCase): > def test_checksum(self): > # Checking primarily the return type for the svn_checksum_create > # function > val = svn.core.svn_checksum_create(svn.core.svn_checksum_md5) > check_val = svn.core.svn_checksum_to_cstring_display(val) > - > # The svn_checksum_to_cstring_display should return a str type object > # from the check_val object passed to it > if(type(check_val) == str): > -# The intialized value created from a checksum should be 0 > -if(int(check_val) != 0): > -self.assertRaises(AssertionError) > +#Check the length of the string check_val > +if(len(check_val) == LENGTH): > +# The intialized value created from a checksum should be 0 > +if(int(check_val) != 0): > +raise This bare "raise" statement without arguments is itself an error. See for yourself: % python -c 'raise' TypeError: exceptions must be old-style classes or derived from BaseException, not NoneType This exception signifies a bug in your program. It has become a RuntimeError in recent Pythons (and, frankly, could become a compile-time error as well --- the compiler knows there's no except: block surrounding this statement). It might work, but not because it's correct. Daniel > +else: > + raise > else: > -self.assertRaises(TypeError, test_checksum) > +raise > > def suite(): > return > unittest.defaultTestLoader.loadTestsFromTestCase(ChecksumTestCases)
1.7.8 up for testing/signing
The 1.7.8 release artifacts are now available for testing/signing. Please get the tarballs from https://dist.apache.org/repos/dist/dev/subversion and add your signatures there. Thanks!
[PATCH] Improve svn_checksum_t bindings in SWIG
Log Message: Improve support for svn_checksum.h in SWIG bindings * subversion/bindings/swig/python/tests/checksum.py: Improved test_checksum Review by: danielsh Modified: subversion/trunk/subversion/bindings/swig/python/tests/checksum.py Regards, Shivani Poddar Bachelors in Computer Sciences and MS in Exact Humanities, Sophomore International Institute of Information Technology, Hyderabad Index: subversion/bindings/swig/python/tests/checksum.py === --- subversion/bindings/swig/python/tests/checksum.py (revision 1419694) +++ subversion/bindings/swig/python/tests/checksum.py (working copy) @@ -20,22 +20,25 @@ # import unittest, setup_path import svn.core - +LENGTH = 32 or 40; class ChecksumTestCases(unittest.TestCase): def test_checksum(self): # Checking primarily the return type for the svn_checksum_create # function val = svn.core.svn_checksum_create(svn.core.svn_checksum_md5) check_val = svn.core.svn_checksum_to_cstring_display(val) - # The svn_checksum_to_cstring_display should return a str type object # from the check_val object passed to it if(type(check_val) == str): -# The intialized value created from a checksum should be 0 -if(int(check_val) != 0): -self.assertRaises(AssertionError) +#Check the length of the string check_val +if(len(check_val) == LENGTH): +# The intialized value created from a checksum should be 0 +if(int(check_val) != 0): +raise +else: + raise else: -self.assertRaises(TypeError, test_checksum) +raise def suite(): return unittest.defaultTestLoader.loadTestsFromTestCase(ChecksumTestCases)
Re: svn commit: r1415864 - /subversion/trunk/subversion/libsvn_ra_serf/update.c
Hi Markus, On Mon, Dec 3, 2012 at 10:43 AM, Markus Schaber wrote: > Hi, > > Just another crazy idea: > > The main problem with the parallelization in ra_serf seems to be the number > of http requests (which potentially causes a high number of authentications > and tcp connections). > > Maybe we could add some partitioned send-all request: > > When the client decides to use 4 connections, it could send 4 requests, with > some parameter like send-all(1/4), send-all(2/4), ..., send-all(4/4). > > Then the server can send one quarter of the complete response on each > connection. > > An advanced server could even share the common state of those 4 requests > through some shared memory / caching scheme, to avoid doing the same work > multiple times. > > Years ago, I implemented a similar scheme between caching GIS web frontend > servers, and the rendering backend server, in the protocol for fetching and > rendering the map tiles. It gave a nearly linear speedup with the number of > connections, up to the point where the CPUs were saturated. > the concept implemented in ra_serf is to parallelize individual GET requests, so that these can be cached by proxies either on the client or on the server side. So we want to avoid using send-all as much as possible, as this creates always one large uncacheable response. I've made a mental note of your idea though, if we need to stick with send-all and further improve it, your suggestion is one way to do this. > > Best regards > > Markus Schaber > thanks, Lieven [..]
Re: SQLite vacuum or auto_vacuum?
On 12/03/2012 07:39 AM, Hyrum K Wright wrote: > I think adding "vacuum" to cleanup is a reasonable first step. "cleanup" is > an explicit operation that a user could reasonably expect to take some > non-trivial amount of time. We already remove unneeded pristines during > cleanup, might as well do the same thing with wc.db space. Yup, agreed. +1. -- C. Michael Pilato CollabNet <> www.collab.net <> Enterprise Cloud Development signature.asc Description: OpenPGP digital signature
svnhooks program (was Re: enforcing LF-normalization for svn:eol-style=native files (issue #4065))
On Mon, Dec 10, 2012 at 3:53 PM, Johan Corveleyn wrote: > On Mon, Dec 10, 2012 at 12:47 PM, Daniel Shahaf > wrote: [...] >> The argument is that a Subversion server should be enforcing >> Subversion's invariants. >> >> That said, I'm not opposed to doing it via standard hooks. It's a good >> way to introduce the feature in a way that allows more easily changing >> it before it hits the APIs and their strict compatibility rules. > > Yes, that would be fine I think. I like Ivan's suggestion of creating > a new standard 'svnhooks' program or something like that, which can > then be part of standard distributions. > I'd like to share my thoughts about 'svnhooks' if someone is going to implement it. Introduce svnhooks program with the separate subcommand for each hook kind: 'pre-commit', 'post-comit', 'pre-lock', 'pre-unlock" and etc. The polices for svnhooks program defined in conf\svnhooks.conf file. This makes very easy for user to use svnhooks: just add line 'svnhooks HOOK-TYPE $*' in each hook and tweak svnhooks.conf configuration file. svnhooks configuration file has separate section for each policy. For example: [[[ [check-eol-normalization] enable = yes [rev-propchange] enable = yes users = svnsync [edit-log-message] enable = yes users = admin, @author ]]] -- Ivan Zhakov
Re: svn commit: r1419560 - /subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c
Julian Foad wrote on Mon, Dec 10, 2012 at 16:57:26 +: > svn_wc__db_op_set_property() uses a txn internally, but is being > called within a larger 'outer txn' (sorry if the terminology is wrong) "Savepoint"? http://www.sqlite.org/lang_savepoint.html
Re: Literals in wc SQLite queries
Daniel Shahaf writes: > Philip - perhaps you can move the relevant definitions to that header? > Then I'll follow up with a transform_sql.py patch. OK, I've done that. I didn't annotate the maps or elements in any way. -- Certified & Supported Apache Subversion Downloads: http://www.wandisco.com/subversion/download
Re: Literals in wc SQLite queries
As I said on IRC, happy to look into this, my main question is how
transform_sql.py would know what files to scan for the MAP_DELETED <->
'base-deleted' mappings.
Do we want a header file with a well-known name
(subversion/include/private/)? Maybe in the same directory as the
source .sql file? Maybe the .sql file should have a directive pointing
to the map file?
Among these I prefer the second one, i.e.,
subversion/libsvn_wc/wc-queries.sql -> subversion/libsvn_wc/token-maps.h
Philip - perhaps you can move the relevant definitions to that header?
Then I'll follow up with a transform_sql.py patch.
Philip Martin wrote on Fri, Dec 07, 2012 at 17:54:16 +:
> Columns such as nodes.kind, nodes.presence, etc. have strings that
> should be one of a discrete set of values. When we bind these columns
> in C code we use something like:
>
> svn_sqlite__bindf("t", presence_map, svn_wc__db_status_normal);
>
> This means we only use known values (svn_wc__db_status_normal) and the
> map converts it to the correct discrete string. This checking happens
> at build time.
>
> We also have queries where the strings are defined as literals in
> wc-queries.sql like:
>
> DELETE FROM nodes
> WHERE wc_id = ?1
> AND IS_STRICT_DESCENDANT_OF(local_relpath, ?2)
> AND (op_depth < ?3
>OR (op_depth = ?3 AND presence = 'base-deleted'))
>
> There is no checking of these literals to catch errors such as
> 'base-delete'.
>
> I've been thinking that transform_sql.py should do some checking.
> Perhaps we could move the maps into a know header, annotate them:
>
> { "base-deleted", svn_wc__db_status_base_deleted }, /* MAP_DELETED */
>
> and then have transform_sql.py parse the header and convert:
>
> OR (op_depth = ?3 AND presence = MAP_DELETED))
>
> into
>
> OR (op_depth = ?3 AND presence = 'base-deleted'))
>
> --
> Philip
Re: svn commit: r1419560 - /subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c
Bert Huijben wrote: >> URL: http://svn.apache.org/viewvc?rev=1419560&view=rev >> Log: >> * subversion/libsvn_wc/wc_db_update_move.c >> (update_working_file): Update a comment. >> @@ -247,8 +247,8 @@ update_working_file(svn_skel_t **work_it >> old_version->props, old_version->props, >> actual_props, propchanges, >> scratch_pool, scratch_pool)); >> - /* ### TODO: Make a WQ item in WORK_ITEMS to set new_actual_props >> ... */ >> - /* ### Not a direct DB op like this... */ >> + /* Install the new actual props. Don't set the conflict_skel yet, >> + because we might need to add a text conflict to it as well. */ >> SVN_ERR(svn_wc__db_op_set_props(db, local_abspath, >> new_actual_props, >> svn_wc__has_magic_property(propchanges), > > This tells me that the change is not atomic, so this really needs some fix-me > comment after all. > > The text and properties should be modified in one sqlite operation. > (Or in other words: the db should be updated to its final state, with the > installing of the wq items in a single transaction) The db_op_set_property() here is just one of a series of DB modifications being made here, all within a single DB txn. svn_wc__db_op_set_property() uses a txn internally, but is being called within a larger 'outer txn' (sorry if the terminology is wrong) set up by the top-level function, svn_wc__db_update_moved_away_conflict_victim(), which executes the whole call tree in this file (wc_db_update_move.c) within svn_wc__db_with_txn(). - Julian > I think the standard svn_wc_merge() function should handle this > for you in one step, so this function should do something similar. > > > It is not a problem to install a conflict skel and then to reinstall it later > with more details. But it would be a problem for the client to crash between > updating the state and installing the conflict. The sqlite transactions > should > catch that.
Re: svn commit: r1417639 - in /subversion/trunk/subversion/mod_dav_svn: dav_svn.h mod_dav_svn.c reports/update.c
On 12/07/2012 02:39 AM, Lieven Govaerts wrote: >> AFAIK, it is only dump where the svnrdump tool fails when using Serf. >> Because of the Ev1 stuff. Using bulk-updates ought to avoid that >> issue. > > Actually, my proposed hack of forcing ra_serf to use bulk update mode > is not going to work. If the server has SVNAllowBulkUpdates set to > Off, bulk update mode isn't available, the server will ignore any > requests from the client and force skelta mode. > > If I remember correctly the issue here was responses arriving out of > order due to multiple parallel connections. So the fix until svnrdump > has moved to Ev2 is to stick to one extra connection for all the file > content requests, so all of those responses arrive in sync. > Don't know how easy that is to do, I wonder if we have a simple > mechanism that allows a client to pass ra implementation specific > options. IIUC, the problem with svnrdump occurs only for the single revision at the start of a non-incremental dump. svnrdump runs svn_ra_do_update2() in such a way as to pretend that it's doing a checkout. After that one revision, it runs a loop around svn_ra_replay_range(). Perhaps there's an opportunity here to solve two problems at once. What if we revved the svn_ra_replay_range() API in such a way that it could now handle this "initial revision" scenario? We might add an 'incremental' flag that parallels what 'svnadmin dump' and 'svnrdump dump' do. This would get us to a single RA API used for revision replays (replay_range, instead of a combination of do_update + replay_range), which simplifies how replays are done from an API perspective. Of course, under the hood, the RA implementations of the new replay_range2() would probably just use the same update mechanics to handle that initial revision when not in incremental mode, but ra_serf's particular implementation would be free to choose "send-all" mode in this one scenario to do exactly that. -- C. Michael Pilato CollabNet <> www.collab.net <> Enterprise Cloud Development signature.asc Description: OpenPGP digital signature
RE: svn commit: r1419560 - /subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c
> -Original Message- > From: julianf...@apache.org [mailto:julianf...@apache.org] > Sent: maandag 10 december 2012 17:29 > To: comm...@subversion.apache.org > Subject: svn commit: r1419560 - > /subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c > > Author: julianfoad > Date: Mon Dec 10 16:28:34 2012 > New Revision: 1419560 > > URL: http://svn.apache.org/viewvc?rev=1419560&view=rev > Log: > * subversion/libsvn_wc/wc_db_update_move.c > (update_working_file): Update a comment. > > Modified: > subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c > > Modified: subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_ > db_update_move.c?rev=1419560&r1=1419559&r2=1419560&view=diff > == > > --- subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c > (original) > +++ subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c Mon > Dec 10 16:28:34 2012 > @@ -247,8 +247,8 @@ update_working_file(svn_skel_t **work_it >old_version->props, old_version->props, >actual_props, propchanges, >scratch_pool, scratch_pool)); > - /* ### TODO: Make a WQ item in WORK_ITEMS to set new_actual_props > ... */ > - /* ### Not a direct DB op like this... */ > + /* Install the new actual props. Don't set the conflict_skel yet, because > + we might need to add a text conflict to it as well. */ >SVN_ERR(svn_wc__db_op_set_props(db, local_abspath, >new_actual_props, >svn_wc__has_magic_property(propchanges), This tells me that the change is not atomic, so this really needs some fix-me comment after all. The text and properties should be modified in one sqlite operation. (Or in other words: the db should be updated to its final state, with the installing of the wq items in a single transaction) I think the standard svn_wc_merge() function should handle this for you in one step, so this function should do something similar. It is not a problem to install a conflict skel and then to reinstall it later with more details. But it would be a problem for the client to crash between updating the state and installing the conflict. The sqlite transactions should catch that. Bert
Re: Buildbot screen per branch?
Filed a ticket:
https://issues.apache.org/jira/browse/INFRA-5623
Daniel Shahaf wrote on Wed, Nov 21, 2012 at 11:42:26 +0200:
> Currently http://subversion.apache.org/buildbot contains a mixture of
> builds for all active branches (trunk, 1.6.x, 1.7.x). That makes it
> unfeasible to track the status of, say, the 1.6.x branch (which had
> a failure tonight - but that will be washed out soon by trunk commits
> and 1.7.x backport merges).
>
> We could split up each builder ("column" in
> http://subversion.apache.org/buildbot) into N builders, one per branch
> --- that adds no maintenance overhead (according to Gavin), and will
> allow us to easily monitor the status (and history) of each branch
> by adding a http://subversion.apache.org/buildbot-17x redirect that
> shows only the 1.7.x builders (at least N recent builds of each).
>
> WDYT?
>
> Daniel
>
> (Implementation-wise --- see mkcassbuilder() in cassandra.conf for an
> example.)
Re: [PATCH] Implement svnadmin verify --keep-going
Julian Foad wrote on Mon, Dec 10, 2012 at 14:45:57 +: > I scanned quickly through your patch and I noticed one place where you > declare a local function without the 'static' keyword. I expect this should > give a warning when you compile it, so please will you compile with and > without your patch applied and check for any additional compiler warnings > that your patch adds. Or you can cheat by using compiler flags that build trunk by default with no warnings: './configure' '--enable-maintainer-mode' '-C' 'CFLAGS= -DSVN_DEPRECATED= -Wformat=0 -Wno-unreachable-code -g' 'CC=x86_64-linux-gnu-gcc' "$@"
Re: [PATCH] Implement svnadmin verify --keep-going
> Index: subversion/tests/cmdline/svnadmin_tests.py
> ===
> --- subversion/tests/cmdline/svnadmin_tests.py(revision 1411074)
> +++ subversion/tests/cmdline/svnadmin_tests.py(working copy)
> @@ -1835,6 +1835,114 @@
>svntest.main.run_svnadmin("recover", sbox.repo_dir)
>
>
> +def verify_keep_going(sbox):
> + "svnadmin verify --keep-going test"
> + test_create(sbox)
> + dumpfile_skeleton = open(os.path.join(os.path.dirname(sys.argv[0]),
> +
> 'svnadmin_tests_data',
> +
> 'skeleton_repos.dump')).read()
> + load_dumpstream(sbox, dumpfile_skeleton, '--ignore-uuid')
> +
> + r2 = fsfs_file(sbox.repo_dir, 'revs', '6')
> + fp = open(r2, 'wb')
> + fp.write("""id: 3-6.0.r6/0
This test will fail when building with
-DSVN_FS_FS_DEFAULT_MAX_FILES_PER_DIR=3 -DPACK_AFTER_EVERY_COMMIT
. Can it recognise the situation and Skip?
(It's fine if it can't: svnadmin_tests 17 has always FAILed with those flags.)
> +""")
> + fp.close()
> + exit_code, output, errput = svntest.main.run_svnadmin("verify",
> +sbox.repo_dir)
> + exit_code, output, errput2 = svntest.main.run_svnadmin("verify",
> + "--keep-going",
> + sbox.repo_dir)
> +
> + if svntest.verify.verify_outputs("Unexpected error while running 'svnadmin
> verify'.",
> + [], errput, None, ".*svnadmin: E165005:
> .*"):
> +raise svntest.Failure
> +
> + if svntest.verify.verify_outputs("Unexpected error while running 'svnadmin
> verify'.",
> + [], errput2, None,
> + ["* Verified revision 0.\n",
> + "* Verified revision 1.\n",
> + "* Verified revision 2.\n",
> + "* Verified revision 3.\n",
Please avoid testing the literal output. It means every single change
to the progress reporting or error reporting will need the test to be
updated.
> + "* Verified revision 4.\n",
> + "* Verified revision 5.\n",
> + "* Error verifying revision 6.\n",
> + "svnadmin: E24: Could not convert ''
> into a number\n",
It might be useful to add a comment here explaining the error --- it's
because the last line in the revision file is blank. Alternatively, you
could make that line contain a sentinel string and then check that the
sentinel appears in the error message; that would be self-documenting.
> + "svnadmin: E165005: Repository
> 'svn-test-work/repositories/svnadmin_tests-31' failed to verify\n"]):
> + raise svntest.Failure
> +
> +{"keep-going",svnadmin__keep_going, 0,
> + N_("continue verifying even if there is a corruption")},
s/even if there is/after detecting/
?
> @@ -744,6 +749,21 @@
> notify->warning_str));
>return;
>
> +case svn_repos_notify_failure:
> + if (notify->revision != SVN_INVALID_REVNUM)
> +svn_error_clear(svn_stream_printf(feedback_stream, scratch_pool,
> + ("* Error verifying revision
> %ld.\n"),
> + notify->revision));
> +/*svn_error_clear(svn_stream_printf(feedback_stream, scratch_pool,
> + _("svnadmin: E%d: Error verifying
> revision %ld\n"),
> + notify->err->apr_err,
> + notify->revision));
> +*/
This debris doesn't belong in the patch. :)
>/** For #svn_repos_notify_load_node_start, the path of the node. */
>const char *path;
>
> + /** For #svn_repos_notify_failure, this error chain indicates what
> + went wrong during verification. */
Add @since please to the docstring.
> + svn_error_t *err;
> +
> Index: subversion/libsvn_repos/dump.c
> ===
> --- subversion/libsvn_repos/dump.c(revision 1411074)
> +++ subversion/libsvn_repos/dump.c(working copy)
> @@ -1359,10 +1359,28 @@
>return close_directory(dir_baton, pool);
> }
>
> +void
static void
> +notify_verification_error(svn_revnum_t rev,
> + svn_error_t *err,
> + svn_repos_notify_func_t notify_func,
> + void *notify_baton,
> + apr_pool_t *pool)
> +{
> + if (notify_func)
> +{
> + svn_repos_notify_t *notify_failure;
> + notify_failure = svn_repos_notify_create(
Re: [PATCH] Implement svnadmin verify --keep-going
Prabhu Gnana Sundar > This patch is a follow up of the long discussion we had in thread [1]. This > patch implements a new switch "--keep-going" to svnadmin verify. > > If "--keep-going" is set(True), svnadmin verify would continue to run > till verifying all the revisions i.e, it would not stop at the very first > occurance of a corruption/error found in the repo and would report > corruptions > wherever found. > > > I have worked on your suggestions and inputs for this patch. Kindly share > your > thoughts. Attaching the patch and the log message with this mail. Thank you for the patch. Please will you summarize the issues that were raised in the previous discussion and how you have chosen to proceed. I'm thinking in particular of the discussion about how the output is notified -- how to display each error, on what output stream, and what to print at the end of the whole verification and what exit code to return. There may have been other issues too. I scanned quickly through your patch and I noticed one place where you declare a local function without the 'static' keyword. I expect this should give a warning when you compile it, so please will you compile with and without your patch applied and check for any additional compiler warnings that your patch adds. - Julian
RE: enforcing LF-normalization for svn:eol-style=native files (issue #4065)
> -Original Message- > From: Branko Čibej [mailto:br...@wandisco.com] > Sent: maandag 10 december 2012 8:04 > To: dev@subversion.apache.org > Subject: Re: enforcing LF-normalization for svn:eol-style=native files (issue > #4065) > > On 10.12.2012 07:35, Bert Huijben wrote: > > I don’t think you have to wait until commit time: You could verify the > > commit base revision’s properties + changes. In the cases where the > > properties change before commit, the commit would fail for being out of > > date. > > That would imply you can't change properties and contents in the same > commit, wouldn't it. Which I suspect is not the case? :) > > The problem is worse, you can theoretically reopen and modify a > transaction any number of times before committing it, so you don't > really know until txn commit time which properties actually apply to > which file contents. On the fs layer this is probably true, but in a normal editor drive through the ra layers this should never happen. On the ra layer the final set of properties is valid in the file_close() callback. I'm just not sure if this is really an event that the server sees for ra_dav, or if this is implied by the committing. Bert
Re: enforcing LF-normalization for svn:eol-style=native files (issue #4065)
On Mon, Dec 10, 2012 at 12:47 PM, Daniel Shahaf wrote: > Branko Čibej wrote on Mon, Dec 10, 2012 at 11:51:08 +0100: >> On 10.12.2012 11:31, Daniel Shahaf wrote: >> > Branko Čibej wrote on Mon, Dec 10, 2012 at 00:26:20 +0100: >> >> On 10.12.2012 00:08, Johan Corveleyn wrote: >> >>> On Sun, Dec 9, 2012 at 11:43 PM, Daniel Shahaf >> >>> wrote: >> Johan Corveleyn wrote on Sun, Dec 09, 2012 at 21:15:24 +0100: >> > 2) Am I the only one who wants to protect his repository against this >> > corruption? Judging from [1], I don't think so. It doesn't make sense >> > that everyone starts writing this pre-commit hook, for something that >> > IMHO is an obvious anti-corruption protection. I think every >> > repository out there deserves to be protected against this. >> > >> FWIW, I suggested a hook because you could implement that in about >> 5 minutes, whereas doing a C code change would take at least 10 times >> that (and several weeks if you have to wait for it to appear in a 1.7.x >> release that you can install at $WORK). I won't object to C code >> verifying the svn:eol-style invariant. >> >>> Thanks. And your pre-commit hook example is much appreciated. >> >>> >> >>> For the moment I get the impression that it's not really doable / >> >>> desirable to implement this in the repository. At least until now >> >>> no-one has suggested how it could be done, and I don't know enough >> >>> myself about the server-side / back-end to figure it out :-). >> >> The first obstacle is that the server does not interpret properties.[1] >> >> Therefore, you'd have to implement this check at transaction-commit >> >> time, because there's no earlier point where you're guaranteed to have >> >> all node properties at their final values. This implies that the time to >> >> reject a commit would be proportional to the size of the commit (which >> >> isn't the case when it comes to conflict detection). >> > Can't you do what stefan2 suggested, but in libsvn_repos? The repos >> > layer commit editor would remember for each file whether its textdelta >> > has added a new 0x0D byte, then at close_edit() it would iterate all >> > files in the commit --- and for each file only compare its svn:eol-style >> > property to the by-now-precomputed "did it it contain a 0x0D" bit. >> > >> > I'm not sure how efficient it would be to parse for 0x0D's in >> > libsvn_repos, though. Maybe we should make this optional. >> >> >> It's not enough to just look for \r in the delta stream. Certainly >> wouldn't help with historically broken files. > > I wasn't trying to fix historically broken files. (That would require > a fulltext scan --- that's not a cheap computation.) Do you see another > problem? Indeed, historically broken files are not the focus here. They're broken in the history of the repository anyway, so that's not really fixable anymore. But if we can prevent new broken files from entering the repository, that would be great. >> Maybe that's a good idea, but /why/ put it in libsvn_repos when it's >> much easier and cleaner to just provide some standard hooks, written in >> C and distributed with releases, that the admin plug in if she feels >> like it? Surely that's what hooks are for. > > The argument is that a Subversion server should be enforcing > Subversion's invariants. > > That said, I'm not opposed to doing it via standard hooks. It's a good > way to introduce the feature in a way that allows more easily changing > it before it hits the APIs and their strict compatibility rules. Yes, that would be fine I think. I like Ivan's suggestion of creating a new standard 'svnhooks' program or something like that, which can then be part of standard distributions. -- Johan
[PATCH] Implement svnadmin verify --keep-going
Hi,
This patch is a follow up of the long discussion we had in thread [1].
This patch implements a new switch "--keep-going" to svnadmin verify.
If "--keep-going" is set(True), svnadmin verify would continue to run
till verifying all the revisions i.e, it would not stop at the very
first occurance of a corruption/error found in the repo and would report
corruptions wherever found.
I have worked on your suggestions and inputs for this patch. Kindly
share your thoughts. Attaching the patch and the log message with this mail.
Thanks and regards
Prabhu
[1] http://svn.haxx.se/dev/archive-2012-10/0271.shtml
Implement svnadmin verify --keep-going, which would continue the verification
even if there is some corruption, after printing the errors to stderr.
* subversion/svnadmin/svnadmin.c
(svnadmin__cmdline_options_t): Add keep-going option.
(svnadmin_opt_state): Add keep-going option.
(subcommand_verify): Switch to the new svn_repos_verify_fs3 function instead
of svn_repos_verify_fs2, and pass the keep-going option.
(repos_notify_handler): Handle svn_repos_notify_failure notification by
printing warnings to stderr with the respective revision number.
* subversion/include/svn_repos.h
(svn_repos_notify_action_t): Add svn_repos_notify_failure to notify failure.
(svn_repos_verify_fs3): Newly added to handle "keep-going" option.
(svn_repos_notify_t): Add "err", the error chain which indicates what went
wrong during verification.
* subversion/libsvn_repos/dump.c
(svn_repos_verify_fs3): Handle "keep-going". If "keep-going" is TRUE, the
error message is notified and verification process continues.
When a repository fails to verify, return an error SVN_ERR_REPOS_CORRUPTED.
(notify_verification_error): New function to notify the verification
failure error message.
* subversion/libsvn_repos/deprecated.c
(svn_repos_verify_fs2): Deprecate. Call svn_repos_verify_fs3 with
keep_going as FALSE by default to keep the old default implementation.
* subversion/libsvn_repos/notify.c
(svn_repos_notify_create): Initialise the error chain "err" to SVN_NO_ERROR.
* subversion/tests/cmdline/svnadmin_tests.py
(verify_keep_going): New test case to test svnadmin verify and the new
switch --keep-going.
Patch by: Prabhu Gnana Sundar
Index: subversion/tests/cmdline/svnadmin_tests.py
===
--- subversion/tests/cmdline/svnadmin_tests.py (revision 1411074)
+++ subversion/tests/cmdline/svnadmin_tests.py (working copy)
@@ -1835,6 +1835,114 @@
svntest.main.run_svnadmin("recover", sbox.repo_dir)
+def verify_keep_going(sbox):
+ "svnadmin verify --keep-going test"
+ test_create(sbox)
+ dumpfile_skeleton = open(os.path.join(os.path.dirname(sys.argv[0]),
+'svnadmin_tests_data',
+'skeleton_repos.dump')).read()
+ load_dumpstream(sbox, dumpfile_skeleton, '--ignore-uuid')
+
+ r2 = fsfs_file(sbox.repo_dir, 'revs', '6')
+ fp = open(r2, 'wb')
+ fp.write("""id: 3-6.0.r6/0
+type: file
+count: 0
+text: 2 0 60 48 02d086e41b03058c5f1af6282c1f483f cc67e4dd7cd8ca83095c8b95f65b6698b39cb263 5-5/_5
+props: 2 73 34 0 25e6c2f7558b7484000d4d090dea5b92
+cpath: /Projects/docs/README
+copyroot: 0 /
+
+PLAIN
+K 6
+README
+V 15
+file 3-6.0.r6/0
+END
+ENDREP
+id: 1-6.0.r6/275
+type: dir
+count: 0
+text: 6 226 36 0 2c3f6410944c6ff8667fa1b3e78f45a2
+cpath: /Projects/docs
+copyroot: 0 /
+
+PLAIN
+K 9
+Project-X
+V 14
+dir 1-3.0.r3/0
+K 9
+Project-Y
+V 14
+dir 1-4.0.r4/0
+K 9
+Project-Z
+V 14
+dir 1-5.0.r5/0
+K 4
+docs
+V 16
+dir 1-6.0.r6/275
+END
+ENDREP
+id: 0-1.0.r6/548
+type: dir
+pred: 0-1.0.r5/195
+count: 4
+text: 6 398 137 0 c758f548da93cc57d68af0610766b549
+cpath: /Projects
+copyroot: 0 /
+
+PLAIN
+K 8
+Projects
+V 16
+dir 0-1.0.r6/548
+K 6
+README
+V 17
+file 0-2.0.r2/120
+END
+ENDREP
+id: 0.0.r6/772
+type: dir
+pred: 0.0.r5/418
+count: 6
+text: 6 686 73 0 353c6bbf43b0f2ae474d85e206337bbd
+cpath: /
+copyroot: 0 /
+
+_1.0.t5-5 add-dir false false /Projects/docs
+
+_3.0.t5-5 add-file true true /Projects/docs/README
+
+
+""")
+ fp.close()
+ exit_code, output, errput = svntest.main.run_svnadmin("verify",
+sbox.repo_dir)
+ exit_code, output, errput2 = svntest.main.run_svnadmin("verify",
+ "--keep-going",
+ sbox.repo_dir)
+
+ if svntest.verify.verify_outputs("Unexpected error while running 'svnadmin verify'.",
+ [], errput, None, ".*svnadmin: E165005: .*"):
+raise svntest.Failure
+
+ if svntest.verify.verify_outputs("Unexpected error while running 'svnadmin verify'.",
+ [], errput2, None,
+ ["* Verified revision 0.\n",
+ "* Ve
Re: enforcing LF-normalization for svn:eol-style=native files (issue #4065)
Branko Čibej wrote on Mon, Dec 10, 2012 at 11:51:08 +0100: > On 10.12.2012 11:31, Daniel Shahaf wrote: > > Branko Čibej wrote on Mon, Dec 10, 2012 at 00:26:20 +0100: > >> On 10.12.2012 00:08, Johan Corveleyn wrote: > >>> On Sun, Dec 9, 2012 at 11:43 PM, Daniel Shahaf > >>> wrote: > Johan Corveleyn wrote on Sun, Dec 09, 2012 at 21:15:24 +0100: > > 2) Am I the only one who wants to protect his repository against this > > corruption? Judging from [1], I don't think so. It doesn't make sense > > that everyone starts writing this pre-commit hook, for something that > > IMHO is an obvious anti-corruption protection. I think every > > repository out there deserves to be protected against this. > > > FWIW, I suggested a hook because you could implement that in about > 5 minutes, whereas doing a C code change would take at least 10 times > that (and several weeks if you have to wait for it to appear in a 1.7.x > release that you can install at $WORK). I won't object to C code > verifying the svn:eol-style invariant. > >>> Thanks. And your pre-commit hook example is much appreciated. > >>> > >>> For the moment I get the impression that it's not really doable / > >>> desirable to implement this in the repository. At least until now > >>> no-one has suggested how it could be done, and I don't know enough > >>> myself about the server-side / back-end to figure it out :-). > >> The first obstacle is that the server does not interpret properties.[1] > >> Therefore, you'd have to implement this check at transaction-commit > >> time, because there's no earlier point where you're guaranteed to have > >> all node properties at their final values. This implies that the time to > >> reject a commit would be proportional to the size of the commit (which > >> isn't the case when it comes to conflict detection). > > Can't you do what stefan2 suggested, but in libsvn_repos? The repos > > layer commit editor would remember for each file whether its textdelta > > has added a new 0x0D byte, then at close_edit() it would iterate all > > files in the commit --- and for each file only compare its svn:eol-style > > property to the by-now-precomputed "did it it contain a 0x0D" bit. > > > > I'm not sure how efficient it would be to parse for 0x0D's in > > libsvn_repos, though. Maybe we should make this optional. > > > It's not enough to just look for \r in the delta stream. Certainly > wouldn't help with historically broken files. I wasn't trying to fix historically broken files. (That would require a fulltext scan --- that's not a cheap computation.) Do you see another problem? > Maybe that's a good idea, but /why/ put it in libsvn_repos when it's > much easier and cleaner to just provide some standard hooks, written in > C and distributed with releases, that the admin plug in if she feels > like it? Surely that's what hooks are for. The argument is that a Subversion server should be enforcing Subversion's invariants. That said, I'm not opposed to doing it via standard hooks. It's a good way to introduce the feature in a way that allows more easily changing it before it hits the APIs and their strict compatibility rules. > > > -- Brane > > > -- > Branko Čibej > Director of Subversion | WANdisco | www.wandisco.com >
Re: enforcing LF-normalization for svn:eol-style=native files (issue #4065)
On Mon, Dec 10, 2012 at 2:51 PM, Branko Čibej wrote: > On 10.12.2012 11:31, Daniel Shahaf wrote: >> Branko Čibej wrote on Mon, Dec 10, 2012 at 00:26:20 +0100: >>> On 10.12.2012 00:08, Johan Corveleyn wrote: On Sun, Dec 9, 2012 at 11:43 PM, Daniel Shahaf wrote: > Johan Corveleyn wrote on Sun, Dec 09, 2012 at 21:15:24 +0100: >> 2) Am I the only one who wants to protect his repository against this >> corruption? Judging from [1], I don't think so. It doesn't make sense >> that everyone starts writing this pre-commit hook, for something that >> IMHO is an obvious anti-corruption protection. I think every >> repository out there deserves to be protected against this. >> > FWIW, I suggested a hook because you could implement that in about > 5 minutes, whereas doing a C code change would take at least 10 times > that (and several weeks if you have to wait for it to appear in a 1.7.x > release that you can install at $WORK). I won't object to C code > verifying the svn:eol-style invariant. Thanks. And your pre-commit hook example is much appreciated. For the moment I get the impression that it's not really doable / desirable to implement this in the repository. At least until now no-one has suggested how it could be done, and I don't know enough myself about the server-side / back-end to figure it out :-). >>> The first obstacle is that the server does not interpret properties.[1] >>> Therefore, you'd have to implement this check at transaction-commit >>> time, because there's no earlier point where you're guaranteed to have >>> all node properties at their final values. This implies that the time to >>> reject a commit would be proportional to the size of the commit (which >>> isn't the case when it comes to conflict detection). >> Can't you do what stefan2 suggested, but in libsvn_repos? The repos >> layer commit editor would remember for each file whether its textdelta >> has added a new 0x0D byte, then at close_edit() it would iterate all >> files in the commit --- and for each file only compare its svn:eol-style >> property to the by-now-precomputed "did it it contain a 0x0D" bit. >> >> I'm not sure how efficient it would be to parse for 0x0D's in >> libsvn_repos, though. Maybe we should make this optional. > > > It's not enough to just look for \r in the delta stream. Certainly > wouldn't help with historically broken files. > > Moreover, if libsvn_repos started looking at svn:eol-style, that'd only > make sense if you verified all normalizations, not just "native". And > why should it just be svn:eol-style, when svn:keywords has potentially > the same problem? Eventually you start verifying everything that affects > file contents. > > Maybe that's a good idea, but /why/ put it in libsvn_repos when it's > much easier and cleaner to just provide some standard hooks, written in > C and distributed with releases, that the admin plug in if she feels > like it? Surely that's what hooks are for. > > New program 'svnhooks' with set of hooks for standard recommended polices would be great. svn:eol-style check is good start. -- Ivan Zhakov
Re: enforcing LF-normalization for svn:eol-style=native files (issue #4065)
On 10.12.2012 11:31, Daniel Shahaf wrote: > Branko Čibej wrote on Mon, Dec 10, 2012 at 00:26:20 +0100: >> On 10.12.2012 00:08, Johan Corveleyn wrote: >>> On Sun, Dec 9, 2012 at 11:43 PM, Daniel Shahaf >>> wrote: Johan Corveleyn wrote on Sun, Dec 09, 2012 at 21:15:24 +0100: > 2) Am I the only one who wants to protect his repository against this > corruption? Judging from [1], I don't think so. It doesn't make sense > that everyone starts writing this pre-commit hook, for something that > IMHO is an obvious anti-corruption protection. I think every > repository out there deserves to be protected against this. > FWIW, I suggested a hook because you could implement that in about 5 minutes, whereas doing a C code change would take at least 10 times that (and several weeks if you have to wait for it to appear in a 1.7.x release that you can install at $WORK). I won't object to C code verifying the svn:eol-style invariant. >>> Thanks. And your pre-commit hook example is much appreciated. >>> >>> For the moment I get the impression that it's not really doable / >>> desirable to implement this in the repository. At least until now >>> no-one has suggested how it could be done, and I don't know enough >>> myself about the server-side / back-end to figure it out :-). >> The first obstacle is that the server does not interpret properties.[1] >> Therefore, you'd have to implement this check at transaction-commit >> time, because there's no earlier point where you're guaranteed to have >> all node properties at their final values. This implies that the time to >> reject a commit would be proportional to the size of the commit (which >> isn't the case when it comes to conflict detection). > Can't you do what stefan2 suggested, but in libsvn_repos? The repos > layer commit editor would remember for each file whether its textdelta > has added a new 0x0D byte, then at close_edit() it would iterate all > files in the commit --- and for each file only compare its svn:eol-style > property to the by-now-precomputed "did it it contain a 0x0D" bit. > > I'm not sure how efficient it would be to parse for 0x0D's in > libsvn_repos, though. Maybe we should make this optional. It's not enough to just look for \r in the delta stream. Certainly wouldn't help with historically broken files. Moreover, if libsvn_repos started looking at svn:eol-style, that'd only make sense if you verified all normalizations, not just "native". And why should it just be svn:eol-style, when svn:keywords has potentially the same problem? Eventually you start verifying everything that affects file contents. Maybe that's a good idea, but /why/ put it in libsvn_repos when it's much easier and cleaner to just provide some standard hooks, written in C and distributed with releases, that the admin plug in if she feels like it? Surely that's what hooks are for. -- Brane -- Branko Čibej Director of Subversion | WANdisco | www.wandisco.com
Re: enforcing LF-normalization for svn:eol-style=native files (issue #4065)
Branko Čibej wrote on Mon, Dec 10, 2012 at 00:26:20 +0100: > On 10.12.2012 00:08, Johan Corveleyn wrote: > > On Sun, Dec 9, 2012 at 11:43 PM, Daniel Shahaf > > wrote: > >> Johan Corveleyn wrote on Sun, Dec 09, 2012 at 21:15:24 +0100: > >>> 2) Am I the only one who wants to protect his repository against this > >>> corruption? Judging from [1], I don't think so. It doesn't make sense > >>> that everyone starts writing this pre-commit hook, for something that > >>> IMHO is an obvious anti-corruption protection. I think every > >>> repository out there deserves to be protected against this. > >>> > >> FWIW, I suggested a hook because you could implement that in about > >> 5 minutes, whereas doing a C code change would take at least 10 times > >> that (and several weeks if you have to wait for it to appear in a 1.7.x > >> release that you can install at $WORK). I won't object to C code > >> verifying the svn:eol-style invariant. > > Thanks. And your pre-commit hook example is much appreciated. > > > > For the moment I get the impression that it's not really doable / > > desirable to implement this in the repository. At least until now > > no-one has suggested how it could be done, and I don't know enough > > myself about the server-side / back-end to figure it out :-). > > The first obstacle is that the server does not interpret properties.[1] > Therefore, you'd have to implement this check at transaction-commit > time, because there's no earlier point where you're guaranteed to have > all node properties at their final values. This implies that the time to > reject a commit would be proportional to the size of the commit (which > isn't the case when it comes to conflict detection). Can't you do what stefan2 suggested, but in libsvn_repos? The repos layer commit editor would remember for each file whether its textdelta has added a new 0x0D byte, then at close_edit() it would iterate all files in the commit --- and for each file only compare its svn:eol-style property to the by-now-precomputed "did it it contain a 0x0D" bit. I'm not sure how efficient it would be to parse for 0x0D's in libsvn_repos, though. Maybe we should make this optional. > > All properties are interpreted by clients. A buggy client will cause > cause problems for other users, so the best course of action is to > report the bug (to SvnKit in this case?). > > Also note that you're using a much too strong term when you talk about > "corrupted files" in this case. There's nothing corrupted about them. > An invariant failed to maintain. Granted it's not an FS_level invariant, though. > > -- Brane > > [1] Not strictly true since mod_dav_svn will look at svn:mime-type when > serving the content at its default, unversioned URL; but it doesn't > actually interpret the value. > > -- > Branko Čibej > Director of Subversion | WANdisco | www.wandisco.com >
Re: svn commit: r1418830 - in /subversion/trunk/subversion/bindings/swig: core.i python/tests/checksum.py python/tests/run_all.py
On Sun, Dec 09, 2012 at 08:05:52AM -, bre...@apache.org wrote: > +class ChecksumTestCases(unittest.TestCase): > +def test_checksum(self): > +# Checking primarily the return type for the svn_checksum_create > +# function > +val = svn.core.svn_checksum_create(svn.core.svn_checksum_md5) > +check_val = svn.core.svn_checksum_to_cstring_display(val) > + > +# The svn_checksum_to_cstring_display should return a str type object > +# from the check_val object passed to it > +if(type(check_val) == str): > +# The intialized value created from a checksum should be 0 Typo in comment. > +if(int(check_val) != 0): It would be better to write: if check_val == '0'*32 (except that the test shouldn't hardcode "32") This will catch a digest of the wrong length, and will avoid doing type equality checking (inheritance checking is preferred). > +self.assertRaises(AssertionError) This line does not cause the test to fail. It returns a context manager --- the language construct implementing the 'with' statement. > +else: > +self.assertRaises(TypeError, test_checksum) Infinite recursion.
