You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Mark Phippard <ma...@gmail.com> on 2012/05/09 15:59:39 UTC

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

On Wed, May 9, 2012 at 8:18 AM,  <st...@apache.org> wrote:
> Author: stsp
> Date: Wed May  9 12:18:37 2012
> New Revision: 1336128
>
> URL: http://svn.apache.org/viewvc?rev=1336128&view=rev
> Log:
> Blind attempt at fixing the JavaHL build after r1336110 broke it.
>
> [ in subversion/bindings/javahl/ ]
>
> * native/SVNClient.h,
>  native/SVNClient.cpp
>  (diff, diff, diff): Add propsOnly parameter.
>
> * native/org_apache_subversion_javahl_SVNClient.cpp
>  (Java_org_apache_subversion_javahl_SVNClient_diff__Ljava_lang_String_2Lorg_apache_subversion_javahl_types_Revision_2Ljava_lang_String_2Lorg_apache_subversion_javahl_types_Revision_2Ljava_lang_String_2Ljava_io_OutputStream_2Lorg_apache_subversion_javahl_types_Depth_2Ljava_util_Collection_2ZZZZZ): Wow! This has to be
>   the longest function name in all of Subversion!
>
> * src/org/apache/subversion/javahl/SVNClient.java
>  (diff, diff): Add propsOnly parameter. Pass 'false' in compat wrappers.
>
> * src/org/apache/subversion/javahl/ISVNClient.java
>  (diff, diff): Add propsOnly parameter.

[snip]


> Modified: subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/ISVNClient.java
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/ISVNClient.java?rev=1336128&r1=1336127&r2=1336128&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/ISVNClient.java (original)
> +++ subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/ISVNClient.java Wed May  9 12:18:37 2012
> @@ -522,13 +522,14 @@ public interface ISVNClient
>      * @param copiesAsAdds  if set, copied files will be shown in their
>      *                      entirety, not as diffs from their sources
>      * @param ignoreProps   don't show property diffs
> +     * @param propsOnly     show property changes only
>      * @throws ClientException
>      */
>     void diff(String target1, Revision revision1, String target2,
>               Revision revision2, String relativeToDir, OutputStream outStream,
>               Depth depth, Collection<String> changelists,
>               boolean ignoreAncestry, boolean noDiffDeleted, boolean force,
> -              boolean copiesAsAdds, boolean ignoreProps)
> +              boolean copiesAsAdds, boolean ignoreProps, boolean propsOnly)
>             throws ClientException;
>
>     void diff(String target1, Revision revision1, String target2,
> @@ -554,6 +555,7 @@ public interface ISVNClient
>      * @param copiesAsAdds  if set, copied files will be shown in their
>      *                      entirety, not as diffs from their sources
>      * @param ignoreProps   don't show property diffs
> +     * @param propsOnly     show property changes only
>      * @throws ClientException
>      */
>     void diff(String target, Revision pegRevision, Revision startRevision,
> @@ -561,7 +563,7 @@ public interface ISVNClient
>               OutputStream outStream,
>               Depth depth, Collection<String> changelists,
>               boolean ignoreAncestry, boolean noDiffDeleted, boolean force,
> -              boolean copiesAsAdds, boolean ignoreProps)
> +              boolean copiesAsAdds, boolean ignoreProps, boolean propsOnly)
>             throws ClientException;

I do not know if these methods are new in 1.8, but if not, then this
is the public API.  So for compatibility you have to add a new method
with the  new additional options.  You do not need to add a "2" or
anything to the method name since Java does not require that, just add
another method to the interface.

In the SVNClient implementation, you can remove the "native" from the
old method and just have it call the new native method with suitable
defaults.

-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

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

Posted by Stefan Sperling <st...@elego.de>.
On Wed, May 09, 2012 at 11:57:29PM -0400, Greg Stein wrote:
> On Wed, May 9, 2012 at 10:07 AM, Stefan Sperling <st...@elego.de> wrote:
> > The parameter is new in 1.8. The ignoreProps parameter is new in 1.8, too,
> > and I modeled my changes on those that added the ignoreProps parameter.
> >
> > So I suppose this change is alright?
> 
> Nope. People coding to the 1.7 API will not be providing that
> parameter. Thus, it will not compile.
> 
> You need to define new methods that include the parameter, to
> supplement the existing methods that do not have it. (yay,
> polymorphism!)

Yes, that's the approach to take when changing the 1.7 variant.

However, according to Mark I was actually changing the 1.8 variant.
The API was already "bumped" earlier by Hyrum (ignoreProps and something
else is new in 1.8, too).

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

Posted by Greg Stein <gs...@gmail.com>.
On Wed, May 9, 2012 at 10:07 AM, Stefan Sperling <st...@elego.de> wrote:
> On Wed, May 09, 2012 at 09:59:39AM -0400, Mark Phippard wrote:
>> On Wed, May 9, 2012 at 8:18 AM,  <st...@apache.org> wrote:
>> > Author: stsp
>> > Date: Wed May  9 12:18:37 2012
>> > New Revision: 1336128
>
>> > @@ -561,7 +563,7 @@ public interface ISVNClient
>> >               OutputStream outStream,
>> >               Depth depth, Collection<String> changelists,
>> >               boolean ignoreAncestry, boolean noDiffDeleted, boolean force,
>> > -              boolean copiesAsAdds, boolean ignoreProps)
>> > +              boolean copiesAsAdds, boolean ignoreProps, boolean propsOnly)
>> >             throws ClientException;
>>
>> I do not know if these methods are new in 1.8, but if not, then this
>> is the public API.  So for compatibility you have to add a new method
>> with the  new additional options.  You do not need to add a "2" or
>> anything to the method name since Java does not require that, just add
>> another method to the interface.
>
> The parameter is new in 1.8. The ignoreProps parameter is new in 1.8, too,
> and I modeled my changes on those that added the ignoreProps parameter.
>
> So I suppose this change is alright?

Nope. People coding to the 1.7 API will not be providing that
parameter. Thus, it will not compile.

You need to define new methods that include the parameter, to
supplement the existing methods that do not have it. (yay,
polymorphism!)

Cheers,
-g

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

Posted by Stefan Sperling <st...@elego.de>.
On Wed, May 09, 2012 at 09:59:39AM -0400, Mark Phippard wrote:
> On Wed, May 9, 2012 at 8:18 AM,  <st...@apache.org> wrote:
> > Author: stsp
> > Date: Wed May  9 12:18:37 2012
> > New Revision: 1336128

> > @@ -561,7 +563,7 @@ public interface ISVNClient
> >               OutputStream outStream,
> >               Depth depth, Collection<String> changelists,
> >               boolean ignoreAncestry, boolean noDiffDeleted, boolean force,
> > -              boolean copiesAsAdds, boolean ignoreProps)
> > +              boolean copiesAsAdds, boolean ignoreProps, boolean propsOnly)
> >             throws ClientException;
> 
> I do not know if these methods are new in 1.8, but if not, then this
> is the public API.  So for compatibility you have to add a new method
> with the  new additional options.  You do not need to add a "2" or
> anything to the method name since Java does not require that, just add
> another method to the interface.

The parameter is new in 1.8. The ignoreProps parameter is new in 1.8, too,
and I modeled my changes on those that added the ignoreProps parameter.

So I suppose this change is alright?