Re: BuildBots

2014-02-18 Thread Hyrum K Wright
On Tue, Feb 18, 2014 at 12:51 PM, Ben Reser b...@reser.org wrote:

 On 2/17/14, 12:38 PM, Hyrum K Wright wrote:
  I know that on the svn-x64-ubuntu-gcc slave (currently residing in my
  basement), the scripts are updated manually.  I may even have local
 edits that
  aren't yet in the repository.

 I suspected as much.  Can I possibly get access to this machine to work on
 resolving this?


Sure, but it might take me a day or two.  I'll ping you in IRC.



  On Fri, Feb 14, 2014 at 6:49 PM, Ben Reser b...@reser.org
  mailto:b...@reser.org wrote:
  versions in SVN?  For that matter is there someplace where we
 document exactly
  who is responsible for which slaves?
 
 
  +1 to documenting who owns what.

 It is documented on the builders page, I just didn't know this.

 E.G.
 https://ci.apache.org/builders/svn-x64-centos-gcc



Re: BuildBots

2014-02-17 Thread Hyrum K Wright
On Fri, Feb 14, 2014 at 6:49 PM, Ben Reser b...@reser.org wrote:

 We need our buildbots to work for branches.  I propose the following
 changes,
 in decreasing order of priority.

 1) If something like bindings is broken on a build bot for branches then
 disable the test on that buildbot.  It is far better to disable bindings
 tests
 than it is to continue to allow the build bot to fail and thus encourage
 us to
 ignore the build bot entirely.

 2) We should find a way to keep functioning build slaves for our branches
 that
 support the same tests as we run for trunk.  If that means providing
 parallel
 installations of dependencies on the same machine or using separate
 machines
 then we should do that.

 3) The build bot should not use the same slave for builds of branch and
 trunk
 changes.  Right now the waterfall view does not give you a good view of the
 state of our branches/trunk.  If I break 1.7.x the bot will show red until
 the
 next build which might be on trunk succeeds.  That's not useful.  I can
 understand sometimes running a test on a branch that's not normally tested
 (like I did with the 1.7.x-diff-translate branch) and using the 1.7.x
 slave.
 But we really ought to have slaves for each release branch we're using.

 Obviously 2 and 3 are longer term propositions.  But #1 is something we
 should
 fix immediately.  To that end I have conducted an audit of what's broken on
 which build bots and which release branches.  My findings are below.

 1.8.x:
   svn-x64-ubuntu-gcc: works
   svn-x64-centos-gcc: works
   bb-openbsd: doesn't build 1.8.x
   svn-windows-local: JavaHL fails.
   svn-windows-ra: Perl bindings fails.

 1.7.x:
   svn-x64-ubuntu-gcc: Ruby bindings fail (build not just tests)
   svn-x64-centos-gcc: works
   bb-openbsd: doesn't build 1.8.x
   svn-windows-local: JavaHL fails.
   svn-windows-ra:  Build fails (can't determine why bot is down right now).

 We should disable the swig-rb tests on 1.7.x on svn-x864-ubuntu-gcc since
 the
 problem there is that the version of Ruby on that host is too new for
 1.7.x.

 Bert said the JavaHL tests on svn-windows-local are known due to some sort
 of
 DLL PATH problem.  We should fix or disable these.

 I recall Bert having said in the past that SWIG-PL doesn't build on Windows
 properly, we should disable that test for swig-windows-ra.

 The build bots are down for Windows right now due to a network issue.  So I
 can't run a specific test to determine what's wrong with svn-windows-ra and
 1.7.x and the logs for the old builds have been removed already.

 We have two ways to handle turning tests off by branch.  First we can pass
 the
 branch into the scripts that it's running for and then turn things off.
  The
 only problem with this is that svncheck.sh for the *nix buildbots already
 takes
 the list of tests to run.  So we'll need to change the scripts to accept
 this
 in parallel with the master changes to pass it.  We could alternatively
 detect
 the branch from the working copy.  But I'm not sure how easy this will be
 to do.

 Or we could just skip all of the above and get the build bots setup with
 separate slaves for each branch and then they can have their own
 independent
 scripts.

 I'd start on some of the above steps but I'm also not clear on how these
 scripts get updated on the slaves.  Does someone have to do that manually,
 are
 they automatically updated?  If they are manually updated are the current


I know that on the svn-x64-ubuntu-gcc slave (currently residing in my
basement), the scripts are updated manually.  I may even have local edits
that aren't yet in the repository.


 versions in SVN?  For that matter is there someplace where we document
 exactly
 who is responsible for which slaves?


+1 to documenting who owns what.


Re: C++ thoughts for Berlin

2013-05-30 Thread Hyrum K Wright
On Wed, May 29, 2013 at 6:27 PM, Branko Čibej br...@wandisco.com wrote:

 On 30.05.2013 03:03, Blair Zajac wrote:
  ... one could stop thinking about memory management.

 Ha, ha, censored ha.

 I've heard that argument any number of times from C++ enthusiasts. I
 still get a kick from seeing their faces after they realize what a load
 of censored it is when you actually get away from Hello, world! to
 something closer to the complexity of real projects.


+1

Spending any amount of time around C++ programmers, and you quickly learn
that they have just as much paranoia about memory as your standard C
programmer.  Actually, it's even worse, since the language hides just
enough detail that you can't really glance at code and know what's going
on, memory-wise (is this thing being copied?  who owns this thing?  is
it a reference or a pointer or something else?).  Using C++ doesn't solve
any memory management problems.

It'd be really painful to write code in anything less than C++11, and as
has been stated before, the platform support for that is very piecemeal.
 In that case, it might even be worth examining Lua or Go or Haskell. :)

Most importantly, I've not yet heard a good statement of the problem that
such a rewrite would solve.  (Though I might not be listening closely
enough.)

-Hyrum

PS - For those that want to waste a bunch of time bashing C++, I suggest
the Frequently Questioned Answers document: http://yosefk.com/c++fqa/  I
bit dated, but entertaining nonetheless.


Re: 1.8.0 release timing

2013-05-30 Thread Hyrum K Wright
On Thu, May 30, 2013 at 2:48 PM, Johan Corveleyn jcor...@gmail.com wrote:

 On Thu, May 30, 2013 at 11:23 PM, Ben Reser b...@reser.org wrote:
  On Thu, May 30, 2013 at 2:00 PM, Greg Stein gst...@gmail.com wrote:
  As noted elsewhere, press releases are ideally on a Tuesday. I'd really
 like
  to see the release/announce go out, coordinated, on a Tuesday.
 
  That's fine the release would need to be after the hackathon then.
  Given that and Ivan's objection to doing testing during the hackathon
  I'd suggest we plan for a release on the 18th.

 Heh, I wouldn't want to be the one finding a show-stopper after the
 party and before the release :-).

 That would just be an excuse to have another party later. :)

-Hyrum


Re: branch 1.8 or at least start making alpha releases?

2013-02-14 Thread Hyrum K Wright
On Thu, Feb 14, 2013 at 12:45 PM, Stefan Sperling s...@elego.de wrote:

 On Thu, Feb 14, 2013 at 12:36:13PM -0500, C. Michael Pilato wrote:
  As such -- and assuming no one else has a feature waiting in the wings
 and
  can make a compelling case for further delaying the release to
 accommodate
  it -- I lean towards branching ASAP.  And yes, +1 on delivering alphas as
  soon as the branch is cut!

 We could make backports to the branch a little less of a burden by
 streamlining the process somewhat, e.g. by requiring only two votes
 during the alpha release phase (i.e. until we start issuing betas).
 IIRC we've done this kind of thing before.


We have done that before, and I think it worked out well.  I
would hesitate to release 1.8 alphas from trunk, but support a streamlined
review process until betas or release candidates start to ship.

I would also urge us to move through the alpha process quickly.  From past
experience, it can feel like we're making progress when we release alphas
and betas---and we are, to an extent---but most end users won't start
paying attention until release candidates start to ship and the soak period
starts.  (And even then, the vast majority won't start caring until we ship
1.8.0.)

-Hyrum


Re: packages tree

2013-01-29 Thread Hyrum K Wright
On Tue, Jan 29, 2013 at 6:41 PM, Justin Erenkrantz jus...@erenkrantz.comwrote:

 On Tue, Jan 29, 2013 at 1:04 PM, Ben Reser b...@reser.org wrote:

 If we're not going to keep these things up to date then I think we
 should just remove them.


 +1 to remove.  (It's all version-controlled anyway!)  -- justin


+1


Re: Running gdb in the build tree Re: OWP: Introduction for Gabriela Gibson

2013-01-21 Thread Hyrum K Wright
On Mon, Jan 21, 2013 at 6:41 AM, Gabriela Gibson
gabriela.gib...@gmail.comwrote:

 On 21/01/13 11:05, Daniel Shahaf wrote:

 On Mon, Dec 10, 2012 at 11:02:22PM -0500, C. Michael Pilato wrote:

 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...)


 A collection of 'how to debug svn' tips would be nice to have, and this
 thread seems like a good place to post them.


We might want to think about putting them on the site or on the wiki, as it
might be hard for newcomers to find them buried in an email thread.  But I
agree that a location collecting debugging tips would be useful.

-Hyrum


Re: Diff Project compilation problem

2013-01-09 Thread Hyrum K Wright
On Wed, Jan 9, 2013 at 11:37 AM, Branko Čibej br...@wandisco.com wrote:

 On 09.01.2013 17:31, Gabriela Gibson wrote:
  I added a new option to the command structure and the compile error I
  get is this:
 
  ../../subversion/libsvn_client/.libs/libsvn_client-1.so: undefined
  reference to `svn_wc__get_wcroot'
  ../../subversion/libsvn_client/.libs/libsvn_client-1.so: undefined
  reference to `svn_wc_add_from_disk2'
 
  What is going on here?  This is totally unexpected!  I grepped for the
  terms and I cannot even see a connection %-)
 
  I attached a patch of the changes I made and the compile log is at the
  bottom of this mail.
 
  I did make clean prior to this, and also make -k, the make here
  was just to cut down the spammy list.
 
  thanks for any advice!

 I think you have to run autogen.sh again to rebuild the generated
 makefiles.


Even just ./gen-make.py should suffice.

fwiw, this probably isn't related to your change, just one that you picked
up upon running 'svn update'.

-Hyrum


Re: [RFC] Deprecate Berkelety DB filesystem backend

2013-01-05 Thread Hyrum K Wright
+1

-Hyrum


On Sat, Jan 5, 2013 at 12:53 PM, Justin Erenkrantz jus...@erenkrantz.comwrote:

 +1!  -- justin
 On Jan 5, 2013 10:40 AM, Greg Stein gst...@gmail.com wrote:

 Is +1 too short of a response?

 :-)
 On Jan 4, 2013 7:35 PM, Branko Čibej br...@wandisco.com wrote:

  For the following reasons

- FSFS has been the default filesystem backend for almost 7 years,
since 1.2.

 - Looking at commit history, I've not seen a single (functional or
performance) improvement, beyond a few bug fixes, in the BDB backend in 
 at
least two years. The last significant change that I'm aware of was 
 released
in 1.4 (support for BDB 4.4. and DB_REGISTER) back in 2006. In effect, 
 BDB
is in barely maintained mode whereas interesting things are happening 
 to
FSFS all the time.

 - I cannot remember seeing a BDB-related bug report in a month of
