On Tue, 29 May 2007, Conor MacNeill wrote:
> This change retains the first thrown exception when attempting to load
> the svnjavahl-1 native library rather than throwing the exception from
> the last attempt. This is important when the user specifies the library
> to load and it cannot be loaded. At present the error that finally gets
> returned to the user is not the one related to the attempt to load their
> On my system for example, the change lets me see this error:
> Could not load svn-javahl:
> Can't load IA 32-bit .dll on a AMD 64-bit platform
Sounds like a good idea, Connor.
> which before I could not. Here is my suggested log message:
> Better error reporting when unable to load the Native JavaHL library
> (loadNativeLibrary): Use the earliest exception to report to the user
> rather than the last
> Let me know if I goofed.
A few issues:
- You continue to use the "if (loadException == null)" check after the
first two attempts to load the library. These will never actually
succeed in setting "loadException" though, will they? (Just as the
second won't succeed if the "subversion.native.library" system
property is set.)
- You unrolled the exception handling from the nested try/catch blocks
which were previously used. While not necessarily a bad change, this
formatting change makes the diff harder to read and review.
- You used tabs instead of spaces, which makes the diff more difficult
to review, and don't conform to the Subversion project's coding
I'm attaching a revised patch which attempts to address the second two
items. I'm not sure the first is worth addressing. Thoughts?
Received on Tue May 29 23:05:32 2007
- application/pgp-signature attachment: stored