JavaHL SVNClient.logMessages() cannot return all revision properties.

[Doros Agathangelou <do...@triadiktyo.com>]

Hi All

I have been trying to get the logMessages of a repository along with ALL
revision properties. Turns out this is not possible through javahl and I
believe this creates an inconsistency between javahl and the svn C api.

This is because in the C API, *svn_client_log5* will accept a NULL argument
for the revprops argument to return all revision properties. Excerpt from
API follows

 * If @a revprops is NULL, retrieve all revision properties; else, retrieve
 * only the revision properties named by the (const char *) array elements
 * (i.e. retrieve none if the array is empty).

Now I believe the problem in javahl is that the class StringArray does not
cater for NULL values and will turn a NULL value into an empty string
array.

This can be seen in file org_apache_subversion_javahl_SVNClient.cpp at
method Java_org_apache_subversion_javahl_SVNClient_logMessages,

If a null is passed to jrevProps then the line:
StringArray revProps(jrevProps);
will initialize a revProps object with an empty collection.

This array generated from the revProps object will subsequently be passed
to svn_client_log5 as an empty array (instead of a NULL) which means that
no revision properties are returned when the intention was to actually
receive all revisions.

I have fixed this in my builds using the attached patch. It is probably not
the best or cleanest solution but it works. I am hoping that a more
experienced person might provide a better solution to this. Or if this is
an acceptable fix maybe commit this patch to the trunk.

Some notes on the attached patch: The reason I am creating a new
StringArray constructor is because StringArray is used in a lot of places
and I am not sure if allowing NULLs everywhere might cause more trouble in
the rest of the API. This new constructor will set a flag of whether the
revProps were NULL and then in the array() method we check the flag to
return NULL instead of an empty array.

Thanks
Doros

Re: JavaHL SVNClient.logMessages() cannot return all revision properties.

[Branko Čibej <br...@apache.org>]

On 11.11.2016 14:56, Branko \u010cibej wrote:
> On 11.11.2016 14:02, Branko \u010cibej wrote:
>> One solution would be to introduce an overload of the
>> ISVNClient.logMessages() method that would have the option to return all
>> revprops.
> In fact, the attached patch does just that. Can you test it, please?

Committed in r1780819 and I'm so thrilled by the O.P.'s dedication and
support.

-- Brane

Re: JavaHL SVNClient.logMessages() cannot return all revision properties.

[Branko Čibej <br...@apache.org>]

On 11.11.2016 14:02, Branko \u010cibej wrote:
> One solution would be to introduce an overload of the
> ISVNClient.logMessages() method that would have the option to return all
> revprops.

In fact, the attached patch does just that. Can you test it, please?

-- Brane

Re: JavaHL SVNClient.logMessages() cannot return all revision properties.

[Branko Čibej <br...@apache.org>]

On 07.11.2016 11:56, Doros Agathangelou wrote:
> Hi All
>
> I have been trying to get the logMessages of a repository along with ALL
> revision properties. Turns out this is not possible through javahl and I
> believe this creates an inconsistency between javahl and the svn C api.
>
> This is because in the C API, *svn_client_log5* will accept a NULL argument
> for the revprops argument to return all revision properties. Excerpt from
> API follows
>
>  * If @a revprops is NULL, retrieve all revision properties; else, retrieve
>  * only the revision properties named by the (const char *) array elements
>  * (i.e. retrieve none if the array is empty).
>
> Now I believe the problem in javahl is that the class StringArray does not
> cater for NULL values and will turn a NULL value into an empty string
> array.
>
> This can be seen in file org_apache_subversion_javahl_SVNClient.cpp at
> method Java_org_apache_subversion_javahl_SVNClient_logMessages,
>
> If a null is passed to jrevProps then the line:
> StringArray revProps(jrevProps);
> will initialize a revProps object with an empty collection.
>
> This array generated from the revProps object will subsequently be passed
> to svn_client_log5 as an empty array (instead of a NULL) which means that
> no revision properties are returned when the intention was to actually
> receive all revisions.
>
> I have fixed this in my builds using the attached patch. It is probably not
> the best or cleanest solution but it works. I am hoping that a more
> experienced person might provide a better solution to this. Or if this is
> an acceptable fix maybe commit this patch to the trunk.

The biggest problem here is that your patch makes an incompatible change
in the public JavaHL API. In other words, everyone who's currently using
JavaHL's ISVNClient.logMessages() method will assume that passing a null
revProps parameter will return no revision properties. With your change,
it would return /all/ revision properties. That kind of change is not
acceptable.

One solution would be to introduce an overload of the
ISVNClient.logMessages() method that would have the option to return all
revprops.

-- Brane

P.S.: If you're going to work on such a patch, please read the relevant
sections of our Community Guide regarding code formatting, notably:
http://subversion.apache.org/docs/community-guide/conventions.html#coding-style
http://subversion.apache.org/docs/community-guide/conventions.html#other-conventions
http://subversion.apache.org/docs/community-guide/conventions.html#log-messages