Sundays (meaning that it's either rock-solid, or not used).

 - The BDB backend is an order of magnitude slower on trunk than FSFS
   - timing parallel make check on my 4x4-core i7+ssd mac:
  - FSFS: real 7m33.213s, user 19m8.075s, sys 10m54.739s
  - BDB: real 35m17.766s, user 15m28.395s, sys 11m58.824s

 I propose that we:

- Declare the BDB backend deprecated in 1.8, adding appropriate
warnings when it's used or manipulated (to svnadmin?)

 - Stop supporting it (including bug fixes) in 1.9

 - Completely remove the BDB-related code in 1.10 (I'm making an
assumption that we'll have the FSv2 API and releated refactoring of FSFS 
 by
then, and at least a draft experimental FSv2 implementation).


 I realize I'm raising this question at a time when we should be focusing
 on branching 1.8. On the other hand, this release may be a good opportunity
 to deprecate the Berkeley DB filesystem.


 --
 Branko Čibej
 Director of Subversion | WANdisco | www.wandisco.com




Re: svn commit: r1425778 - in /subversion/trunk: build.conf tools/dev/fsfs-access-map.c

2012-12-26 Thread Hyrum K Wright
On Wed, Dec 26, 2012 at 8:31 AM, Stefan Fuhrmann 
stefanfuhrm...@alice-dsl.de wrote:

  On 12/26/2012 12:54 PM, Bert Huijben wrote:

  One of the build.conf changes applies to __ALL_TESTS__, not to libs.
 (There is no such section).


 I took that from the diff output libs = __ALL__.
 Corrected the log message now.


  I’m not sure why we add tools that are not used by the test tools to the
 __ALL_TESTS__ target? Do you intend to run it from the test suite. (On
 which platforms?)


 No. I don't expect that tool to be used in tests.
 I only want to get it built. __ALL_TESTS__ seems
 to be the target we use for this purpose already
 (e.g. svn-rep-sharing-stats).

 Broken window? Maybe, we need a new just
 build it target instead of misusing __ALL_TESTS__
 as the kitchen sink?


Possibly.  It may also be useful to consider whether or not we should ship
tools like this.  (Though, I expect that packagers would probably them out
into separate packages anyway, so maybe it's moot.)


Re: RFC: Build system changes

2012-12-13 Thread Hyrum K Wright
On Thu, Dec 13, 2012 at 1:13 PM, Branko Čibej br...@wandisco.com wrote:

 The attached patch makes several changes to how we discover compilers
 and set flags on *nix:

   * Search for clang as well as the default gcc/cc, and prefer clang(++)
 over gcc/g++.
   * Set standards-compliance mode (C90/C++11) even without maintainer-mode.


It seems a bit odd to allow use of C++11 features and yet still use C90 for
the rest of the codebase.  I realize the C++ code is largely limited to
interfaces with lower-level libraries and bindings, but I would lean toward
C++98, at least initially.  (That is, unless you've got a compelling reason
for rvalue references.  :P  )

-Hyrum


Re: Start the process for releasing 1.7.8 (Was: FW: 1.7.7 svn client x64 crashes while committing with credentials provided on the command line.)

2012-12-03 Thread Hyrum K Wright
On Mon, Dec 3, 2012 at 5:10 AM, Bert Huijben b...@qqmail.nl wrote:



  -Original Message-
  From: Thorsten Schöning [mailto:tschoen...@am-soft.de]
  Sent: maandag 3 december 2012 07:48
  To: us...@subversion.apache.org
  Subject: Re: 1.7.7 svn client x64 crashes while committing with
 credentials
  provided on the command line.
 
  Guten Tag Mikhail Pereiaslavski,
  am Montag, 3. Dezember 2012 um 07:25 schrieben Sie:
 
   not sure whether it was already reported or not, but just in case it's
 not
  yet.
 
  It's a known issue:
 
  http://mail-archives.apache.org/mod_mbox/subversion-
  users/201211.mbox/%3c20121104101302.gd27...@ted.stsp.name%3E

 Hi,

 I think we receive a crash report on the users list for this specific
 problem about every day now.

 Maybe it is time to release a Subversion 1.7.8 with this fix in?
 (Or even a 1.7.8 with just this fix)


+1.  Not volunteering to shepherd the release, but if it impacts this many
users, it ought to be fixed post haste.

-Hyrum


Re: SQLite vacuum or auto_vacuum?

2012-12-03 Thread Hyrum K Wright
On Mon, Dec 3, 2012 at 7:08 AM, Philip Martin philip.mar...@wandisco.comwrote:

 Prompted by a question on users I wondered how SQLite's vacuum
 (http://sqlite.org/lang_vacuum.html) would affect wc.db size.  On a
 Subversion trunk working copy I have been using for months the size was
 reduced from 2.3MB to 1.3MB which isn't really a significant change.

 For a further test I checked-out a ^/subversion/branches working copy
 for a wc.db of 93MB with 121738 rows, I made it sparse with 66046 rows
 and it was still 93MB, then I ran vacuum and it was reduced to 51MB.  I
 have a gcc working copy with some subtrees switched to an empty
 directory.  There vacuum reduced wc.db from 47MB to 8.1MB.
 So it appears that vacuum is interesting if the number of rows decreases
 dramatically.

 SQLite has auto_vacuum but it comes with a warning that it may make
 fragmentation worse (http://sqlite.org/pragma.html#pragma_auto_vacuum)
 so it's not clear whether we should enable it.  Perhaps we should add a
 vacuum to cleanup?  A full vacuum rewrites all the tables so it's not
 a trivial operation but it is reasonably fast for the working copies on
 local disk that I tried.


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.

-Hyrum


trunk 'svn merge' takes a very long time to respond.

2012-12-03 Thread Hyrum K Wright
'svn merge' appears to hang when running a simple merge:

[[[
$ svnd merge ^/subversion/trunk/
^Csubversion/svn/util.c:913: (apr_err=4)
subversion/svn/merge-cmd.c:163: (apr_err=4)
subversion/libsvn_client/merge.c:11856: (apr_err=4)
subversion/libsvn_client/merge.c:11829: (apr_err=4)
subversion/libsvn_client/merge.c:9295: (apr_err=4)
subversion/libsvn_client/merge.c:9001: (apr_err=4)
subversion/libsvn_client/merge.c:8762: (apr_err=4)
subversion/libsvn_client/merge.c:8599: (apr_err=4)
subversion/libsvn_client/merge.c:6388: (apr_err=4)
subversion/libsvn_ra_serf/log.c:607: (apr_err=4)
subversion/libsvn_ra_serf/util.c:819: (apr_err=4)
subversion/libsvn_ra_serf/util.c:786: (apr_err=4)
svn: E04: Error running context: Interrupted system call
$
]]]

This is using a trunk client build at r1416744 on Mac OS X.  I also see the
same problem using a Linux client, same vintage.  It was unresponsive for
something on the order of 5 minutes before I finally killed the process
with the above output.

I don't know enough about what's going on under the hood, but it appears
the serf is lost.  None of my CPUs are pegged, though, so it looks like
it's just waiting on network?  Hard to believe, as I've got a 15/5 fiber
connection.

Running it again, I start to get a response after 7 minutes.  Something
strange going on here...

-Hyrum


Re: 1.8 Progress

2012-12-01 Thread Hyrum K Wright
On Thu, Nov 29, 2012 at 4:52 PM, C. Michael Pilato cmpil...@collab.netwrote:

  2) Ev2.  The notes say this is believed to be in a releasable state?  Is
  there any work needed to verify this?  Do we need to remove the use of
 Ev2
  in any place to avoid releasing with compatibility shims in use? Are we
  comfortable that the API is complete?

 Julian expressed doubt about whether the API was ready for prime-time.

 C-Mike expressed concern about the extremely low bus factor.

 Hyrum acknowledged both, and continued with:  We can always shuffle
 headers
 around or document the things as experimental, so committing ourselves to
 the API as this point isn't my concern.  The only real limiting around Ev2
 and 1.8 is issue #4116 which is svnrdump failures over ra_serf.  In the
 issue, I propose using Ev2 to get around the problem, since the dumpfile
 format is so incongruent with the editor.  Of course, we don't *have* to do
 that, but as I've thought about it, any solution will require a bit o'
 caching---which we've already implemented as part of the Ev2 shims.  We
 *might* be able to implement the svnrdump editor as Ev2, shim the thing on
 the client side (which gives us the required caching) and release that way.
  Or there might be a better solution I'm overlooking because I've got Ev2
 on
 the brain.


This is basically boils down to rdump isn't completely Delta Editor
friendly, which interacts badly with Serf.  This problem is only
tangentially related to Ev2, but it was proposed as one of the possible
solutions.  It's probably better to try and pursue other solutions to this
independent of Ev2.

As for Ev2 itself, I don't see anything that should be blocking 1.8.  If
people are uncomfortable shipping the API, some documentation and/or header
hackery should be sufficient to make it mutable in future releases.  As far
as I know, all the Ev2 work is entirely self-contained within Subversion.

OWNERSHIP:  Hyrum's got the most experience here, but due to his time
 contention, we may very well have no owner for this at all.  That's bad.


Sadly, true.

-Hyrum


Re: svn commit: r1414728 - in /subversion/trunk: ./ build/generator/ subversion/bindings/cxxhl/ subversion/bindings/cxxhl/include/ subversion/bindings/cxxhl/include/types/ subversion/bindings/cxxhl/sr

2012-11-29 Thread Hyrum K Wright
On Wed, Nov 28, 2012 at 3:10 PM, Branko Čibej br...@wandisco.com wrote:

 On 28.11.2012 17:15, Philip Martin wrote:
  Branko Čibej br...@wandisco.com writes:
 
  On 28.11.2012 15:39, br...@apache.org wrote:
  Author: brane
  Date: Wed Nov 28 14:39:42 2012
  New Revision: 1414728
 
  URL: http://svn.apache.org/viewvc?rev=1414728view=rev
  Log:
  Getting sidetracked for a bit: create placeholder and update build for
  a future high-level C++ API for Subversion, based on the same
 principles
  as JavaHL.
  Lest anyone become too worried about this: I'm not going to let this
  project divert my attention from getting 1.8 out the door. I had this
  idea on the back burner for a long time, and just now found a bit of
  time when I had to clear my head; so I decided to at least get the build
  infrastructure in place.
  How does this compare to Hyrum's object model?
 
notes/object-model.txt
^/subversion/branches/object-model

 I don't know yet. I'll be looking at that more closely when the time
 comes, but I'll also take notes from JavaHL. I feel it's important that
 we hide the details of the C API (for example, its dependency on APR)
 from this high-level interface.


The object model stuff is probably a bit dated, but some of the ideas
probably still hold.  If there's anything I learned through that
experience, it was that we'll probably need to use something like pimpl for
any type of backward compatibility within the C++ bindings themselves.  Ugh.

-Hyrum


[RFC] Ev2 implementations in ev2 subdirectory

2012-11-27 Thread Hyrum K Wright
Greetings all!

r1413184 introduced the --enable-ev2-impl flag to configure on trunk to
allow interested users to compile with the Ev2 implementations of various
operations.  While the set of things that supports Ev2 is not nearly
complete, it is (slowly) growing, and I'd like to do that development on
trunk, rather than the current ev2-export branch.

To that end, I plan on selectively moving the current bits from the
ev2-export branch to trunk, guarded by this configure-time flag.  I also
hope to add a buildslave which runs the tests with --enable-ev2-impl to
catch failures soon after they are introduced.

For the immediate future I'd like for this code to be compiled, but not in
the call path by default.  We can use #ifndef guards, but that means the
code won't even be compiled.  If we unconditionally compile the code in the
same file, we get unused symbol warnings, and have to worry about clashing
function names (Ev1 add_directory vs. Ev2 add_directory).  It also creates
something of a mess for people trying to determine which code is actually
being used.

Instead, I propose subdirectories named ev2 in the various libsvn_foo
directories which would hold Ev2 implementations.  They would eventually go
away if/when the Ev2 implementations replace the existing ones.  We haven't
historically used subdirectories in this way, but I think this type of
change would make implementation and maintenance easier.

Thoughts?

-Hyrum


Re: [RFC] Ev2 implementations in ev2 subdirectory

2012-11-27 Thread Hyrum K Wright
On Tue, Nov 27, 2012 at 9:07 AM, Daniel Shahaf d...@daniel.shahaf.namewrote:

 Hyrum K Wright wrote on Tue, Nov 27, 2012 at 07:49:04 -0500:
  Instead, I propose subdirectories named ev2 in the various libsvn_foo
  directories which would hold Ev2 implementations.  They would eventually
 go

 Not objected to subdirs, but have you considered just naming your files
 libsvn_repos/ev2_replay.c and so on?  That doesn't need any build.conf
 changes...


I thought about that, but didn't want to double the number of files in
something like libsvn_client where there are several editor implementations.

I know we already have subdirectories in at least a couple of places
(mod_dav_svn comes to mind).  Are the build.conf changes that invasive?

 away if/when the Ev2 implementations replace the existing ones.  We
 haven't
  historically used subdirectories in this way, but I think this type of
  change would make implementation and maintenance easier.
 
  Thoughts?
 
  -Hyrum



Re: [RFC] Ev2 implementations in ev2 subdirectory

2012-11-27 Thread Hyrum K Wright
On Tue, Nov 27, 2012 at 9:21 AM, Daniel Shahaf d...@daniel.shahaf.namewrote:

 Hyrum K Wright wrote on Tue, Nov 27, 2012 at 09:11:50 -0500:
  On Tue, Nov 27, 2012 at 9:07 AM, Daniel Shahaf d...@daniel.shahaf.name
 wrote:
 
   Hyrum K Wright wrote on Tue, Nov 27, 2012 at 07:49:04 -0500:
Instead, I propose subdirectories named ev2 in the various libsvn_foo
directories which would hold Ev2 implementations.  They would
 eventually
   go
  
   Not objected to subdirs, but have you considered just naming your files
   libsvn_repos/ev2_replay.c and so on?  That doesn't need any build.conf
   changes...
  
 
  I thought about that, but didn't want to double the number of files in
  something like libsvn_client where there are several editor
 implementations.
 
  I know we already have subdirectories in at least a couple of places
  (mod_dav_svn comes to mind).  Are the build.conf changes that invasive?

 No, they aren't invasive; they're just one more moving part --- and I've
 spent the last couple of days reducing the number of moving parts (in
 various other contexts), so I quite appreciate the difference between
 0. Profit! and 0. Foo 1. Profit!.


I completely understand (and agree!), but I think in this case it'd be
worth it to make the segregation more pronounced by using the
directory hierarchy as a namespace mechanism, rather than prepending
something to the filename.

-Hyrum


Re: Test code coverage

2012-11-26 Thread Hyrum K Wright
On Sun, Nov 25, 2012 at 5:17 PM, Stefan Fuhrmann 
stefan.fuhrm...@wandisco.com wrote:

 Hi there,

 I was wondering whether we could have one or two of
 our UNIX build bots create a code coverage profile
 and make the results available online.

 Here is what I use in my test runner script:

 [[[
 env CFLAGS='-fprofile-arcs -ftest-coverage' ./configure --disable-shared
 --enable-maintainer-mode $moreopts

 make -sj 2 /dev/null  /dev/null
 make svnserveautocheck PARALLEL=1

 lcov -d . -b . -c -o tests.lcov  lcov.log
 genhtml tests.lcov -o html  genhtml.log
 ]]]


+1 to test coverage stats.

We have an ancient patch in our repo for making Subversion build and use
gcov to produce coverage information.  I think it's in
tools/dev/gcov.patch.  I don't know if it's useful or not, but as you're
looking at this topic, it might be a good time to revisit whether we need
it there or not.

-Hyrum


Re: svn commit: r1411629 - /subversion/trunk/subversion/tests/libsvn_fs/fs-test.c

2012-11-24 Thread Hyrum K Wright
Sure, if philip thinks it's appropriate.


On Sat, Nov 24, 2012 at 1:08 AM, Daniel Shahaf d...@daniel.shahaf.namewrote:

 Revert r1412515 then?

 Hyrum K Wright wrote on Fri, Nov 23, 2012 at 15:29:32 -0500:
  I did a little poking and fixed this in r1413046.
 
 
  On Fri, Nov 23, 2012 at 2:18 PM, Hyrum K Wright hy...@hyrumwright.org
 wrote:
 
   This test isn't cleaning up after itself (or before a subsequent run).
The first time I run it in a working copy, it passes, but the next
 time, I
   get the following error:
  
   $ ./fs-test 37
   subversion/tests/libsvn_fs/fs-test.c:4908: (apr_err=160033)
   subversion/tests/svn_test_fs.c:183: (apr_err=160033)
   subversion/tests/svn_test_fs.c:121: (apr_err=160033)
   svn_tests: E160033: cannot create fs 'test-delete-fs' there is already
 a
   directory of that name
   subversion/libsvn_fs/fs-loader.c:515: (apr_err=160033)
   subversion/libsvn_fs/fs-loader.c:322: (apr_err=160033)
   subversion/libsvn_fs/fs-loader.c:162: (apr_err=160033)
   svn_tests: E160033: Failed to load module for FS type 'bdb'
   FAIL:  fs-test 37: test svn_fs_delete_fs
  
   I suspect there's some boilerplate somewhere that was left out.
  
   -Hyrum
  
  
   On Tue, Nov 20, 2012 at 6:52 AM, phi...@apache.org wrote:
  
   Author: philip
   Date: Tue Nov 20 11:52:56 2012
   New Revision: 1411629
  
   URL: http://svn.apache.org/viewvc?rev=1411629view=rev
   Log:
   Explicitly test svn_fs_delete_fs.
  
   * subversion/tests/libsvn_fs/fs-test.c
 (delete_fs): New test.
 (test_list): Add new test.
  
   Modified:
   subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
  
   Modified: subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
   URL:
  
 http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_fs/fs-test.c?rev=1411629r1=1411628r2=1411629view=diff
  
  
 ==
   --- subversion/trunk/subversion/tests/libsvn_fs/fs-test.c (original)
   +++ subversion/trunk/subversion/tests/libsvn_fs/fs-test.c Tue Nov 20
   11:52:56 2012
   @@ -4896,6 +4896,26 @@ node_history(const svn_test_opts_t *opts
  return SVN_NO_ERROR;
}
  
   +/* Test svn_fs_delete_fs(). */
   +static svn_error_t *
   +delete_fs(const svn_test_opts_t *opts,
   + apr_pool_t *pool)
   +{
   +  svn_fs_t *fs;
   +  const char *path;
   +  svn_node_kind_t kind;
   +
   +  SVN_ERR(svn_test__create_fs(fs, test-delete-fs, opts, pool));
   +  path = svn_fs_path(fs, pool);
   +  SVN_ERR(svn_io_check_path(path, kind, pool));
   +  SVN_TEST_ASSERT(kind != svn_node_none);
   +  SVN_ERR(svn_fs_delete_fs(path, pool));
   +  SVN_ERR(svn_io_check_path(path, kind, pool));
   +  SVN_TEST_ASSERT(kind == svn_node_none);
   +
   +  return SVN_NO_ERROR;
   +}
   +
  
  
/*
  
  */
   @@ -4979,5 +4999,7 @@ struct svn_test_descriptor_t test_funcs[
   create and modify small file),
SVN_TEST_OPTS_PASS(node_history,
   test svn_fs_node_history),
   +SVN_TEST_OPTS_PASS(delete_fs,
   +   test svn_fs_delete_fs),
SVN_TEST_NULL
  };
  
  
  
  



Re: svn commit: r1413109 - in /subversion/trunk: ./ subversion/svnrdump/svnrdump.c subversion/svnrdump/svnrdump.h

2012-11-24 Thread Hyrum K Wright
On Fri, Nov 23, 2012 at 7:44 PM, Branko Čibej br...@wandisco.com wrote:

 On 24.11.2012 01:39, hwri...@apache.org wrote:
  Author: hwright
  Date: Sat Nov 24 00:39:38 2012
  New Revision: 1413109
 
  URL: http://svn.apache.org/viewvc?rev=1413109view=rev
  Log:
  Cherrypick r1413107 from the ev2-export branch.  I'm going to be brave
 here
  and develop on trunk.  The plan is to implement the Ev2 editor in
 parallel
  with the existing implementation, which should negatively impact 1.8
  releasability.

 I'm going to assume you meant should *not* in that last line.

 -- Brane


Correct.  Fixed.


Re: svn commit: r1411629 - /subversion/trunk/subversion/tests/libsvn_fs/fs-test.c

2012-11-23 Thread Hyrum K Wright
This test isn't cleaning up after itself (or before a subsequent run).  The
first time I run it in a working copy, it passes, but the next time, I get
the following error:

$ ./fs-test 37
subversion/tests/libsvn_fs/fs-test.c:4908: (apr_err=160033)
subversion/tests/svn_test_fs.c:183: (apr_err=160033)
subversion/tests/svn_test_fs.c:121: (apr_err=160033)
svn_tests: E160033: cannot create fs 'test-delete-fs' there is already a
directory of that name
subversion/libsvn_fs/fs-loader.c:515: (apr_err=160033)
subversion/libsvn_fs/fs-loader.c:322: (apr_err=160033)
subversion/libsvn_fs/fs-loader.c:162: (apr_err=160033)
svn_tests: E160033: Failed to load module for FS type 'bdb'
FAIL:  fs-test 37: test svn_fs_delete_fs

I suspect there's some boilerplate somewhere that was left out.

-Hyrum


On Tue, Nov 20, 2012 at 6:52 AM, phi...@apache.org wrote:

 Author: philip
 Date: Tue Nov 20 11:52:56 2012
 New Revision: 1411629

 URL: http://svn.apache.org/viewvc?rev=1411629view=rev
 Log:
 Explicitly test svn_fs_delete_fs.

 * subversion/tests/libsvn_fs/fs-test.c
   (delete_fs): New test.
   (test_list): Add new test.

 Modified:
 subversion/trunk/subversion/tests/libsvn_fs/fs-test.c

 Modified: subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
 URL:
 http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_fs/fs-test.c?rev=1411629r1=1411628r2=1411629view=diff

 ==
 --- subversion/trunk/subversion/tests/libsvn_fs/fs-test.c (original)
 +++ subversion/trunk/subversion/tests/libsvn_fs/fs-test.c Tue Nov 20
 11:52:56 2012
 @@ -4896,6 +4896,26 @@ node_history(const svn_test_opts_t *opts
return SVN_NO_ERROR;
  }

 +/* Test svn_fs_delete_fs(). */
 +static svn_error_t *
 +delete_fs(const svn_test_opts_t *opts,
 + apr_pool_t *pool)
 +{
 +  svn_fs_t *fs;
 +  const char *path;
 +  svn_node_kind_t kind;
 +
 +  SVN_ERR(svn_test__create_fs(fs, test-delete-fs, opts, pool));
 +  path = svn_fs_path(fs, pool);
 +  SVN_ERR(svn_io_check_path(path, kind, pool));
 +  SVN_TEST_ASSERT(kind != svn_node_none);
 +  SVN_ERR(svn_fs_delete_fs(path, pool));
 +  SVN_ERR(svn_io_check_path(path, kind, pool));
 +  SVN_TEST_ASSERT(kind == svn_node_none);
 +
 +  return SVN_NO_ERROR;
 +}
 +


  /*
  */
 @@ -4979,5 +4999,7 @@ struct svn_test_descriptor_t test_funcs[
 create and modify small file),
  SVN_TEST_OPTS_PASS(node_history,
 test svn_fs_node_history),
 +SVN_TEST_OPTS_PASS(delete_fs,
 +   test svn_fs_delete_fs),
  SVN_TEST_NULL
};





Re: svn commit: r1411629 - /subversion/trunk/subversion/tests/libsvn_fs/fs-test.c

2012-11-23 Thread Hyrum K Wright
I did a little poking and fixed this in r1413046.


On Fri, Nov 23, 2012 at 2:18 PM, Hyrum K Wright hy...@hyrumwright.orgwrote:

 This test isn't cleaning up after itself (or before a subsequent run).
  The first time I run it in a working copy, it passes, but the next time, I
 get the following error:

 $ ./fs-test 37
 subversion/tests/libsvn_fs/fs-test.c:4908: (apr_err=160033)
 subversion/tests/svn_test_fs.c:183: (apr_err=160033)
 subversion/tests/svn_test_fs.c:121: (apr_err=160033)
 svn_tests: E160033: cannot create fs 'test-delete-fs' there is already a
 directory of that name
 subversion/libsvn_fs/fs-loader.c:515: (apr_err=160033)
 subversion/libsvn_fs/fs-loader.c:322: (apr_err=160033)
 subversion/libsvn_fs/fs-loader.c:162: (apr_err=160033)
 svn_tests: E160033: Failed to load module for FS type 'bdb'
 FAIL:  fs-test 37: test svn_fs_delete_fs

 I suspect there's some boilerplate somewhere that was left out.

 -Hyrum


 On Tue, Nov 20, 2012 at 6:52 AM, phi...@apache.org wrote:

 Author: philip
 Date: Tue Nov 20 11:52:56 2012
 New Revision: 1411629

 URL: http://svn.apache.org/viewvc?rev=1411629view=rev
 Log:
 Explicitly test svn_fs_delete_fs.

 * subversion/tests/libsvn_fs/fs-test.c
   (delete_fs): New test.
   (test_list): Add new test.

 Modified:
 subversion/trunk/subversion/tests/libsvn_fs/fs-test.c

 Modified: subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
 URL:
 http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_fs/fs-test.c?rev=1411629r1=1411628r2=1411629view=diff

 ==
 --- subversion/trunk/subversion/tests/libsvn_fs/fs-test.c (original)
 +++ subversion/trunk/subversion/tests/libsvn_fs/fs-test.c Tue Nov 20
 11:52:56 2012
 @@ -4896,6 +4896,26 @@ node_history(const svn_test_opts_t *opts
return SVN_NO_ERROR;
  }

 +/* Test svn_fs_delete_fs(). */
 +static svn_error_t *
 +delete_fs(const svn_test_opts_t *opts,
 + apr_pool_t *pool)
 +{
 +  svn_fs_t *fs;
 +  const char *path;
 +  svn_node_kind_t kind;
 +
 +  SVN_ERR(svn_test__create_fs(fs, test-delete-fs, opts, pool));
 +  path = svn_fs_path(fs, pool);
 +  SVN_ERR(svn_io_check_path(path, kind, pool));
 +  SVN_TEST_ASSERT(kind != svn_node_none);
 +  SVN_ERR(svn_fs_delete_fs(path, pool));
 +  SVN_ERR(svn_io_check_path(path, kind, pool));
 +  SVN_TEST_ASSERT(kind == svn_node_none);
 +
 +  return SVN_NO_ERROR;
 +}
 +


  /*
  */
 @@ -4979,5 +4999,7 @@ struct svn_test_descriptor_t test_funcs[
 create and modify small file),
  SVN_TEST_OPTS_PASS(node_history,
 test svn_fs_node_history),
 +SVN_TEST_OPTS_PASS(delete_fs,
 +   test svn_fs_delete_fs),
  SVN_TEST_NULL
};






Re: 1.8 Progress

2012-11-01 Thread Hyrum K Wright
On Thu, Nov 1, 2012 at 3:11 PM, C. Michael Pilato cmpil...@collab.net wrote:
 On 11/01/2012 02:42 PM, Ben Reser wrote:
 2) Ev2.  The notes say this is believed to be in a releasable state?  Is
 there any work needed to verify this?  Do we need to remove the use of Ev2
 in any place to avoid releasing with compatibility shims in use? Are we
 comfortable that the API is complete?

 Honestly, I'm a bit concerned that with Hyrum and Greg both effectively
 inactive, Ev2 runs the risk of appearing in our public API without any
 remaining active champions/implementors/maintainers of it.  There's a
 non-zero fear factor here.

Understandably so.  I've been trying to leave breadcrumbs and beg for
help over the past several months, but nobody has taken me up on it.
I've been hacking around on Ev2 as I can, but Real Life is leaving
little time for that these days.  I can consult, direct and review,
but my hacking time is very limited.

We can always shuffle headers around or document the things as
experimental, so committing ourselves to the API as this point isn't
my concern.  The only real limiting around Ev2 and 1.8 is issue #4116
which is svnrdump failures over ra_serf.  In the issue, I propose
using Ev2 to get around the problem, since the dumpfile format is so
incongruent with the editor.

Of course, we don't *have* to do that, but as I've thought about it,
any solution will require a bit o' caching---which we've already
implemented as part of the Ev2 shims.  We *might* be able to implement
the svnrdump editor as Ev2, shim the thing on the client side (which
gives us the required caching) and release that way.  Or there might
be a better solution I'm overlooking because I've got Ev2 on the
brain.

 3) libsvn_ra_serf stabilization.  I know there have been a couple concerns
 that Philip has raised (EAGAIN and the random failures).  Plus there are
 several issues here (not all of the issues here are serf
 issues):

 Who can drive these issues to completion?  Is there any additional testing
 work we should do to try and determine the stability of serf in light of
 the fact that we're planning to remove neon?

 I've been steadily working on the ra_serf stuff, but there are some issues
 which appear to need some deeper focus by members of the Serf dev-team.
 Lieven mentioned a bit ago that he was planning to invest a block of time
 here soon (ApacheCon Europe hackathan, perhaps it was?) -- I'm hoping we'll
 see some significant progress on ra_serf stabilization as a result.

 5) Inherited properties/Server-dictated configuration.  This is marked as
 completed but I see some discussion over property names still ongoing.

 To the best of my knowledge, it's only this superficial property name
 discussion that's still unresolved.

 Beyond that we have the ordinary reviews of tests (pburba has said he's
 working on this), new apis and issue triage (cmpilato seems to have been
 doing some issue triage).

 Also at the risk of opening a can of worms we need to decide on the wc
 upgrade issue?  I can say that the impression I got from Subversion Live was
 that a lot of people use multiple clients and that auto-upgrade seems bad.
  But we also discussed trying to handle reads from an older wcng style wc
 without requiring a wc upgrade.  Can someone drive this?

 I distinctly recall stsp offering to take up the charge on this. :-)

 Lastly I don't want to give the impression that I'm rushing 1.8.  However, I
 would like us to see us focus on the things we want to get done with 1.8.

 Rushing 1.8?  No, that's not this community's style.  But I'm starting to
 see feedback from our user base that the slippage we've allowed in the
 release already is not inspiring confidence.  I'm trying to keep my focus on
 1.8.  There are plenty of other things I'd love to get my fingers into
 (FSv2, master passphrase, etc.) but at some point we need to remain good
 stewards of our user base.

+1, and I'm sorry I can't help more.


Re: [Subversion Wiki] Update of FS2_Design by brane

2012-10-24 Thread Hyrum K Wright
On Wed, Oct 24, 2012 at 11:25 PM, Apache subversion Wiki
comm...@subversion.apache.org wrote:
 Dear Wiki user,

 You have subscribed to a wiki page or wiki category on Subversion Wiki for 
 change notification.

 The FS2_Design page has been changed by brane:
 http://wiki.apache.org/subversion/FS2_Design?action=diffrev1=2rev2=3

 Comment:
 Describe versioned filesystem concepts.

   = The Versioned Filesystem =

 + Subversion's versioned filesystem is a set of snapshots of the state of a 
 tree of nodes. Changes to the tree are described by ''transactions'', which 
 are a set of mutations of the tree state. Discrete states of the tree are 
 called ''revisions'' and are represented by immutable transactions. Each 
 snapshot (except for the first, which has none and is called ''revision 0'') 
 has exactly one predecessor and at most one immutable successor. Any number 
 of mutable transactions can exist at the same time. The set of revisions is a 
 temporally ordered sequence; the last or ''youngest'' revision is called the 
 ''HEAD''. New revisions are created by ''merging'' the changes described by a 
 mutable transaction with the tree state represented by HEAD and appending the 
 resulting revision to the sequence, creating a new HEAD.
 +
 + [[attachment:vfs-snapshot-sequence.svg]]

This might be more helpful if it was an image type the wiki could
embed.  When I went to the page, it didn't want to show me the image.

 +
 + Nodes are either ''files'', which are containers of unstructured data, or 
 ''directories'', which are lists of named files and directories. The root 
 node of this tree is always a directory. At any point in time during the 
 filesystem's lifetime, each node (except the root) will have exactly one 
 parent.
 +


Re: svn commit: r1400545 - /subversion/trunk/subversion/libsvn_ra/ra_loader.c

2012-10-22 Thread Hyrum K Wright
On Mon, Oct 22, 2012 at 7:55 AM, Philip Martin
philip.mar...@wandisco.com wrote:
 hwri...@apache.org writes:

 Author: hwright
 Date: Sun Oct 21 01:18:26 2012
 New Revision: 1400545

 URL: http://svn.apache.org/viewvc?rev=1400545view=rev
 Log:
 Refactor the pre-1.5 fallback code for replay_ranges into a separate 
 function.

 * subversion/libsvn_ra/ra_loader.c
   (replay_range_from_replays): New.
   (svn_ra_replay_range): Call the factored out code.

 This causes svnrdump_tests.py 43 and lots of svnsync_tests.py to FAIL
 over ra_svn.  It looks like an innocuous change, I can't see the error
 in the code.

Thanks for pointing out the failures.  I won't be able to get around
to fixing it for (at least) several hours, so feel free to revert in
the meantime if it's a large issue.

-Hyrum


Re: svn commit: r1400545 - /subversion/trunk/subversion/libsvn_ra/ra_loader.c

2012-10-22 Thread Hyrum K Wright
On Mon, Oct 22, 2012 at 11:46 AM, Philip Martin
philip.mar...@wandisco.com wrote:
 Hyrum K Wright hy...@hyrumwright.org writes:

 On Mon, Oct 22, 2012 at 7:55 AM, Philip Martin
 philip.mar...@wandisco.com wrote:
 hwri...@apache.org writes:

 Author: hwright
 Date: Sun Oct 21 01:18:26 2012
 New Revision: 1400545

 URL: http://svn.apache.org/viewvc?rev=1400545view=rev
 Log:
 Refactor the pre-1.5 fallback code for replay_ranges into a separate 
 function.

 * subversion/libsvn_ra/ra_loader.c
   (replay_range_from_replays): New.
   (svn_ra_replay_range): Call the factored out code.

 This causes svnrdump_tests.py 43 and lots of svnsync_tests.py to FAIL
 over ra_svn.  It looks like an innocuous change, I can't see the error
 in the code.

 Thanks for pointing out the failures.  I won't be able to get around
 to fixing it for (at least) several hours, so feel free to revert in
 the meantime if it's a large issue.

 Worked it out:

   svn_error_t *err =
 session-vtable-replay_range(session, start_revision, end_revision,
   low_water_mark, text_deltas,
   revstart_func, revfinish_func,
   replay_baton, pool);

   if (err  (err-apr_err != SVN_ERR_RA_NOT_IMPLEMENTED))
 return svn_error_trace(err);

   svn_error_clear(err);
   return svn_error_trace(replay_range_from_replays(session, start_revision,
end_revision,
low_water_mark,
text_deltas,
revstart_func,
revfinish_func,
replay_baton, pool));

 When the vtable call returns SVN_NO_ERROR the code should return but
 instead goes on to call replay_range_from_replays.

Thanks.  I generally like to use the early-return rather than
big-if-statement construction as I think it highlights readability,
but not if I sacrifice correctness to get there!  Thanks for fixing.

-Hyrum


Re: Sparse Externals

2012-10-18 Thread Hyrum K Wright
On Thu, Oct 18, 2012 at 5:30 PM, Ben Reser b...@reser.org wrote:
 Regarding this issue: http://subversion.tigris.org/issues/show_bug.cgi?id=3311

 We don't support using --depth options other than infinity with
 externals.  Bert mentions that wc-ng should make it easier to
 implement this.

 However, given the way svn:externals can be defined I think that it's
 really hard to implement this without a performance hit or without
 having users frustrated by the way it works.  In particular you can
 define svn:externals at a different level than where you're putting
 them.  If we wanted this to work the way most people I think would
 expect them to work you'd end up needing to walk all the way up to the
 repo root looking for externals on every checkout/update.  Which
 stinks.

 We could punt on that but I'm not sure I like the idea of just
 continuing to apply hacks upon a poor design.  So I'm wondering if it
 wouldn't be better to just replace externals with something that
 resolves a lot of other issues and gives us a more consistent
 behavior.  While leaving the existing externals implementation alone.

 What does everyone else think?  Anyone have put any thought into
 replacing externals?

