[ 
https://issues.apache.org/jira/browse/SVN-4628?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Marek Parfianowicz updated SVN-4628:
------------------------------------
    Description: 
In this commit:

https://github.com/apache/subversion/commit/fd00c8271282c7d7b0283f5e994feba0f00417be#diff-16c43c60e5af2f2a802701c33d649b26

a static code has been added to load the native library to all classes having 
native methods. 

This is superfluous. Please note that subclasses can provide a non-native 
implementation of these native methods. So having a native library is actually 
not mandatory.

There's a real problem with SVNKit, which provides a pure Java-based 
implementation of SVN protocol, that you cannot use it without having SVN 
binaries installed. 

Please have a look at this bug report for SVNKit 1.9.x:

https://issues.tmatesoft.com/issue/SVNKIT-662

which actually should be fixed on SVN side...



================================================================================

You have a permission scheme in your JIRA configured incorrectly as I cannot 
add comments. So let me answer your questions here:


bq. (Please provide references to commits in the ASF Subversion repository. The 
thing on GitHub is just a read-only mirror and there's no reference to the 
original revision there.)

It's this change: http://svn.apache.org/viewvc?view=revision&revision=1525922

bq. I added the static initialisers because otherwise those classes would not 
work correctly. Loading the native method implementations is not superfluous in 
any sense; JavaHL does not provide a mandatory initialisation method that could 
be used for loading the native library.

Branko, correct me if I am wrong, but my understanding is that JavaHL 
(https://svn.apache.org/repos/asf/subversion/trunk/subversion/bindings/javahl/README)
 consists of high level Java API for Subversion and concrete, native, 
implementation of this API using core Subversion C API.

Until subversion 1.9.0 it was possible to use high level API in a way described 
in https://issues.tmatesoft.com/issue/SVNKIT-662 issue, without the need to 
have native libraries installed on the system.   Since the cs:1525922 change 
though, those native libraries are required.

This causes issues in products like Atlassian FishEye, where SVNKit library is 
bundled and used out of the box as the Subversion JavaHL implementation.  Seems 
like change introduced in subversion 1.9.0 would require us to bundle OS 
specific native libraries, just to check the JavaHL version in runtime.  Seems 
like an overkill, especially knowing that in default configuration those 
binaries wouldn't be used anyway as SVNKit implementation will be used.

bq. JavaHL does not provide a mandatory initialisation method that could be 
used for loading the native library.

I see your concern here, not sure if loading native libraries in every API 
class is the best solution to the problem.  Perhaps a JavaDoc explaining the 
responsibility of the client to call NativeResources.loadNativeLibrary(); is 
sufficient?



  was:
In this commit:

https://github.com/apache/subversion/commit/fd00c8271282c7d7b0283f5e994feba0f00417be#diff-16c43c60e5af2f2a802701c33d649b26

a static code has been added to load the native library to all classes having 
native methods. 

This is superfluous. Please note that subclasses can provide a non-native 
implementation of these native methods. So having a native library is actually 
not mandatory.

There's a real problem with SVNKit, which provides a pure Java-based 
implementation of SVN protocol, that you cannot use it without having SVN 
binaries installed. 

Please have a look at this bug report for SVNKit 1.9.x:

https://issues.tmatesoft.com/issue/SVNKIT-662

which actually should be fixed on SVN side...



================================================================================

You have a permission scheme in your JIRA configured incorrectly as I cannot 
add comments. So let me answer your questions here:


bq. (Please provide references to commits in the ASF Subversion repository. The 
thing on GitHub is just a read-only mirror and there's no reference to the 
original revision there.)

It's this change: http://svn.apache.org/viewvc?view=revision&revision=1525922

bq. I added the static initialisers because otherwise those classes would not 
work correctly. Loading the native method implementations is not superfluous in 
any sense; JavaHL does not provide a mandatory initialisation method that could 
be used for loading the native library.

Branko, correct me if I am wrong, but my understanding is that JavaHL 
(https://svn.apache.org/repos/asf/subversion/trunk/subversion/bindings/javahl/README)
 consists of high level Java API for Subversion and concrete, native, 
implementation of this API using core Subversion C API.

Until subversion 1.9.0 it was possible to use high level API in a way described 
in https://issues.tmatesoft.com/issue/SVNKIT-662 issue, without the need to 
have native libraries installed on the system.   Since the cs:1525922 change 
though, those native libraries are required.

This causes issues in products like Atlassian FishEye, where SVNKit library is 
bundled and used out of the box as the Subversion JavaHL implementation.  Seems 
like change introduced in subversion 1.9.0 would require us to bundle OS 
specific native libraries, just the check the JavaHL version in runtime.  Seems 
like an overkill, especially knowing that in default configuration those 
binaries wouldn't be used anyway as SVNKit implementation will be used.

bq. JavaHL does not provide a mandatory initialisation method that could be 
used for loading the native library.

I see your concern here, not sure if loading native libraries in every API 
class is the best solution to the problem.  Perhaps a JavaDoc explaining the 
responsibility of the client to call NativeResources.loadNativeLibrary(); is 
sufficient?




> Classes with native methods unnecessarily load the svnjavahl-1 native library
> -----------------------------------------------------------------------------
>
>                 Key: SVN-4628
>                 URL: https://issues.apache.org/jira/browse/SVN-4628
>             Project: Subversion
>          Issue Type: Bug
>          Components: bindings_javahl
>    Affects Versions: 1.9.0, 1.9.1, 1.9.2, 1.9.3
>            Reporter: Marek Parfianowicz
>            Assignee: Branko Čibej
>
> In this commit:
> https://github.com/apache/subversion/commit/fd00c8271282c7d7b0283f5e994feba0f00417be#diff-16c43c60e5af2f2a802701c33d649b26
> a static code has been added to load the native library to all classes having 
> native methods. 
> This is superfluous. Please note that subclasses can provide a non-native 
> implementation of these native methods. So having a native library is 
> actually not mandatory.
> There's a real problem with SVNKit, which provides a pure Java-based 
> implementation of SVN protocol, that you cannot use it without having SVN 
> binaries installed. 
> Please have a look at this bug report for SVNKit 1.9.x:
> https://issues.tmatesoft.com/issue/SVNKIT-662
> which actually should be fixed on SVN side...
> ================================================================================
> You have a permission scheme in your JIRA configured incorrectly as I cannot 
> add comments. So let me answer your questions here:
> bq. (Please provide references to commits in the ASF Subversion repository. 
> The thing on GitHub is just a read-only mirror and there's no reference to 
> the original revision there.)
> It's this change: http://svn.apache.org/viewvc?view=revision&revision=1525922
> bq. I added the static initialisers because otherwise those classes would not 
> work correctly. Loading the native method implementations is not superfluous 
> in any sense; JavaHL does not provide a mandatory initialisation method that 
> could be used for loading the native library.
> Branko, correct me if I am wrong, but my understanding is that JavaHL 
> (https://svn.apache.org/repos/asf/subversion/trunk/subversion/bindings/javahl/README)
>  consists of high level Java API for Subversion and concrete, native, 
> implementation of this API using core Subversion C API.
> Until subversion 1.9.0 it was possible to use high level API in a way 
> described in https://issues.tmatesoft.com/issue/SVNKIT-662 issue, without the 
> need to have native libraries installed on the system.   Since the cs:1525922 
> change though, those native libraries are required.
> This causes issues in products like Atlassian FishEye, where SVNKit library 
> is bundled and used out of the box as the Subversion JavaHL implementation.  
> Seems like change introduced in subversion 1.9.0 would require us to bundle 
> OS specific native libraries, just to check the JavaHL version in runtime.  
> Seems like an overkill, especially knowing that in default configuration 
> those binaries wouldn't be used anyway as SVNKit implementation will be used.
> bq. JavaHL does not provide a mandatory initialisation method that could be 
> used for loading the native library.
> I see your concern here, not sure if loading native libraries in every API 
> class is the best solution to the problem.  Perhaps a JavaDoc explaining the 
> responsibility of the client to call NativeResources.loadNativeLibrary(); is 
> sufficient?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to