Re: 1.8 new public API review (mostly) complete.

[Ben Reser]

On Mon, Apr 1, 2013 at 10:34 AM, Ben Reser  wrote:
> On Mon, Apr 1, 2013 at 9:27 AM, Julian Foad  
> wrote:
>> First we need to clarify what "@a path (a file,)" means.  From the 
>> discussion below I gather "a file" means an absolute local filesystem path, 
>> so let's say so.
>
> Yes absolute OS filesystem path. I prefer saying OS filesystem path
> because it may not actually be local if it's some sort of network
> mount, so I think local clouds the issue.  I'm not sure what we've
> been using in our API docs, but if we've been using local that way
> then that's fine.

I changed it to say "a dirent" which fits with our naming scheme for
types of paths.

>> That could work, but it is *much* nicer in my opinion to get rid of the 
>> relative-URL option and get rid of the 'repos_root' argument (which was 
>> Ben's original suggestion IIUC).  It's simple, cheap, and easy.
>
> Yes, that was my original suggestion.  I'll go ahead and do that.

Done along with the doc change mentioned above in r1463374.

Re: [svnbench] Revision: 1463069 compiled Apr 1 2013, 00:22:00 on x86_64-unknown-linux-gnu

[Daniel Shahaf]

Daniel Shahaf wrote on Tue, Apr 02, 2013 at 02:42:52 +0300:
> Neels Hofmeyr wrote on Tue, Apr 02, 2013 at 01:29:09 +0200:
> > Somewhere between r1454789 and r1457253 on trunk, that's three to
> > four weeks ago, 'commit', 'copy' and 'merge' got dramatically faster: as
> > much as 80%, and more! Amazing.
> 
> Ahem, you should probably correlate that with whenever our VM was moved
> to the shiny new VMs host.

Bingo.  The VM was moved on March 12 (see private@), that's after
r1454789 and before r1457253.

P.S.  Sorry for the triplicate sending.

Re: [svnbench] Revision: 1463069 compiled Apr 1 2013, 00:22:00 on x86_64-unknown-linux-gnu

[Daniel Shahaf]

Daniel Shahaf wrote on Tue, Apr 02, 2013 at 02:42:52 +0300:
> Neels Hofmeyr wrote on Tue, Apr 02, 2013 at 01:29:09 +0200:
> > Somewhere between r1454789 and r1457253 on trunk, that's three to
> > four weeks ago, 'commit', 'copy' and 'merge' got dramatically faster: as
> > much as 80%, and more! Amazing.
> 
> Ahem, you should probably correlate that with whenever our VM was moved
> to the shiny new VMs host.

Probably the easiest way to do this is by re-running the benchmark
against r1454789?

Re: [svnbench] Revision: 1463069 compiled Apr 1 2013, 00:22:00 on x86_64-unknown-linux-gnu

[Daniel Shahaf]

Neels Hofmeyr wrote on Tue, Apr 02, 2013 at 01:29:09 +0200:
> Somewhere between r1454789 and r1457253 on trunk, that's three to
> four weeks ago, 'commit', 'copy' and 'merge' got dramatically faster: as
> much as 80%, and more! Amazing.

Ahem, you should probably correlate that with whenever our VM was moved
to the shiny new VMs host.

Re: [PATCH] run_test.py appearance changes

[Daniel Shahaf]

Gabriela Gibson wrote on Mon, Apr 01, 2013 at 22:36:57 +0100:
> [[[
>
> Disable ANSI color for dumb terminals, fix logic bug where
> dimensions of (0,0) was treated as a positive result,
> re-factored _run_tests to remove repeated calculation of
> line_length.
>
> * build/run_tests.py
>   (_get_term_width): Add test condition.
>   (TestHarness): Add new variable.
>   (_run_test): Add new parameter, remove function call.
>
> ]]]
>
>

> Index: build/run_tests.py
> ===
> --- build/run_tests.py(revision 1463012)
> +++ build/run_tests.py(working copy)
> @@ -100,6 +100,11 @@ def _get_term_width():
>os.close(fd)
>  except:
>pass
> +  ## Some terminals (eg emacs *shell*, emacs *compilation*)
> +  ## seem to return (0,0) from ioctl_GWINSZ, so let's deal
> +  ## with that case...
> +  if cr == (0, 0):
> +cr = None

I'm not sure how to interpret a return value of (0,0) that's not
accompanied by an error flag (C errno!=0, or a Python exception).  Is
that normal behaviour, a bug we should be working around in our code, or
an indication of a bug in our logic in the preceding lines?

i.e., if (0,0) means "_get_term_width() was going 35mph in a school zone
at the time of the call to fcntl.ioctl()", we should fix that.  But if
(0,0) is just a terminal emulator bug, or expected behaviour, then +1 to
commit.

>if not cr:
>  try:
>cr = (os.environ['LINES'], os.environ['COLUMNS'])
> @@ -178,7 +183,8 @@ class TestHarness:
>  self.log = None
>  self.ssl_cert = ssl_cert
>  self.http_proxy = http_proxy
> -if not sys.stdout.isatty() or sys.platform == 'win32':
> +if not sys.stdout.isatty() or sys.platform == 'win32' or \
> +   os.getenv("TERM") == "dumb":
>TextColors.disable()
>  
>def run(self, list):
> @@ -186,8 +192,9 @@ class TestHarness:
> there is a log file. Return zero iff all test programs passed.'''
>  self._open_log('w')
>  failed = 0
> +line_length = _get_term_width()
>  for cnt, prog in enumerate(list):
> -  failed = self._run_test(prog, cnt, len(list)) or failed
> +  failed = self._run_test(prog, cnt, len(list), line_length) or failed
>  

This change will actually change the output if you resize the terminal
window while running tests interactively (at least in terminal emulators
that support the ioctl_GWINSZ approach).  I'm not objected to it, but
it's not clear to me what it gains either (shaves 100 syscalls from
a 'make check' run?).

>  if self.log is None:
>return failed
> @@ -550,7 +557,7 @@ class TestHarness:
>  
>  return failed
>  
> -  def _run_test(self, prog, test_nr, total_tests):
> +  def _run_test(self, prog, test_nr, total_tests, line_length):
>  "Run a single test. Return the test's exit code."
>  
>  if self.log:
> @@ -587,7 +594,6 @@ class TestHarness:
>  
>  progabs = os.path.abspath(os.path.join(self.srcdir, prog))
>  old_cwd = os.getcwd()
> -line_length = _get_term_width()
>  dots_needed = line_length \
>  - len(test_info) \
>  - len('success')

Re: [svnbench] Revision: 1463069 compiled Apr 1 2013, 00:22:00 on x86_64-unknown-linux-gnu

[Neels Hofmeyr]

On Mon, 01 Apr 2013 00:44:12 + ne...@apache.org wrote:
> 1.7.0@1181106 vs. trunk@1462907
> Started at Mon Apr  1 00:26:11 UTC 2013
> 
> *DISCLAIMER* - This tests only file://-URL access on a GNU/Linux VM.
> This is intended to measure changes in performance of the local
> working copy layer, *only*. These results are *not* generally true
> for everyone.
> 
> Charts of this data are available at
> http://svn-qavm.apache.org/charts/

It's time for another chart reading summary, as there has been a
dramatic change that is shown to be consistent for the past three
benchmark runs.

Whoever is reading this, keep in mind that all of these findings only
apply to a local non-network connection to the repository, using
file:// URLs only, and even then: Your Mileage May Vary. Thank you.

Somewhere between r1454789 and r1457253 on trunk, that's three to
four weeks ago, 'commit', 'copy' and 'merge' got dramatically faster: as
much as 80%, and more! Amazing.

Commit and copy got faster across the board, merge only in the 5x5
working copy scenario (5x5 means: 5 dir levels, with 5 files and 5 more
subdirs per dir -- an evenly spread, "realistic" working copy).

The most noticeable speed increase is seen for a commit in the 5x5
working copy: of 1.7.0's average 25.5 seconds of runtime, trunk has
lost around 21 seconds! ... That's twenty-one full seconds!

A merge in a 5x5 WC with trunk has now lost about seven of 1.7.0's ten
seconds of runtime. 70%!

Four weeks ago, trunk's copy in a 5x5 WC has actually been slightly
slower than 1.7.0. Now it's 90% faster than 1.7.0, dropping from a
quarter of a second in 1.7.0 to near instantaneous completion on trunk.

The benchmark's grand totals now suggest that trunk's libsvn_wc is --
drumrolls -- on average 62% faster than 1.7.0! Just four weeks ago that
was "a mere" 25%.

Above speed gains have been consistent for the past three benchmark
runs, i.e. the past three weeks.

BTW, if you noticed some red bars for the same three test runs with a
speed *loss* of up to 90%, relax: all of those account for far less
than 0.1 seconds; just take a look on the right-hand-side charts.

I hope you enjoyed this summary from your friendly neighborhood chart
interpreter. Good work lads!!1!1 (It was definitely not me, that's for
sure.)

Meep meep!
  -- Roadrunner

~Neels


signature.asc
Description: PGP signature

[PATCH] Adding tests for some functions in svn_checksum.h in SWIG bindings for python

[Shivani Poddar]

Log:
[[[ Followup to r1420334: Adding new tests.

* subversiom/bindings/swig/python/tests/checksum.py
Writing new tests to ensure robustness of svn_checksum_match(),
svn_checksum_dup() and svn_checksum_empty_checksum().

Patch by: Shivani Poddar  ]]]


Regards,
Shivani Poddar,
irc-nick - Pods







-- 
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 1448005)
+++ subversion/bindings/swig/python/tests/checksum.py   (working copy)
@@ -20,7 +20,7 @@
 #
 import unittest, setup_path
 import svn.core
-
+import random
 class ChecksumTestCases(unittest.TestCase):
 def test_checksum(self):
 # Checking primarily the return type for the svn_checksum_create
@@ -28,14 +28,56 @@
 kind, expected_length = svn.core.svn_checksum_md5, 128/8
 val = svn.core.svn_checksum_create(kind)
 check_val = svn.core.svn_checksum_to_cstring_display(val)
-
+rand_checksum = svn.core.svn_checksum_t()
+rand_checksum.kind = svn.core.svn_checksum_md5
+#Generating a random digest value, length of which is 16 bytes for 
checkum kind = md5 checksum.
+rand_checksum.digest  = ''.join(chr(random.randrange(256)) for x in 
xrange(16)).encode('base64')
+rand_checksum2 = svn.core.svn_checksum_t()
+rand_checksum2.kind = svn.core.svn_checksum_md5
+rand_checksum2.digest  = ''.join(chr(random.randrange(256)) for x in 
xrange(16)).encode('base64')
+#To check if both the random checksums are not identical
+while rand_checksum.digest == rand_checksum2.digest:
+rand_checksum2.digest  = ''.join(chr(random.randrange(256)) for x 
in xrange(16)).encode('base64')
+
+check_rand = svn.core.svn_checksum_to_cstring(rand_checksum)
+duplicate = svn.core.svn_checksum_dup(rand_checksum)
+duplicate_val = svn.core.svn_checksum_dup(val)
+check_rand2 = svn.core.svn_checksum_to_cstring(rand_checksum2)
+check_dup = svn.core.svn_checksum_to_cstring(duplicate)
 self.assertTrue(isinstance(check_val, str),
-  "Type of digest not string")
+"Type of digest not string")
 self.assertEqual(len(check_val), 2*expected_length,
- "Length of digest does not match kind")
+"Length of digest does not match kind")
 self.assertEqual(int(check_val), 0,
- "Value of initialized digest is not 0")
+"Value of initialized digest is not 0")
+#Checking the svn_checksum_dup() function's returned object's 
attributes
+self.assertEqual(type(rand_checksum),type(duplicate),
+"Duplicate checksum returned is not of the type 
svn_checksum_t*")
+self.assertEqual(rand_checksum.kind,duplicate.kind,
+"Duplicate returned is not of the same 
svn_checksum_kind_t as the parent checksum")
+self.assertEqual(check_rand,check_dup,
+"Duplicate returned does not have the same digest 
value as the parent checksum")
+#Checking svn_checksum_match()
+#Unequal checksums rand_checksum and rand_checksum2
+
self.assertEqual(str(svn.core.svn_checksum_match(rand_checksum2,rand_checksum)),'0',
+"Unequal checksums return match")
+#Duplicate non zero checksums
+
self.assertIs(str(svn.core.svn_checksum_match(rand_checksum,duplicate)),'1',
+"Equal Checksums return false for checksum match")
+#Border case to check if match returns a zero for all checksums
+self.assertIs(str(svn.core.svn_checksum_match(val,duplicate_val)),'1',
+"Zero checksums should return 1 in svn_checksum_match")
+#Checking for 1 zero and 1 non zero checksum
+self.assertIs(str(svn.core.svn_checksum_match(val,rand_checksum)),'1',
+"0 not returned for zero checksum argument in 
svn_checksum_match")
 
+#Testing for svn_checksum_is_empty_checksum()
+#Checking for an empty checksum
+self.assertEqual(str(svn.core.svn_checksum_is_empty_checksum(val)),'1',
+"False is returned for svn_checksum_is_empty_checksum 
for empty checksum")
+#Checking for a non-empty checksum
+
self.assertEqual(str(svn.core.svn_checksum_is_empty_checksum(rand_checksum)),'0',
+"True is returned for svn_checksum_is_empty_checksum 
for non-empty checksum")
 def suite():
 return unittest.defaultTestLoader.loadTestsFromTestCase(ChecksumTestCases)
 

Re: 1.8 new public API review (mostly) complete.

[Blair Zajac]

On 4/1/13 10:25 AM, C. Michael Pilato wrote:

On 03/31/2013 12:31 AM, Blair Zajac wrote:

Question on svn_repos_load_fs4() and svn_repos_get_fs_build_parser4() (maybe
other functions have a similar text):

  * @note If @a start_rev and @a end_rev are valid revisions, this
  * function presumes the revisions as numbered in @a dumpstream only
  * increase from the beginning of the stream to the end.  Gaps in the
  * number sequence are ignored, but upon finding a revision number
  * younger than the specified range, this function may stop loading
  * new revisions regardless of their number.

What does 'may stop' mean?  Does it flips a coin ;)  Seriously, will it or
will it not stop, or under which conditions.


It won't stop.  I've removed this @note from both APIs in r1463213.


Great, thanks!

Blair

Re: [PATCH] run_test.py appearance changes

[Gabriela Gibson]

On 01/04/13 01:31, Daniel Shahaf wrote:
I reworked hunk 2 because I found a bug.

The actual problem I saw was caused because the script didn't handle the 
case where the terminal dimension returns as (0,0).  I've fixed

this in the new patch, and cleaned up inefficient code.

> I did note, though, that you missed adding the SVN_DEPRECATED
> marker to the being-deprecated function.

Thanks :)  I'll fix that once I have some feedback on the rest.

[[[

Disable ANSI color for dumb terminals, fix logic bug where
dimensions of (0,0) was treated as a positive result,
re-factored _run_tests to remove repeated calculation of
line_length.

* build/run_tests.py
  (_get_term_width): Add test condition.
  (TestHarness): Add new variable.
  (_run_test): Add new parameter, remove function call.

]]]


Index: build/run_tests.py
===
--- build/run_tests.py	(revision 1463012)
+++ build/run_tests.py	(working copy)
@@ -100,6 +100,11 @@ def _get_term_width():
   os.close(fd)
 except:
   pass
+  ## Some terminals (eg emacs *shell*, emacs *compilation*)
+  ## seem to return (0,0) from ioctl_GWINSZ, so let's deal
+  ## with that case...
+  if cr == (0, 0):
+cr = None
   if not cr:
 try:
   cr = (os.environ['LINES'], os.environ['COLUMNS'])
@@ -178,7 +183,8 @@ class TestHarness:
 self.log = None
 self.ssl_cert = ssl_cert
 self.http_proxy = http_proxy
-if not sys.stdout.isatty() or sys.platform == 'win32':
+if not sys.stdout.isatty() or sys.platform == 'win32' or \
+   os.getenv("TERM") == "dumb":
   TextColors.disable()
 
   def run(self, list):
@@ -186,8 +192,9 @@ class TestHarness:
there is a log file. Return zero iff all test programs passed.'''
 self._open_log('w')
 failed = 0
+line_length = _get_term_width()
 for cnt, prog in enumerate(list):
-  failed = self._run_test(prog, cnt, len(list)) or failed
+  failed = self._run_test(prog, cnt, len(list), line_length) or failed
 
 if self.log is None:
   return failed
@@ -550,7 +557,7 @@ class TestHarness:
 
 return failed
 
-  def _run_test(self, prog, test_nr, total_tests):
+  def _run_test(self, prog, test_nr, total_tests, line_length):
 "Run a single test. Return the test's exit code."
 
 if self.log:
@@ -587,7 +594,6 @@ class TestHarness:
 
 progabs = os.path.abspath(os.path.join(self.srcdir, prog))
 old_cwd = os.getcwd()
-line_length = _get_term_width()
 dots_needed = line_length \
 - len(test_info) \
 - len('success')

Re: 1.6.21 up for testing/signing

[C. Michael Pilato]

On 03/28/2013 11:27 PM, Ben Reser wrote:
> The 1.6.21 release artifacts are now available for testing/signing.
> Please get the tarballs from
>   https://dist.apache.org/repos/dist/dev/subversion

Summary:

   +1 to release.

Platform:

   Linux 3.2.0-39-generic-pae
   Ubuntu 12.04.2 (precise) Linux (x86)
   Python 2.7.3
   Perl 5.14.2
   Ruby 1.8.7
   Java 1.6.0_27

Verified:

   I tested the following (with pre-installed dependencies):

  ((svn, neon, local) x (fsfs, bdb))
 + swig-py + ctypes-python + swig-rb

SHA1 Checksums:

   bb7c4692216adf0eab89cd3e5d58bbc5908b639c  subversion-1.6.21.tar.gz
   c62b0f9c4dff7202bd5e00876135557b5f5b5f55  subversion-1.6.21.tar.bz2

Signatures added to the appropriate files.

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development



signature.asc
Description: OpenPGP digital signature

Re: Fold the merge_automatic API into the existing merge_peg API

[Julian Foad]

Branko Čibej wrote:

> On 28.03.2013 22:07, Julian Foad wrote:
>>  Branko Čibej wrote:
>>>  On 28.03.2013 21:38, Julian Foad wrote:
 I like the focused API, but I also see how the automatic merge can be 
  considered to fill in a bit of missing functionality that merge_peg
 ought to   provide.
[...]
>>>  I like it. Apparently the encapsulation is even simpler than I expected.
>> 
>>  Heads up: that patch is broken.  merge_automatic_tests.py 7 though 14 all 
>> fail.  However, it's most likely broken in a rather trivial way so I expect 
>> the corrected version will still be simple.

It was indeed a simple goof.

>>>  For JavaHL, a simple overload of ISVNClient.merge can provide the
>>>  "focused" interface without inventing yet another type of merge API.
>>>  Even better, passing null for the revision ranges in the existing
>>>  merge-peg overload can be made to yield the same effect, without
>>>  affecting the API signature at all. (Currently IIUC passing a null
>>>  ranges array will cause an error.)
>> 
>>  Making the C API accept NULL for the revision-ranges array argument
>> would  be a totally sensible extension.  I'll do that.
> 
> I was thinking of the JavaHL API, but you're right -- the C API could
> benefit from the same simplification.

OK, I've got that working and I'm happy with it.  Committed in 1463250.  Any 
further review or changes can come later.

TODO: Decide whether to keep or make private or delete the dedicated 'automatic 
merge' APIs.

This reduces the verbosity of 'svn merge --verbose' for an automatic merge.  We 
can consider putting some verbosity back in in another way.  One option is to 
add some new notifications for the notifier callback, although I don't like 
that.  I don't think this is important, so will leave it as it is unless we 
come up with a good suggestion.

- Julian

Re: svn commit: r1462828 - in /subversion/trunk/subversion/include: svn_fs.h svn_ra_svn.h

[Stefan Fuhrmann]

On Mon, Apr 1, 2013 at 7:22 PM, Bert Huijben  wrote:

>
>
> > -Original Message-
> > From: stef...@apache.org [mailto:stef...@apache.org]
> > Sent: zaterdag 30 maart 2013 19:07
> > To: comm...@subversion.apache.org
> > Subject: svn commit: r1462828 - in /subversion/trunk/subversion/include:
> > svn_fs.h svn_ra_svn.h
> >
> > Author: stefan2
> > Date: Sat Mar 30 18:06:47 2013
> > New Revision: 1462828
> >
> > URL: http://svn.apache.org/r1462828
> > Log:
> > Incorporate API review feedback for svn_fs.h and svn_ra_svn.h
> >
>


> > + * Only if @c r0 has been included in the range of revisions to check,
> > + * are global invariants guaranteed to get verified.
>
> Before this patch the comment documented that global invariants *may be*
> checked by this function if r0 is included. After this patch it guarantees
> that *all* global invariants are verified.
> (I'm not sure how we would ever be able to guarantee that?)
>

The user question I try to answer here is: "what do I have to
specify to make sure that global invariants (i.e. not linked to
any specific revision) get checked, too?". There is no guarantee
that checks in svn_fs_verify are complete in the first place.

It will hopefully catch most issues but there will be conditions
that are "implicit" etc. and won't be checked. Think about
warnings that we might issue in later versions.

-- Stefan^2.

-- 
*Join one of our free daily demo sessions on* *Scaling Subversion for the
Enterprise *
*

*

Re: 1.8 new public API review (mostly) complete.

[Ben Reser]

On Mon, Apr 1, 2013 at 9:27 AM, Julian Foad  wrote:
> First we need to clarify what "@a path (a file,)" means.  From the discussion 
> below I gather "a file" means an absolute local filesystem path, so let's say 
> so.

Yes absolute OS filesystem path. I prefer saying OS filesystem path
because it may not actually be local if it's some sort of network
mount, so I think local clouds the issue.  I'm not sure what we've
been using in our API docs, but if we've been using local that way
then that's fine.

> Erm, not so trivial: svn_path_resolve_repos_relative_url() assumes the 
> "repos-relative URL" is URI-encoded, whereas a relpath is by definition not 
> URI-encoded.

Good point.

> That could work, but it is *much* nicer in my opinion to get rid of the 
> relative-URL option and get rid of the 'repos_root' argument (which was Ben's 
> original suggestion IIUC).  It's simple, cheap, and easy.

Yes, that was my original suggestion.  I'll go ahead and do that.

Re: 1.8 new public API review (mostly) complete.

[C. Michael Pilato]

On 03/31/2013 12:31 AM, Blair Zajac wrote:
> Question on svn_repos_load_fs4() and svn_repos_get_fs_build_parser4() (maybe
> other functions have a similar text):
> 
>  * @note If @a start_rev and @a end_rev are valid revisions, this
>  * function presumes the revisions as numbered in @a dumpstream only
>  * increase from the beginning of the stream to the end.  Gaps in the
>  * number sequence are ignored, but upon finding a revision number
>  * younger than the specified range, this function may stop loading
>  * new revisions regardless of their number.
> 
> What does 'may stop' mean?  Does it flips a coin ;)  Seriously, will it or
> will it not stop, or under which conditions.

It won't stop.  I've removed this @note from both APIs in r1463213.


-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development



signature.asc
Description: OpenPGP digital signature

RE: svn commit: r1462828 - in /subversion/trunk/subversion/include: svn_fs.h svn_ra_svn.h

[Bert Huijben]

> -Original Message-
> From: stef...@apache.org [mailto:stef...@apache.org]
> Sent: zaterdag 30 maart 2013 19:07
> To: comm...@subversion.apache.org
> Subject: svn commit: r1462828 - in /subversion/trunk/subversion/include:
> svn_fs.h svn_ra_svn.h
> 
> Author: stefan2
> Date: Sat Mar 30 18:06:47 2013
> New Revision: 1462828
> 
> URL: http://svn.apache.org/r1462828
> Log:
> Incorporate API review feedback for svn_fs.h and svn_ra_svn.h
> 
> * subversion/include/svn_fs.h
>   (svn_fs_verify): rewrite the docstring for @a start and @a end;
>add cross-references
> 
> * subversion/include/svn_ra_svn.h
>   (svn_ra_svn_create_conn3): document @a zero_copy_limit parameter
> 
> Modified:
> subversion/trunk/subversion/include/svn_fs.h
> subversion/trunk/subversion/include/svn_ra_svn.h
> 
> Modified: subversion/trunk/subversion/include/svn_fs.h
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_fs.
> h?rev=1462828&r1=1462827&r2=1462828&view=diff
> ==
> 
> --- subversion/trunk/subversion/include/svn_fs.h (original)
> +++ subversion/trunk/subversion/include/svn_fs.h Sat Mar 30 18:06:47 2013
> @@ -2425,13 +2425,17 @@ svn_fs_pack(const char *db_path,
>   * to the Subversion filesystem (mainly the meta-data) located in the
>   * directory @a path.  Use @a scratch_pool for temporary allocations.
>   *
> - * @a start and @a end are used to limit the amount of checks being done
> - * to data that is relevant to that range of revisions.  However, this is
> - * only a lower limit to the actual amount of checks being done.  The
> - * backend may not even be able to limit the errors begin reported.
> - * @a start and @a end may be #SVN_INVALID_REVNUM, in which case
> - * svn_repos_verify_fs2()'s semantics apply.  When @c r0 is being
> - * verified, global invariants may be verified as well.
> + * Repository (meta) data forms a tightly knit network of references.
> + * A full check can be expensive and may not always be required.  If not
> + * equal to #SVN_INVALID_REVNUM, @a start and @a end define the
> range of
> + * revisions to check, @c r0 and @a HEAD being the respective defaults.
> + * Due to the references among repository (meta) data, the
> implementation
> + * may however need to also check elements that are not strictly part of
> + * the selected range of revisions.  Thus, it is perfectly legal for a FS
> + * implementation to ignore the @a start and @a end parameters entirely.
> + *
> + * Only if @c r0 has been included in the range of revisions to check,
> + * are global invariants guaranteed to get verified.

Before this patch the comment documented that global invariants *may be* 
checked by this function if r0 is included. After this patch it guarantees that 
*all* global invariants are verified.
(I'm not sure how we would ever be able to guarantee that?)

Bert 

Re: 1.8 new public API review (mostly) complete.

[Julian Foad]

Here's the current doc string of svn_repos_authz_read2():

/**

 * Read authz configuration data from @a path (a file, repos relative
 * url, an absolute file url, or a registry path) into @a *authz_p,
 * allocated in @a pool.
 *
 * If @a groups_path (a file, repos relative url, an absolute file url,
 * or a registry path) is set, use the global groups parsed from it.
 *
 * [...] If @a path is a repos relative URL then @a repos_root must be
 * set to the root of the repository [...].
svn_error_t *
svn_repos_authz_read2(svn_authz_t **authz_p,
  const char *path,
  const char *groups_path,
  svn_boolean_t must_exist,
  const char *repos_root,
  apr_pool_t *pool);

First we need to clarify what "@a path (a file,)" means.  From the discussion 
below I gather "a file" means an absolute local filesystem path, so let's say 
so.

Ben Reser wrote:
> On Sun, Mar 31, 2013 at 5:58 AM, Daniel Shahaf  wrote:
>>  How about forbidding PATH to be a repos-relative URL, but permitting it
>>  to be a relpath, that is then interpreted relative to the repos root.
>>  The conversion is trivial, just +2 on the argument, but it avoids

Erm, not so trivial: svn_path_resolve_repos_relative_url() assumes the 
"repos-relative URL" is URI-encoded, whereas a relpath is by definition not 
URI-encoded.

>>  "non-canonical" and "^/ URLs" in the repos API.
> 
> This could work because we don't accept a relative path for the OS
> file system paths.  The funny thing about this is then I don't think I
> need the repos-relative APIs that I made public.

That could work, but it is *much* nicer in my opinion to get rid of the 
relative-URL option and get rid of the 'repos_root' argument (which was Ben's 
original suggestion IIUC).  It's simple, cheap, and easy.

- Julian


>>  (I would say permit it to be an fspath, which is by definition absolute,
>>  but fspaths aren't part of the public API.)
> 
> Not a good idean, on unix platforms the fspath would be
> indistinguishable from an absolute OS file system path.

Re: 1.8 new public API review (mostly) complete.

[Ben Reser]

On Sun, Mar 31, 2013 at 5:58 AM, Daniel Shahaf  wrote:
> How about forbidding PATH to be a repos-relative URL, but permitting it
> to be a relpath, that is then interpreted relative to the repos root.
> The conversion is trivial, just +2 on the argument, but it avoids
> "non-canonical" and "^/ URLs" in the repos API.

This could work because we don't accept a relative path for the OS
file system paths.  The funny thing about this is then I don't think I
need the repos-relative APIs that I made public.

> (I would say permit it to be an fspath, which is by definition absolute,
> but fspaths aren't part of the public API.)

Not a good idean, on unix platforms the fspath would be
indistinguishable from an absolute OS file system path.

Re: 1.8 new public API review (mostly) complete.

[C. Michael Pilato]

On 04/01/2013 11:25 AM, C. Michael Pilato wrote:
> On 03/31/2013 12:31 AM, Blair Zajac wrote:
>> Question on svn_repos_load_fs4() and svn_repos_get_fs_build_parser4() (maybe
>> other functions have a similar text):
>>
>>  * @note If @a start_rev and @a end_rev are valid revisions, this
>>  * function presumes the revisions as numbered in @a dumpstream only
>>  * increase from the beginning of the stream to the end.  Gaps in the
>>  * number sequence are ignored, but upon finding a revision number
>>  * younger than the specified range, this function may stop loading
>>  * new revisions regardless of their number.
>>
>> What does 'may stop' mean?  Does it flips a coin ;)  Seriously, will it or
>> will it not stop, or under which conditions.
> 
> Heh.  It's been a long while -- I'll look into that again.  On first (again)
> glance, I don't see any code which would stop anything.

Okay, so the behavior is straightforward -- if a revision in the dump stream
is outside the range, it is ignored (skipped) and processing continues.

I remember now what I was thinking when I composed that docstring.  I was
thinking about how much time would be wasted trudging the whole way through
a dump of, say, revisions 0-10 when the load operation was limited to
the revision range 0-10.  Today, the API will keep examining the addition
99,990 revisions just in case one of them has a revision number within the
desired range.  I was building in some future-proofing to allow us to decide
later that that's silly -- that the minute we see a revision that's younger
than the desired range, we could bail out on processing the dumpstream
immediately.

Any strong opinions on which behavior we want for this API?

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development



signature.asc
Description: OpenPGP digital signature

Re: 1.8 new public API review (mostly) complete.

[C. Michael Pilato]

On 03/31/2013 12:31 AM, Blair Zajac wrote:
> On 03/29/2013 01:17 PM, C. Michael Pilato wrote:
>> Devs,
>>
>> I've just completed my review of the new-in-1.8 public APIs, minus the bits
>> that Philip reviewed (thanks!) and the new merge-related stuff which, if I
>> understand from recent threads correctly, is still subject to some churn.
>>
>> The results of my review revealed overwhelmingly positive results which, in
>> my approximation, are non-contentious.  I had (for some definition of "had")
>> to touch up quite a few docstring in the process, but by and large those
>> were stylistic nits with the occasionally overlooked item.
> 
> Question on svn_repos_load_fs4() and svn_repos_get_fs_build_parser4() (maybe
> other functions have a similar text):
> 
>  * @note If @a start_rev and @a end_rev are valid revisions, this
>  * function presumes the revisions as numbered in @a dumpstream only
>  * increase from the beginning of the stream to the end.  Gaps in the
>  * number sequence are ignored, but upon finding a revision number
>  * younger than the specified range, this function may stop loading
>  * new revisions regardless of their number.
> 
> What does 'may stop' mean?  Does it flips a coin ;)  Seriously, will it or
> will it not stop, or under which conditions.

Heh.  It's been a long while -- I'll look into that again.  On first (again)
glance, I don't see any code which would stop anything.

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development



signature.asc
Description: OpenPGP digital signature

Re: 1.8 new public API review (mostly) complete.

[C. Michael Pilato]

On 03/30/2013 02:26 PM, Stefan Fuhrmann wrote:
> r1462828 addresses the issues listed for svn_fs.h and svn_ra_svn.h.
> Please review.

The zero-copy-limit stuff looks good.  I don't quite understand the bit
about svn_fs_verify() possibly ignoring the 'start' and 'end' parameters
altogether.  From code inspection, it certainly *looks* like FSFS will quite
literally walk the reps between start and end, inclusive.  So if you're
simply trying to say that additional revs (outside of the START:END range)
might be checked as a side-effect of verifying the START:END range, then
let's say that.

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development



signature.asc
Description: OpenPGP digital signature

Re: 1.6.21 up for testing/signing

[Johan Corveleyn]

On Fri, Mar 29, 2013 at 4:27 AM, Ben Reser  wrote:
> The 1.6.21 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. I plan to try and release on April
> 4th so please try and get your votes/signatures in place by April
> 3rd EOD.  Thanks!
>
> Please note the following known test failures (perhaps most easily
> triggered with APR-1.4.6):
>
>  log_tests.py 30: log -g should ignore cyclic merges
>  Can fail because 1.6.x is missing r1293229, which fixed occasionally
>  missing merged-via notifications.
>
>  diff_tests.py 32: repos-wc diff showing added entries with props
>  Can fail because this test is missing various tweaks to account
>  for random output order of 'svn diff' (this is independent of the
>  APR 1.4.6 hash order problem).
>
>  ruby swig test 'test_dump'
>  Can fail because 1.6.x is missing r966458, which sorted property
>  listings in dump files in alphabetical order, and this test does
>  not parse dump files and sort properties for comparison.

Summary
---
+1 to release

Platform

Windows XP (32 bit) SP3
Microsoft Visual Studio 2010 SP1

Verified

Signature and sha1 for subversion-1.6.21.zip.

Contents of subversion-1.6.21.zip are identical to tags/1.6.21, and to
branches/1.6.x@1462351 (except for expected differences in svn_version.h).

Tested
--
[ Release build ] x [ fsfs ] x [ file | svn | neon | serf ]

Results
---
All tests pass (which is expected because I'm still using APR 1.4.5)

For serf I disabled my local AVG Surf-Shield to make it work
(issue #4175 - ra_serf crashes on Windows with AVG 2012 Surf-Shield
 http://subversion.tigris.org/issues/show_bug.cgi?id=4175)

Dependencies

Httpd 2.2.22
Apr 1.4.5
Apr-Util 1.4.1
Apr-Iconv 1.2.1
Neon 0.29.6
OpenSSL 1.0.0d
SQLite 3.7.14.1
ZLib 1.2.6
Serf 0.7.1

Signature
-

subversion-1.6.21.zip:
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (MingW32)

iQIcBAABCgAGBQJRWJn8AAoJELWc5tYBDIqtHygP/i2YTryHJgTFFnIWAPb0klnK
xC7PNfD4MHSYVLoT4WpOdSF/DMMrZ1fxDO5PB3zOKYemXH/w1e7PVV/wJJA8HILx
KZytLPdQfIHB/LiZGycaADGfDp9TtNM/bofJJfAg9QzHXwgvSqxz64ytLAVWj0oa
MO9XV+TOI4zBLGa/U7fY1AZWC/LDjJB+HUSxn84ZDo1d/YPOmuql+hVszj1uUXTx
0qHijsRz5yX/2Zw/hKnxGiHzuewWCXADXLUaje0cMGFTw+CFdvBb8YYmkBcs4Yxa
KJs4jOBWKy9Fj0d3jNAFKcda8xFeEN18u7i4mtWSPz31P6C/x7NevE5BPcMNa2Kg
8jGim93Zh3IqJW9Yf8MUuyRopRzxHnGPWxdyOafskiYcglheXoOqib1YvNgiIGPo
0qVa8UYiU9P+fk5jWmKSsnRQzcNTjqhEuwo2amhsYl8LB435Tj6lkcnPiQL/QdcW
aWKKher5EX9mYRX2HObMLKyr/VhnOxSep5LbIKrg2AtnNukJG7DknDQDN8IDc4im
fWYTJmPRkS+koX/Qf5Q1svuE4ghk9aF3PTwe70QVs/qRdg1uytd5sj1n1no93HAv
nbKqzBWdMlagxii8rM8TiwBlpHcQO5CJBI65c68+JbOVMxPLs6kCAIIH/3Fo+QJh
phY/U/G2Tar/pLI+3LLe
=kZ6N
-END PGP SIGNATURE-

--
Johan

Re: [PATCH] Make conflict message translatable

[Mattias Engdegård]

29 mar 2013 kl. 03.03 skrev Julian Foad:


Mattias Engdegård wrote:

The patch assumes that svn_node_none, svn_node_unknown or  
svn_wc_operation_none

cannot occur here. Please confirm or refute.


I haven't checked whether these can occur in this message.   
svn_node_unknown can certainly occur for the local kind of node --  
such as in a tree conflict when a merge tries to edit a path which  
doesn't exist in the local working copy.  I *think* the node kind  
that appears in these messages can be 'none' for this reason  I can  
check this and fix if necessary and commit it, probably on Monday if  
nobody does it sooner.


Please do - I haven't been able to produce such a situation myself,  
but I'm unsure of the invariants in effect at this point.


My only other suggestion would be to include the comma in the  
English strings instead of in the format string -- _("local file  
edit,") instead of _("local file edit") -- so that the other  
translations can use different (or no) punctuation.


To me it seemed unlikely that the punctuation would vary between  
different cases, but I could be wrong about that. Changing it in the  
way you propose wouldn't do any harm, so please do that if you like.


Thanks for looking at the patch!