In theory, the wc-ng data model allows arbitrary nodes in the working
copy to reference arbitrary repositories and paths in those
repository.  This would be a great way to *implement* externals,
leaving the property merely a UI to set and change them.  We could
also introduce a new UI for doing so.

The problem, though is backwards compat with a heterogenous client
environment, so I'd think the property still has to be shuffled
around. (*sigh*)

-Hyrum


Regular expressions in Subversion

2012-10-17 Thread Hyrum K Wright
There are several places where regular expressions would be useful in
Subversion.  Off hand, the new log --search feature and svn:ignore
properties feel like they'd be use candidates for regexs, and they
could probably also apply to authz rules eventually.  I'm sure there
are more.

Historically, the argument against using regexes in Subversion was
that they would be a potential DoS target, or could lead to unexpected
performance problems.  However, I recently ran across a new regex
engine, RE2, which claims to have linear time complexity in the size
of the input with the ability to also limit memory consumption[1].
These come at the expenses of a couple of less-used regex features,
and it feels like it'd be a good fit for Subversion.

There are a few downsides:
 * RE2 is written in C++; we'd need a C wrapper to use it within Subversion.
 * RE2 packages don't exist for a number of platforms, though we might
be able to embedded it in Subversion.
 * RE2 doesn't claim to compile on Windows. :)

Anyway, I was just wondering what folks feelings were about this
possibility, and whether it's finally time to start thinking about
proper regex support within Subversion.

-Hyrum

[1] https://code.google.com/p/re2/


Re: Regular expressions in Subversion

2012-10-17 Thread Hyrum K Wright
On Wed, Oct 17, 2012 at 1:22 PM, Daniel Shahaf d...@daniel.shahaf.name wrote:
 Hyrum K Wright wrote on Wed, Oct 17, 2012 at 12:20:20 -0400:
 These come at the expenses of a couple of less-used regex features,

 PLease be objective/specific: it doesn't support backreferences and
 zero-width lookarounds.

Correct (sorry for the hand waving).  I envision our use cases being
more pattern matching-like, and not search-and-replace.

-Hyrum


Re: svn commit: r1399064 - in /subversion/trunk/subversion/bindings/javahl: native/ src/org/apache/subversion/javahl/

2012-10-16 Thread Hyrum K Wright
On Tue, Oct 16, 2012 at 9:36 PM, Mark Phippard markp...@gmail.com wrote:

 On Oct 16, 2012, at 9:25 PM, Hyrum Wright hwri...@apache.org wrote:

 On Tue, Oct 16, 2012 at 9:19 PM,  hwri...@apache.org wrote:
 Author: hwright
 Date: Wed Oct 17 01:19:21 2012
 New Revision: 1399064

 URL: http://svn.apache.org/viewvc?rev=1399064view=rev
 Log:
 JavaHL: Punch additional changelist parameter for property fetching to
 the Java layer.

 [ in subversion/bindings/javahl/ ]
 * native/SVNClient.cpp,
  native/SVNClient.h
  (SVNClient::propertyGet): Add changelists param.

 * native/org_apache_subversion_javahl_SVNClient.cpp
  (Java_org_apache_subversion_javahl_SVNClient_propertyGet):
Add changelists param, pass to C++ layer.

 * src/org/apache/subversion/javahl/ISVNClient.java,
  src/org/apache/subversion/javahl/SVNClient.java
  (propertyGet): Add overload to accept changelists param.

 Mark,
 This revision doesn't provide a way to return the inherited props,
 which the underlying API does.  Right now the obvious method is a new
 return object which contains both the property and any inherited
 props, but I'd hate to create an object with just two fields.  Any
 other suggestions?


 How about returning a Map where the path is key?

Well, we have to return both the existing property *and* the inherited
props.  I don't think a Map will meet those criteria.

-Hyrum


Re: svn commit: r1387696 - /subversion/trunk/subversion/libsvn_wc/conflicts.c

2012-10-11 Thread Hyrum K Wright
On Wed, Sep 19, 2012 at 2:06 PM,  s...@apache.org wrote:
 Author: stsp
 Date: Wed Sep 19 18:06:13 2012
 New Revision: 1387696

 URL: http://svn.apache.org/viewvc?rev=1387696view=rev
 Log:
 * subversion/libsvn_wc/conflicts.c
   (read_prop_conflicts): New helper to prepare for the times when new
 conflict storage is enabled. Currently, this describes each property
 conflict in a separate svn_wc_conflict_description2_t, within the
 limitations of svn_wc_conflict_description2_t. These limitations
 and suggested improvements have been recorded in comments.
   (svn_wc__read_conflicts): Use the new read_prop_conflicts() helper
instead of creating just one conflict description.

 Modified:
 subversion/trunk/subversion/libsvn_wc/conflicts.c

 Modified: subversion/trunk/subversion/libsvn_wc/conflicts.c
 URL: 
 http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/conflicts.c?rev=1387696r1=1387695r2=1387696view=diff
 ==
 --- subversion/trunk/subversion/libsvn_wc/conflicts.c (original)
 +++ subversion/trunk/subversion/libsvn_wc/conflicts.c Wed Sep 19 18:06:13 2012
 @@ -1907,6 +1907,153 @@ svn_wc__conflict_invoke_resolver(svn_wc_
return SVN_NO_ERROR;
  }

 +/* Read all property conflicts contained in CONFLICT_SKEL into
 + * individual conflict descriptions, and append those descriptions
 + * to the CONFLICTS array. Allocate results in RESULT_POOL.
 + * SCRATCH_POOL is used for temporary allocations. */
 +static svn_error_t *
 +read_prop_conflicts(apr_array_header_t *conflicts,
 +svn_wc__db_t *db,
 +const char *local_abspath,
 +svn_skel_t *conflict_skel,
 +apr_pool_t *result_pool,
 +apr_pool_t *scratch_pool)
 +{
 +  const char *prop_reject_file;
 +  apr_hash_t *my_props;
 +  apr_hash_t *their_old_props;
 +  apr_hash_t *their_props;
 +  apr_hash_t *conflicted_props;
 +  apr_hash_index_t *hi;
 +  apr_pool_t *iterpool;
 +
 +  SVN_ERR(svn_wc__conflict_read_prop_conflict(prop_reject_file,
 +  my_props,
 +  their_old_props,
 +  their_props,
 +  conflicted_props,
 +  db, local_abspath,
 +  conflict_skel,
 +  scratch_pool, scratch_pool));
 +
 +  if (apr_hash_count(conflicted_props) == 0)
 +{
 +  /* Legacy prop conflict with only a .reject file. */
 +  svn_wc_conflict_description2_t *desc;
 +
 +  desc  = svn_wc_conflict_description_create_prop2(local_abspath,
 +   svn_node_unknown,
 +   , result_pool);
 +
 +  /* ### This should be changed. The prej file should be stored
 +   * ### separately from the other files. We need to rev the
 +   * ### conflict description struct for this. */
 +  desc-their_abspath = apr_pstrdup(result_pool, prop_reject_file);
 +
 +  APR_ARRAY_PUSH(conflicts, svn_wc_conflict_description2_t*) = desc;
 +
 +  return SVN_NO_ERROR;
 +}
 +
 +  iterpool = svn_pool_create(scratch_pool);
 +  for (hi = apr_hash_first(scratch_pool, conflicted_props);
 +   hi;
 +   hi = apr_hash_next(hi))
 +{
 +  const char *propname = svn__apr_hash_index_key(hi);
 +  svn_string_t *old_value;
 +  svn_string_t *my_value;
 +  svn_string_t *their_value;
 +  svn_wc_conflict_description2_t *desc;
 +
 +  svn_pool_clear(iterpool);
 +
 +  desc  = svn_wc_conflict_description_create_prop2(local_abspath,
 +   svn_node_unknown,
 +   propname,
 +   result_pool);
 +
 +  desc-property_name = apr_pstrdup(result_pool, propname);
 +
 +  my_value = apr_hash_get(my_props, propname, APR_HASH_KEY_STRING);
 +  their_value = apr_hash_get(their_props, propname,
 + APR_HASH_KEY_STRING);
 +
 +  /* Compute the incoming side of the conflict ('action'). */
 +  if (their_value == NULL)
 +desc-action = svn_wc_conflict_action_delete;
 +  else if (my_value == NULL)
 +desc-action = svn_wc_conflict_action_add;

Wrong enum type assignment here, the source is an
svn_wc_conflict_reason_t, but the destination is an
svn_wc_conflict_action_t.  While this might work now, it feels like a
real opportunity for obscure bugs later.  (And my compiler complains
about it. :) )

 +  else
 +desc-action = svn_wc_conflict_action_edit;

Same.

 +
 +  /* Compute the local side of the conflict ('reason'). */
 +  if (my_value == NULL)
 +

Re: svn commit: r1390438 - in /subversion/trunk: ./ subversion/include/private/ subversion/libsvn_ra_svn/ subversion/libsvn_subr/ tools/client-side/svn-bench/ tools/dist/

2012-10-11 Thread Hyrum K Wright
On Wed, Oct 10, 2012 at 1:46 PM, Hyrum K Wright hy...@hyrumwright.org wrote:
 On Wed, Sep 26, 2012 at 8:39 AM,  stef...@apache.org wrote:
 Author: stefan2
 Date: Wed Sep 26 12:38:59 2012
 New Revision: 1390438

 URL: http://svn.apache.org/viewvc?rev=1390438view=rev
 Log:
 Merge second batch of changes from the 10Gb branch. These add the
 svn-bench tool.
 ...
 Modified: subversion/trunk/subversion/libsvn_subr/string.c
 URL: 
 http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/string.c?rev=1390438r1=1390437r2=1390438view=diff
 ==
 --- subversion/trunk/subversion/libsvn_subr/string.c (original)
 +++ subversion/trunk/subversion/libsvn_subr/string.c Wed Sep 26 12:38:59 2012
 @@ -1093,3 +1093,44 @@ svn__i64toa(char * dest, apr_int64_t num
*dest = '-';
return svn__ui64toa(dest + 1, (apr_uint64_t)(0-number)) + 1;
  }
 +
 +static void
 +ui64toa_sep(apr_uint64_t number, char seperator, char *buffer)
 +{
 +  apr_size_t length = svn__ui64toa(buffer, number);
 +  apr_size_t i;
 +
 +  for (i = length; i  3; i -= 3)
 +{
 +  memmove(buffer[i - 2], buffer[i - 3], length - i + 3);
 +  buffer[i-3] = seperator;
 +  length++;
 +}
 +
 +  buffer[length] = 0;
 +}
 +
 +char *
 +svn__ui64toa_sep(apr_uint64_t number, char seperator, apr_pool_t *pool)
 +{
 +  char buffer[2 * SVN_INT64_BUFFER_SIZE];
 +  ui64toa_sep(number, seperator, buffer);
 +
 +  return apr_pstrdup(pool, buffer);
 +}
 +
 +char *
 +svn__i64toa_sep(apr_uint64_t number, char seperator, apr_pool_t *pool)

 This function name looks like it takes a signed integer, but the
 actual argument type is an unsigned int...

 +{
 +  char buffer[2 * SVN_INT64_BUFFER_SIZE];
 +  if (number  0)

 ...which means this conditional will never evaluate as true.

 +{
 +  buffer[0] = '-';
 +  ui64toa_sep((apr_uint64_t)(-number), seperator, buffer[1]);
 +}
 +  else
 +ui64toa_sep((apr_uint64_t)(number), seperator, buffer);
 +
 +  return apr_pstrdup(pool, buffer);
 +}
 ...

Talked to Stefan about this in person, and updated in r1397148.

-Hyrum


Re: svn commit: r1387696 - /subversion/trunk/subversion/libsvn_wc/conflicts.c

2012-10-11 Thread Hyrum K Wright
On Thu, Oct 11, 2012 at 3:01 PM, Stefan Sperling s...@elego.de wrote:
 On Thu, Oct 11, 2012 at 12:20:32PM -0400, Hyrum K Wright wrote:
  +  /* Compute the incoming side of the conflict ('action'). */
  +  if (their_value == NULL)
  +desc-action = svn_wc_conflict_action_delete;
  +  else if (my_value == NULL)
  +desc-action = svn_wc_conflict_action_add;

 Wrong enum type assignment here, the source is an
 svn_wc_conflict_reason_t, but the destination is an
 svn_wc_conflict_action_t.

 Really? I see neither svn_wc_conflict_action_delete nor
 svn_wc_conflict_action_add in the definition of svn_wc_conflict_reason_t.

Whoops, sorry, I should have referenced a different line below,
currently line 2003.


 While this might work now, it feels like a
 real opportunity for obscure bugs later.  (And my compiler complains
 about it. :) )

  +  else
  +desc-action = svn_wc_conflict_action_edit;

 Same.

 Seems fine to me. Both are saying 'action'... did you mean to
 quote a different part of the code?

Yes, I meant to quote a different part.

This is the compiler warning I see:
subversion/libsvn_wc/conflicts.c:2003:24: warning: implicit conversion from
  enumeration type 'enum svn_wc_conflict_reason_t' to different
enumeration type
  'svn_wc_conflict_action_t' (aka 'enum svn_wc_conflict_action_t')
  [-Wconversion]
desc-action = svn_wc_conflict_reason_added;
 ~ ^~~~
subversion/libsvn_wc/conflicts.c:2005:24: warning: implicit conversion from
  enumeration type 'enum svn_wc_conflict_reason_t' to different
enumeration type
  'svn_wc_conflict_action_t' (aka 'enum svn_wc_conflict_action_t')
  [-Wconversion]
desc-action = svn_wc_conflict_reason_edited;
 ~ ^

Does this (untested) patch make sense?

