Re: svn commit: r1827100 - /subversion/branches/1.10.x/STATUS

2018-03-17 Thread Branko Čibej
On 17.03.2018 20:24, br...@apache.org wrote:
>  
> + * r1825979
> +   Minor clarification to docstring.
> +   Justification:
> + Trivial documentation fix.
> +   Votes:
> + +1: danielsh, jamessan
> + -1: brane (the docstring "clarification" is wrong, since
> +we do not propagate the compression level to LZ4)


It seems I failed to this change on the list when it was originally made
on trunk. I believe r1825979 should be reverted because the change is
not correct.

-- Brane



Re: svnadmin load error with 1.10 that seems to be fsfs cache related

2018-03-17 Thread Branko Čibej
On 17.03.2018 13:57, Philip Martin wrote:
> Julian Foad  writes:
>
>> But does that mean this code isn't being tested by any of our manual
>> or buildbot testing in the last 6 months?
> Yes, I think so.
>
> For svnserve the code only gets enabled when svnserve has block-read
> enabled and svnserveautocheck doesn't do that.  By invoking svnserve
> manually it is possible to enable it and lots of tests fail with the
> zlib error:
>
> subversion/svnserve/svnserve -Tdr subversion/tests/cmdline --block-read 1
> make check BASE_URL=svn://localhost
>
> For http the code only gets enabled when mod_dav_svn has block read
> enabled and davautocheck doesn't do that.  Patching davautocheck.sh
>
> Index: ../src/subversion/tests/cmdline/davautocheck.sh
> ===
> --- ../src/subversion/tests/cmdline/davautocheck.sh   (revision 1827068)
> +++ ../src/subversion/tests/cmdline/davautocheck.sh   (working copy)
> @@ -548,6 +548,7 @@
>Require   valid-user
>SVNAdvertiseV2Protocol ${ADVERTISE_V2_PROTOCOL}
>SVNCacheRevProps  ${CACHE_REVPROPS_SETTING}
> +  SVNBlockRead on
>${SVN_PATH_AUTHZ_LINE}
>  
>
> causes lots of tests to fail with the zlib error.
>
> There doesn't seen to be a way to enable the block-read code for svn
> file:// access but it could be enabled for svnadmin by making the cache
> greater than 64MB.  The regression tests use the default 16MB so
> patching main.py:
>
> Index: ../src/subversion/tests/cmdline/svntest/actions.py
> ===
> --- ../src/subversion/tests/cmdline/svntest/actions.py(revision 
> 1827068)
> +++ ../src/subversion/tests/cmdline/svntest/actions.py(working copy)
> @@ -378,7 +378,7 @@
>  
>  def run_and_verify_dump(repo_dir, deltas=False):
>"Runs 'svnadmin dump' and reports any errors, returning the dump content."
> -  args = ()
> +  args = ('-M65')
>if deltas:
>  args += ('--deltas',)
>exit_code, output, errput = run_and_verify_svnadmin(
>
> causes tests to fail with the zlib error.
>
>> Extending our testing is one option. I would much prefer to apply the
>> principle of testable design: "if this code only runs sometimes,
>> change it so it runs always". Can we do that?
> There are a lot of tuning options for caching.  Do we want to add
> mechanisms to enable them individually in the testsuite?  Do we want to
> change the defaults in our binaries?  Do we want to eliminate the tuning
> options?

As long as we have tuning options, we should test them. It's not that
hard to add buildslave configurations.

As for eliminating them: it would be nice if the server could auto-tune
these parameters, but that is probably not a worthwhile goal to spend
our efforts at (and that would make testing behaviour with different
options harder).

-- Brane



Re: svnadmin load error with 1.10 that seems to be fsfs cache related

2018-03-17 Thread Philip Martin
Julian Foad  writes:

> But does that mean this code isn't being tested by any of our manual
> or buildbot testing in the last 6 months?

Yes, I think so.

For svnserve the code only gets enabled when svnserve has block-read
enabled and svnserveautocheck doesn't do that.  By invoking svnserve
manually it is possible to enable it and lots of tests fail with the
zlib error:

subversion/svnserve/svnserve -Tdr subversion/tests/cmdline --block-read 1
make check BASE_URL=svn://localhost

For http the code only gets enabled when mod_dav_svn has block read
enabled and davautocheck doesn't do that.  Patching davautocheck.sh

Index: ../src/subversion/tests/cmdline/davautocheck.sh
===
--- ../src/subversion/tests/cmdline/davautocheck.sh (revision 1827068)
+++ ../src/subversion/tests/cmdline/davautocheck.sh (working copy)
@@ -548,6 +548,7 @@
   Require   valid-user
   SVNAdvertiseV2Protocol ${ADVERTISE_V2_PROTOCOL}
   SVNCacheRevProps  ${CACHE_REVPROPS_SETTING}
+  SVNBlockRead on
   ${SVN_PATH_AUTHZ_LINE}
 

causes lots of tests to fail with the zlib error.

There doesn't seen to be a way to enable the block-read code for svn
file:// access but it could be enabled for svnadmin by making the cache
greater than 64MB.  The regression tests use the default 16MB so
patching main.py:

Index: ../src/subversion/tests/cmdline/svntest/actions.py
===
--- ../src/subversion/tests/cmdline/svntest/actions.py  (revision 1827068)
+++ ../src/subversion/tests/cmdline/svntest/actions.py  (working copy)
@@ -378,7 +378,7 @@
 
 def run_and_verify_dump(repo_dir, deltas=False):
   "Runs 'svnadmin dump' and reports any errors, returning the dump content."
-  args = ()
+  args = ('-M65')
   if deltas:
 args += ('--deltas',)
   exit_code, output, errput = run_and_verify_svnadmin(

causes tests to fail with the zlib error.

> Extending our testing is one option. I would much prefer to apply the
> principle of testable design: "if this code only runs sometimes,
> change it so it runs always". Can we do that?

There are a lot of tuning options for caching.  Do we want to add
mechanisms to enable them individually in the testsuite?  Do we want to
change the defaults in our binaries?  Do we want to eliminate the tuning
options?

-- 
Philip


Re: [PATCH] Hackathon project: Dumping viewspec

2018-03-17 Thread Branko Čibej
On 16.03.2018 18:33, Julian Foad wrote:
> Johan Corveleyn wrote:
>> Julian Foad wrote:
>>> We need to look at using the viewspec as input next. Semantically
>>> speaking,
>>> these sorts of things:
>>>
>>> * a way to check out a new WC to match the spec;
>>> * a way to modify an existing WC to match the spec;
>>> * a way to modify/checkout a WC of a *different* branch, to match
>>> the spec
>>> except for its URLs (maybe switch URLs pointing inside this 'branch'
>>> or WC
>>> get adjusted as if they are relative, and other switch URLs stay
>>> absolute?);
>>
>> Ack.
>>
>> FWIW, the viewspec format of svn-viewspec.py [1] (which can serve as
>> inspiration I guess) requires the viewspec to have a "Url", and then a
>> series of path rules. The Url serves as a base url, the path rules are
>> relative to that base url.
>
> I implemented an output format compatible with 'svn-viewspec.py' in
> r1826990. Then I updated that output format to also support 'switched'
> and revisions, in r1826993. This version outputs a header declaring
> 'Format: 2', and svn-viewspec.py currently barfs on reading that.
>
> Suggested exercises for the reader (you, plural):
>
>   * implement a UI to choose the output format (currently 'svn info
> --viewspec' is hard-coded to produce that 'format 2')

Why? 'svn-viewspec.py' is not a supported tool; 'svn info --viewspec'
does not exist in 1.10. We don't have any compatibility guarantees to
worry about.

>   * update 'svn-viewspec.py' to implement those 'format 2' extensions
>
>   * start implementing the API functions that 'svn-viewspec.py' needs

Again, why? 'svn-viewspec.py' uses the standard command-line tools. It
follows that all API functions it needs already exist.

>   * expose those API functions in the Python bindings and convert
> 'svn-viewspec.py' to use them

Frankly, if all necessary functionality exists in the command-line
client, what's the benefit of requiring python bindings for
svn-viewspec.py? What _should_ be done is to stop using the system()
call to invoke the command-line and use the popen module instead (as we
do in our test suite).

-- Brane