[[[
Index: subversion/libsvn_wc/conflicts.c
===
--- subversion/libsvn_wc/conflicts.c(revision 1397318)
+++ subversion/libsvn_wc/conflicts.c(working copy)
@@ -2000,9 +2000,9 @@
   if (my_value == NULL)
 desc-reason = svn_wc_conflict_reason_deleted;
   else if (their_value == NULL)
-desc-action = svn_wc_conflict_reason_added;
+desc-reason = svn_wc_conflict_reason_added;
   else
-desc-action = svn_wc_conflict_reason_edited;
+desc-reason = svn_wc_conflict_reason_edited;

   /* ### This should be changed. The prej file should be stored
* ### separately from the other files. We need to rev the
]]]

-Hyrum


Re: svn commit: r1396826 - /subversion/trunk/subversion/libsvn_fs_fs/tree.c

2012-10-11 Thread Hyrum K Wright
Updated in r1397331.

On Thu, Oct 11, 2012 at 1:09 AM, Ben Reser b...@reser.org wrote:
 They are indeed.  It's always bothered me that we didn't use a stable
 type for our revnums:

 $ grep svn_revnum_t subversion/include/svn_types.h | grep typedef
 typedef long int svn_revnum_t;

 On Wed, Oct 10, 2012 at 3:31 PM, Greg Stein gst...@gmail.com wrote:
 Eh? I thought revnums were longs. Not 64-bit (except by happenstance)

 On Oct 10, 2012 3:06 PM, hwri...@apache.org wrote:

 Author: hwright
 Date: Wed Oct 10 22:05:34 2012
 New Revision: 1396826

 URL: http://svn.apache.org/viewvc?rev=1396826view=rev
 Log:
 Avoid integer size mismatch warning.

 * subversion/libsvn_fs_fs/tree.c
   (cache_entry_t, cache_lookup): Adjust hash_value to match the size of
 the
 revision number, which is an input to the hash value.

 Modified:
 subversion/trunk/subversion/libsvn_fs_fs/tree.c

 Modified: subversion/trunk/subversion/libsvn_fs_fs/tree.c
 URL:
 http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/tree.c?rev=1396826r1=1396825r2=1396826view=diff

 ==
 --- subversion/trunk/subversion/libsvn_fs_fs/tree.c (original)
 +++ subversion/trunk/subversion/libsvn_fs_fs/tree.c Wed Oct 10 22:05:34
 2012
 @@ -148,7 +148,7 @@ typedef struct cache_entry_t
  {
/* hash value derived from PATH, REVISION.
   Used to short-circuit failed lookups. */
 -  apr_uint32_t hash_value;
 +  apr_uint64_t hash_value;

/* revision to which the NODE belongs */
svn_revnum_t revision;
 @@ -337,7 +337,7 @@ cache_lookup( fs_fs_dag_cache_t *cache
  {
apr_size_t i, bucket_index;
apr_size_t path_len = strlen(path);
 -  apr_uint32_t hash_value = revision;
 +  apr_uint64_t hash_value = revision;

/* optimistic lookup: hit the same bucket again? */
cache_entry_t *result = cache-buckets[cache-last_hit];





Re: svn commit: r1387696 - /subversion/trunk/subversion/libsvn_wc/conflicts.c

2012-10-11 Thread Hyrum K Wright
On Thu, Oct 11, 2012 at 7:15 PM, Stefan Sperling s...@elego.de wrote:
 On Thu, Oct 11, 2012 at 05:25:03PM -0400, Hyrum K Wright wrote:
 Does this (untested) patch make sense?

 Oh yes, very much! Thanks!

r1397425

-Hyrum


 [[[
 Index: subversion/libsvn_wc/conflicts.c
 ===
 --- subversion/libsvn_wc/conflicts.c  (revision 1397318)
 +++ subversion/libsvn_wc/conflicts.c  (working copy)
 @@ -2000,9 +2000,9 @@
if (my_value == NULL)
  desc-reason = svn_wc_conflict_reason_deleted;
else if (their_value == NULL)
 -desc-action = svn_wc_conflict_reason_added;
 +desc-reason = svn_wc_conflict_reason_added;
else
 -desc-action = svn_wc_conflict_reason_edited;
 +desc-reason = svn_wc_conflict_reason_edited;

/* ### This should be changed. The prej file should be stored
 * ### separately from the other files. We need to rev the
 ]]]

 -Hyrum


Re: Dead code or bug in gpg_agent.c

2012-10-10 Thread Hyrum K Wright
On Tue, Oct 9, 2012 at 4:34 AM, Daniel Shahaf d...@daniel.shahaf.name wrote:
 Hyrum K Wright wrote on Mon, Oct 08, 2012 at 21:51:02 -0400:
 Poking around subversion/libsvn_subr/gpg_agent.c I found this snippet
 (at around line 325):

 [[[
   /* Send DISPLAY to the gpg-agent daemon. */
   display = getenv(DISPLAY);
   if (display != NULL)
 {
   request = apr_psprintf(pool, OPTION display=%s\n, display);
   if (!send_option(sd, buffer, BUFFER_SIZE, display, display, pool))
 ...

 I don't know enough about what's going on here.  Is the first time the
 variable is set supposed to be combined with the second, or is it just
 a superfluous assignment which we can safely remove?


 Remove it.  The definition of send_option() recomputes REQUEST and sends
 it over the socket --- the copy here is not needed.

r1396534

-Hyrum


Re: svn commit: r1390438 - in /subversion/trunk: ./ subversion/include/private/ subversion/libsvn_ra_svn/ subversion/libsvn_subr/ tools/client-side/svn-bench/ tools/dist/

2012-10-10 Thread Hyrum K Wright
On Wed, Sep 26, 2012 at 8:39 AM,  stef...@apache.org wrote:
 Author: stefan2
 Date: Wed Sep 26 12:38:59 2012
 New Revision: 1390438

 URL: http://svn.apache.org/viewvc?rev=1390438view=rev
 Log:
 Merge second batch of changes from the 10Gb branch. These add the
 svn-bench tool.
...
 Modified: subversion/trunk/subversion/libsvn_subr/string.c
 URL: 
 http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/string.c?rev=1390438r1=1390437r2=1390438view=diff
 ==
 --- subversion/trunk/subversion/libsvn_subr/string.c (original)
 +++ subversion/trunk/subversion/libsvn_subr/string.c Wed Sep 26 12:38:59 2012
 @@ -1093,3 +1093,44 @@ svn__i64toa(char * dest, apr_int64_t num
*dest = '-';
return svn__ui64toa(dest + 1, (apr_uint64_t)(0-number)) + 1;
  }
 +
 +static void
 +ui64toa_sep(apr_uint64_t number, char seperator, char *buffer)
 +{
 +  apr_size_t length = svn__ui64toa(buffer, number);
 +  apr_size_t i;
 +
 +  for (i = length; i  3; i -= 3)
 +{
 +  memmove(buffer[i - 2], buffer[i - 3], length - i + 3);
 +  buffer[i-3] = seperator;
 +  length++;
 +}
 +
 +  buffer[length] = 0;
 +}
 +
 +char *
 +svn__ui64toa_sep(apr_uint64_t number, char seperator, apr_pool_t *pool)
 +{
 +  char buffer[2 * SVN_INT64_BUFFER_SIZE];
 +  ui64toa_sep(number, seperator, buffer);
 +
 +  return apr_pstrdup(pool, buffer);
 +}
 +
 +char *
 +svn__i64toa_sep(apr_uint64_t number, char seperator, apr_pool_t *pool)

This function name looks like it takes a signed integer, but the
actual argument type is an unsigned int...

 +{
 +  char buffer[2 * SVN_INT64_BUFFER_SIZE];
 +  if (number  0)

...which means this conditional will never evaluate as true.

 +{
 +  buffer[0] = '-';
 +  ui64toa_sep((apr_uint64_t)(-number), seperator, buffer[1]);
 +}
 +  else
 +ui64toa_sep((apr_uint64_t)(number), seperator, buffer);
 +
 +  return apr_pstrdup(pool, buffer);
 +}
...

-Hyrum


Re: Subversion 1.8 -- where are we?

2012-10-10 Thread Hyrum K Wright
On Wed, Oct 10, 2012 at 11:30 AM, C. Michael Pilato cmpil...@collab.net wrote:
 It's been a bit since I prodded the community for a status check on
 Subversion 1.8, so here we go.

 Let's start with roadmap.html.  We've got several green lights on the
 grid.  We've got several yellows, too:

 - Improved handling of local moves/renames -  looks like we're down to
 multi-layer move issues right now.  Is that correct?

 - Editor V2 - Hyrum sez that the work is sufficiently protected on trunk as
 to be releasable.  But I was reminded this morning about the svnrdump
 situation:  svnrdump appears to be broken under ra_serf w/ editor v1
 (http://subversion.tigris.org/issues/show_bug.cgi?id=4116).  This doesn't
 sound releasable.

I've been somewhat AWOL on this the past few months due to a number of
Real Life factors.  While I can't dedicate the amount of time I
previously had, my hope is that I can spend some time looking at this
over the next few days while at Subversion Live.

It may take a bit to page in the state, though.  If somebody else
wants to run with it, I'll happily act as a reference.

 - libsvn_ra_serf stabilization - we're making progress here, but there are
 still open issues (such as the aforementioned svnrdump one!)

 - Symmetric merge - Julian has called for review of this work (as well as
 for a new term to describe it).  Is anyone doing this?

 - Inherited properties - This is, as far as I know, finished and should be
 shown as such.  Paul?

 And then there are all the typical red items.  API reviews, performance
 reviews (is anyone really signing up for this?), issue triage (which I will
 probably begin myself soon), etc.

 What else?

 - Paul is working on some of the server-dictated configuration stuff on his
 branch(es).

 - Julian has also requested code review of his 'svn mergeinfo' graph display
 stuff.

 1.7.0 was released a year ago tomorrow.  How can we ensure that we aren't
 still asking about the status of 1.8 another six months from now?

Concrete, actionable steps with individual ownership.  Not ownership
of code, of course, but somebody who will be accountable for getting X
done for the release.

-Hyrum


Re: Request svn.a.o upgrade from 1.7.0 to 1.7.7?

2012-10-10 Thread Hyrum K Wright
On Wed, Oct 10, 2012 at 4:06 PM, Julian Foad julianf...@btopenworld.com wrote:
 I just noticed asf.a.o is running 1.7.0, according to the footer on pages
 like http://svn.apache.org/repos/asf/subversion/.

 Shall we request infra upgrade it to 1.7.7?  Seems like a good idea to me
 (eating our own dogfood), but not sure of our policy or history.

+1

At one point in the not-too-distant past, we were even running RCs on
svn.apache.org.

-Hyrum


Re: PGP Keys

2012-10-08 Thread Hyrum K Wright
On Fri, Oct 5, 2012 at 6:10 PM, Ben Reser b...@reser.org wrote:
 Given that we're coming up on a couple of opportunities for various
 developers to get together an potentially sign keys I thought I'd
 bring this subject up.

 1) SHA-1 based keys should be migrated off of.  The US Government's
 requirement of agencies was to stop using SHA-1 by the end of 2010.
 We're nearly 2 years past that date and there are actually several
 people still signing releases with such keys.  In particular if you're
 still using a 1024 DSA key that means you.  You can check by looking
 at your looking at how GPG represents your key, if it says 1024D then
 you need to replace that key.  Details on a sane way of migrating keys
 can details about the situation can be found on this blog:
 http://www.debian-administration.org/users/dkg/weblog/48

 If you have any questions about this I'll do my best to answer them.

 2) There is going to be 2 opportunities in the coming months when
 several of us are together that it may be useful to carry out a key
 signing party.

   a) Greenwich, Connecticut USA October 13th - 15th at the
 mini-hackathon before SVN Live.
   b) Sinsheim, Germany November 5th - 8th at ApacheCon EU 2012.

 I plan on organizing key signing at both events if there is sufficient
 people interested and there will be keys that need signing.  Given the
 issue the SHA-1 issue described above and the key signing party
 options.  Now might be a excellent time to generate a new key,
 especially if you're planning on attending one of those events.

 If you're interested in participating in something like that at one of
 those locations, please reply and indicate which location(s) you'll be
 available to attend and the dates you'll be available (since some
 people may not be available the whole time).  Based on this
 information I'll try to coordinate something that hits the maximum
 number of people and generates the biggest web of trust.

 This is not just an opportunity for developers to sign each others
 keys but also an opportunity for some of our users to sign our keys
 and potentially enhance their trust of our signatures.  So feel free
 to pass this information along to anyone that's interested.

 I'd like to plan the details for the Greenwich, Connecticut
 opportunity no later than Tuesday October 8th, so please reply ASAP if
 you're interested in that.  I'll post more details once I've figured
 them out.

I am interested in participating in the keysigning in Greenwich.
Though I will note that October 8th is a Monday this year, not a
Tuesday. :)

-Hyrum


Dead code or bug in gpg_agent.c

2012-10-08 Thread Hyrum K Wright
Poking around subversion/libsvn_subr/gpg_agent.c I found this snippet
(at around line 325):

[[[
  /* Send DISPLAY to the gpg-agent daemon. */
  display = getenv(DISPLAY);
  if (display != NULL)
{
  request = apr_psprintf(pool, OPTION display=%s\n, display);
  if (!send_option(sd, buffer, BUFFER_SIZE, display, display, pool))
{
  close(sd);
  return SVN_NO_ERROR;
}
}
]]]

Among other things, it looks to be setting the request variable.
However, that variable is never read before it is reset a few lines
later:

[[[
  request = apr_psprintf(pool,
 GET_PASSPHRASE --data %s--repeat=1 
 %s X %s %s\n,
 non_interactive ? --no-ask  : ,
 cache_id,
 escape_blanks(password_prompt),
 escape_blanks(realm_prompt));
]]]

I don't know enough about what's going on here.  Is the first time the
variable is set supposed to be combined with the second, or is it just
a superfluous assignment which we can safely remove?

-Hyrum


Re: buildbot failure in ASF Buildbot on svn-x64-ubuntu-gcc

2012-09-25 Thread Hyrum K Wright
On Tue, Sep 25, 2012 at 12:29 AM, Ben Reser b...@reser.org wrote:
 On Mon, Sep 24, 2012 at 8:27 PM, Hyrum K Wright hy...@hyrumwright.org wrote:
 For the time being, the Ruby build step and tests have been commented
 out of the relevant scripts.

 I think there is value in running a bot on a stock (albeit currently
 beta) install of a popular Linux distribution.  We don't control our
 users' machines, and pretty soon many of them will be upgrading to the
 latest Ubuntu.  A not-insignificant-number of those people will
 discover their Subversion Ruby scripts don't work anymore.  This is
 unfortunate, and the sooner we can find these problems, the better.
 Running a bot in this type of environment is one way to do so.  (Plus,
 I haven't committed a line of code in 3 months: I have to do
 _something_ to feel more useful than just kibitzing on dev@. :) )

 You are, of course, welcome to run yet another bot with its own
 configuration, but I recommend we still use this one.

 Well I agree there's value in that configuration, since as you rightly
 point out it does alert us to upcoming issues.  Coming from a
 continuous delivery workplace here recently I was thinking that the
 primary purpose of the buildbots was to alert us to breakage after
 code changes.  If the environment the build bot is running in is
 changing out from under it, you can't tell at a glance that the build
 was broken due to a code change or an environmental change on the
 build bot.

Good point: it does require extra dev time to answer that question in
the case of false negatives.

Incidentally, I'm still seeing swig-pl build errors on that bot.  From
reading above, I thought these were addressed.  Is that correct?

-Hyrum


Re: buildbot failure in ASF Buildbot on svn-x64-ubuntu-gcc

2012-09-24 Thread Hyrum K Wright
On Mon, Sep 24, 2012 at 8:02 PM, Ben Reser b...@reser.org wrote:
 On Wed, Sep 19, 2012 at 2:14 AM, Philip Martin
 philip.mar...@wandisco.com wrote:
 In build/ac-macros/swig.m4 the -ansi flag is explicltly removed for the
 Ruby bindings so that gcc accepts things like that.

 Thanks, that worked.  I've committed something to that effect in r1389658.

 I also tracked down the root cause in SWIG and submitted a bug report
 with a patch attached so they can resolve the problem:
 https://sourceforge.net/tracker/?func=detailaid=3571361group_id=1645atid=101645

The svn-x64-ubuntu-gcc bot currently lives in my basement, and was
recently upgraded to the latest Ubuntu beta release, which bumped a
number of dependency versions.  The biggest issue with the bindings is
the bump to Ruby 1.9 which we explicitly don't support because the
tests are not currently compatible with it.  It seems that Ruby 1.9 is
the Next Big Thing, and 1.8 is soon to unsupported, so a number of
distros are moving to it as the default.

I've spent the evening digging into what it would take to support Ruby
1.9, but my Ruby-fu is so weak that it'll be more than an evening for
me to actually make progress on the issue.  I invite more capable
hands to join in the effort.

-Hyrum


Re: [svnbench] Revision: 1389172 compiled Sep 24 2012, 00:21:39 on x86_64-unknown-linux-gnu

2012-09-24 Thread Hyrum K Wright
On Mon, Sep 24, 2012 at 7:26 PM, Ivan Zhakov i...@visualsvn.com wrote:
 On Mon, Sep 24, 2012 at 10:03 PM, Stefan Sperling s...@elego.de wrote:
 On Mon, Sep 24, 2012 at 01:50:36PM -0400, C. Michael Pilato wrote:
 That's correct.

 And Philip, I see this as really two issues:

 1. we auto-upgrade working copies (at all)
 2. we auto-upgrade working copies that are arguably not the true targets of
 an operation.

 I can live with the first problem if I must.  It's the second that's the
 more egregious of the two, in my book.  So yes, I think it makes (as you
 suggested elsethread) to add a 'read-only' mode to the WCDB, and to use that
 mode in the initial exploratory phases of a checkout operation.  Maybe we
 provide a way to upgrade that to read/write programmatically rather than
 closing and re-opening the DB ... no opinion there.  Whatever makes the most
 sense.

 Or we just disable auto-upgrade. I think we've seen enough reasons
 now why it's just a plain stupid idea in practice.

 +1 to disable auto-upgrade. WC upgrade is non reversible operation and
 performing it silently is very bad idea.

Yet again I give my hearty +1 (and wonder why this is still even a
question several months after first discussing the issue. :/ )

-Hyrum


Re: buildbot failure in ASF Buildbot on svn-x64-ubuntu-gcc

2012-09-24 Thread Hyrum K Wright
On Mon, Sep 24, 2012 at 11:15 PM, Ben Reser b...@reser.org wrote:
 On Mon, Sep 24, 2012 at 7:38 PM, Hyrum K Wright hy...@hyrumwright.org wrote:
 The svn-x64-ubuntu-gcc bot currently lives in my basement, and was
 recently upgraded to the latest Ubuntu beta release, which bumped a
 number of dependency versions.  The biggest issue with the bindings is
 the bump to Ruby 1.9 which we explicitly don't support because the
 tests are not currently compatible with it.  It seems that Ruby 1.9 is
 the Next Big Thing, and 1.8 is soon to unsupported, so a number of
 distros are moving to it as the default.

 I've spent the evening digging into what it would take to support Ruby
 1.9, but my Ruby-fu is so weak that it'll be more than an evening for
 me to actually make progress on the issue.  I invite more capable
 hands to join in the effort.

 Yeah I noticed the Ruby failure.  If the machine running this buildbot
 is being shared by some other use for you maybe we should split this
 off.  I have a KVM setup dedicated for Subversion work and would be
 happy to host this slave as a dedicated guest.  This would allow us to
 have upgrades only when we wanted them for the specific build slave.

 Obviously the work to move to 1.9 would still need to be done, but
 there's really not much value in having our build bot fail on
 something we're not expecting to work.

For the time being, the Ruby build step and tests have been commented
out of the relevant scripts.

I think there is value in running a bot on a stock (albeit currently
beta) install of a popular Linux distribution.  We don't control our
users' machines, and pretty soon many of them will be upgrading to the
latest Ubuntu.  A not-insignificant-number of those people will
discover their Subversion Ruby scripts don't work anymore.  This is
unfortunate, and the sooner we can find these problems, the better.
Running a bot in this type of environment is one way to do so.  (Plus,
I haven't committed a line of code in 3 months: I have to do
_something_ to feel more useful than just kibitzing on dev@. :) )

You are, of course, welcome to run yet another bot with its own
configuration, but I recommend we still use this one.

-Hyrum


Re: Finalizing the definition of 1.8.0

2012-08-16 Thread Hyrum K Wright
On Wed, Aug 15, 2012 at 12:21 PM, C. Michael Pilato cmpil...@collab.net wrote:
 Hello, all.

 Echoing in the back of my head are promises we devs have made to avoid long
 release cycles, and 1.7.0 recently turned 10 months old.  There are a few
 ongoing bits of feature work happening on branches or in various states of
 completion on the trunk, and I'd really like to avoid 1.8.0 remaining a
 moving target for much longer.

 Also echoing in my head are recent conversations with some pretty large
 Subversion-using corporate community members who are starting to sense that
 it's about time for another Subversion release, but have no idea what such a
 release might contain or when it will ship.  It's the common complaint about
 our poor management of outward-bound communication -- a stale roadmap.html
 page, no real user-targeting blog or similar info stream, etc.

 It's time to make a dedicated push, not so much to release 1.8.0 -- that
 would be premature, I sense -- but to at least better define it.

 After I send this mail, I'll take a shot at doing some updates to the
 Release Status portion of roadmap.html.  But in the meantime, allow me to
 start the ball rolling with some sound-offs on outstanding
 works-in-progress, hopefully so we as a community can ultimately come to an
 agreement about which bodies of effort should be aimed at 1.8.0, and which
 should be deferred.

 ===

 1.8.0 Issues:  Per http://goo.gl/uo0CN, there are currently 21
 1.8.0-milestoned (that is, per our convention, 1.8-blocking) issues.  Most
 of those are related to...

 Serf Stabilization:  There's clearly a body of work required here to get
 ra_serf up to snuff for service as our sole HTTP communication library.

 Conflict Storage:  I get the sense that there was energy invested on this
 for 1.8, possibly culminating in the trunk changes to defer interactive
 conflict handling until the tail end of update/merge operations.  Maybe Bert
 or Stefan can update this roadmap.html item?

 Server-dictated Configuration:  See Inherited Properties.

 Inherited Properties:  Paul has the basic property inheritence and local
 caching mechanism working on his branch, and almost ready to begin
 validating his approach by implementing one of our wishlist items (inherited
 default ignores, or inherited default auto-props).  Paul, Mark and I agree
 that we needn't verify that server-dictated configuration is All Good(tm)
 before deeming the inherited properties work trunk-worthy, but we also
 realize that

 Symmetic Merge:  Looks like Julian and Paul are sufficiently satisfied with
 this feature as to go live in trunk with it.  I don't, however, know what
 1.8-must-have work remains here.

 EditorV2:  I have this memory that EditorV2 adoption/migration work sits in
 a half-finished state.  That shims exist for adapting v1 to v2 and
 vice-versa, but that certain bits of the codebase were suffering today
 performance-wise or scalability-wise due to the use of those shims.  And
 there was something interesting about svnrdump, too.  But I could be
 *completely* misremembering.

I believe that the work is sufficiently protected on trunk as to be
releasable.  There is certainly more that could be done, but my
energies are elsewhere at the moment, and I don't know who else is
involved.

 Master Passphrase:  I have on my branch abstracted the authn storage logic
 into a set of private APIs, and then implemented two stores -- one based on
 the current ~/.subversion/auth/ config-like code, and a one that uses our
 serialized hash format and the new encrypted string support.  A
 'use-master-passphrase' runtime configuration item dictates which store is
 used, and all the extant providers just use the authn store abstraction
 layer.  I highly doubt that my particular encrypted store implementation
 will fly for production code, and my approach in general might not withstand
 intense developer scrutiny.  But ... that's where things sit today.  It
 partially because of where this feature sits that I'm intentionally turning
 my head towards 1.8.0 -- I value us getting a good release out the door in a
 timely matter, and don't want to be the cause of that release slipping.

 Those are the things that come to mind immediately.  What else?

 -- C-Mike

 PS:  Is it safe to assume that commit shelving/checkpointing is *not*
 going to happen for 1.8?  ;-)

 --
 C. Michael Pilato cmpil...@collab.net
 CollabNet  www.collab.net  Enterprise Cloud Development



Re: Is there a way to post a comment to a subversion issue without agreeing to colabnet terms?

2012-08-14 Thread Hyrum K Wright
On Sat, Aug 11, 2012 at 2:11 AM, Greg Stein gst...@gmail.com wrote:
 To be fair, Apache Bloodhound (incubating) is just one option. We
 could preserve issue numbers with a migration to Bugzilla, Jira, or
 Google Code. (not sure if other options have been raised as options,
 in the past)

 My leanings are towards Google Code's tracker, but I like the idea
 that Apache Bloodhound could integrate with our version control (where
 GC never could).

 The migration tool is an important point. We're still on IssueZilla at
 tigris, primarily because we don't have a migration tool.

And the migration tool will probably have to be hand-rolled, iirc.

-Hyrum


Re: 1.7.6 Candidates

2012-08-08 Thread Hyrum K Wright
On Wed, Aug 8, 2012 at 1:40 PM, Philip Martin
philip.mar...@wandisco.com wrote:
 Subversion 1.7.6 tarballs are now available for testing/signing by
 committers. To obtain them please check out a working copy from
 https://dist.apache.org/repos/dist/dev/subversion

 Please add your signatures to the .asc files there.
 You can use the release.py script for this:

  release.py sign-candidates --target /path/to/dist/dev/subversion/wc 1.7.6

 Downstream packagers, please keep in mind that this release is not
 blessed yet. Please do not distribute binaries compiled from these
 sources before the release has been officially announced. This release
 may still be pulled and supplanted by a different one (with a new
 version number) in case of unforeseen problems during the testing phase.

Thanks for rolling these, I'll give 'em a spin.

Don't forget to create the tag and bump the version numbers on the branch.

-Hyrum


Re: 1.7.6 Plan?

2012-08-03 Thread Hyrum K Wright
On Fri, Aug 3, 2012 at 4:41 AM, Greg Stein gst...@gmail.com wrote:

 On Aug 3, 2012 4:31 AM, Philip Martin philip.mar...@wandisco.com wrote:

 Justin Erenkrantz jus...@erenkrantz.com writes:

  On Thu, Aug 2, 2012 at 6:10 PM, Hyrum K Wright hy...@hyrumwright.org
  wrote:
  So, I'm still on semi-hiatus, but I've been seeing all this stuff
  being merged to 1.7.x and I can't help but wonder what the plan for
  the 1.7.6 release is.  I haven't found any discussion on list.  The
  release process has changed enough since I RM'd that I'm not quite
  confident I can volunteer to shepherd the release.  But I am
  interested to know if anybody else is.  :)
 
  If no one else has a burning desire to do so, I could take a swing at
  being RM for 1.7.6 next week.  -- justin

 I was considering doing it next week as well, but I'm happy to let you
 do it.

 FIGHT! FIGHT!

Just don't let the fight result in a stalemate. :)

-Hyrum


1.7.6 Plan?

2012-08-02 Thread Hyrum K Wright
So, I'm still on semi-hiatus, but I've been seeing all this stuff
being merged to 1.7.x and I can't help but wonder what the plan for
the 1.7.6 release is.  I haven't found any discussion on list.  The
release process has changed enough since I RM'd that I'm not quite
confident I can volunteer to shepherd the release.  But I am
interested to know if anybody else is.  :)

-Hyrum


Re: Deterministic FSFS revision files

2012-07-30 Thread Hyrum K Wright
On Mon, Jul 30, 2012 at 1:19 PM, C. Michael Pilato cmpil...@collab.net wrote:
 On 07/30/2012 12:47 PM, Philip Martin wrote:
 I'm considering changing the commit code so that hashes are written in a
 stable order and the revision files are repeatable.  Does anyone think
 this would be a bad idea?

 I can't think of any reason not to do this.  The performance overhead will
 be minimal, I should think, and you've already outlined the benefits.

As long as the performance overhead is negligible, I think having
deterministic ordering is very desirable.  There's just something in
me that longs to have a One True Ordering in dumpfiles and revfiles
and output and everything else.  I can't really say why, but it feels
like by having a canonical ordering, we open ourselves up for more
opportunities to cache work moving forward.

(How's that for nebulous and hand wavy?)

-Hyrum


Re: Format bump for 1.8?

2012-07-13 Thread Hyrum K Wright
On Fri, Jul 13, 2012 at 5:32 AM, Branko Čibej br...@wandisco.com wrote:
...
 And really, there's no reason why the Subversion command-line client
 shouldn't do the same thing -- in fact, this is by far the easiest way
 to support multiple working copy formats, even if it is less than
 optimal as far as installed binary size is concerned (not to mention
 release management and version juggling and testing).

But why should we, as an open source project that only ships source
code, worry about this?  It sounds like a job for the folks that
package binaries, either via OS package management systems or the
various other binary providers around.

If the solution you suggest is ship several versions of the libraries
in the same binary, it just doesn't feel like something we should do
(or worry about, frankly).

-Hyrum


Re: Format bump for 1.8?

2012-07-11 Thread Hyrum K Wright
On Wed, Jul 11, 2012 at 9:23 AM, C. Michael Pilato cmpil...@collab.net wrote:
 On 07/11/2012 09:50 AM, Johan Corveleyn wrote:
 Yes, I agree we'd have to make it clear in our docs that feature X
 depends on working copy format Y. That's an additional effort, yes.
 But for the book or the FAQ it's not a lot different from phrases
 where they now say: If you've got Subversion 1.7 or higher, you'll
 get  Then they'll have to say If your working copy format is 1.8
 or higher, you'll get 

 Documentation-wise, I don't see this as much more problematic than If
 you're server was created with Subversion 1.5 or better, the mergeinfo
 feature is available.

 As I said elsethread, most end-users I know consider the manual
 upgrade in 1.7 a clear improvement over auto-upgrading. It makes the
 entire process less scary. It avoids accidents like the scenario I
 mentioned, where the user has 3 svn clients (a commandline client, a
 GUI tool, and an IDE plugin), which all have their own release cycles.
 In that situation, an auto-upgrade by one of the three, while another
 can't be upgraded immediatly, can wreak havoc. It's much better to
 error out with 'working copy format too old, run svn upgrade'.

 Agreed.  Working copy auto-upgrade has been a repeated point of concern by
 our users over the years, for the scenario you gave as well as for the
 situation where users share working copies.  (A similar situation drives the
 reason why we don't auto-upgrade repositories -- multiple users hitting a
 repository over ra_local with different client versions.)

 That said, I'm not really in favor of us maintaining multiple codepaths
 (based on WC version) in the client.  I just don't see the cost justifying
 the benefit.  I mean, it makes sense to do so in the repositories as we do
 today, because the cost of any Subversion upgrade could be equivalent to the
 cost of a full dump/load of the repository.  I think that's too much to ask
 of our administrators.  But for working copies?

I agree with Mike on all of the above.

 I actually feel like the 1.7 situation was the best we've had to date:
 restrictive WC format client support, an explicit upgrade step, minimal
 surprises.

Agreed.  Most of the feedback I've heard was that the 1.7 upgrade
experience was actually better than previous cycles, even though
technically t was the most radical.

-Hyrum


Re: Format bump for 1.8?

2012-07-11 Thread Hyrum K Wright
On Wed, Jul 11, 2012 at 12:09 PM, Johan Corveleyn jcor...@gmail.com wrote:
 ...
 For instance, we could simply package two set of binaries/libraries in
 one package (the 1.8.0 version together with 1.7.x (taken from the
 corresponding tag)), and implement a main wrapper that delegates
 everything to the appropriate version. The 1.8.0 code doesn't need to
 care about the 1.7-compat stuff, we just have a dispatcher and package
 the old code together with it. I'm just fantasizing of course, for
 illustrative purposes only :-). But ... it depends. Anyone have a
 creative idea how this could technically be done and what the pros and
 cons would be?

Or some third-party could just do this, and we wouldn't have to mess
with it at all.  If there is a demand, somebody will step up and do
it, but I don't think we (as the Apache Subversion project) should too
much about it.

-Hyrum


Re: Format bump for 1.8?

2012-07-11 Thread Hyrum K Wright
On Wed, Jul 11, 2012 at 12:25 PM, Johan Corveleyn jcor...@gmail.com wrote:
 On Wed, Jul 11, 2012 at 7:14 PM, Hyrum K Wright hy...@hyrumwright.org wrote:
 On Wed, Jul 11, 2012 at 12:09 PM, Johan Corveleyn jcor...@gmail.com wrote:
 ...
 For instance, we could simply package two set of binaries/libraries in
 one package (the 1.8.0 version together with 1.7.x (taken from the
 corresponding tag)), and implement a main wrapper that delegates
 everything to the appropriate version. The 1.8.0 code doesn't need to
 care about the 1.7-compat stuff, we just have a dispatcher and package
 the old code together with it. I'm just fantasizing of course, for
 illustrative purposes only :-). But ... it depends. Anyone have a
 creative idea how this could technically be done and what the pros and
 cons would be?

 Or some third-party could just do this, and we wouldn't have to mess
 with it at all.  If there is a demand, somebody will step up and do
 it, but I don't think we (as the Apache Subversion project) should too
 much about it.

 That's true, but one of the problems I'm trying to solve here is users
 having different clients on their system, each with their own release
 cycle. Maybe one of those offers this back-compat feature (e.g.
 currently the one that uses svnkit under the hood), but not the other
 two. That problem can only be solved, I think, if the core project
 makes this a built-in supported feature.

We can not, and should not attempt to, solve every problem for every
user.  Using heterogenous versions in the same environment has a set
of known risks (which were somewhat alleviated by the manual upgrade
step in 1.7, btw).  We attempt to catch and prevent old clients from
corrupting newer working copies, but that's as far as I think we need
to go.

You have a different opinion, and I respect that, but the amount of
effort required to simultaneously support multiple active working copy
formats will probably prove prohibitive.  Unless you are willing to
bear that burden yourself, it will probably  be difficult convincing
the rest of the community that this departure from historic precedent
is worth the maintenance burden.

-Hyrum


Re: Format bump for 1.8?

2012-07-11 Thread Hyrum K Wright
On Wed, Jul 11, 2012 at 3:19 PM, Johan Corveleyn jcor...@gmail.com wrote:
 On Wed, Jul 11, 2012 at 8:07 PM, Hyrum K Wright hy...@hyrumwright.org wrote:
 On Wed, Jul 11, 2012 at 12:25 PM, Johan Corveleyn jcor...@gmail.com wrote:
 On Wed, Jul 11, 2012 at 7:14 PM, Hyrum K Wright hy...@hyrumwright.org 
 wrote:
 On Wed, Jul 11, 2012 at 12:09 PM, Johan Corveleyn jcor...@gmail.com 
 wrote:
 ...
 For instance, we could simply package two set of binaries/libraries in
 one package (the 1.8.0 version together with 1.7.x (taken from the
 corresponding tag)), and implement a main wrapper that delegates
 everything to the appropriate version. The 1.8.0 code doesn't need to
 care about the 1.7-compat stuff, we just have a dispatcher and package
 the old code together with it. I'm just fantasizing of course, for
 illustrative purposes only :-). But ... it depends. Anyone have a
 creative idea how this could technically be done and what the pros and
 cons would be?

 Or some third-party could just do this, and we wouldn't have to mess
 with it at all.  If there is a demand, somebody will step up and do
 it, but I don't think we (as the Apache Subversion project) should too
 much about it.

 That's true, but one of the problems I'm trying to solve here is users
 having different clients on their system, each with their own release
 cycle. Maybe one of those offers this back-compat feature (e.g.
 currently the one that uses svnkit under the hood), but not the other
 two. That problem can only be solved, I think, if the core project
 makes this a built-in supported feature.

 We can not, and should not attempt to, solve every problem for every
 user.  Using heterogenous versions in the same environment has a set
 of known risks (which were somewhat alleviated by the manual upgrade
 step in 1.7, btw).  We attempt to catch and prevent old clients from
 corrupting newer working copies, but that's as far as I think we need
 to go.

 You seem to be implying that users having multiple svn clients on
 their system are the exception, rather than the rule. I think it's the
 other way around (though I haven't done the market research to back
 that up of course :-)). And all too often, regular users do *not* know
 the risks (but the manual upgrade is indeed a big help here).

 But I understand, we can't solve every problem on the planet (there
 needs to be some room left for educating users too :-)).

 You have a different opinion, and I respect that, but the amount of
 effort required to simultaneously support multiple active working copy
 formats will probably prove prohibitive.  Unless you are willing to
 bear that burden yourself, it will probably  be difficult convincing
 the rest of the community that this departure from historic precedent
 is worth the maintenance burden.

 I'm obviously not about to bear the burden myself, this thread was all
 about convincing you guys that this could be a burden worth bearing
 (and that maybe, with some creativity, the burden could be kept
 light).

 But okay, it seems I'm totally outnumbered (not a single voice of
 support in this thread). Fine. I concede.

I wasn't trying to push your opinion to the side, just pointing out
that you've a long row to hoe in trying to change the status quo.  If
you feel passionate about it, by all means please continue to fight
the good fight.

(As for me, my view has now been made quite vociferously known, so I
will now fade back into the shadows.)

-Hyrum


Re: Revprop packing implemented

2012-07-07 Thread Hyrum K Wright
On Fri, Jul 6, 2012 at 3:32 AM, Stefan Fuhrmann
stefan.fuhrm...@wandisco.com wrote:
 Hi devs,

 This week I had one of my how hard can it be? moments
 and finally implemented revprop packing (did that mainly
 offline). It passes all tests and seems to work pretty well.

 It's design deviates from the existing revprop packing branch
 in that it is more scalable and simpler to implement. Key
 differences:

 * Have a configurable limit (default 64k) to the pack file size.
   Concatenate revprops only up to that limit, but at least one
   revision's revprops per file. Have the manifest map revs to
   revprop pack files. IOW, let the OS do all the heavy lifting
   of storage management.
 * Make revprop packing a mandatory part of FSFS packing
   in format 6 repos. There is no need to track revprop packing
   separately nor to give it a separate format info.

Do you have an overview of the entire design, rather than just a delta
between your implementation and the existing one?

 ...

-Hyrum


Re: Ev2 design questions

2012-06-27 Thread Hyrum K Wright
On Wed, Jun 27, 2012 at 3:57 AM, Greg Stein gst...@gmail.com wrote:
 Let's not over complicate this people. get_commit_ev2() has cancellation
 params for no other reason than I was unaware of the session variables.

As I suspected. :P

-Hyrum


Re: Format bump for 1.8?

2012-06-27 Thread Hyrum K Wright
On Wed, Jun 27, 2012 at 9:13 PM, Branko Čibej branko.ci...@wandisco.com wrote:
 On 28.06.2012 01:32, Greg Stein wrote:
 On Wed, Jun 27, 2012 at 7:19 PM, Johan Corveleyn jcor...@gmail.com wrote:
 On Tue, Jun 26, 2012 at 10:51 AM, Stefan Sperling s...@elego.de wrote:
 On Mon, Jun 25, 2012 at 07:51:59PM -0400, Greg Stein wrote:
 ...
 I would prefer to by default keep working copy upgrades manual from now on.
 +1, let's please keep it an explicit action by the user.
 Not sure about that. The user will type 'svn move' and not get the
 benefits. All the docs will say it *should* work, but it doesn't.

 Not to mention feature-invariant updates, such as the new MD5 index. if
 you don't have it, nothing breaks except your patience. :)

 Realistically, we've taught users and especially packagers to expect
 silent updates (and have said loudly enough that 1.7/WC-NG is an
 exception). I think we should just keep on doing that.

Agreed, and I'm against the auto-update as well.

I understand that people run a heterogenous collection of tools, and
they aren't all upgraded at the same time.  Doing a manual upgrade
ensures that people know they are upgrading, and that they hopefully
are aware enough to upgrade their other clients.  People running
multiple disparate clients are doing so under their own providence,
and we can't be expected to save them from themselves.

-Hyrum


Re: Bigger --deltas dump with 1.7.5 than with 1.6.17

2012-06-26 Thread Hyrum K Wright
On Sat, Jun 23, 2012 at 9:15 AM, Branko Čibej br...@apache.org wrote:
 On 23.06.2012 00:23, Vincent Lefevre wrote:
 Thanks for the explanations.

 On 2012-06-22 00:18:50 +0200, Stefan Fuhrmann wrote:
 xdelta uses fixed-size 100kByte deltification windows.
 The Changelog file in question is 400k, i.e. 4+ windows.
 You insert about 2k at the beginning of the file, moving
 the older parts by a similar distance. At the beginning
 of each delta window, those 2+k don't have deltification
 partner. Expected delta size:  4 x 2kBytes.
 Wouldn't it be possible to change this size of deltification windows,
 with a command-line option (for svnadmin dump) and/or an option in
 the config file?

 The gain would be quite important on some kinds of changes (typically
 ChangeLog update), and a dump of a revision is something that could
 be done once and for all, so that it would be worth to spend time on
 optimizing it.

 It's not that simple, unfortunately. Currently both the delta generator
 and especially the delta combiner expect the windows to be a known,
 constant size. You can't just change them without affecting a /lot/ of code.

 Frankly I don't know how to make all the delta code paths gracefully
 deal with variable window sizes. I'm sure it can be done, but the ROI at
 this point is not good at all.

It's a shame that we didn't include the window size in the encoding of
xdelta windows back in the day.  Oh, well, a lesson for the future, I
suppose.

-Hyrum



-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: Ev2 design questions

2012-06-26 Thread Hyrum K Wright
On Sun, Jun 24, 2012 at 6:09 PM, Vladimir Berezniker v...@hitechman.com wrote:
 Hi All,

 I have been taking a peek at ev2 code to see what it would take for JavaHL
 implementation and I have following questions that do not seem do be covered
 by the docs:

   * svn_ra__get_commit_ev2() takes svn_cancel_func_t as a parameter, however
     session already has a cancellation callback specified in the
     svn_ra_callbacks2_t structure. What is the relationship between the two?
     Is one of these going away?

I've not looked at the session struct, but can explain a bit about the
reason get_commit_ev2() takes a cancellation func/baton.
Historically, we've had a number of places where we'd wrap an editor
with a cancellation editor, which was just a pain.  Ev2 folds the
(optional) cancellation editor into the editor itself.

My guess is that in most places, the session cancellation callback and
the one provided to get_commit_ev2() will be the same, though they
will be invoked in different places.

   * svn_editor_add_directory() function requires that
     const apr_array_header_t *children and apr_hash_t *props parameters
     cannot be NULL, instead an empty structure should be passed. However,
     svn_editor_alter_directory() permits the same parameters to be NULL.
     What is the reason for this difference?

In the case of add_directory(), callers MUST provide the list of
children and properties, since they are not known a priori.  However,
in the case of alter_directory(), if there are no changes, a NULL
parameter may be used to indicate that.  (It would be rather pointless
to require clients to fetch the list of children and then replay them
back to the server if there are no changes.)

-Hyrum


-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: [RFC] JavaHL: Moving some of the C++ object address logic into Java

2012-06-26 Thread Hyrum K Wright
On Sun, Jun 24, 2012 at 9:20 PM, Vladimir Berezniker v...@hitechman.com wrote:
 Hi All,

 In the current JavaHL code the C++ objects are attached via pointer stored in
 the long cppAddr field in java object.  The way C++ code gets hold of the
 cppAddr value is to read this field using the GetLongField(). In summary

 Java:

 class JHLClass
 {
  long cppAddr;
  public native method();
 }


 JNI Stub:

 Java_method(JNIEnv *env, jobject jthis)
 {
  JHLClass cppObj = JHLClass::getCppObject(jthis);
 }

 C++:

 class JHLClass
 {
  JHLClass getCppObject(jobject jthis) {  }
 }


 I was thinking why not simplify this by doing all object-jlong lookup in the
 java land. What takes about 20 lines of C++ takes 1 line in java, at a cost of
 implementing 3 line java wrapper method that converts from caller visible
 method signature to native method signature. As follows:

 class JHLClass
 {
  long cppAddr;

  private native static method(long cppAddr);
  method() {
    method(cppAddr);
  }
 }

 JNI Stub:

 Java_method(JNIEnv *env, jlong cppAdder)
 {
  JHLClass cppObj = reinterpret_castSVNFile *(fileCppAddr);
 }

 C++: No additional code necessary

 This will require a related change in JNIStackElement, as it won't have jthis
 anymore. But this logic also can move up to java code in a similar manner.

 What do others think? Any objections at least of doing this in RA functions?

While I can't comment on the specific implications of your plan,
generally the more we can write in Java in the JavaHL bindings, the
better.  Hopefully that nugget of wisdom gives you some insight. :)

-Hyrum



-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: [PATCH] JavaHL: Support commit callback across method invocations

2012-06-26 Thread Hyrum K Wright
On Sun, Jun 24, 2012 at 8:31 PM, Vladimir Berezniker v...@hitechman.com wrote:
 In case of delta editor, commit callback is provided at the time of the editor
 creation and used during the close_edit() call. For that there is a need to
 keep a global reference to the underlying java object so that it does not get
 GCed meanwhile.

 Attached please find the patch that adds such capabilities.

 Thank you,

 Vladimir

 [[[
 JavaHL: Support keeping global reference to the callback java object for cases
 when callback is being used across method calls

 [ in subversion/bindings/javahl/native ]

 * CommitCallback.cpp,
  CommitCallback.h
  (CommitCallback, ~CommitCallback): Add handling of additional parameter and
    state when requesting a global reference to be kept
 ]]]

I don't like putting the burden on the caller that constructs the
Commit callback to define whether a global reference should be held or
not.  Can't the thing consuming the callback reference make that
determination?

(Alternatively, could we have a subclass which keeps a global
reference around, and let the caller instantiate that if needed?)

-Hyrum



-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: Ev2 design questions

2012-06-26 Thread Hyrum K Wright
On Tue, Jun 26, 2012 at 7:10 PM, Vladimir Berezniker v...@hitechman.com wrote:
 On Tue, Jun 26, 2012 at 3:17 PM, Hyrum K Wright
 hyrum.wri...@wandisco.com wrote:
 On Sun, Jun 24, 2012 at 6:09 PM, Vladimir Berezniker v...@hitechman.com 
 wrote:
 Hi All,

 I have been taking a peek at ev2 code to see what it would take for JavaHL
 implementation and I have following questions that do not seem do be covered
 by the docs:

   * svn_ra__get_commit_ev2() takes svn_cancel_func_t as a parameter, however
     session already has a cancellation callback specified in the
     svn_ra_callbacks2_t structure. What is the relationship between the two?
     Is one of these going away?

 I've not looked at the session struct, but can explain a bit about the
 reason get_commit_ev2() takes a cancellation func/baton.
 Historically, we've had a number of places where we'd wrap an editor
 with a cancellation editor, which was just a pain.  Ev2 folds the
 (optional) cancellation editor into the editor itself.
 Understood

 My guess is that in most places, the session cancellation callback and
 the one provided to get_commit_ev2() will be the same, though they
 will be invoked in different places.

 But how come other ra functions grab the cancellation callback from the ra
 session, but ev2 editor wants to get one passed directly to it rather
 than taking
 one from the ra session?

Ev2 is a generic API interface used across an large number of layers
in Subversion.  (There is an editor interface for committing changes
to the FS layer, for example.)  In many cases, these layers don't have
the concept of an ra session, so it would be a mistake for the Ev2
interface to use the ra session directly.

However, I think I see your point: svn_ra__get_commit_ev2() takes both
an ra_session_t *and* a cancellation func/baton.  Since that API is
specific to the ra layer, I think we can drop the cancellation
func/baton from that interface, and just use those in the session
object when we create the Ev2 object.  But looking at the definition
for svn_ra_session_t, it doesn't look like there is a cancellation
func/baton in there.  Am I missing something?


   * svn_editor_add_directory() function requires that
     const apr_array_header_t *children and apr_hash_t *props parameters
     cannot be NULL, instead an empty structure should be passed. However,
     svn_editor_alter_directory() permits the same parameters to be NULL.
     What is the reason for this difference?

 In the case of add_directory(), callers MUST provide the list of
 children and properties, since they are not known a priori.  However,
 in the case of alter_directory(), if there are no changes, a NULL
 parameter may be used to indicate that.  (It would be rather pointless
 to require clients to fetch the list of children and then replay them
 back to the server if there are no changes.)

 I think I was not very clear in asking the question.  Lets take
 following use cases.

 1) Add new directory without any children, with 1 property
 2) Alter a directory, by adding 1 property

 In both cases caller needs to indicate that there are 0 children at
 play.  In call to
 add_directory() 0 size array means no affected children. In call to
 alter_directory()
 NULL means no affected children.

In case (1), the caller needs to affirm that there are zero children
forthcoming, by providing a zero-length array.  In case (2), since no
children are added or deleted, no array needs to be provided.  The
difference is that in case (1), there isn't any existing list of
children, so one is required.

 Or another way, what is the behavior of alter_directory() if passed a
 0 size children
 array rather than NULL?

If alter_directory() gets an array of size 0, it means this directory
now has zero children; you should expect to see a delete() for each of
those children.  If alter_directory() gets a NULL array it means
none of the children are added or deleted.  Big difference.

 Thank you for your time, as I might just being dense here.

No worries; I am often quite dense myself.

-Hyrum


Re: Open mailing lists

2012-06-18 Thread Hyrum K Wright
It should also be pointed out that a spammer could easily subscribe
directly to the list and get all the address information that way,
completely by passing any archives.

For completeness, the ASF's public archive policy, which we adhere to,
is here: http://www.apache.org/foundation/public-archives.html

Best,
-Hyrum

On Mon, Jun 18, 2012 at 3:20 AM, Greg Stein gst...@gmail.com wrote:
 Darren,

 Over a dozen sites mirror our archives, usually by grabbing our published
 mbox for the list. As a result, we cannot control how they publish the email
 addresses contained within. It is also important for those mboxes to retain
 the email addresses for archival purposes, and so those third-party systems
 can allow proper replies (hopefully, only by humans, but as you've
 discovered... they are not all perfect).

 Sorry for any inconvenience, but please don't blame us. We do try to respect
 your privacy in our own web archive system.

 Cheers,
 -g

 On Jun 18, 2012 5:10 AM, s...@feb17.org wrote:

 Less than 2 months after using this mailing list I've started getting spam
 to the custom email address I used to post here.  I think it's terrible
 practice to openly publish email addresses in easily harvestable form.  I'll
 be /dev/nulling this address and unsubscribing.  I hope you could reconsider
 that policy,

 Darren

 On Tue, Mar 13, 2012 at 10:05:52PM -0700, daz wrote:
  On Tue, Mar 13, 2012 at 07:58:10AM -0500, Hyrum K Wright wrote:
   On Tue, Mar 13, 2012 at 5:45 AM, Philip Martin
   philip.mar...@wandisco.com wrote:
s...@feb17.org writes:
   
A little more information on this.  I have probably rebuilt svn
about 20 times tonight from scratch, with
 
  Thanks to everyone who contributed useful clues on this.  Using the
  current code tree and rebuilding with different versions and combinations 
  of
  libraries I narrowed the problem down to the apr version.  Either the build
  of my earlier apr 1.3.9 or the version itself was the problem.  The test
  suite was super helpful and the explanation about XFAIL vs FAIL.   I have a
  build using apr 1.4.6 that passes all the tests it should pass and more
  importantly actually works.    It might be helpful to print a reminder at
  the end of the default make step suggesting running the tests if this is a
  common problem.  There are a lot of dependencies and some of them seem to 
  be
  a bit finicky.
 
  Thanks!
  Darren



-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Hyrum limited connectivity for a couple of weeks

2012-06-15 Thread Hyrum K Wright
Hi all,
After a great time at the hackathon this week, I need a vacation.
I'll be with limited connectivity over the next few weeks as my family
and I road trip around the US.  I'll probably do mail in batch every
few days.

That shouldn't stop Ev2 work from continuing though, and I'd encourage
folks who are interested to jump in and help out.  Items that can be
worked on are:
 * Finish Ev2 implementation of svn_repos__replay_ev2().
 * Update ra_serf and ra_svn to support the commit editor.
 * Optionally update ra_serf and ra_svn to support the replay editor.
 * Implement svnrdump using an Ev2 editor.

I'm sure there are other things, and there aren't many dependencies
between them, but this should be sufficient for people looking to get
involved.  If there are questions, Greg can probably help, and I may
be able to as well, but on a limited basis.  None of this blocks 1.8,
but it's all got to be done at some point, so if you are looking for a
project, here's your chance!

-Hyrum


-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: svn commit: r1349944 - /subversion/trunk/gen-make.py

2012-06-14 Thread Hyrum K Wright
On Thu, Jun 14, 2012 at 1:23 AM, Bert Huijben b...@qqmail.nl wrote:


 -Original Message-
 From: Hyrum K Wright [mailto:hyrum.wri...@wandisco.com]
 Sent: Thursday, June 14, 2012 12:33 AM
 To: Bert Huijben
 Cc: Michael Pilato; Subversion Development
 Subject: Re: svn commit: r1349944 - /subversion/trunk/gen-make.py

 On Wed, Jun 13, 2012 at 9:45 PM, Bert Huijben b...@qqmail.nl wrote:
 
 
  -Original Message-
  From: C. Michael Pilato [mailto:cmpil...@collab.net]
  Sent: Wednesday, June 13, 2012 9:34 PM
  To: dev@subversion.apache.org
  Cc: Hyrum K Wright; comm...@subversion.apache.org
  Subject: Re: svn commit: r1349944 - /subversion/trunk/gen-make.py
 
  On 06/13/2012 07:36 PM, Hyrum K Wright wrote:
   On Wed, Jun 13, 2012 at 6:10 PM,  rhuij...@apache.org wrote:
   Author: rhuijben
   Date: Wed Jun 13 16:10:25 2012
   New Revision: 1349944
  
   URL: http://svn.apache.org/viewvc?rev=1349944view=rev
   Log:
   * gen-make.py
    Make the --with-neon= and --without-neon arguments on gen-
 make.py
  optional,
    just like how ./configure handles those. Windows doesn't use
 ./configure
  and
    this breaks tools that are friendly enough to provide hints.
  
   I don't like this change.  ./configure *doesn't* work this way: it
   errors when attempting to provide the now-defunct neon options.
 
  Perhaps Bert meant only that, in general, configure allows you to specify
  options that it doesn't itself understand (while printing a warning about
  this fact).
 
  I think most distributions at least share some build code between versions.
 
  The buildbots only have a single set of dependencies for all the branches
 we are building. Only once we released 1.9.0 neon will be gone and before
 that we still have to support it (and even after that we still have to 
 support all
 the Subversion 1.0-1.7 clients that still use neon)

 So, what you're saying is that we have single slaves, with single sets
 of scripts building multiple branches.  And because of this, we can't
 change the dependencies or script arguments on trunk, for fear of
 breaking other branches.  That sounds a bit like the tail wagging the
 dog.

 What would happen if we added a flag on trunk that one of the slaves
 needed to build, but didn't exist (and hence error'd) on other
 branches?  Would we need to backport that flag as a no-op, simply to
 make the build slaves happen?  That's exactly what this commit does,
 only in the forward direction.

  Things get very easy if you just break the few remaining buildbots without
 caring about the consequences, but it doesn’t make Subversion a better
 product.

 Making decisions simply to please the bots doesn't make Subversion a
 better product, either.  They exist to help facilitate development.
 If there is a problem with the bots, then FIX THE BOTS, rather than
 introducing hacks in an attempt to please them.

 In this specific case that might mean a slightly more complex setup,
 which build each branch with a separate set of dependencies / scripts
 / inputs, since that's what end users will be doing anyway.

 Well, maybe you could help writing these scripts instead of breaking them 
 because they have to broken anyway in the future.

I choose not to maintain the bots you choose to host.  The slave I
host is currently offline, if/when it comes back on, I will address
these issues which impact them.  I have helped fix the centos
buildslave, which I have knowledge of and access to.

 Breaking them 2 minutes after telling you that breaking them will cause huge 
 problems that take much time to solve isn't helping things.

You pointed out a deficiency in my prior patches, which I attempted to fix.

 Why do you prefer 2 of the 3 buildbots to be broken for at least the rest of 
 the hackathon?

I don't.  Please don't put words in my mouth.

 Fixing the scripts for the bots, for SharpSvn, for the Slik distribution, for 
 CollabNet's build and several test scenarios takes time certainly more 
 time than the 5 minutes you provided.

Those aren't my responsibility.  While I think we should be respectful
of downstream users, people who choose to build directly against trunk
on a regular basis have to know they are working against a moving
target, and such breakage is possible, and even quasi-expected.

 And probably more than a few weeks if I'm not delaying other work, which I 
 would have to do if as all my build environments are broken by this change.

Note that I have not asked that you revert this change.  I just said
that I don't like it, because we should think strongly about the
precedent this sets.  The flags to gen-make.py and other build scripts
are *not* part of our backward compat guarantees, and we shouldn't
feel required to support them indefinitely.  (I noticed your response
doesn't address the questions I asked along these lines.)

-Hyrum


-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: [PATCH] JavaHL: New method for creating java objects linked to their C++ counterpart

2012-06-14 Thread Hyrum K Wright
On Thu, Jun 14, 2012 at 10:16 AM, Mat Booth mat.bo...@wandisco.com wrote:
 On 12 June 2012 03:11, Vladimir Berezniker v...@hitechman.com wrote:


 On Mon, Jun 11, 2012 at 8:39 AM, Vladimir Berezniker v...@hitechman.com
 wrote:



 On Mon, Jun 11, 2012 at 6:20 AM, Hyrum K Wright
 hyrum.wri...@wandisco.com wrote:

 On Thu, Jun 7, 2012 at 4:03 AM, Vladimir Berezniker v...@hitechman.com
 wrote:
  Hi All,
 
  The intention if this patch is to introduce reusable logic for creating
  java
  objects from within C++.  This keeps object creation logic fully within
  C++
  while leaving to java the decision as to when they will be destroyed.
  It
  will be used by RA code to allocate container object for items like
  SVNRa,
  Editor, Directory and File.
 
  Thank you,
 
  Vladimir
 
  [[[
  JavaHL: New method for creating java objects linked to their C++
  counterpart
 
  [ in subversion/bindings/javahl/native ]
 
  * SVNBase.cpp, SVNBase.h
    (createCppBoundObject): New method for creating java objects linked
  to
  their
      C++ counterpart
 
  [ in subversion/bindings/javahl/src/org/tigris/subversion/javahl/ ]
 
  * JNIObject.java: Base class for JNI linked java objects
  ]]]

 Looks good.  Is there a way to mark the new Java class as private?


 The original plan is to put RA java classes in a ra/ sub package, so
 package visible
 would not work in such case.  It is an abstract class, even if someone
 instantiates
 it will be harmless, as it will not create any C++ instances.  Would
 putting a good
 javadoc explaining it is not part of the public API address your concerns?



 How do you plan to use this functionality?


 As compared to what SVNClient class does, where caller creates the java
 object which
 in turn calls native method call to create a matching native object. With
 RA layer,
 consumer calls a factory method:

 /**
 * Crates RA session for a given url with provided context
 * @param url to connect to
 * @param uuid of the remote repository, can be null if uuid check is not
 desired
 * @param config configuration to use for the session.
 * @return RA session
 */
 public static native ISVNRa createRaSession(String url, String uuid,
 ISVNRaConfig config);
 }

 that method is responsible for instantiation of the related C++ and Java
 classes.
 I think it is cleaner because:

 It better encapsulates implementation class: User only sees factory
 and ISVNRa interface
 Instantiation logic resides in a single language
 It avoids chicken and the egg with not year having cppAddr when java
 object is constructed: Allows enforcement that parameter is supplied via
 constructor argument

  Relatedly, do you have a

 high-level plan for this series of patches?



 More detailed plan, full version is in the BRANCH-README on javahl-ra
 branch:

    * Prepare existing code for merging of SVNRa editor       [IN PROGRESS]

       01. JavaHL: Changed return value from the
           java svn_stream_t read function to be compatible
           with the txdelta_next_window function              [trunk@1342720]

       02. JavaHL: Explicitly pass jobject jthis when
           processing dispose() call rather  than stashing a
           reference in the SVNBase class where it can be
           misused later                                      [trunk@1342810]

       03. JavaHL: Add SVN_JNI_STRING macro to reduce amount
           of code necessary to declare JNIStringHolder and
           check for exceptions                               [IN REVIEW]

       04. JavaHL: New method for creating java objects
           linked to their C++ counterpart                    [IN REVIEW]

       05. JavaHL: Factored out common context for later use
           by SVNRa class                                     [IN REVIEW]

       06. JavaHL: Minimal implementation of SVNRa            [TODO]

       07. JavaHL: Merge existing SVNRepoAccess into SVNRA    [TODO]


    * Implement other ra functions not requiring addition code
      refactoring                                             [TODO]


    * Apply editor support patches                            [TODO]
       xx. JavaHL: Support returning non const, empty rather
           than NULL hash as required by
           (svn_ra_get_commit_editor3)
           apr_hash_t *revprop_table parameter                [br.@1343456
 TBR]

       xx. JavaHL: Support keeping global reference to the
           callback java object as required by the RA API due
           to callback being used across method calls         [TODO]

       xx. JavaHL: Added support for creating of svn_string_t
           from JNIByteArray                                  [TODO]

       xx. JavaHL: Added SVN_JNI_BYTE_ARRAY macro to reduce
           amount of duplicate code dealing with jbyteArray
           wrapper and checking for exceptions                [TODO]

       xx. JavaHL: Factor out common java string map
           processing into StringsTable class from
           svn_string_t specific processing

Re: [PATCH] JavaHL: New method for creating java objects linked to their C++ counterpart

2012-06-14 Thread Hyrum K Wright
On Mon, Jun 11, 2012 at 2:39 PM, Vladimir Berezniker v...@hitechman.com wrote:


 On Mon, Jun 11, 2012 at 6:20 AM, Hyrum K Wright hyrum.wri...@wandisco.com
 wrote:

 On Thu, Jun 7, 2012 at 4:03 AM, Vladimir Berezniker v...@hitechman.com
 wrote:
  Hi All,
 
  The intention if this patch is to introduce reusable logic for creating
  java
  objects from within C++.  This keeps object creation logic fully within
  C++
  while leaving to java the decision as to when they will be destroyed. It
  will be used by RA code to allocate container object for items like
  SVNRa,
  Editor, Directory and File.
 
  Thank you,
 
  Vladimir
 
  [[[
  JavaHL: New method for creating java objects linked to their C++
  counterpart
 
  [ in subversion/bindings/javahl/native ]
 
  * SVNBase.cpp, SVNBase.h
    (createCppBoundObject): New method for creating java objects linked to
  their
      C++ counterpart
 
  [ in subversion/bindings/javahl/src/org/tigris/subversion/javahl/ ]
 
  * JNIObject.java: Base class for JNI linked java objects
  ]]]

 Looks good.  Is there a way to mark the new Java class as private?


 The original plan is to put RA java classes in a ra/ sub package, so package
 visible
 would not work in such case.  It is an abstract class, even if someone
 instantiates
 it will be harmless, as it will not create any C++ instances.  Would putting
 a good
 javadoc explaining it is not part of the public API address your concerns?

Yes, just documenting that it is private should work.  My primary
concern is backward compat, rather than technical issues.



 How do you plan to use this functionality?


 As compared to what SVNClient class does, where caller creates the java
 object which
 in turn calls native method call to create a matching native object. With RA
 layer,
 consumer calls a factory method:

 /**
 * Crates RA session for a given url with provided context
 * @param url to connect to
 * @param uuid of the remote repository, can be null if uuid check is not
 desired
 * @param config configuration to use for the session.
 * @return RA session
 */
 public static native ISVNRa createRaSession(String url, String uuid,
 ISVNRaConfig config);
 }

 that method is responsible for instantiation of the related C++ and Java
 classes.
 I think it is cleaner because:

 It better encapsulates implementation class: User only sees factory
 and ISVNRa interface
 Instantiation logic resides in a single language
 It avoids chicken and the egg with not year having cppAddr when java object
 is constructed: Allows enforcement that parameter is supplied via
 constructor argument

If you think that's cleaning, I'm happy to support it.  It's been a
while since I've been in that part of the code, and I don't think many
folks around here have spent much time reasoning about it lately.
You're probably the domain expert. :)

  Relatedly, do you have a

 high-level plan for this series of patches?


 Let me get the up to date version published. But high level summary for now:

 1. Have the following 3 patches reviewed, so that minimal version of SVNRa
 implementation
 can be committed to the javahl-ra branch and merged with the existing code
 base.

 [PATCH] JavaHL: Add SVN_JNI_STRING macro to reduce amount of code necessary
 to declare JNIStringHolder and check for exceptions
 [PATCH] JavaHL: New method for creating java objects linked to their C++
 counterpart
 [PATCH] JavaHL: Factor out common context to be shared between SVNClient and
 SVNRa classes

 2. Commit additional RA function calls that do not require additional
 enhancements to the
 common code to the branch. These are mostly pass-through calls to the C
 functions.

 3. Submit another ~6 patches for review om shared code that will support RA
 Editor code

 4. Commit revised RA Editor implementation to the branch

 5. Present proposal on making RA layer use Runtime exceptions rather than
 checked ones

Thanks for that, and updating the branch.

A couple more things, now that I'm thinking about it:
* There is a nascent project on google code called SVNJ
(http://code.google.com/p/svnj/) which had some kind of editor
implementation.  That code is Apache licensed, so we should be able to
use it.  You may want to see if anything there is useful.
* We're in the middle of a rather large effort to move from the
current delta editor to something called Ev2.  Ev2 should be simpler
from a bindings perspective, but it won't be fully implemented for a
while.  When it is implemented, it's something you'll want to support,
but we don't have a concrete timeline for it, yet.

Thanks for being patient with my somewhat sporadic responses. :)

-Hyrum


-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: [PATCH] JavaHL: New method for creating java objects linked to their C++ counterpart

2012-06-14 Thread Hyrum K Wright
On Thu, Jun 14, 2012 at 2:40 PM, Vladimir Berezniker v...@hitechman.com wrote:


 On Thu, Jun 14, 2012 at 8:13 AM, Hyrum K Wright hyrum.wri...@wandisco.com
 wrote:
 * We're in the middle of a rather large effort to move from the
 current delta editor to something called Ev2.  Ev2 should be simpler
 from a bindings perspective, but it won't be fully implemented for a
 while.  When it is implemented, it's something you'll want to support,
 but we don't have a concrete timeline for it, yet.


 I saw the emails and took a peek on the branch, it did look simpler to me.
 I did not look at it in huge detail though. I figured I need to get
 javahl-ra to a
 stable point and then see what it takes to port it to ev2 branch, once the
 editor
  v2 API considered stable enough (which it could be already).

The best place to look would be svn_editor.h, which defines the formal
API specification.  The API itself is very stable by this point, it's
just simply* a matter converting the existing systems over to it.
Because the delta editor is deeply embedded across a large part of the
codebase, this is an ongoing piece of work.

The RA layer (on trunk) already exposes Ev2 via the
svn_ra__get_commit_ev2() API.  Feel free to use it, knowing that its
implementation is still quite experimental, and that it may change /
go away at some point (but not the Ev2 concept).

-Hyrum

* Famous last words


-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: [PATCH] JavaHL: New method for creating java objects linked to their C++ counterpart

2012-06-14 Thread Hyrum K Wright
On Thu, Jun 14, 2012 at 3:11 PM, Greg Stein gst...@gmail.com wrote:
 On Thu, Jun 14, 2012 at 8:54 AM, Vladimir Berezniker v...@hitechman.com 
 wrote:
 On Thu, Jun 14, 2012 at 8:48 AM, Hyrum K Wright hyrum.wri...@wandisco.com
 wrote:
...
 The RA layer (on trunk) already exposes Ev2 via the
 svn_ra__get_commit_ev2() API.  Feel free to use it, knowing that its
 implementation is still quite experimental, and that it may change /
 go away at some point (but not the Ev2 concept).

 That is great, it will make it much easier being able to work with it
 straight on the javahl-branch.

 Dunno why Hyrum thinks it might go away... I think it is pretty solid :-)

I was referring to the fact that the API itself my change names, and
we don't promise that code depending upon it won't have to be updated
sometime in the future.

But, you probably already knew that. :P

-Hyrum



-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: svn commit: r1349661 - /subversion/trunk/subversion/libsvn_subr/cache-memcache.c

2012-06-13 Thread Hyrum K Wright
On Wed, Jun 13, 2012 at 9:22 AM,  gst...@apache.org wrote:
 Author: gstein
 Date: Wed Jun 13 07:22:00 2012
 New Revision: 1349661

 URL: http://svn.apache.org/viewvc?rev=1349661view=rev
 Log:
 Followup to r1349277: old APRs do not have APR_INT16_MAX

 * subversion/libsvn_subr/cache-memcache.c:
  (...): define APR_INT16_MAX if it is undefined

 Modified:
    subversion/trunk/subversion/libsvn_subr/cache-memcache.c

 Modified: subversion/trunk/subversion/libsvn_subr/cache-memcache.c
 URL: 
 http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cache-memcache.c?rev=1349661r1=1349660r2=1349661view=diff
 ==
 --- subversion/trunk/subversion/libsvn_subr/cache-memcache.c (original)
 +++ subversion/trunk/subversion/libsvn_subr/cache-memcache.c Wed Jun 13 
 07:22:00 2012
 @@ -36,6 +36,11 @@

  #include apr_memcache.h

 +/* Older APRs do not have this.  */
 +#ifndef APR_INT16_MAX
 +#define APR_INT16_MAX   (0x7fff)
 +#endif
 +

Instead defining this macro here, just include
private/svn_dep_compat.h, which conditionally defines this macro if
the APR version isn't high enough.

-Hyrum

  /* A note on thread safety:

    The apr_memcache_t object does its own mutex handling, and nothing





-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: Patch to remove libsvn_ra_neon

2012-06-13 Thread Hyrum K Wright
On Wed, Jun 13, 2012 at 9:33 AM, Greg Stein gst...@gmail.com wrote:
 On Tue, Jun 12, 2012 at 7:47 AM, Hyrum K Wright
 hyrum.wri...@wandisco.com wrote:
 We've had the should we remove neon? discussion before, and the
 consensus has felt to resolve in the affirmative.  Now is the time for
 action.

 I've got a 586 kb patch which removes libsvn_ra_neon from trunk.  It
 doesn't not remove the --http-library runtime option or config file
 code, but does remove the various build stuff which supports building
 neon.  I obviously haven't tested this on all platforms, and I
 wouldn't be surprised if it causes some short-term breakage.  My plan
 is to commit this patch Real Soon Now, pending any very vocal
 objections from folks on this list.

 Maybe do it as a number of reviewable pieces, rather than one mother patch?

I could do that, but most of the patch is just 'svn rm
subversion/libsvn_ra_neon' and the associated build system cruft:

$ svn diff | diffstat
 Makefile.in   |   18
 build.conf|   51
 build/ac-macros/neon.m4   |  169 -
 build/generator/gen_msvc_dsp.py   |1
 build/generator/gen_vcnet_vcproj.py   |1
 build/generator/gen_win.py|   99
 build/generator/templates/neon.dsp.ezt|   96
 build/generator/templates/neon.vcproj.ezt |   85
 build/generator/templates/neon.vcxproj.ezt|   64
 build/win32/make_dist.py  |6
 build/win32/vc6-build.bat.in  |4
 configure.ac  |   49
 subversion/libsvn_client/repos_diff.c |7
 subversion/libsvn_ra/ra_loader.c  |   29
 subversion/libsvn_ra_neon/README  |  112
 subversion/libsvn_ra_neon/commit.c| 1637 
 subversion/libsvn_ra_neon/fetch.c | 2825 --
 subversion/libsvn_ra_neon/file_revs.c |  388 ---
 subversion/libsvn_ra_neon/get_dated_rev.c |  169 -
 subversion/libsvn_ra_neon/get_deleted_rev.c   |  173 -
 subversion/libsvn_ra_neon/get_location_segments.c |  216 -
 subversion/libsvn_ra_neon/get_locations.c |  195 -
 subversion/libsvn_ra_neon/get_locks.c |  438 ---
 subversion/libsvn_ra_neon/lock.c  |  609 
 subversion/libsvn_ra_neon/log.c   |  572 
 subversion/libsvn_ra_neon/merge.c |  777 --
 subversion/libsvn_ra_neon/mergeinfo.c |  255 -
 subversion/libsvn_ra_neon/options.c   |  480 ---
 subversion/libsvn_ra_neon/props.c | 1520 ---
 subversion/libsvn_ra_neon/ra_neon.h   | 1199 -
 subversion/libsvn_ra_neon/replay.c|  508 ---
 subversion/libsvn_ra_neon/session.c   | 1271 -
 subversion/libsvn_ra_neon/util.c  | 1667 
 33 files changed, 23 insertions(+), 15667 deletions(-)
$

-Hyrum


-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: svn commit: r1349944 - /subversion/trunk/gen-make.py

2012-06-13 Thread Hyrum K Wright
On Wed, Jun 13, 2012 at 6:10 PM,  rhuij...@apache.org wrote:
 Author: rhuijben
 Date: Wed Jun 13 16:10:25 2012
 New Revision: 1349944

 URL: http://svn.apache.org/viewvc?rev=1349944view=rev
 Log:
 * gen-make.py
  Make the --with-neon= and --without-neon arguments on gen-make.py optional,
  just like how ./configure handles those. Windows doesn't use ./configure and
  this breaks tools that are friendly enough to provide hints.

I don't like this change.  ./configure *doesn't* work this way: it
errors when attempting to provide the now-defunct neon options.

Neon is gone, dead, buried.  We shouldn't be special casing it in this
manner: people who rely on neon will have more work to do than just
updating their scripts, and we don't need to coddle them along the
way.

-Hyrum


 Modified:
    subversion/trunk/gen-make.py

 Modified: subversion/trunk/gen-make.py
 URL: 
 http://svn.apache.org/viewvc/subversion/trunk/gen-make.py?rev=1349944r1=1349943r2=1349944view=diff
 ==
 --- subversion/trunk/gen-make.py (original)
 +++ subversion/trunk/gen-make.py Wed Jun 13 16:10:25 2012
 @@ -257,6 +257,13 @@ if __name__ == '__main__':
                             'disable-shared',
                             'installed-libs=',
                             'vsnet-version=',
 +
 +                            # Keep distributions that help by adding a path
 +                            # working. On unix this would be filtered by
 +                            # configure, but on Windows gen-make.py is used
 +                            # directly.
 +                            'with-neon=',
 +                            'without-neon',
                             ])
     if len(args)  1:
       _usage_exit(Too many arguments)
 @@ -281,6 +288,9 @@ if __name__ == '__main__':
         if opt != '--debug':
           rest.add(opt, val)
       del prev_conf
 +    elif opt == '--with-neon' or opt == '--without-neon':
 +      # Provide a warning that we ignored these arguments
 +      print(Ignoring no longer supported argument '%s' % opt)
     else:
       rest.add(opt, val)






-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: svn commit: r1349944 - /subversion/trunk/gen-make.py

2012-06-13 Thread Hyrum K Wright
On Wed, Jun 13, 2012 at 9:45 PM, Bert Huijben b...@qqmail.nl wrote:


 -Original Message-
 From: C. Michael Pilato [mailto:cmpil...@collab.net]
 Sent: Wednesday, June 13, 2012 9:34 PM
 To: dev@subversion.apache.org
 Cc: Hyrum K Wright; comm...@subversion.apache.org
 Subject: Re: svn commit: r1349944 - /subversion/trunk/gen-make.py

 On 06/13/2012 07:36 PM, Hyrum K Wright wrote:
  On Wed, Jun 13, 2012 at 6:10 PM,  rhuij...@apache.org wrote:
  Author: rhuijben
  Date: Wed Jun 13 16:10:25 2012
  New Revision: 1349944
 
  URL: http://svn.apache.org/viewvc?rev=1349944view=rev
  Log:
  * gen-make.py
   Make the --with-neon= and --without-neon arguments on gen-make.py
 optional,
   just like how ./configure handles those. Windows doesn't use ./configure
 and
   this breaks tools that are friendly enough to provide hints.
 
  I don't like this change.  ./configure *doesn't* work this way: it
  errors when attempting to provide the now-defunct neon options.

 Perhaps Bert meant only that, in general, configure allows you to specify
 options that it doesn't itself understand (while printing a warning about
 this fact).

 I think most distributions at least share some build code between versions.

 The buildbots only have a single set of dependencies for all the branches we 
 are building. Only once we released 1.9.0 neon will be gone and before that 
 we still have to support it (and even after that we still have to support all 
 the Subversion 1.0-1.7 clients that still use neon)

So, what you're saying is that we have single slaves, with single sets
of scripts building multiple branches.  And because of this, we can't
change the dependencies or script arguments on trunk, for fear of
breaking other branches.  That sounds a bit like the tail wagging the
dog.

What would happen if we added a flag on trunk that one of the slaves
needed to build, but didn't exist (and hence error'd) on other
branches?  Would we need to backport that flag as a no-op, simply to
make the build slaves happen?  That's exactly what this commit does,
only in the forward direction.

 Things get very easy if you just break the few remaining buildbots without 
 caring about the consequences, but it doesn’t make Subversion a better 
 product.

Making decisions simply to please the bots doesn't make Subversion a
better product, either.  They exist to help facilitate development.
If there is a problem with the bots, then FIX THE BOTS, rather than
introducing hacks in an attempt to please them.

In this specific case that might mean a slightly more complex setup,
which build each branch with a separate set of dependencies / scripts
/ inputs, since that's what end users will be doing anyway.

-Hyrum


-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Patch to remove libsvn_ra_neon

2012-06-12 Thread Hyrum K Wright
We've had the should we remove neon? discussion before, and the
consensus has felt to resolve in the affirmative.  Now is the time for
action.

I've got a 586 kb patch which removes libsvn_ra_neon from trunk.  It
doesn't not remove the --http-library runtime option or config file
code, but does remove the various build stuff which supports building
neon.  I obviously haven't tested this on all platforms, and I
wouldn't be surprised if it causes some short-term breakage.  My plan
is to commit this patch Real Soon Now, pending any very vocal
objections from folks on this list.

-Hyrum


-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: svn commit: r1336833 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_client/merge.c libsvn_wc/deprecated.c libsvn_wc/merge.c

2012-06-12 Thread Hyrum K Wright
On Thu, May 31, 2012 at 4:32 PM, Hyrum K Wright
hyrum.wri...@wandisco.com wrote:
 On Thu, May 10, 2012 at 2:13 PM,  rhuij...@apache.org wrote:
 Author: rhuijben
 Date: Thu May 10 19:13:11 2012
 New Revision: 1336833

 URL: http://svn.apache.org/viewvc?rev=1336833view=rev
 Log:
 Make the text and property merge handling of 'svn merge' of a single file an
 atomic operation by moving the handling into a single libsvn_wc call that
 installs or doesn't install the working queue items.

 * subversion/include/svn_wc.h
  (svn_wc_merge5): New function.
  (svn_wc_merge4): Deprecate function.

 * subversion/libsvn_client/merge.c
  (merge_file_changed): Update caller.

 * subversion/libsvn_wc/deprecated.c
  (svn_wc_merge4): New function. Wraps svn_wc_merge5().

 * subversion/libsvn_wc/merge.c
  (svn_wc_merge4): Rename to ...
  (svn_wc_merge5): ... this and add support for merging properties in the same
    operation. At the same time avoid a few more unneeded db operations.

 ...
 Modified: subversion/trunk/subversion/libsvn_wc/merge.c
 URL: 
 http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/merge.c?rev=1336833r1=1336832r2=1336833view=diff
 ==
 --- subversion/trunk/subversion/libsvn_wc/merge.c (original)
 +++ subversion/trunk/subversion/libsvn_wc/merge.c Thu May 10 19:13:11 2012
 ...
 @@ -1476,7 +1476,8 @@ svn_wc__internal_merge(svn_skel_t **work


  svn_error_t *
 -svn_wc_merge4(enum svn_wc_merge_outcome_t *merge_outcome,
 +svn_wc_merge5(enum svn_wc_merge_outcome_t *merge_content_outcome,
 +              enum svn_wc_notify_state_t *merge_props_outcome,
               svn_wc_context_t *wc_ctx,
               const char *left_abspath,
               const char *right_abspath,
 @@ -1489,6 +1490,7 @@ svn_wc_merge4(enum svn_wc_merge_outcome_
               svn_boolean_t dry_run,
               const char *diff3_cmd,
               const apr_array_header_t *merge_options,
 +              apr_hash_t *original_props,
               const apr_array_header_t *prop_diff,
               svn_wc_conflict_resolver_func2_t conflict_func,
               void *conflict_baton,
 @@ -1497,8 +1499,11 @@ svn_wc_merge4(enum svn_wc_merge_outcome_
               apr_pool_t *scratch_pool)
  {
   const char *dir_abspath = svn_dirent_dirname(target_abspath, scratch_pool);
 +  svn_skel_t *prop_items = NULL;
   svn_skel_t *work_items;
 -  apr_hash_t *actual_props;
 +  apr_hash_t *pristine_props = NULL;
 +  apr_hash_t *actual_props = NULL;
 +  apr_hash_t *new_actual_props = NULL;

   SVN_ERR_ASSERT(svn_dirent_is_absolute(left_abspath));
   SVN_ERR_ASSERT(svn_dirent_is_absolute(right_abspath));
 @@ -1508,37 +1513,86 @@ svn_wc_merge4(enum svn_wc_merge_outcome_
   if (!dry_run)
     SVN_ERR(svn_wc__write_check(wc_ctx-db, dir_abspath, scratch_pool));

 -  /* Sanity check:  the merge target must be under revision control,
 -   * unless the merge target is a copyfrom text, which lives in a
 -   * temporary file and does not exist in ACTUAL yet. */
 +  /* Sanity check:  the merge target must be a file under revision control 
 */
   {
 +    svn_wc__db_status_t status;
     svn_kind_t kind;
 -    svn_boolean_t hidden;
 -    SVN_ERR(svn_wc__db_read_kind(kind, wc_ctx-db, target_abspath, TRUE,
 -                                 scratch_pool));
 +    svn_boolean_t had_props;
 +    svn_boolean_t props_mod;

 -    if (kind == svn_kind_unknown)
 +    SVN_ERR(svn_wc__db_read_info(status, kind, NULL, NULL, NULL, NULL, 
 NULL,
 +                                 NULL, NULL, NULL, NULL, NULL, NULL, NULL,
 +                                 NULL, NULL, NULL, NULL, NULL, NULL, NULL,
 +                                 NULL, had_props, props_mod, NULL, NULL,
 +                                 NULL,
 +                                 wc_ctx-db, target_abspath,
 +                                 scratch_pool, scratch_pool));
 +
 +    if (kind != svn_kind_file || (status != svn_wc__db_status_normal
 +                                   status != svn_wc__db_status_added))
       {
 -        *merge_outcome = svn_wc_merge_no_merge;
 +        *merge_content_outcome = svn_wc_merge_no_merge;
 +        if (merge_props_outcome)
 +          *merge_props_outcome = svn_wc_merge_no_merge;

 There is a type mismatch here: *merge_props_outcome is an enum of type
 svn_wc_notify_state_t, but svn_wc_merge_no_merge is one of
 svn_wc_merge_outcome_t.

Bert,
Any response on this?  I'm concerned about this seemingly arbitrary
type mismatch, and the effects it could have.  This probably only work
by luck at this point...

-Hyrum


-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: svn commit: r1349316 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

2012-06-12 Thread Hyrum K Wright
On Tue, Jun 12, 2012 at 3:22 PM,  stef...@apache.org wrote:
 Author: stefan2
 Date: Tue Jun 12 13:22:40 2012
 New Revision: 1349316

 URL: http://svn.apache.org/viewvc?rev=1349316view=rev
 Log:
 Fix unreachable code warning under Windows. In the for (i=0;iretries;++i)
 loop header, the ++i would never be executed. So, replace the condition and
 increment with a target-dependent macro.

 * subversion/libsvn_fs_fs/fs_fs.c
  (RECOVERABLE_RETRY_COUNT): drop
  (RECOVERABLE_RETRY_LOOP): new macro
  (read_current, revision_proplist, get_and_increment_txn_key_body):
   use the new macro to guard retries

 Modified:
    subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

 Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
 URL: 
 http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1349316r1=1349315r2=1349316view=diff
 ==
 --- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
 +++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Tue Jun 12 13:22:40 2012
 @@ -1451,8 +1451,6 @@ svn_fs_fs__upgrade(svn_fs_t *fs, apr_poo
  * these macros do not.
  */

 -#define RECOVERABLE_RETRY_COUNT 10
 -
  #ifdef ESTALE
  /* Do not use do-while due to the embedded 'continue'.  */
  #define RETRY_RECOVERABLE(err, filehandle, expr)                \
 @@ -1481,9 +1479,12 @@ svn_fs_fs__upgrade(svn_fs_t *fs, apr_poo
           return svn_error_trace(err);                         \
       }                                                         \
   } else
 +#define RECOVERABLE_RETRY_LOOP \
 +  i  RECOVERABLE_RETRY_COUNT; i++
  #else
  #define RETRY_RECOVERABLE(err, filehandle, expr)  SVN_ERR(expr)
  #define IGNORE_RECOVERABLE(err, expr) SVN_ERR(expr)
 +#define RECOVERABLE_RETRY_LOOP ;
  #endif

  /* Long enough to hold: svn_revnum_t node id copy id\0
 @@ -1508,7 +1509,7 @@ read_current(const char *fname, char **b

   *buf = apr_palloc(pool, CURRENT_BUF_LEN);
   iterpool = svn_pool_create(pool);
 -  for (i = 0; i  RECOVERABLE_RETRY_COUNT; i++)
 +  for (i = 0; RECOVERABLE_RETRY_LOOP)
     {
       svn_pool_clear(iterpool);

 @@ -3206,7 +3207,7 @@ revision_proplist(apr_hash_t **proplist_

       proplist = apr_hash_make(pool);
       iterpool = svn_pool_create(pool);
 -      for (i = 0; i  RECOVERABLE_RETRY_COUNT; i++)
 +      for (i = 0; RECOVERABLE_RETRY_LOOP)
         {
           svn_pool_clear(iterpool);

 @@ -5038,7 +5039,7 @@ get_and_increment_txn_key_body(void *bat
   cb-txn_id = apr_palloc(cb-pool, MAX_KEY_SIZE);

   iterpool = svn_pool_create(pool);
 -  for (i = 0; i  RECOVERABLE_RETRY_COUNT; ++i)
 +  for (i = 0; RECOVERABLE_RETRY_LOOP)
     {
       svn_pool_clear(iterpool);



On non-windows platforms, this gives:
[[[
subversion/libsvn_fs_fs/fs_fs.c:1512:15: error: use of undeclared identifier
  'RECOVERABLE_RETRY_COUNT'
  for (i = 0; RECOVERABLE_RETRY_LOOP)
  ^
subversion/libsvn_fs_fs/fs_fs.c:1483:7: note: expanded from macro
  'RECOVERABLE_RETRY_LOOP'
  i  RECOVERABLE_RETRY_COUNT; i++
  ^
subversion/libsvn_fs_fs/fs_fs.c:3210:19: error: use of undeclared identifier
  'RECOVERABLE_RETRY_COUNT'
  for (i = 0; RECOVERABLE_RETRY_LOOP)
  ^
subversion/libsvn_fs_fs/fs_fs.c:1483:7: note: expanded from macro
  'RECOVERABLE_RETRY_LOOP'
  i  RECOVERABLE_RETRY_COUNT; i++
  ^
subversion/libsvn_fs_fs/fs_fs.c:5042:15: error: use of undeclared identifier
  'RECOVERABLE_RETRY_COUNT'
  for (i = 0; RECOVERABLE_RETRY_LOOP)
  ^
subversion/libsvn_fs_fs/fs_fs.c:1483:7: note: expanded from macro
  'RECOVERABLE_RETRY_LOOP'
  i  RECOVERABLE_RETRY_COUNT; i++
  ^
3 errors generated.
]]]

-Hyrum



-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: Patch to remove libsvn_ra_neon

2012-06-12 Thread Hyrum K Wright
On Tue, Jun 12, 2012 at 11:57 PM, Stefan Sperling s...@elego.de wrote:
 On Tue, Jun 12, 2012 at 01:47:13PM +0200, Hyrum K Wright wrote:
 We've had the should we remove neon? discussion before, and the
 consensus has felt to resolve in the affirmative.  Now is the time for
 action.

 We should move this issue into the 1.8.0 milestone if neon is deleted:
 http://subversion.tigris.org/issues/show_bug.cgi?id=4116

We should probably discuss that issue in a separate thread, but my
experience in attempting to rewrite svnrdump to use proper file and
directory batons is that it probably won't happen.  The dump file
format is much less amenable to streamy generation from the piecemeal
callback paradigm that Ev1 provides, and the right (and probably
easiest) thing to do at this point is just implement svnrdump using
Ev2.  The work I'm currently doing with replay would hopefully pave
the way for that.

-Hyrum


-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: svn commit: r1348770 - /subversion/trunk/subversion/libsvn_subr/pool.c

2012-06-11 Thread Hyrum K Wright
On Mon, Jun 11, 2012 at 11:33 AM,  i...@apache.org wrote:
 Author: ivan
 Date: Mon Jun 11 09:33:36 2012
 New Revision: 1348770

 URL: http://svn.apache.org/viewvc?rev=1348770view=rev
 Log:
 Fix compiler warning.

 * subversion/libsvn_subr/pool.c
  (abort_on_pool_failure): Add return statement to make compiler happy.

 Modified:
    subversion/trunk/subversion/libsvn_subr/pool.c

 Modified: subversion/trunk/subversion/libsvn_subr/pool.c
 URL: 
 http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/pool.c?rev=1348770r1=1348769r2=1348770view=diff
 ==
 --- subversion/trunk/subversion/libsvn_subr/pool.c (original)
 +++ subversion/trunk/subversion/libsvn_subr/pool.c Mon Jun 11 09:33:36 2012
 @@ -53,6 +53,7 @@ abort_on_pool_failure(int retcode)
      And we don't have any of it... */
   printf(Out of memory - terminating application.\n);
   abort();
 +  return 0; /* not reached */
  }



This change actually *adds* the following warning for (clang on Darwin):
[[[
clang: warning: argument unused during compilation: '-no-cpp-precomp'
subversion/libsvn_subr/pool.c:56:10: warning: will never be executed
  [-Wunreachable-code]
  return 0; /* not reached */
 ^
1 warning generated.
]]]

I know this isn't the only place in our code where we have such
constructions.  Can we do something compiler-specific which then
applies to all such places?

-Hyrum


-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: svn commit: r1348782 - /subversion/trunk/subversion/tests/cmdline/blame_tests.py

2012-06-11 Thread Hyrum K Wright
Greg,
As soon as I do the merge, this will fix the last test failure on the
ev2-export branch.  It looks like we've got a solid commit editor
driving Ev2.

My next target, hopefully for this week, is to implement replay using
Ev2.  Since this is about reading data, and is mainly server-side, I
plan on implementing it on trunk.

-Hyrum

On Mon, Jun 11, 2012 at 12:08 PM,  hwri...@apache.org wrote:
 Author: hwright
 Date: Mon Jun 11 10:08:17 2012
 New Revision: 1348782

 URL: http://svn.apache.org/viewvc?rev=1348782view=rev
 Log:
 In blame test 7, ensure we don't have any pre-existing properties which may
 interfere with the running of subsequent loops.

 * subversion/tests/cmdline/blame_tests.py
  (blame_eol_styles): For each eol-style, delete the property before
    manipulating the file.

 Modified:
    subversion/trunk/subversion/tests/cmdline/blame_tests.py

 Modified: subversion/trunk/subversion/tests/cmdline/blame_tests.py
 URL: 
 http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/blame_tests.py?rev=1348782r1=1348781r2=1348782view=diff
 ==
 --- subversion/trunk/subversion/tests/cmdline/blame_tests.py (original)
 +++ subversion/trunk/subversion/tests/cmdline/blame_tests.py Mon Jun 11 
 10:08:17 2012
 @@ -333,6 +333,7 @@ def blame_eol_styles(sbox):

   # do the test for each eol-style
   for eol in ['CR', 'LF', 'CRLF', 'native']:
 +    svntest.main.run_svn(None, 'propdel', 'svn:eol-style', file_path)
     svntest.main.file_write(file_path, This is no longer the file 'iota'.\n)

     for i in range(1,3):





-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: [PATCH] JavaHL: New method for creating java objects linked to their C++ counterpart

2012-06-11 Thread Hyrum K Wright
On Thu, Jun 7, 2012 at 4:03 AM, Vladimir Berezniker v...@hitechman.com wrote:
 Hi All,

 The intention if this patch is to introduce reusable logic for creating java
 objects from within C++.  This keeps object creation logic fully within C++
 while leaving to java the decision as to when they will be destroyed. It
 will be used by RA code to allocate container object for items like SVNRa,
 Editor, Directory and File.

 Thank you,

 Vladimir

 [[[
 JavaHL: New method for creating java objects linked to their C++ counterpart

 [ in subversion/bindings/javahl/native ]

 * SVNBase.cpp, SVNBase.h
   (createCppBoundObject): New method for creating java objects linked to
 their
     C++ counterpart

 [ in subversion/bindings/javahl/src/org/tigris/subversion/javahl/ ]

 * JNIObject.java: Base class for JNI linked java objects
 ]]]

Looks good.  Is there a way to mark the new Java class as private?

How do you plan to use this functionality?  Relatedly, do you have a
high-level plan for this series of patches?  (Or, perhaps it was in
your first set of mail, and I've just overlooked it...)

-Hyrum


-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: [PATCH] JavaHL: Factor out common context to be shared between SVNClient and SVNRa classes

2012-06-11 Thread Hyrum K Wright
On Thu, Jun 7, 2012 at 5:09 AM, Vladimir Berezniker v...@hitechman.com wrote:
 Greetings,

 This patch signifies a point post which I can merge the existing logic on
 the
 javahl-ra branch with starting parts of my proposed enhancements. So I hope
 people still have some patience left to review them.

 This patch moves parts of client context that are shared with ra context
 into
 common classes.  As I cannot get svn to generate patches that deal with
 copies
 of existing file, before applying below the following svn copy command have
 to
 be issued:

 svn cp
 subversion/bindings/javahl/src/org/apache/subversion/javahl/SVNClient.java
 subversion/bindings/javahl/src/org/apache/subversion/javahl/CommonContext.java
 svn cp subversion/bindings/javahl/native/ClientContext.h
 subversion/bindings/javahl/native/CommonContext.h
 svn cp subversion/bindings/javahl/native/ClientContext.cpp
 subversion/bindings/javahl/native/CommonContext.cpp


 Also please note that java part of this patch is already applied in r1343452
  r1347345
 on javahl-ra branch, due to my earlier mistake.

If this work is specific to the things happening on the branch, I
think you're fine committing it directly there.

One question, though.  The ClientContext object directly mirrors the
svn_client_context_t struct in the C layer (the analogs are probably
obvious).  To my knowledge, we don't have a similar struct for the C
ra layer.  I'm just a little confused why we need to break this clear
correlation with the C layer in Java, when we don't need to in C.
I'll need to think on this a bit...

-Hyrum


 Thank you for your help,

 Vladimir

 [[[
 JavaHL: Factor out common context to be shared between SVNClient and SVNRa
 classes

 [ in subversion/bindings/javahl/src/org/tigris/subversion/javahl/ ]

 * CommonContext.java,
   SVNClient.java
   (ClientContext): Move to progress listener into CommonContext

 [ in subversion/bindings/javahl/native ]

 * CommonContext.cpp,
   CommonContext.h,
   ClientContext.cpp,
   ClientContext.h
   (username, password, getConfigDirectory, setConfigDirectory, setPrompt,
    cancelOperation, progress): Move from ClientContext to CommonContext

 * CommonContext.cpp,
   CommonContext.h
   (attachJavaObject): New function to hold common logic of attaching to the
     java CommonContext class used for callbacks
   (getConfigData, getAuthBaton): Split getContext into separate
 configuration
     data setup and authentication data setup to better reflect their
 different life cycles
   (getClientName): New function providing client name to be used in
 callbacks

 * ClientContext.cpp,
   ClientContext.h
   (ClientContext, getContext): Use the factored out CommonContext member
     variables and functions
 ]]]




-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: [PATCH] JavaHL: Factor out common context to be shared between SVNClient and SVNRa classes

2012-06-11 Thread Hyrum K Wright
On Mon, Jun 11, 2012 at 12:56 PM, Hyrum K Wright
hyrum.wri...@wandisco.com wrote:
 On Thu, Jun 7, 2012 at 5:09 AM, Vladimir Berezniker v...@hitechman.com 
 wrote:
 Greetings,

 This patch signifies a point post which I can merge the existing logic on
 the
 javahl-ra branch with starting parts of my proposed enhancements. So I hope
 people still have some patience left to review them.

 This patch moves parts of client context that are shared with ra context
 into
 common classes.  As I cannot get svn to generate patches that deal with
 copies
 of existing file, before applying below the following svn copy command have
 to
 be issued:

 svn cp
 subversion/bindings/javahl/src/org/apache/subversion/javahl/SVNClient.java
 subversion/bindings/javahl/src/org/apache/subversion/javahl/CommonContext.java
 svn cp subversion/bindings/javahl/native/ClientContext.h
 subversion/bindings/javahl/native/CommonContext.h
 svn cp subversion/bindings/javahl/native/ClientContext.cpp
 subversion/bindings/javahl/native/CommonContext.cpp


 Also please note that java part of this patch is already applied in r1343452
  r1347345
 on javahl-ra branch, due to my earlier mistake.

 If this work is specific to the things happening on the branch, I
 think you're fine committing it directly there.

 One question, though.  The ClientContext object directly mirrors the
 svn_client_context_t struct in the C layer (the analogs are probably
 obvious).  To my knowledge, we don't have a similar struct for the C
 ra layer.  I'm just a little confused why we need to break this clear
 correlation with the C layer in Java, when we don't need to in C.
 I'll need to think on this a bit...

Oh, one other thing: we like to separate white-space and functional
changes into separate commits.  I noticed a large part of your patch
is reindenting various files to conform with the rest of the codebase
(good!), and I'm wondering if we could apply that portion independent
of the others.

-Hyrum


 Thank you for your help,

 Vladimir

 [[[
 JavaHL: Factor out common context to be shared between SVNClient and SVNRa
 classes

 [ in subversion/bindings/javahl/src/org/tigris/subversion/javahl/ ]

 * CommonContext.java,
   SVNClient.java
   (ClientContext): Move to progress listener into CommonContext

 [ in subversion/bindings/javahl/native ]

 * CommonContext.cpp,
   CommonContext.h,
   ClientContext.cpp,
   ClientContext.h
   (username, password, getConfigDirectory, setConfigDirectory, setPrompt,
    cancelOperation, progress): Move from ClientContext to CommonContext

 * CommonContext.cpp,
   CommonContext.h
   (attachJavaObject): New function to hold common logic of attaching to the
     java CommonContext class used for callbacks
   (getConfigData, getAuthBaton): Split getContext into separate
 configuration
     data setup and authentication data setup to better reflect their
 different life cycles
   (getClientName): New function providing client name to be used in
 callbacks

 * ClientContext.cpp,
   ClientContext.h
   (ClientContext, getContext): Use the factored out CommonContext member
     variables and functions
 ]]]




 --

 uberSVN: Apache Subversion Made Easy
 http://www.uberSVN.com/



-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


XPASSes in merge symetric tests

2012-06-11 Thread Hyrum K Wright
Julian,
I'm seeing the following XPASSs on trunk:
At least one test XPASSED, checking /Users/Hyrum/dev/svn-trunk2/tests.log
XPASS: merge_symmetric_tests.py 7: merge_to_and_fro_1_1
XPASS: merge_symmetric_tests.py 8: merge_to_and_fro_1_2
XPASS: merge_symmetric_tests.py 9: merge_to_and_fro_2_1
XPASS: merge_symmetric_tests.py 10: merge_to_and_fro_2_2
XPASS: merge_symmetric_tests.py 11: merge_to_and_fro_3_1
XPASS: merge_symmetric_tests.py 12: merge_to_and_fro_3_2
XPASS: merge_symmetric_tests.py 13: merge_to_and_fro_4_1
XPASS: merge_symmetric_tests.py 14: merge_to_and_fro_4_2

They all look like they are part of your symmetric merge test suite.
While I know that code is experimental, it would be nice if they
didn't clutter the test output.

-Hyrum

-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: What mechanisms for code comparison and merge between versions on svn are used?

2012-06-11 Thread Hyrum K Wright
On Sun, Jun 10, 2012 at 8:51 AM, Carlos Andrade carlosvia...@gmail.com wrote:
 Hi,

 I was interested on learning more about the mechanisms used on svn to verify
 if a certain code has been modified or not and how the merge function works.
 Is there any non-code documentation available for this? I would also be
 interested on knowing if svn employs any abstract syntax tree comparisons
 during this comparison between versions and merging (or maybe anywhere else)
 for any of its usage.

I'm not sure what you are asking for, but I'll try.  Subversion just
treats files as streams of bits, and compares them (often using hashes
and mtimes) to see what has changed.  Subversion does special case
known text files, based upon mimetype and some simple heuristics, and
when merging them uses line granularity to do so.  We don't use any
language-specific comparison or merging.

 (I hope I hit the right mailing list since this is not usage related).

It's not development-related, either. :)

-Hyrum



-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: XPASSes in merge symetric tests

2012-06-11 Thread Hyrum K Wright
On Mon, Jun 11, 2012 at 2:13 PM, Philip Martin
philip.mar...@wandisco.com wrote:
 Hyrum K Wright hyrum.wri...@wandisco.com writes:

 Julian,
 I'm seeing the following XPASSs on trunk:
 At least one test XPASSED, checking /Users/Hyrum/dev/svn-trunk2/tests.log
 XPASS: merge_symmetric_tests.py 7: merge_to_and_fro_1_1
 XPASS: merge_symmetric_tests.py 8: merge_to_and_fro_1_2
 XPASS: merge_symmetric_tests.py 9: merge_to_and_fro_2_1
 XPASS: merge_symmetric_tests.py 10: merge_to_and_fro_2_2
 XPASS: merge_symmetric_tests.py 11: merge_to_and_fro_3_1
 XPASS: merge_symmetric_tests.py 12: merge_to_and_fro_3_2
 XPASS: merge_symmetric_tests.py 13: merge_to_and_fro_4_1
 XPASS: merge_symmetric_tests.py 14: merge_to_and_fro_4_2

 They all look like they are part of your symmetric merge test suite.
 While I know that code is experimental, it would be nice if they
 didn't clutter the test output.

 The tests PASS when SVN_DEBUG is defined because that causes
 SVN_WITH_SYMMETRIC_MERGE to be defined.  In a non-debug build build the
 tests will FAIL.  I'm not sure how else these tests could be handled,
 some python to detect SVN_WITH_SYMMETRIC_MERGE (how?) and then SKIP the
 tests?  I suppose the --symmetric option could be made conditional and
 the python could detect that.

Don't unconditionally define SVN_WITH_SYMMETRIC_MERGE with SVN_DEBUG.
We could easily add a configure flag, or interested parties could
define the value at compile time (we used both of these for the Ev2
shims).  The majority of developers aren't using the SYMMETRIC_MERGE
stuff, but they are likely using maintainer mode.  We should make
things easier for the most people.

-Hyrum



-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: [PATCH] JavaHL: Factor out common context to be shared between SVNClient and SVNRa classes

2012-06-11 Thread Hyrum K Wright
On Mon, Jun 11, 2012 at 2:58 PM, Vladimir Berezniker v...@hitechman.com wrote:


 On Mon, Jun 11, 2012 at 6:56 AM, Hyrum K Wright hyrum.wri...@wandisco.com
 wrote:

 On Thu, Jun 7, 2012 at 5:09 AM, Vladimir Berezniker v...@hitechman.com
 wrote:
  Greetings,
 
  This patch signifies a point post which I can merge the existing logic
  on
  the
  javahl-ra branch with starting parts of my proposed enhancements. So I
  hope
  people still have some patience left to review them.
 
  This patch moves parts of client context that are shared with ra context
  into
  common classes.  As I cannot get svn to generate patches that deal with
  copies
  of existing file, before applying below the following svn copy command
  have
  to
  be issued:
 
  svn cp
 
  subversion/bindings/javahl/src/org/apache/subversion/javahl/SVNClient.java
 
  subversion/bindings/javahl/src/org/apache/subversion/javahl/CommonContext.java
  svn cp subversion/bindings/javahl/native/ClientContext.h
  subversion/bindings/javahl/native/CommonContext.h
  svn cp subversion/bindings/javahl/native/ClientContext.cpp
  subversion/bindings/javahl/native/CommonContext.cpp
 
 
  Also please note that java part of this patch is already applied in
  r1343452
   r1347345
  on javahl-ra branch, due to my earlier mistake.

 If this work is specific to the things happening on the branch, I
 think you're fine committing it directly there.


 Similar to the other patches so far, while the logic is used by RA
 layer. The re factoring is applicable to the trunk on its own. Though
 not as useful as it does not take care of the additional flexibility on
 its own. It does lean more towards RA specific enhancements than the
 other patches.



 One question, though.  The ClientContext object directly mirrors the
 svn_client_context_t struct in the C layer (the analogs are probably
 obvious).  To my knowledge, we don't have a similar struct for the C
 ra layer.  I'm just a little confused why we need to break this clear
 correlation with the C layer in Java, when we don't need to in C.
 I'll need to think on this a bit...


 The refactoring represents the common elements between
 the svn_client_ctx_t,
 svn_ra_callbacks2_t and svn_commit_callback2_t needed by the
 (svn_ra_get_commit_editor3) function. This overlap is demonstrated by the
 existing code in the (svn_client__open_ra_session_internal) function that
 goes
 between the client context and ra session. Specifically client and ra layers
 share
 the following objects:

    * svn_auth_baton_t
    * apr_hash_t (config) needed to create svn_auth_baton_t
    * svn_ra_progress_notify_func_t

 Hope this helps,

It helps a lot!  Should we then call the new object something a bit
more descriptive?  Maybe CommitContext?

-Hyrum



-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: [PATCH] JavaHL: Support logging of the static method calls

2012-06-06 Thread Hyrum K Wright
Looks good, +1 to commit to trunk.

-Hyrum

On Wed, Jun 6, 2012 at 1:39 AM, Vladimir Berezniker v...@hitechman.com wrote:
 [[[
 JavaHL: Support logging of the static method calls

 [ in subversion/bindings/javahl/native ]

 * JNIStackElement.cpp
   (JNIStackElement): Add logic to deal with NULL jthis, which happens with
     static method calls
 ]]]



-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Mac OS X buildslaves

2012-06-06 Thread Hyrum K Wright
Hello infra!

In the past, the Subversion project has had build slave configured to
run our testsuite on Mac OS.  These slaves, svn-x64-macosx-gnu-shared,
svn-x64-macosx-gnu-shared-daily-ra_serf, were useful, but privately
owned and have been dark for several months.  What type of resources
exist within Infra to set up a Mac OS-based buildslave?

Thanks,
-Hyrum


MacOS buildslave?

2012-06-04 Thread Hyrum K Wright
We used to have a buildslave for MacOS, but it went offline and hasn't
been replaced.  Do we know if infrastructure has the resources to
provide a slave for testing Subversion on MacOS?  I consistently see a
local failure in stat test 19 on a clean working copy, but since it
only appears on MacOS, and we don't have a slave for that platform, it
doesn't feel like it's a priority for people to fix.  I think getting
a slave would make the problem more visible (and more likely to be
fixed).

-Hyrum

-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: validity check error? huh?

2012-06-01 Thread Hyrum K Wright
On Fri, Jun 1, 2012 at 6:19 AM, Bert Huijben b...@qqmail.nl wrote:
...

 I don’t see a problem with bumping the requirement to 3.7.9, but that
 decision should be made here on this list. Not by me :)

We've talked elsewhere about bumping the required version to 3.7.12 to
take advantage of further improvements which dramatically impact us.
My only reason for not doing so just yet is that the buildslaves
aren't yet equipped with 3.7.12, so making it required would cause
them all to start failing, which defeats their utility.  I don't know
about 3.7.9, but suspect it is new enough the slaves haven't caught up
to it as well.

At some point, though, we can't let that drive the decision, and
should just bump the requirement.

-Hyrum



-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: svn commit: r1336833 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_client/merge.c libsvn_wc/deprecated.c libsvn_wc/merge.c

2012-05-31 Thread Hyrum K Wright
On Thu, May 10, 2012 at 2:13 PM,  rhuij...@apache.org wrote:
 Author: rhuijben
 Date: Thu May 10 19:13:11 2012
 New Revision: 1336833

 URL: http://svn.apache.org/viewvc?rev=1336833view=rev
 Log:
 Make the text and property merge handling of 'svn merge' of a single file an
 atomic operation by moving the handling into a single libsvn_wc call that
 installs or doesn't install the working queue items.

 * subversion/include/svn_wc.h
  (svn_wc_merge5): New function.
  (svn_wc_merge4): Deprecate function.

 * subversion/libsvn_client/merge.c
  (merge_file_changed): Update caller.

 * subversion/libsvn_wc/deprecated.c
  (svn_wc_merge4): New function. Wraps svn_wc_merge5().

 * subversion/libsvn_wc/merge.c
  (svn_wc_merge4): Rename to ...
  (svn_wc_merge5): ... this and add support for merging properties in the same
    operation. At the same time avoid a few more unneeded db operations.

...
 Modified: subversion/trunk/subversion/libsvn_wc/merge.c
 URL: 
 http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/merge.c?rev=1336833r1=1336832r2=1336833view=diff
 ==
 --- subversion/trunk/subversion/libsvn_wc/merge.c (original)
 +++ subversion/trunk/subversion/libsvn_wc/merge.c Thu May 10 19:13:11 2012
...
 @@ -1476,7 +1476,8 @@ svn_wc__internal_merge(svn_skel_t **work


  svn_error_t *
 -svn_wc_merge4(enum svn_wc_merge_outcome_t *merge_outcome,
 +svn_wc_merge5(enum svn_wc_merge_outcome_t *merge_content_outcome,
 +              enum svn_wc_notify_state_t *merge_props_outcome,
               svn_wc_context_t *wc_ctx,
               const char *left_abspath,
               const char *right_abspath,
 @@ -1489,6 +1490,7 @@ svn_wc_merge4(enum svn_wc_merge_outcome_
               svn_boolean_t dry_run,
               const char *diff3_cmd,
               const apr_array_header_t *merge_options,
 +              apr_hash_t *original_props,
               const apr_array_header_t *prop_diff,
               svn_wc_conflict_resolver_func2_t conflict_func,
               void *conflict_baton,
 @@ -1497,8 +1499,11 @@ svn_wc_merge4(enum svn_wc_merge_outcome_
               apr_pool_t *scratch_pool)
  {
   const char *dir_abspath = svn_dirent_dirname(target_abspath, scratch_pool);
 +  svn_skel_t *prop_items = NULL;
   svn_skel_t *work_items;
 -  apr_hash_t *actual_props;
 +  apr_hash_t *pristine_props = NULL;
 +  apr_hash_t *actual_props = NULL;
 +  apr_hash_t *new_actual_props = NULL;

   SVN_ERR_ASSERT(svn_dirent_is_absolute(left_abspath));
   SVN_ERR_ASSERT(svn_dirent_is_absolute(right_abspath));
 @@ -1508,37 +1513,86 @@ svn_wc_merge4(enum svn_wc_merge_outcome_
   if (!dry_run)
     SVN_ERR(svn_wc__write_check(wc_ctx-db, dir_abspath, scratch_pool));

 -  /* Sanity check:  the merge target must be under revision control,
 -   * unless the merge target is a copyfrom text, which lives in a
 -   * temporary file and does not exist in ACTUAL yet. */
 +  /* Sanity check:  the merge target must be a file under revision control */
   {
 +    svn_wc__db_status_t status;
     svn_kind_t kind;
 -    svn_boolean_t hidden;
 -    SVN_ERR(svn_wc__db_read_kind(kind, wc_ctx-db, target_abspath, TRUE,
 -                                 scratch_pool));
 +    svn_boolean_t had_props;
 +    svn_boolean_t props_mod;

 -    if (kind == svn_kind_unknown)
 +    SVN_ERR(svn_wc__db_read_info(status, kind, NULL, NULL, NULL, NULL, 
 NULL,
 +                                 NULL, NULL, NULL, NULL, NULL, NULL, NULL,
 +                                 NULL, NULL, NULL, NULL, NULL, NULL, NULL,
 +                                 NULL, had_props, props_mod, NULL, NULL,
 +                                 NULL,
 +                                 wc_ctx-db, target_abspath,
 +                                 scratch_pool, scratch_pool));
 +
 +    if (kind != svn_kind_file || (status != svn_wc__db_status_normal
 +                                   status != svn_wc__db_status_added))
       {
 -        *merge_outcome = svn_wc_merge_no_merge;
 +        *merge_content_outcome = svn_wc_merge_no_merge;
 +        if (merge_props_outcome)
 +          *merge_props_outcome = svn_wc_merge_no_merge;

There is a type mismatch here: *merge_props_outcome is an enum of type
svn_wc_notify_state_t, but svn_wc_merge_no_merge is one of
svn_wc_merge_outcome_t.

-Hyrum

         return SVN_NO_ERROR;
       }

 -    SVN_ERR(svn_wc__db_node_hidden(hidden, wc_ctx-db, target_abspath,
 -                                   scratch_pool));
 +    if (merge_props_outcome  had_props)
 +      {
 +        SVN_ERR(svn_wc__db_read_pristine_props(pristine_props,
 +                                               wc_ctx-db, target_abspath,
 +                                               scratch_pool, scratch_pool));
 +      }
 +    else if (merge_props_outcome)
 +      pristine_props = apr_hash_make(scratch_pool);

 -    if (hidden)
 +    if (props_mod)
       {
 -        *merge_outcome = svn_wc_merge_no_merge;
 -      

Re: [PATCH] JavaHL: Reduce amount of duplicate code used to check C++ pointer extracted from the java object

2012-05-31 Thread Hyrum K Wright
On Thu, May 31, 2012 at 2:35 AM, Greg Stein gst...@gmail.com wrote:
 On Thu, May 31, 2012 at 12:43 AM, Vladimir Berezniker
 v...@hitechman.com wrote:
 Patch 01 - Introduce macro

 [[[
 JavaHL: Added CPPADDR_NULL_PTR macro to reduce amount of duplicate code
 checking C++ pointer extracted from the java object

 [ in subversion/bindings/javahl/native ]

 * JNIUtil.h
   (CPPADDR_NULL_PTR): New macro to test for NULL pointer and raise java
     exception if necessary
 ]]]

 Replying to just this patch. The second patch seems pretty mechanical.
 And we're only looking at minor nits.

 (sorry, but the patch doesn't inline into this response, so let's just
 be flexible here...)


 The macro argument substitutions need to be parenthesized for safety.
 So it would be: (expr) == NULL, and it would be: return (ret_val);

 Next bit: the indentation in the diff seems to be off. Are there TAB
 characters in there? the JNIUTIL:: and the return lines have different
 indents in the patch that I'm looking at. That is either sloppy SPC
 character indents, or a TAB is present.

 Lastly, there is an extra space character before the ; in the return
 statement. That should be eliminated.


 Fix the above three problems, and I'm +1 for you to commit just patch #1.

 I have not reviewed #2, but the first patch seems reasonable to
 simplify (as done in #2). I also await others to comment on the
 applicability of patch #2.

 I do seem to recall that C++ tried to do away with the preprocessor.
 It would be nice to follow that ideal, but looking at this macro... I
 have no idea how to map it into proper, non-CPP concepts. If you
 know, that would be better. Otherwise... meh. CPP is just fine with me
 (and screw the C++ academic purity).

Like most things, C++ tried to reinvent the preprocessor and ended up
with a half-baked solution which only partially replaces the thing it
was intended to fix.  We have several similar helper macros in the
JavaHL C++ layer which do are similar in checking for error conditions
(SVN_JNI_NULL_PTR_EX, for example) and possibly raising an exception.
I don't know if the proposed macro is for a common situation, but
there is precedent.

-Hyrum



-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: [PATCH] JavaHL: Reduce amount of duplicate code used to check C++ pointer extracted from the java object

2012-05-31 Thread Hyrum K Wright
On Thu, May 31, 2012 at 4:36 AM, Julian Foad julianf...@btopenworld.com wrote:




 - Original Message -
 From: Greg Stein gst...@gmail.com
 To: Vladimir Berezniker v...@hitechman.com
 Cc: dev@subversion.apache.org
 Sent: Thursday, 31 May 2012, 8:35
 Subject: Re: [PATCH] JavaHL: Reduce amount of duplicate code used to check 
 C++ pointer extracted from the java object

 On Thu, May 31, 2012 at 12:43 AM, Vladimir Berezniker
 v...@hitechman.com wrote:
  Patch 01 - Introduce macro

  [[[
  JavaHL: Added CPPADDR_NULL_PTR macro to reduce amount of duplicate code
  checking C++ pointer extracted from the java object

  [ in subversion/bindings/javahl/native ]

  * JNIUtil.h
    (CPPADDR_NULL_PTR): New macro to test for NULL pointer and raise java
      exception if necessary
  ]]]

 Replying to just this patch. The second patch seems pretty mechanical.
 And we're only looking at minor nits.

 (sorry, but the patch doesn't inline into this response, so let's just
 be flexible here...)


 The macro argument substitutions need to be parenthesized for safety.
 So it would be: (expr) == NULL, and it would be: return (ret_val);

 I notice the second patch relies on being able to pass an empty 
 (whitespace-only) second argument in order to generate return; in the 
 macro, so putting parentheses there wouldn't work.  Actually I didn't know it 
 was possible to pass an empty (or, rather, whitespace-only) argument to a 
 macro, but apparently it is.  Is it standardized?  If so, this seems fine to 
 me, to use the argument without adding parentheses around it.

We use this same trick for other macros, such as
SVN_JNI_NULL_PTR_EX().  I don't know if it is standardized, but we've
been using it for years.  We do have to omit the parenthesis around
the return value in the macro definition, as return (); is not valid
syntax.

-Hyrum


-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: svn commit: r1343456 - in /subversion/branches/javahl-ra/subversion/bindings/javahl/native: RevpropTable.cpp RevpropTable.h

2012-05-29 Thread Hyrum K Wright
On Tue, May 29, 2012 at 4:11 PM, Daniel Shahaf d...@daniel.shahaf.name wrote:
 Greg Stein wrote on Tue, May 29, 2012 at 16:28:19 -0400:
 I would suggest any logical change that can apply to trunk should be
 submitted to dev@. If it helps trunk, or is at least neutral, then I'd
 support it.

 We couldn't digest your initial 27 patches :-), but some minor ones showing
 up should be just fine. I would guess you'll be looking for a +1 from Mark
 or Hyrum. Most others don't really know JavaHL :-(


 Why don't we ask Vladimir to commit patches to the branch and then ask
 for a +1 to cherry-pick them to trunk?

 He could even maintain a STATUS file in the branch of revisions
 nominated for cherry picking...

So you're suggesting a change in policy from the traditional method of
committing stuff directly to trunk (after the requisite +1 from a full
committer, if required)?

I think the dev@ + review technique we've historically employed is
better for a couple of reasons.  Firstly, it ensures stuff gets to
trunk by the quickest route possible, and encourages the contributor
to work with the community.  Second, it also ensures that the code
will be released, even if the branch doesn't pan out.  Thirdly, I
think it gives the contributor more visibility, and hopefully
encourages reviewers to offer the relevant commit access more rapidly.

In light of those reasons, I still prefer Vladimir to submit generally
relevant patches to dev@ for commit to trunk, rather than commit to
the branch and pull them from there.  (And this isn't specific to just
this instance of course.)

-Hyrum


-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: Coverity and Apache buildbots.

2012-05-26 Thread Hyrum K Wright
On Fri, May 25, 2012 at 3:09 PM, Pedro Giffuni p...@apache.org wrote:
 Hello guys;

 Sorry to contact you about something somewhat off-topic but perhaps
 someone here can give me details (in private is OK) on how the Coverity
 scans are generated?

 On another Apache project we want to use coverity but infra@ is not
 aware about anything on their side that is required to enable it.

The Coverity scans are all done on their infrastructure, with reports
limited to people whom they have authorized to view them.

To be honest, it's been a long while since we as a project have done
anything meaningful with the Coverity reports.  Their system works by
substituting their scanning compiler for the normal one and then
running the project build system.  Some time ago, something about our
build system changed which broke their automation to the point where
the vast majority of the project wasn't being covered.  To compound
problems, the link to login to fetch results went bad a few months
after that, and efforts to contact them to determine a fix have been
futile.  While I personally am appreciative of the static analysis
tools Coverity provides, the lack of responsiveness has negated that
benefit.

In short, you need to contact Coverity directly, but it may take a lot
of effort.

If you are looking for static analysis tools, you may be interested in
the Clang static analyzer.  I have found it to be pretty useful in
finding many of the same issues Coverity claims to find.  You can find
more information about it here:
http://clang-analyzer.llvm.org/

Best,
-Hyrum



-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/


Re: svn commit: r1342676 - in /subversion/branches/javahl-ra/subversion/bindings/javahl: native/ src/org/apache/subversion/javahl/ tests/org/apache/subversion/javahl/

2012-05-25 Thread Hyrum K Wright
A couple stylistic nits on the log message:

On Fri, May 25, 2012 at 10:12 AM,  v...@apache.org wrote:
 Author: vmpn
 Date: Fri May 25 15:12:56 2012
 New Revision: 1342676

 URL: http://svn.apache.org/viewvc?rev=1342676view=rev
 Log:
 On the javahl-ra branch:

 Brought RA implementation up to date with changes merged from trunk in 
 r1329205

 [in subversion/bindings/javahl/native]

   * SVNReposAccess.cpp
      (SVNReposAccess): Drop the global pool mutex as it is not necessary, as 
 per r1154119
      (getDatedRev, getLocks, checkPath): Use getPool() instead of pool as per 
 r1154155

 [in subversion/bindings/javahl/src/org/apache/subversion/javahl/]

   * ISVNReposAccess.java, SVNReposAccess.java: Added imports for 
 org.apache.subversion.javahl.types.*  because classes moved from the 
 org.apache.subversion.javahl package

When including the same comment for multiple files, please put the
file names on separate lines.  Also, please wrap comments to
80-character line widths.

You can propedit this log message to fix.

-Hyrum


 [in subversion/bindings/javahl/test/org/apache/subversion/javahl/]

   * SVNRATests.java: Added imports for org.apache.subversion.javahl.types.*  
 because classes moved from the org.apache.subversion.javahl package

 Modified:
    
 subversion/branches/javahl-ra/subversion/bindings/javahl/native/SVNReposAccess.cpp
    
 subversion/branches/javahl-ra/subversion/bindings/javahl/src/org/apache/subversion/javahl/ISVNReposAccess.java
    
 subversion/branches/javahl-ra/subversion/bindings/javahl/src/org/apache/subversion/javahl/SVNReposAccess.java
    
 subversion/branches/javahl-ra/subversion/bindings/javahl/tests/org/apache/subversion/javahl/SVNRATests.java

 Modified: 
 subversion/branches/javahl-ra/subversion/bindings/javahl/native/SVNReposAccess.cpp
 URL: 
 http://svn.apache.org/viewvc/subversion/branches/javahl-ra/subversion/bindings/javahl/native/SVNReposAccess.cpp?rev=1342676r1=1342675r2=1342676view=diff
 ==
 --- 
 subversion/branches/javahl-ra/subversion/bindings/javahl/native/SVNReposAccess.cpp
  (original)
 +++ 
 subversion/branches/javahl-ra/subversion/bindings/javahl/native/SVNReposAccess.cpp
  Fri May 25 15:12:56 2012
 @@ -36,7 +36,6 @@

  SVNReposAccess::SVNReposAccess(const char *repos_url)
  {
 -  JNICriticalSection criticalSection(*JNIUtil::getGlobalPoolMutex());
   m_sess_pool = svn_pool_create(JNIUtil::getPool());

   svn_ra_callbacks2_t *cbtable =
 @@ -74,7 +73,7 @@ SVNReposAccess::getDatedRev(apr_time_t t
   svn_revnum_t rev;

   SVN_JNI_ERR(svn_ra_get_dated_revision(m_ra_session, rev, tm,
 -                                        requestPool.pool()),
 +                                        requestPool.getPool()),
               SVN_INVALID_REVNUM);

   return rev;
 @@ -87,10 +86,10 @@ SVNReposAccess::getLocks(const char *pat
   apr_hash_t *locks;

   SVN_JNI_ERR(svn_ra_get_locks2(m_ra_session, locks, path, depth,
 -                                requestPool.pool()),
 +                                requestPool.getPool()),
               NULL);

 -  return CreateJ::LockMap(locks, requestPool.pool());
 +  return CreateJ::LockMap(locks, requestPool.getPool());
  }

  jobject
 @@ -101,7 +100,7 @@ SVNReposAccess::checkPath(const char *pa

   SVN_JNI_ERR(svn_ra_check_path(m_ra_session, path,
                                 revision.revision()-value.number,
 -                                kind, requestPool.pool()),
 +                                kind, requestPool.getPool()),
               NULL);

   return EnumMapper::mapNodeKind(kind);

 Modified: 
 subversion/branches/javahl-ra/subversion/bindings/javahl/src/org/apache/subversion/javahl/ISVNReposAccess.java
 URL: 
 http://svn.apache.org/viewvc/subversion/branches/javahl-ra/subversion/bindings/javahl/src/org/apache/subversion/javahl/ISVNReposAccess.java?rev=1342676r1=1342675r2=1342676view=diff
 ==
 --- 
 subversion/branches/javahl-ra/subversion/bindings/javahl/src/org/apache/subversion/javahl/ISVNReposAccess.java
  (original)
 +++ 
 subversion/branches/javahl-ra/subversion/bindings/javahl/src/org/apache/subversion/javahl/ISVNReposAccess.java
  Fri May 25 15:12:56 2012
 @@ -26,6 +26,12 @@ package org.apache.subversion.javahl;
  import java.util.Date;
  import java.util.Map;

 +import org.apache.subversion.javahl.types.Depth;
 +import org.apache.subversion.javahl.types.Lock;
 +import org.apache.subversion.javahl.types.NodeKind;
 +import org.apache.subversion.javahl.types.Revision;
 +import org.apache.subversion.javahl.types.Version;
 +
  /**
  * This interface is an interface to interact with a remote Subversion
  * repository via the repository access method.

 Modified: 
 subversion/branches/javahl-ra/subversion/bindings/javahl/src/org/apache/subversion/javahl/SVNReposAccess.java
 URL: 
 

  1   2   3   4   5   6   7   8   9   10   >